[prev in list] [next in list] [prev in thread] [next in thread] 

List:       wikitech-l
Subject:    Re: [Wikitech-l] [MediaWiki-CVS] SVN:  [23537] trunk/phase3
From:       Brion Vibber <brion () wikimedia ! org>
Date:       2007-06-29 13:25:21
Message-ID: 46850841.6070106 () wikimedia ! org
[Download RAW message or body]

-----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.

- -- brion vibber (brion @ wikimedia.org)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGhQhBwRnhpk1wk44RAlFgAJ9HPkd9o3bLbo272qaDM8V+QjcIqQCgkKQG
H0H29izL+vUqWc855dn/ci8=
=Q1JP
-----END PGP SIGNATURE-----

_______________________________________________
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
http://lists.wikimedia.org/mailman/listinfo/wikitech-l
[prev in list] [next in list] [prev in thread] [next in thread]