Fixes for ResourceLoaderWikiModule r72776. No serious bugs found, do not merge before...

* Specify page titles as strings instead of split NS/DBK, as suggested by Roan on CR. It seemed sensible to me.
* Pass a Title object to getContent() instead of a string, to avoid unnecessary object construction overhead
* "*" and "/" are valid title characters. Check module input for JS comment end tokens.
* Fixed inappropriate conversion to boolean, when checking result of getContent(). Presumably the idea was to omit empty sections and errors, so that's what I did. Maybe an informative error message would be better in the error case.
* Use LinkBatch for selecting multiple page rows instead of Database::makeWhereFrom2d().
* Fixed assignment expression.

diff --git a/includes/resourceloader/ResourceLoaderSiteModule.php b/includes/resourceloader/ResourceLoaderSiteModule.php
index 2336acb..977d16b 100644 (file)
--- a/includes/resourceloader/ResourceLoaderSiteModule.php
+++ b/includes/resourceloader/ResourceLoaderSiteModule.php
@@ -36,15 +36,14 @@ class ResourceLoaderSiteModule extends ResourceLoaderWikiModule {
                global $wgHandheldStyle;
 
                $pages = array(
-                       'Common.js' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'script' ),
-                       'Common.css' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'style' ),
-                       ucfirst( $context->getSkin() ) . '.js' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'script' ),
-                       ucfirst( $context->getSkin() ) . '.css' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'style' ),
-                       'Print.css' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'style', 'media' => 'print' ),
+                       'MediaWiki:Common.js' => array( 'type' => 'script' ),
+                       'MediaWiki:Common.css' => array( 'type' => 'style' ),
+                       'MediaWiki:' . ucfirst( $context->getSkin() ) . '.js' => array( 'type' => 'script' ),
+                       'MediaWiki:' . ucfirst( $context->getSkin() ) . '.css' => array( 'type' => 'style' ),
+                       'MediaWiki:Print.css' => array( 'type' => 'style', 'media' => 'print' ),
                );
                if ( $wgHandheldStyle ) {
-                       $pages['Handheld.css'] = array( 
-                               'ns' => NS_MEDIAWIKI, 
+                       $pages['MediaWiki:Handheld.css'] = array( 
                                'type' => 'style', 
                                'media' => 'handheld' );
                }

diff --git a/includes/resourceloader/ResourceLoaderUserModule.php b/includes/resourceloader/ResourceLoaderUserModule.php
index 4791a0f..f2413fd 100644 (file)
--- a/includes/resourceloader/ResourceLoaderUserModule.php
+++ b/includes/resourceloader/ResourceLoaderUserModule.php
@@ -32,12 +32,12 @@ class ResourceLoaderUserModule extends ResourceLoaderWikiModule {
                if ( $context->getUser() ) {
                        $username = $context->getUser();
                        return array(
-                               "$username/common.js" => array( 'ns' => NS_USER, 'type' => 'script' ),
-                               "$username/" . $context->getSkin() . '.js' => 
-                                       array( 'ns' => NS_USER, 'type' => 'script' ),
-                               "$username/common.css" => array( 'ns' => NS_USER, 'type' => 'style' ),
-                               "$username/" . $context->getSkin() . '.css' => 
-                                       array( 'ns' => NS_USER, 'type' => 'style' ),
+                               "User:$username/common.js" => array( 'type' => 'script' ),
+                               "User:$username/" . $context->getSkin() . '.js' => 
+                                       array( 'type' => 'script' ),
+                               "User:$username/common.css" => array( 'type' => 'style' ),
+                               "User:$username/" . $context->getSkin() . '.css' => 
+                                       array( 'type' => 'style' ),
                        );
                }
                return array();

diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php
index 0abd74f..d8cd79b 100644 (file)
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -45,12 +45,12 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
        
        /* Protected Methods */
        
-       protected function getContent( $page, $ns ) {
-               if ( $ns === NS_MEDIAWIKI ) {
-                       return wfEmptyMsg( $page ) ? '' : wfMsgExt( $page, 'content' );
+       protected function getContent( $title ) {
+               if ( $title->getNamespace() === NS_MEDIAWIKI ) {
+                       $dbkey = $title->getDBkey();
+                       return wfEmptyMsg( $dbkey ) ? '' : wfMsgExt( $dbkey, 'content' );
                }
-               $title = Title::newFromText( $page, $ns );
-               if ( !$title || !$title->isCssJsSubpage() ) {
+               if ( !$title->isCssJsSubpage() ) {
                        return null;
                }
                $revision = Revision::newFromTitle( $title );
@@ -64,14 +64,20 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
 
        public function getScript( ResourceLoaderContext $context ) {
                $scripts = '';
-               foreach ( $this->getPages( $context ) as $page => $options ) {
+               foreach ( $this->getPages( $context ) as $titleText => $options ) {
                        if ( $options['type'] !== 'script' ) {
                                continue;
                        }
-                       $script = $this->getContent( $page, $options['ns'] );
-                       if ( $script ) {
-                               $ns = MWNamespace::getCanonicalName( $options['ns'] );
-                               $scripts .= "/* $ns:$page */\n$script\n";
+                       $title = Title::newFromText( $titleText );
+                       if ( !$title ) {
+                               continue;
+                       }
+                       $script = $this->getContent( $title );
+                       if ( strval( $script ) !== '' ) {
+                               if ( strpos( $titleText, '*/' ) === false ) {
+                                       $scripts .=  "/* $titleText */\n";
+                               }
+                               $scripts .= $script . "\n";
                        }
                }
                return $scripts;
@@ -80,13 +86,17 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
        public function getStyles( ResourceLoaderContext $context ) {
                
                $styles = array();
-               foreach ( $this->getPages( $context ) as $page => $options ) {
+               foreach ( $this->getPages( $context ) as $titleText => $options ) {
                        if ( $options['type'] !== 'style' ) {
                                continue;
                        }
+                       $title = Title::newFromText( $titleText );
+                       if ( !$title ) {
+                               continue;
+                       }                       
                        $media = isset( $options['media'] ) ? $options['media'] : 'all';
-                       $style = $this->getContent( $page, $options['ns'] );
-                       if ( !$style ) {
+                       $style = $this->getContent( $title );
+                       if ( strval( $style ) === '' ) {
                                continue;
                        }
                        if ( $this->getFlip( $context ) ) {
@@ -95,8 +105,10 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
                        if ( !isset( $styles[$media] ) ) {
                                $styles[$media] = '';
                        }
-                       $ns = MWNamespace::getCanonicalName( $options['ns'] );
-                       $styles[$media] .= "/* $ns:$page */\n$style\n";
+                       if ( strpos( $titleText, '*/' ) === false ) {
+                               $styles[$media] .=  "/* $titleText */\n";
+                       }
+                       $styles[$media] .= $style . "\n";
                }
                return $styles;
        }
@@ -107,17 +119,16 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
                        return $this->modifiedTime[$hash];
                }
 
-               $titles = array();
-               foreach ( $this->getPages( $context ) as $page => $options ) {
-                       $titles[$options['ns']][$page] = true;
+               $batch = new LinkBatch;
+               foreach ( $this->getPages( $context ) as $titleText => $options ) {
+                       $batch->addObj( Title::newFromText( $titleText ) );
                }
 
                $modifiedTime = 1; // wfTimestamp() interprets 0 as "now"
-
-               if ( $titles ) {
+               if ( !$batch->isEmpty() ) {
                        $dbr = wfGetDB( DB_SLAVE );
                        $latest = $dbr->selectField( 'page', 'MAX(page_touched)',
-                               $dbr->makeWhereFrom2d( $titles, 'page_namespace', 'page_title' ),
+                               $batch->constructSet( 'page', $dbr ),
                                __METHOD__ );
 
                        if ( $latest ) {
@@ -125,6 +136,7 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
                        }
                }
 
-               return $this->modifiedTime[$hash] = $modifiedTime;
+               $this->modifiedTime[$hash] = $modifiedTime;
+               return $modifiedTime;
        }
 }