r95632 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r95631‎ | r95632 | r95633 >
Date:17:57, 28 August 2011
Author:hashar
Status:ok
Tags:
Comment:
MFT to REL1_18
r94277 (bug 30322) “SVG metadata is read incorrectly”
r94517 fix NS_MAIN subpage test
r94738 reverts r79892
r95023 installer dont recheck db availability
r95072 Factor out code duplication (introduce wfMatchesDomainList())
r95073 undefined var in r90849
r95327 (bug 30335) Fix for HTMLForms using GET with unfriendly URLs
-> manually added to 1.18 release notes
r95422 r95426 Various fixes for MWNamespaceTest.php
Modified paths:
  • /branches/REL1_18/phase3 (modified) (history)
  • /branches/REL1_18/phase3/RELEASE-NOTES-1.18 (modified) (history)
  • /branches/REL1_18/phase3/includes (modified) (history)
  • /branches/REL1_18/phase3/includes/GlobalFunctions.php (modified) (history)
  • /branches/REL1_18/phase3/includes/HTMLForm.php (modified) (history)
  • /branches/REL1_18/phase3/includes/OutputPage.php (modified) (history)
  • /branches/REL1_18/phase3/includes/Skin.php (modified) (history)
  • /branches/REL1_18/phase3/includes/installer/Installer.php (modified) (history)
  • /branches/REL1_18/phase3/includes/media/SVGMetadataExtractor.php (modified) (history)
  • /branches/REL1_18/phase3/includes/parser/Parser.php (modified) (history)
  • /branches/REL1_18/phase3/tests/phpunit/includes/MWNamespaceTest.php (modified) (history)

Diff [purge]

Index: branches/REL1_18/phase3/RELEASE-NOTES-1.18
——@@ -224,8 +224,6 @@
225225 * If an edit summary exceeds 250 bytes and is truncated, add an ellipse.
226226 * (bug 26638) Database error pages display correctly in RTL languages.
227227 * (bug 26187) Confirmrecreate no longer parses the edit summary.
228 -* (bug 25506) Exception is thrown if OutputPage::parse is called inside a tag
229 - hook, which would reset parser state.
230228 * (bug 26208) Mark directionality of some interlanguage links.
231229 * (bug 26034) Make the "View / Read" tab in content_navigation style tabs remain
232230 selected when the action is "purge".
——@@ -431,6 +429,8 @@
432430 * $wgSVGMaxSize is now applied to the smaller of width or height, making very wide pano/timeline/diagram SVGs renderable at saner sizes
433431 * (bug 30074) Moving user JS subpages resulted in JS errors because
434432 #REDIRECT [[Foo]] is invalid JS
 433+* (bug 30335) Fix for HTMLForms using GET breaking when non-friendly URLs
 434+ are used
