r89648 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89647‎ | r89648 | r89649 >
Date:15:12, 7 June 2011
Author:pcopp
Status:reverted (Comments)
Tags:
Comment:
Another try at fixing bug 93 "tilde signatures inside nowiki tags sometimes get expanded (<includeonly><nowiki>~~~~</nowiki></includeonly>)"

* Change the preprocessor to insert strip items for <ignore> nodes during pre-save-transform, just like <comment> nodes are handled already. This effectively disables all pre-save-transform steps inside <includeonly> tags.
* Adapt parser tests to the new behavior.
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/parser/Preprocessor_DOM.php (modified) (history)
  • /trunk/phase3/includes/parser/Preprocessor_Hash.php (modified) (history)
  • /trunk/phase3/tests/parser/parserTests.txt (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
——@@ -86,6 +86,8 @@
8787 * (bug 29101) Special:FileDuplicateSearch shows silly message
8888 * (bug 29048) jQuery.tabIndex: firstTabIndex() should not output the same
8989 as lastTabIndex().
 90+* (bug 93) tilde signatures inside nowiki tags sometimes get expanded
 91+ (<includeonly><nowiki>~~~~</nowiki></includeonly>)
9092
9193 === API changes in 1.19 ===
9294 * BREAKING CHANGE: action=watch now requires POST and token.
Index: trunk/phase3/tests/parser/parserTests.txt
——@@ -3926,7 +3926,7 @@
39273927 !! result
39283928 * [[Special:Contributions/127.0.0.1|127.0.0.1]]
39293929 * <noinclude>[[Special:Contributions/127.0.0.1|127.0.0.1]]</noinclude>
3930 -* <includeonly>[[Special:Contributions/127.0.0.1|127.0.0.1]]</includeonly>
 3930+* <includeonly>~~~</includeonly>
39313931 * <onlyinclude>[[Special:Contributions/127.0.0.1|127.0.0.1]]</onlyinclude>
39323932 !! end
39333933
——@@ -3934,7 +3934,7 @@
39353935 !! test
39363936 pre-save transform: Signature expansion in nowiki tags (bug 93)
39373937 !! options
3938 -pst disabled
 3938+pst
39393939 !! input
39403940 Shall not expand:
39413941
Index: trunk/phase3/includes/parser/Preprocessor_Hash.php
——@@ -1019,8 +1019,11 @@
10201020 # OT_WIKI will only respect <ignore> in substed templates.
10211021 # The other output types respect it unless NO_IGNORE is set.
10221022 # extractSections() sets NO_IGNORE and so never respects it.
1023 - if ( ( !isset( $this->parent ) && $this->parser->ot['wiki'] ) || ( $flags & PPFrame::NO_IGNORE ) ) {
 1023+ if ( $flags & PPFrame::NO_IGNORE ) {
10241024 $out .= $contextNode->firstChild->value;
 1025+ # Add a strip marker in PST mode so that pstPass2() can run some old-fashioned regexes on the result
 1026+ } elseif ( !isset( $this->parent ) && $this->parser->ot['wiki'] ) {
 1027+ $out .= $this->parser->insertStripItem( $contextNode->firstChild->value );
10251028 } else {
10261029 //$out .= '';
10271030 }
Index: trunk/phase3/includes/parser/Preprocessor_DOM.php
——@@ -1089,8 +1089,11 @@
10901090 # OT_WIKI will only respect <ignore> in substed templates.
10911091 # The other output types respect it unless NO_IGNORE is set.
10921092 # extractSections() sets NO_IGNORE and so never respects it.
1093 - if ( ( !isset( $this->parent ) && $this->parser->ot['wiki'] ) || ( $flags & PPFrame::NO_IGNORE ) ) {
 1093+ if ( $flags & PPFrame::NO_IGNORE ) {
10941094 $out .= $contextNode->textContent;
 1095+ # Add a strip marker in PST mode so that pstPass2() can run some old-fashioned regexes on the result
 1096+ } elseif ( !isset( $this->parent ) && $this->parser->ot['wiki'] ) {
 1097+ $out .= $this->parser->insertStripItem( $contextNode->textContent );
10951098 } else {
10961099 $out .= '';
10971100 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r89662Provisional revert of r89648: "Another try at fixing bug 93 "tilde signatures...brion17:34, 7 June 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r5295BUG#93 Fix handling of <nowiki> in templatesjeluf23:14, 18 September 2004
r5296BUG#93 Fix handling of <nowiki> in templates. From HEADjeluf23:17, 18 September 2004
r13917(bug 93) <nowiki> tags and tildes in templatesrobchurch02:20, 29 April 2006
r88997fix for Bug #93 “tilde signatures inside nowiki tags sometimes get expanded...mah20:55, 27 May 2011
r89308Revert r88997: fix for bug 93 caused some additional problems....brion00:32, 2 June 2011

Comments

#Comment by Brion VIBBER (talk | contribs)   17:35, 7 June 2011

Reverted in r89662; this disables PST on 'noinclude' sections in their entirety, rather than 'nowiki' sections within 'noinclude' as requested by the bug. A parser test case caught the regression but was changed to hide it...

Commit also failed to update all the preprocessor classes; there's also now Preprocessor_HipHop.hphp.

#Comment by P.Copp (talk | contribs)   21:50, 7 June 2011

Actually it disables PST on includeonly sections which was intentional. I mentioned it in the commit summary but perhaps I wasn't clear enough. I proposed this change on the linked bug in November 2009 and still think this would be the right approach.

At the moment many templates use hacks like 

~~<includeonly></includeonly>~~

to include signatures in substed templates without expanding them right on the spot. With the proposed change you could simply write 

<includeonly>~~~~</includeonly>

Please do also note, that unlike r88997 this change won't break any existing page, because it doesn't expand anything that wasn't expanded before. As for the parser tests: There was only one test, that had to be changed and it was only introduced in r89191.

So, if there are any objections to the proposed behavior I'd like to hear them :)

Status & tagging log