r98422 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r98421‎ | r98422 | r98423 >
Date:17:51, 29 September 2011
Author:mah
Status:ok
Tags:
Comment:
Attempted fix for Bug #26881: noinclude tag breaks Proofread under
Internet Explorer

From Philippe Elie's comments explaining this patch
(https://bugzilla.wikimedia.org/26881#c10)

re3 = /^([\s\S]*?)<noinclude>([\s\S]*?)<\/noinclude>/;

here there is two competing non-greedy group, I think FF is right
by making the first one maximal, it's the same case as greedy, if
there is choice for multiple ways to do a match, the match at the
same level of greediness must be maximal from left to right. I
guess this is why the comment talk about lookahead, because with
non-greedy competition, match must be moved from right to left to
implement that. The trouble is that Thomas didn't think the same
things can occur with re3 if the page contains more than two
<noinclude></noinclude> sequence. More boring re3 miss a final $
so only a part of the remaining part is matched and some data are
lost.

This works on Opera, by making both group greedy:

re3 = /^([\s\S]*)<noinclude>([\s\S]*)<\/noinclude>\s*$/;

I included too a \s*$ at the end to ensure if we match, we match
the whole data, this way if for some reason the match fail, we
will go to the

if (m3) { …
} else {
pageBody = m2[2];
pageFooter = '';
}

which ensure than no data can be lost (re2 is terminated by a $).
Modified paths:
  • /trunk/extensions/ProofreadPage/proofread.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ProofreadPage/proofread.js
@@ -42,7 +42,7 @@
4343 pageHeader = m2[1];
4444 // apparently lookahead is not supported by all browsers
4545 // so let us do another regexp
46 - re3 = /^([\s\S]*?)<noinclude>([\s\S]*?)<\/noinclude>/;
 46+ re3 = /^([\s\S]*)<noinclude>([\s\S]*)<\/noinclude>\s*$/;
4747 m3 = m2[2].match( re3 );
4848 if( m3 ) {
4949 pageBody = m3[1];

Status & tagging log