Last modified: 2012-04-22 14:31:03 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T36453, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 34453 - Extension:PageCSS parser tag hook needs to return a value (even if it's an empty string) for MediaWiki 1.18+ compatibility
Extension:PageCSS parser tag hook needs to return a value (even if it's an em...
Status: REOPENED
Product: MediaWiki extensions
Classification: Unclassified
PageCSS (Other open bugs)
unspecified
All All
: Normal normal (vote)
: ---
Assigned To: Ævar Arnfjörð Bjarmason
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-16 19:39 UTC by Carl Austin Bennett
Modified: 2012-04-22 14:31 UTC (History)
2 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
new PageCSS.php from [[mw:Extension:NewPageCSS/Source]] (1.46 KB, application/x-httpd-php)
2012-02-16 19:39 UTC, Carl Austin Bennett
Details

Description Carl Austin Bennett 2012-02-16 19:39:10 UTC
Created attachment 10026 [details]
new PageCSS.php from [[mw:Extension:NewPageCSS/Source]]

[[mw:Extension:NewPageCSS]] (which has source code on-wiki) is not a fully-new extension but a minor fix to [[mw:Extension:PageCSS]], an extension already in SVN.

It's now tagged {{Extension code in wiki}} as if it were a separate extension, but it differs from the PageCSS currently in SVN only in bug fixes needed to keep the extension working on MediaWiki 1.16-1.17 (replace raw Unicode characters in the author's name with HTML æ - style codes) and MediaWiki 1.18-1.20 (return the empty string, "", where the original would have had the tag hook stuff the CSS into the page headers and return nothing at all, leaving annoying UNIQ-123456789-css-0000000-QINU tags in the displayed page. Parser tags *must* return a value, so this is just a bug fix.

I propose that NewPageCSS be merged back to PageCSS and the current (repaired) code placed back into SVN at the original name. There is no need to list this on-wiki as two extensions if one is a trivial debugged copy of the other and no need to propose a new extension be created in SVN when fixing the existing file suffices.

I'm not sure whether I should be proposing this here or on-wiki but, since this is SVN content, a {{merge}} template on-wiki didn't seem to be quite suited as both the wiki and SVN would be updated by any attempt to merge the two versions.
Comment 1 Carl Austin Bennett 2012-02-19 19:49:46 UTC
I've looked at the PageCSS/PageCSS.php currently in trunk and it appears that the only thing which needs to change for it to work correctly is for the parser tag hook to return a value - a one-line change to return the empty string when done.

Nothing else which was changed between PageCSS and NewPageCSS actually makes a difference - whatever restrictions might have been encountered for Unicode in the description strings were specific to one previous MediaWiki version and have been resolved, rendering [[mw:extension:NewPageCSS]] unnecessary.

At this point, this is a bug fix for the PageCSS/PageCSS.php currently in trunk.

	public static function parse( $content, array $args, Parser $parser ) {
		$css = htmlspecialchars( trim( Sanitizer::checkCss( $content ) ) );
		$parser->mOutput->addHeadItem( <<<EOT
<style type="text/css">
/*<![CDATA[*/
{$css}
/*]]>*/
</style>
EOT
		);
		return htmlspecialchars("");
	}

I've changed the status of this item to be a one-line bug fix... not an enhancement.
Comment 2 Carl Austin Bennett 2012-02-19 20:01:57 UTC
The diff for PageCSS.php is (trivially):

40a41
> 		return htmlspecialchars("");
Comment 3 Max Semenik 2012-04-22 09:38:20 UTC
So why open a bug if you can change the code on-wiki? ;) I've fixed a reundancy in the new version cause htmlspecialchars("") === ''.
Comment 4 Carl Austin Bennett 2012-04-22 14:31:03 UTC
The original [[mw:extension:PageCSS]] has its code in version control, not on-wiki. The on-wiki code is a pointless fork of this code to [[mw:extension:NewPageCSS]]. We shouldn't have to fork code like this just for one trivial bugfix. As such, I've re-opened this to request the one-line fix in SVN that was needed two months ago (or needed since MW 1.18 came out last year).

http://svn.wikimedia.org/svnroot/mediawiki/trunk/extensions/PageCSS/PageCSS.php still returns nothing from the final routine public static function CssHook::parse() and needs to return the empty string. This needs to be fixed in SVN (and git if this code turns up there, which so far it has not).

As for htmlspecialchars(""), that makes no difference either way - constant input will give constant output in this case - but we do need to return *something*, anything, even if it is an empty string or this will give UNIQ...QINU rubbish in the rendered output on MW 1.18+.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links