Wikimedia Bugzilla is closed!

I already pointed this out on the list, but you can't simply turn off escaping. It's there for security reasons, not to prevent people from adding markeup. What if someone enters this?

<script>alert('xss');</script>

It'll just be executed by the browser. See this for more info: https://en.wikipedia.org/wiki/Cross-site_scripting

Other then that it looks ok. One minor thing though: it's better to use setMessage( 'foo' ) then setDescription( wfMsg( 'foo' ) ), as it allows handling code to get the message in languages other then the one of the user executing the code.

Content of message is passed trough MediwKiki parser, so:

1. Unsafe HTML element will not be recognized (but escaped instead):

{{ #info: <a href="…">link</a> | escape = no }}

produces literal "<a href="…">link</a>".

2. Unsafe attributes causes warnings:

{{ #info: <span style="…">link</span> | escape = no }}

produces "Error: style is not allowed" (wording is not exact (translated back
to English from another language)). In particular, 

{{ #info: <script>alert('xss');</script> | escape = no }}

produces nothing more than just "<script>alert('xss');</script>". Literally.

Right. I ought to review more carefully :)

In that case I don't see the point of not having the escaping thing off altogether - where do you anticipate this breaking compat? This icon param has not been there long, so even if it changes something, I doubt it'll affect many people.

> …where do you anticipate this breaking compat?

I would like `escape' if off by default. Actually, I do not need it at all, if escaping is off. You decide.

> …it's better to use setMessage( 'foo' ) then setDescription( wfMsg( 'foo' ) ), as it allows handling code to get the message in languages other then the one of the user executing the code.

Sure I will use setDescription… Will you in turn update comments in class
`Parameter'? From the code it is not clear what the difference between them:

    /**
     * Sets a description message for the parameter.
     * 
     * @since 0.4.3
     * 
     * @param string $descriptionMessage
     */
    public function setDescription( $descriptionMessage ) {
        $this->description = $descriptionMessage;
    }

    /**
     * Sets a message for the parameter that will act as description.
     * 
     * @since 0.4.9
     * 
     * @param string $message
     */
    public function setMessage( $message ) {
        $this->message = $message;
    }

As soon as you decide approach to escaping (whether we need a parameter; default value if it is a parameter), I will prepare a new version of the patch.

Just drop the param, I don't think it's needed.

Ok. Few more questions. This code is not clear to me:

// Starting from MW 1.16, there is a more suited method available: Title::isSpecialPage
$title = $this->parser->getTitle();
if ( !is_null( $title ) && $title->getNamespace() == NS_SPECIAL ) {
    global $wgOut;
    SMWOutputs::commitToOutputPage( $wgOut );
}
else {
    SMWOutputs::commitToParser( $this->parser );
}

Note it was "global $wgTitle;" before, but they say $wgTitle is obsolete and will be deleted very soon, so I replaced it with 

$title = $this->parser->getTitle();

Is that ok?

Also confirm that SMWOutputs::commitTo{Parser,OutputPage} is about extra resources (like scripts and style sheets) and `render' result is always passed through the parser, even on special pages.

Created attachment 9263 [details]
Patch: New #info implementation, v2.

Changes from the first version:

1. Parameter `escape' removed; message is not escaped.
2. `setMessage' is used instead of `setDescription'.

Applied in r100382

Verified on r102853.