r65967 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r65966‎ | r65967 | r65968 >
Date:21:58, 5 May 2010
Author:platonides
Status:ok (Comments)
Tags:
Comment:
Recovered parser test whose content got broken in r12975, functionality broken in r14635.

Changed what used to be EXT_LINK_TEXT_CLASS to
define( 'EXT_LINK_TEXT_CLASS', '[^\]\\x00-\\x08\\x0a-\\x1F]' );
In plain English, do not allow control characters in description. We still allow tabs
there, since they may arrive from a paste.

We should probably make the space between the text and the description mandatory, it needs testing.

These tests only work with the Hash Preprocessor, since the DOM Preprocessor changes them into the
replacement character. Should we autolink an URL with a FFFD? That will require changing some regex
into unicode ones.
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/maintenance/ExtraParserTests.txt (added) (history)
  • /trunk/phase3/maintenance/parserTests.inc (modified) (history)
  • /trunk/phase3/maintenance/parserTests.txt (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/parserTests.inc
@@ -304,10 +304,10 @@
305305 /**
306306 * Get a Parser object
307307 */
308 - function getParser() {
 308+ function getParser($preprocessor = null) {
309309 global $wgParserConf;
310310 $class = $wgParserConf['class'];
311 - $parser = new $class( $wgParserConf );
 311+ $parser = new $class( array( 'preprocessorClass'=>$preprocessor ) + $wgParserConf );
312312 foreach( $this->hooks as $tag => $callback ) {
313313 $parser->setHook( $tag, $callback );
314314 }
@@ -352,7 +352,8 @@
353353
354354 $noxml = isset( $opts['noxml'] );
355355 $local = isset( $opts['local'] );
356 - $parser = $this->getParser();
 356+ $preprocessor = @$opts['preprocessor'];
 357+ $parser = $this->getParser( $preprocessor );
357358 $title = Title::newFromText( $titleText );
358359
359360 $matches = array();
Index: trunk/phase3/maintenance/parserTests.txt
@@ -865,18 +865,6 @@
866866 !!end
867867
868868 !! test
869 -External links: invalid character
870 -Fixme: the missing char seems to have gone missing
871 -!! options
872 -disabled
873 -!! input
874 -[http://www.example.com test]
875 -!! result
876 -<p>[<a href="http://www.example.com" class="external free" rel="nofollow">http://www.example.com</a> test]
877 -</p>
878 -!! end
879 -
880 -!! test
881869 External links: multiple legal whitespace is fine, Magnus. Don't break it please. (bug 5081)
882870 !! input
883871 [http://www.example.com test]
Index: trunk/phase3/maintenance/ExtraParserTests.txt
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/phase3/maintenance/ExtraParserTests.txt
___________________________________________________________________
Name: svn:mime-type
884872 + application/octet-stream
Index: trunk/phase3/includes/parser/Parser.php
@@ -129,7 +129,7 @@
130130 $this->mDefaultStripList = $this->mStripList = array();
131131 $this->mUrlProtocols = wfUrlProtocols();
132132 $this->mExtLinkBracketedRegex = '/\[(\b(' . wfUrlProtocols() . ')'.
133 - '[^][<>"\\x00-\\x20\\x7F]+) *([^\]\\x0a\\x0d]*?)\]/S';
 133+ '[^][<>"\\x00-\\x20\\x7F]+) *([^\]\\x00-\\x08\\x0a-\\x1F]*?)\]/S';
134134 $this->mVarCache = array();
135135 if ( isset( $conf['preprocessorClass'] ) ) {
136136 $this->mPreprocessorClass = $conf['preprocessorClass'];
Index: trunk/phase3/includes/DefaultSettings.php
@@ -4015,6 +4015,7 @@
40164016 */
40174017 $wgParserTestFiles = array(
40184018 "$IP/maintenance/parserTests.txt",
 4019+ "$IP/maintenance/ExtraParserTests.txt"
40194020 );
40204021
40214022 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r78358Fixed typos in file names from r65967tstarling08:14, 14 December 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r12975Fix for parser test error "Image link to nonexistent file (bug 1850)", both "...magnus_manske17:50, 7 February 2006
r14635* (bug 6230) Regression fix: <nowiki> in [URL link text]brion09:18, 7 June 2006

Comments

#Comment by MarkAHershberger (talk | contribs)   23:57, 5 May 2010

Should avoid the use of @ to hide warnings. Fixed in r65973 (but I forgot to put that in the log msg).

#Comment by Tim Starling (talk | contribs)   08:13, 14 December 2010

The space is not mandatory to support autonumbered links, like [1]. Obviously that could be done another way. But I think the main priority for maintenance to replaceExternalLinks() should be to replace the preg_split() with a preg_match() loop, since preg_split() will use excessive amounts of memory.

Status & tagging log