r112285 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r112284‎ | r112285 | r112286 >
Date:01:17, 24 February 2012
Author:awjrichards
Status:ok (Comments)
Tags:
Comment:
Added support and corresponding test for creating mobile URLs where the mobile-ness of the URL is denoted in its path. This just adds the method to do this - it is not implemented elsewhere in the MobileFrontend code.
Modified paths:
  • /trunk/extensions/MobileFrontend/MobileFrontend.body.php (modified) (history)
  • /trunk/extensions/MobileFrontend/tests/MobileFrontendTest.php (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/tests/MobileFrontendTest.php
——@@ -85,4 +85,21 @@
8686 $updateMobileUrlHost->invokeArgs( $wgExtMobileFrontend, array( &$parsedUrl ) );
8787 $this->assertEquals( "http://en.m.wikipedia.org/wiki/Gustavus_Airport", wfAssembleUrl( $parsedUrl ) );
8888 }
 89+
 90+ public function testUpdateMobileUrlPath() {
 91+ global $wgMobileUrlTemplate, $wgExtMobileFrontend, $wgScriptPath;
 92+ $wgScriptPath = '/wiki';
 93+ $updateMobileUrlHost = self::getMethod( "updateMobileUrlPath" );
 94+ $wgMobileUrlTemplate = "/mobile/%p";
 95+
 96+ // check for constructing a templated URL
 97+ $parsedUrl = wfParseUrl( "http://en.wikipedia.org/wiki/Gustavus_Airport" );
 98+ $updateMobileUrlHost->invokeArgs( $wgExtMobileFrontend, array( &$parsedUrl ) );
 99+ $this->assertEquals( "http://en.wikipedia.org/wiki/mobile/Gustavus_Airport", wfAssembleUrl( $parsedUrl ) );
 100+
 101+ // check for maintaining an already templated URL
 102+ $parsedUrl = wfParseUrl( "http://en.wikipedia.org/wiki/mobile/Gustavus_Airport" );
 103+ $updateMobileUrlHost->invokeArgs( $wgExtMobileFrontend, array( &$parsedUrl ) );
 104+ $this->assertEquals( "http://en.wikipedia.org/wiki/mobile/Gustavus_Airport", wfAssembleUrl( $parsedUrl ) );
 105+ }
89106 }
Index: trunk/extensions/MobileFrontend/MobileFrontend.body.php
——@@ -1329,7 +1329,7 @@
13301330 global $wgMobileUrlTemplate;
13311331 $parsedUrl = wfParseUrl( $url );
13321332 $this->updateMobileUrlHost( $parsedUrl );
1333 - $this->updateMobileUrlPath( $parsedUrl );
 1333+ $this->updateMobileUrlPath( $parsedUrl );
13341334 return wfAssembleUrl( $parsedUrl );
13351335 }
13361336
——@@ -1373,11 +1373,24 @@
13741374 * Result of parseUrl() or wfParseUrl()
13751375 */
13761376 protected function updateMobileUrlPath( &$parsedUrl ) {
1377 - $mobileUrlHostTemplate = $this->parseMobileUrlTemplate( 'path' );
1378 - if ( !strlen( $mobileUrlHostTemplate ) ) {
 1377+ global $wgScriptPath;
 1378+
 1379+ $mobileUrlPathTemplate = $this->parseMobileUrlTemplate( 'path' );
 1380+ if ( !strlen( $mobileUrlPathTemplate ) ) {
13791381 return;
13801382 }
1381 - return;
 1383+
 1384+ // find out if we already have a templated path
 1385+ $templatePathOffset = strpos( $mobileUrlPathTemplate, '%p' );
 1386+ $templatePathSansToken = substr( $mobileUrlPathTemplate, 0, $templatePathOffset );
 1387+ if ( substr_compare( $parsedUrl[ 'path' ], $wgScriptPath . $templatePathSansToken, 0 ) > 0 ) {
 1388+ return;
 1389+ }
 1390+
 1391+ $scriptPathLength = strlen( $wgScriptPath );
 1392+ // the "+ 1" removes the preceding "/" from the path sans $wgScriptPath
 1393+ $pathSansScriptPath = substr( $parsedUrl[ 'path' ], $scriptPathLength + 1 );
 1394+ $parsedUrl[ 'path' ] = $wgScriptPath . $templatePathSansToken . $pathSansScriptPath;
13821395 }
13831396
13841397 /**

Comments

#Comment by MaxSem (talk | contribs)   16:54, 24 February 2012

Looks ok except for non-standard spacing like $parsedUrl[ 'path' ].

#Comment by Awjrichards (talk | contribs)   18:24, 24 February 2012

Since there's no mention of how array keys should be spaced in the coding conventions for Mediawiki, I've always taken a liberal interpretation of "MediaWiki favors a heavily-spaced style for optimum readability". My personal preference is for spaces around the square bracket and array key for easier readability. Both $arr[ 'key' ] and $arr['key'] are used throughout MW core.

Status & tagging log