On 29/06/07, Brion Vibber brion@wikimedia.org wrote:

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

robchurch@svn.wikimedia.org wrote:

...

+'ShowRawCssJs': When presenting raw CSS and JavaScript during page views +&$text: Text being shown +$title: Title of the custom script/stylesheet page +$output: Current OutputPage object

[snip]

...

+// Give hooks a chance to do formatting... +if( wfRunHooks( 'ShowRawCssJs', array( &$text, $this->mTitle, $wgOut ) ) ) {

• // Wrap the whole lot in a <pre> and don't parse
  

• preg_match( '!\.(css|js)$!u', $this->mTitle->getText(), $m );
  

• $wgOut->addHtml( "<pre class=\"mw-code mw-{$m[1]}\" dir=\"ltr\">\n" );
  

• $wgOut->addHtml( htmlspecialchars( $text ) );
  

• $wgOut->addHtml( "\n</pre>\n" );
  

+} else {

• // Wrap hook output in a <div> with the right direction attribute
  

• $wgOut->addHtml( "<div dir=\"ltr\">\n{$text}\n</div>" );
  

+}

I find I'm a bit leery of this hook. The $text parameter is source text on input, and may be *either* source text *or* HTML on output.

This sort of thing feels "unsafe by default"; not only does the variable change type, but it changes in an unsafe direction (eg, a safe text string may be unsafe HTML).

I'd rather have the hook either do its own output on $output when returning false, or return an HTML string via another parameter.

Addressed these concerns in r23553 (and r23554), and updated the extension in r23555.

Rob Church