435435
436436 === API changes in 1.18 ===
437437 * BREAKING CHANGE: action=watch now requires POST and token.
Property changes on: branches/REL1_18/phase3/RELEASE-NOTES-1.18
___________________________________________________________________
Modified: svn:mergeinfo
438438 Merged /trunk/phase3/RELEASE-NOTES-1.18:r94277,94517,94738,95023,95072-95073,95327,95422,95426,95601
Index: branches/REL1_18/phase3/tests/phpunit/includes/MWNamespaceTest.php
——@@ -12,16 +12,10 @@
1313 */
1414 class MWNamespaceTest extends MediaWikiTestCase {
1515 /**
16 - * @var MWNamespace
17 - */
18 - protected $object;
19 -
20 - /**
2116 * Sets up the fixture, for example, opens a network connection.
2217 * This method is called before a test is executed.
2318 */
2419 protected function setUp() {
25 - $this->object = new MWNamespace;
2620 }
2721
2822 /**
——@@ -87,27 +81,35 @@
8882 * the function testGetTalkExceptions()
8983 */
9084 public function testGetTalk() {
91 - $this->assertEquals( MWNamespace::getTalk( NS_MAIN), NS_TALK );
 85+ $this->assertEquals( NS_TALK, MWNamespace::getTalk( NS_MAIN ) );
9286 }
9387
9488 /**
9589 * Exceptions with getTalk()
96 - * NS_MEDIA and NS_SPECIAL do not have talk pages. MediaWiki raise an exception for them.
 90+ * NS_MEDIA does not have talk pages. MediaWiki raise an exception for them.
9791 * @expectedException MWException
9892 */
99 - public function testGetTalkExceptions() {
 93+ public function testGetTalkExceptionsForNsMedia() {
10094 $this->assertNull( MWNamespace::getTalk( NS_MEDIA ) );
 95+ }
 96+
 97+ /**
 98+ * Exceptions with getTalk()
 99+ * NS_SPECIAL does not have talk pages. MediaWiki raise an exception for them.
 100+ * @expectedException MWException
 101+ */
 102+ public function testGetTalkExceptionsForNsSpecial() {
101103 $this->assertNull( MWNamespace::getTalk( NS_SPECIAL ) );
102104 }
103105
104106 /**
105 - * Regular getAssociated() calls
 107+ * Regular getAssociated() calls
106108 * Namespaces without an associated page (NS_MEDIA, NS_SPECIAL) are tested in
107109 * the function testGetAssociatedExceptions()
108110 */
109111 public function testGetAssociated() {
110 - $this->assertEquals( MWNamespace::getAssociated( NS_MAIN ), NS_TALK );
111 - $this->assertEquals( MWNamespace::getAssociated( NS_TALK ), NS_MAIN );
 112+ $this->assertEquals( NS_TALK, MWNamespace::getAssociated( NS_MAIN ) );
 113+ $this->assertEquals( NS_MAIN, MWNamespace::getAssociated( NS_TALK ) );
112114
113115 }
114116
——@@ -120,6 +122,7 @@
121123 public function testGetAssociatedExceptionsForNsMedia() {
122124 $this->assertNull( MWNamespace::getAssociated( NS_MEDIA ) );
123125 }
 126+
124127 /**
125128 * @expectedException MWException
126129 */
——@@ -131,11 +134,11 @@
132135 */
133136 public function testGetSubject() {
134137 // Special namespaces are their own subjects
135 - $this->assertEquals( MWNamespace::getSubject( NS_MEDIA ), NS_MEDIA );
136 - $this->assertEquals( MWNamespace::getSubject( NS_SPECIAL ), NS_SPECIAL );
 138+ $this->assertEquals( NS_MEDIA, MWNamespace::getSubject( NS_MEDIA ) );
 139+ $this->assertEquals( NS_SPECIAL, MWNamespace::getSubject( NS_SPECIAL ) );
137140
138 - $this->assertEquals( MWNamespace::getSubject( NS_TALK ), NS_MAIN );
139 - $this->assertEquals( MWNamespace::getSubject( NS_USER_TALK ), NS_USER );
 141+ $this->assertEquals( NS_MAIN, MWNamespace::getSubject( NS_TALK ) );
 142+ $this->assertEquals( NS_USER, MWNamespace::getSubject( NS_USER_TALK ) );
140143 }
141144
142145 /**
——@@ -225,7 +228,7 @@
226229 * Similar to testIsContent() but alters the $wgContentNamespaces
227230 * global variable.
228231 */
229 - public function testIsContentWithAdditionsInWgContentNamespaces() {
 232+ public function testIsContentWithAdditionsInWgContentNamespaces() {
230233 // NS_MAIN is a content namespace per DefaultSettings.php
231234 // and per function definition.
232235 $this->assertTrue( MWNamespace::isContent( NS_MAIN ) );
——@@ -271,15 +274,26 @@
272275 $this->assertFalse( MWNamespace::hasSubpages( NS_SPECIAL ) );
273276
274277 // namespaces without subpages
 278+ # save up global
275279 global $wgNamespacesWithSubpages;
276 - if( array_key_exists( NS_MAIN, $wgNamespacesWithSubpages )
277 - && $wgNamespacesWithSubpages[NS_MAIN] === true
278 - ) {
279 - $this->markTestSkipped( "Main namespace has subpages enabled" );
280 - } else {
281 - $this->assertFalse( MWNamespace::hasSubpages( NS_MAIN ) );
 280+ $saved = null;
 281+ if( array_key_exists( NS_MAIN, $wgNamespacesWithSubpages ) ) {
 282+ $saved = $wgNamespacesWithSubpages[NS_MAIN];
 283+ unset( $wgNamespacesWithSubpages[NS_MAIN] );
282284 }
283285
 286+ $this->assertFalse( MWNamespace::hasSubpages( NS_MAIN ) );
 287+
 288+ $wgNamespacesWithSubpages[NS_MAIN] = true;
 289+ $this->assertTrue( MWNamespace::hasSubpages( NS_MAIN ) );
 290+ $wgNamespacesWithSubpages[NS_MAIN] = false;
 291+ $this->assertFalse( MWNamespace::hasSubpages( NS_MAIN ) );
 292+
 293+ # restore global
 294+ if( $saved !== null ) {
 295+ $wgNamespacesWithSubpages[NS_MAIN] = $saved;
 296+ }
 297+
284298 // Some namespaces with subpages
285299 $this->assertTrue( MWNamespace::hasSubpages( NS_TALK ) );
286300 $this->assertTrue( MWNamespace::hasSubpages( NS_USER ) );
——@@ -290,8 +304,8 @@
291305 */
292306 public function testGetContentNamespaces() {
293307 $this->assertEquals(
 308+ array( NS_MAIN ),
294309 MWNamespace::getcontentNamespaces(),
295 - array( NS_MAIN),
296310 '$wgContentNamespaces is an array with only NS_MAIN by default'
297311 );
298312
——@@ -299,37 +313,37 @@
300314
301315 # test !is_array( $wgcontentNamespaces )
302316 $wgContentNamespaces = '';
303 - $this->assertEquals( MWNamespace::getcontentNamespaces(), NS_MAIN );
 317+ $this->assertEquals( NS_MAIN, MWNamespace::getcontentNamespaces() );
304318 $wgContentNamespaces = false;
305 - $this->assertEquals( MWNamespace::getcontentNamespaces(), NS_MAIN );
 319+ $this->assertEquals( NS_MAIN, MWNamespace::getcontentNamespaces() );
306320 $wgContentNamespaces = null;
307 - $this->assertEquals( MWNamespace::getcontentNamespaces(), NS_MAIN );
 321+ $this->assertEquals( NS_MAIN, MWNamespace::getcontentNamespaces() );
308322 $wgContentNamespaces = 5;
309 - $this->assertEquals( MWNamespace::getcontentNamespaces(), NS_MAIN );
 323+ $this->assertEquals( NS_MAIN, MWNamespace::getcontentNamespaces() );
310324
311 - # test $wgContentNamespaces === array()
 325+ # test $wgContentNamespaces === array()
312326 $wgContentNamespaces = array();
313 - $this->assertEquals( MWNamespace::getcontentNamespaces(), NS_MAIN );
 327+ $this->assertEquals( NS_MAIN, MWNamespace::getcontentNamespaces() );
314328
315329 # test !in_array( NS_MAIN, $wgContentNamespaces )
316330 $wgContentNamespaces = array( NS_USER, NS_CATEGORY );
317331 $this->assertEquals(
318 - MWNamespace::getcontentNamespaces(),
319332 array( NS_MAIN, NS_USER, NS_CATEGORY ),
 333+ MWNamespace::getcontentNamespaces(),
320334 'NS_MAIN is forced in wgContentNamespaces even if unwanted'
321335 );
322336
323337 # test other cases, return $wgcontentNamespaces as is
324338 $wgContentNamespaces = array( NS_MAIN );
325339 $this->assertEquals(
326 - MWNamespace::getcontentNamespaces(),
327 - array( NS_MAIN )
 340+ array( NS_MAIN ),
 341+ MWNamespace::getcontentNamespaces()
328342 );
329343
330344 $wgContentNamespaces = array( NS_MAIN, NS_USER, NS_CATEGORY );
331345 $this->assertEquals(
332 - MWNamespace::getcontentNamespaces(),
333 - array( NS_MAIN, NS_USER, NS_CATEGORY )
 346+ array( NS_MAIN, NS_USER, NS_CATEGORY ),
 347+ MWNamespace::getcontentNamespaces()
334348 );
335349
336350 }
——@@ -440,7 +454,7 @@
441455 $this->assertFalse( MWNamespace::hasGenderDistinction( NS_SPECIAL ) );
442456 $this->assertFalse( MWNamespace::hasGenderDistinction( NS_MAIN ) );
443457 $this->assertFalse( MWNamespace::hasGenderDistinction( NS_TALK ) );
444 -
 458+
445459 }
446460 }
447461
Index: branches/REL1_18/phase3/includes/GlobalFunctions.php
——@@ -598,6 +598,26 @@
599599 }
600600
601601 /**
 602+ * Check whether a given URL has a domain that occurs in a given set of domains
 603+ * @param $url string URL
 604+ * @param $domains array Array of domains (strings)
 605+ * @return bool True if the host part of $url ends in one of the strings in $domains
 606+ */
 607+function wfMatchesDomainList( $url, $domains ) {
 608+ $bits = wfParseUrl( $url );
 609+ if ( is_array( $bits ) && isset( $bits['host'] ) ) {
 610+ foreach ( (array)$domains as $domain ) {
 611+ // FIXME: This gives false positives. http://nds-nl.wikipedia.org will match nl.wikipedia.org
 612+ // We should use something that interprets dots instead
 613+ if ( substr( $bits['host'], -strlen( $domain ) ) === $domain ) {
 614+ return true;
 615+ }
 616+ }
 617+ }
 618+ return false;
 619+}
 620+
 621+/**
602622 * Sends a line to the debug log if enabled or, optionally, to a comment in output.
603623 * In normal operation this is a NOP.
604624 *
Property changes on: branches/REL1_18/phase3/includes/GlobalFunctions.php
___________________________________________________________________
Modified: svn:mergeinfo
605625 Merged /trunk/phase3/includes/GlobalFunctions.php:r94277,94517,94738,95023,95072-95073,95327,95422,95426,95601
Index: branches/REL1_18/phase3/includes/parser/Parser.php
——@@ -1584,23 +1584,12 @@
15851585 */
15861586 function getExternalLinkAttribs( $url = false ) {
15871587 $attribs = array();
1588 - global $wgNoFollowLinks, $wgNoFollowNsExceptions;
 1588+ global $wgNoFollowLinks, $wgNoFollowNsExceptions, $wgNoFollowDomainExceptions;
15891589 $ns = $this->mTitle->getNamespace();
1590 - if ( $wgNoFollowLinks && !in_array( $ns, $wgNoFollowNsExceptions ) ) {
 1590+ if ( $wgNoFollowLinks && !in_array( $ns, $wgNoFollowNsExceptions ) &&
 1591+ !wfMatchesDomainList( $url, $wgNoFollowDomainExceptions ) )
 1592+ {
15911593 $attribs['rel'] = 'nofollow';
1592 -
1593 - global $wgNoFollowDomainExceptions;
1594 - if ( $wgNoFollowDomainExceptions ) {
1595 - $bits = wfParseUrl( $url );
1596 - if ( is_array( $bits ) && isset( $bits['host'] ) ) {
1597 - foreach ( $wgNoFollowDomainExceptions as $domain ) {
1598 - if ( substr( $bits['host'], -strlen( $domain ) ) == $domain ) {
1599 - unset( $attribs['rel'] );
1600 - break;
1601 - }
1602 - }
1603 - }
1604 - }
16051594 }
16061595 if ( $this->mOptions->getExternalLinkTarget() ) {
16071596 $attribs['target'] = $this->mOptions->getExternalLinkTarget();
Index: branches/REL1_18/phase3/includes/HTMLForm.php
——@@ -429,12 +429,18 @@
430430 * @return String HTML.
431431 */
432432 function getHiddenFields() {
 433+ global $wgUsePathInfo;
 434+
433435 $html = '';
434436 if( $this->getMethod() == 'post' ){
435437 $html .= Html::hidden( 'wpEditToken', $this->getUser()->editToken(), array( 'id' => 'wpEditToken' ) ) . "\n";
436438 $html .= Html::hidden( 'title', $this->getTitle()->getPrefixedText() ) . "\n";
437439 }
438440
 441+ if ( !$wgUsePathInfo && $this->getMethod() == 'get' ) {
 442+ $html .= Html::hidden( 'title', $this->getTitle()->getPrefixedText() ) . "\n";
 443+ }
 444+
439445 foreach ( $this->mHiddenFields as $data ) {
440446 list( $value, $attribs ) = $data;
441447 $html .= Html::hidden( $attribs['name'], $value, $attribs ) . "\n";
Index: branches/REL1_18/phase3/includes/OutputPage.php
——@@ -1500,13 +1500,6 @@
15011501 * @return String: HTML
15021502 */
15031503 public function parse( $text, $linestart = true, $interface = false, $language = null ) {
1504 - // Check one for one common cause for parser state resetting
1505 - $callers = wfGetAllCallers( 10 );
1506 - if ( strpos( $callers, 'Parser::extensionSubstitution' ) !== false ) {
1507 - throw new MWException( "wfMsg* function with parsing cannot be used " .
1508 - "inside a tag hook. Should use parser->recursiveTagParse() instead" );
1509 - }
1510 -
15111504 global $wgParser;
15121505
15131506 if( is_null( $this->getTitle() ) ) {
Property changes on: branches/REL1_18/phase3/includes/OutputPage.php
___________________________________________________________________
Modified: svn:mergeinfo
15141507 Merged /trunk/phase3/includes/OutputPage.php:r94277,94517,94738,95023,95072-95073,95327,95422,95426,95601
Index: branches/REL1_18/phase3/includes/installer/Installer.php
——@@ -332,12 +332,14 @@
333333 $this->settings[$var] = $GLOBALS[$var];
334334 }
335335
 336+ $compiledDBs = array();
336337 foreach ( self::getDBTypes() as $type ) {
337338 $installer = $this->getDBInstaller( $type );
338339
339340 if ( !$installer->isCompiled() ) {
340341 continue;
341342 }
 343+ $compiledDBs[] = $type;
342344
343345 $defaults = $installer->getGlobalDefaults();
344346
——@@ -349,6 +351,7 @@
350352 }
351353 }
352354 }
 355+ $this->setVar( '_CompiledDBs', $compiledDBs );
353356
354357 $this->parserTitle = Title::newFromText( 'Installer' );
355358 $this->parserOptions = new ParserOptions; // language will be wrong :(
——@@ -621,19 +624,13 @@
622625 protected function envCheckDB() {
623626 global $wgLang;
624627
625 - $compiledDBs = array();
626628 $allNames = array();
627629
628630 foreach ( self::getDBTypes() as $name ) {
629 - if ( $this->getDBInstaller( $name )->isCompiled() ) {
630 - $compiledDBs[] = $name;
631 - }
632 - $allNames[] = wfMsg( 'config-type-' . $name );
 631+ $allNames[] = wfMsg( "config-type-$name" );
633632 }
634633
635 - $this->setVar( '_CompiledDBs', $compiledDBs );
636 -
637 - if ( !$compiledDBs ) {
 634+ if ( !$this->getVar( '_CompiledDBs' ) ) {
638635 $this->showError( 'config-no-db', $wgLang->commaList( $allNames ) );
639636 // @todo FIXME: This only works for the web installer!
640637 return false;
Index: branches/REL1_18/phase3/includes/media/SVGMetadataExtractor.php
——@@ -55,7 +55,7 @@
5656 $size = filesize( $source );
5757 if ( $size === false ) {
5858 throw new MWException( "Error getting filesize of SVG." );
59 - }
 59+ }
6060
6161 if ( $size > $wgSVGMetadataCutoff ) {
6262 $this->debug( "SVG is $size bytes, which is bigger than $wgSVGMetadataCutoff. Truncating." );
——@@ -157,7 +157,7 @@
158158 }
159159 $keepReading = $this->reader->read();
160160 while( $keepReading ) {
161 - if( $this->reader->localName == $name && $this->namespaceURI == self::NS_SVG && $this->reader->nodeType == XmlReader::END_ELEMENT ) {
 161+ if( $this->reader->localName == $name && $this->reader->namespaceURI == self::NS_SVG && $this->reader->nodeType == XmlReader::END_ELEMENT ) {
162162 break;
163163 } elseif( $this->reader->nodeType == XmlReader::TEXT ){
164164 $this->metadata[$metafield] = trim( $this->reader->value );
Index: branches/REL1_18/phase3/includes/Skin.php
——@@ -1255,24 +1255,13 @@
12561256
12571257 if ( preg_match( '/^(?:' . wfUrlProtocols() . ')/', $link ) ) {
12581258 $href = $link;
1259 - //Parser::getExternalLinkAttribs won't work here because of the Namespace things
1260 - global $wgNoFollowLinks;
1261 - if ( $wgNoFollowLinks ) {
 1259+
 1260+ // Parser::getExternalLinkAttribs won't work here because of the Namespace things
 1261+ global $wgNoFollowLinks, $wgNoFollowDomainExceptions;
 1262+ if ( $wgNoFollowLinks && !wfMatchesDomainList( $href, $wgNoFollowDomainExceptions ) ) {
12621263 $extraAttribs['rel'] = 'nofollow';
1263 -
1264 - global $wgNoFollowDomainExceptions;
1265 - if ( $wgNoFollowDomainExceptions ) {
1266 - $bits = wfParseUrl( $url );
1267 - if ( is_array( $bits ) && isset( $bits['host'] ) ) {
1268 - foreach ( $wgNoFollowDomainExceptions as $domain ) {
1269 - if ( substr( $bits['host'], -strlen( $domain ) ) == $domain ) {
1270 - unset( $extraAttribs['rel'] );
1271 - break;
1272 - }
1273 - }
1274 - }
1275 - }
12761264 }
 1265+
12771266 global $wgExternalLinkTarget;
12781267 if ( $wgExternalLinkTarget) {
12791268 $extraAttribs['target'] = $wgExternalLinkTarget;
Property changes on: branches/REL1_18/phase3/includes
___________________________________________________________________
Modified: svn:mergeinfo
12801269 Merged /trunk/phase3/includes:r94277,94517,94738,95023,95072-95073,95327,95422,95426,95601
Property changes on: branches/REL1_18/phase3
___________________________________________________________________
Modified: svn:mergeinfo
12811270 Merged /trunk/phase3:r94277,94517,94738,95023,95072-95073,95327,95422,95426,95601

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r79892* (bug 25506) Exception is thrown if OutputPage::parse is called inside a tag...nikerabbit12:46, 9 January 2011
r90849Apply rgcjones' patch for Bug#29533:...mah19:52, 26 June 2011
r94277Fix Bug #30322 “SVG metadata is read incorrectly” by applying supplied patchmah19:51, 11 August 2011
r94517Fix up NS_MAIN subpage tests...hashar14:17, 15 August 2011
r94738Self-revert r79892: only got one opposition and apparently too hairy for anyo...nikerabbit09:55, 17 August 2011
r95023Don't check for each database's availability twice. This halves the number of...maxsem18:27, 19 August 2011
r95072Per r90849, factor out most of the code that's duplicated between Parser::get...catrope10:18, 20 August 2011
r95073Fix serious bug in r90849 that didn't show up in the tests until I restructur...catrope10:25, 20 August 2011
r95327(bug 30335) Fix for HTMLForms using GET breaking when non-friendly URLs are usedjohnduhart19:56, 23 August 2011
r95422Followup r82577...reedy19:32, 24 August 2011
r95426Fix testGetTalkExceptions from r82577reedy19:52, 24 August 2011

Status & tagging log