Per bug 28738 comment 4, pack ResourceLoader URLs by encoding foo.bar|foo.baz|bar...
authorRoan Kattouw <catrope@users.mediawiki.org>
Thu, 5 May 2011 13:46:47 +0000 (13:46 +0000)
committerRoan Kattouw <catrope@users.mediawiki.org>
Thu, 5 May 2011 13:46:47 +0000 (13:46 +0000)
* Expand these URLs in ResourceLoaderContext
* Build and emit these URLs in OutputPage::makeResourceLoaderLink() and in mw.loader
* Throw an exception in ResourceLoader::register() for module names that contain pipe characters or commas. Commas need to be forbidden for this packing feature to work. Pipes were already forbidden but weren't checked for

includes/OutputPage.php
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderContext.php
resources/mediawiki/mediawiki.js

index 9385c1f..2f8d85b 100644 (file)
@@ -2396,7 +2396,6 @@ class OutputPage {
                        $wgResourceLoaderInlinePrivateModules;
                // Lazy-load ResourceLoader
                // TODO: Should this be a static function of ResourceLoader instead?
-               // TODO: Divide off modules starting with "user", and add the user parameter to them
                $baseQuery = array(
                        'lang' => $this->getContext()->getLang()->getCode(),
                        'debug' => ResourceLoader::inDebugMode() ? 'true' : 'false',
@@ -2475,7 +2474,7 @@ class OutputPage {
                                continue;
                        }
 
-                       $query['modules'] = implode( '|', array_keys( $modules ) );
+                       $query['modules'] = ResourceLoader::makePackedModulesString( array_keys( $modules ) );
 
                        // Support inlining of private modules if configured as such
                        if ( $group === 'private' && $wgResourceLoaderInlinePrivateModules ) {
index 18a0504..3afe69c 100644 (file)
@@ -199,6 +199,7 @@ class ResourceLoader {
         *   this may also be a ResourceLoaderModule object. Optional when using 
         *   multiple-registration calling style.
         * @throws MWException: If a duplicate module registration is attempted
+        * @throws MWException: If a module name contains illegal characters (pipes or commas)
         * @throws MWException: If something other than a ResourceLoaderModule is being registered
         * @return Boolean: False if there were any errors, in which case one or more modules were not
         *     registered
@@ -223,6 +224,11 @@ class ResourceLoader {
                                'Another module has already been registered as ' . $name
                        );
                }
+               
+               // Check $name for illegal characters
+               if ( preg_match( '/[|,]/', $name ) ) {
+                       throw new MWException( "ResourceLoader module name '$name' is invalid. Names may not contain pipes (|) or commas (,)" );
+               }
 
                // Attach module
                if ( is_object( $info ) ) {
@@ -698,6 +704,31 @@ class ResourceLoader {
                return Xml::encodeJsCall( 'mediaWiki.config.set', array( $configuration ) );
        }
        
+       /**
+        * Convert an array of module names to a packed query string.
+        * 
+        * For example, array( 'foo.bar', 'foo.baz', 'bar.baz', 'bar.quux' )
+        * becomes 'foo.bar,baz|bar.baz,quux'
+        * @param $modules array of module names (strings)
+        * @return string Packed query string
+        */
+       public static function makePackedModulesString( $modules ) {
+               $groups = array(); // array( prefix => array( suffixes ) )
+               foreach ( $modules as $module ) {
+                       $pos = strrpos( $module, '.' );
+                       $prefix = $pos === false ? '' : substr( $module, 0, $pos );
+                       $suffix = $pos === false ? $module : substr( $module, $pos + 1 );
+                       $groups[$prefix][] = $suffix;
+               }
+               
+               $arr = array();
+               foreach ( $groups as $prefix => $suffixes ) {
+                       $p = $prefix === '' ? '' : $prefix . '.';
+                       $arr[] = $p . implode( ',', $suffixes );
+               }
+               return implode( '|', $arr );
+       }
+       
        /**
         * Determine whether debug mode was requested
         * Order of priority is 1) request param, 2) cookie, 3) $wg setting
index b40c3a9..5600304 100644 (file)
@@ -51,7 +51,7 @@ class ResourceLoaderContext {
                // Interpret request
                // List of modules
                $modules = $request->getVal( 'modules' );
-               $this->modules   = $modules ? explode( '|', $modules ) : array();
+               $this->modules   = $modules ? self::expandModuleNames( $modules ) : array();
                // Various parameters
                $this->skin      = $request->getVal( 'skin' );
                $this->user      = $request->getVal( 'user' );
@@ -63,6 +63,34 @@ class ResourceLoaderContext {
                        $this->skin = $wgDefaultSkin;
                }
        }
+       
+       /**
+        * Expand a string of the form jquery.foo,bar|jquery.ui.baz,quux to
+        * an array of module names like array( 'jquery.foo', 'jquery.bar',
+        * 'jquery.ui.baz', 'jquery.ui.quux' )
+        * @param $modules String Packed module name list
+        * @return array of module names
+        */
+       public static function expandModuleNames( $modules ) {
+               $retval = array();
+               $exploded = explode( '|', $modules );
+               foreach ( $exploded as $group ) {
+                       if ( strpos( $group, ',' ) === false ) {
+                               // This is not a set of modules in foo.bar,baz notation
+                               // but a single module
+                               $retval[] = $group;
+                       } else {
+                               // This is a set of modules in foo.bar,baz notation
+                               $pos = strrpos( $group, '.' );
+                               $prefix = substr( $group, 0, $pos ); // 'foo'
+                               $suffixes = explode( ',', substr( $group, $pos + 1 ) ); // array( 'bar', 'baz' )
+                               foreach ( $suffixes as $suffix ) {
+                                       $retval[] = "$prefix.$suffix";
+                               }
+                       }
+               }
+               return $retval;
+       }
 
        public function getResourceLoader() {
                return $this->resourceLoader;
index fe27fba..4630b76 100644 (file)
@@ -863,6 +863,20 @@ window.mediaWiki = new ( function( $ ) {
                        }
                        return sorted;
                }
+               
+               /**
+                * Converts a module map of the form { foo: [ 'bar', 'baz' ], bar: [ 'baz, 'quux' ] }
+                * to a query string of the form foo.bar,baz|bar.baz,quux
+                */
+               function buildModulesString( moduleMap ) {
+                       var arr = [];
+                       for ( var prefix in moduleMap ) {
+                               var p = prefix === '' ? '' : prefix + '.';
+                               arr.push( p + moduleMap[prefix].join( ',' ) );
+                       }
+                       return arr.join( '|' );
+               }
+               
 
                /* Public Methods */
 
@@ -920,32 +934,38 @@ window.mediaWiki = new ( function( $ ) {
                                        var reqBaseLength = $.param( reqBase ).length;
                                        var reqs = [];
                                        var limit = mw.config.get( 'wgResourceLoaderMaxQueryLength', -1 );
-                                       if ( limit > 0 ) {
-                                               // We may need to split up the request to honor the query string length limit
-                                               // So build it piece by piece
-                                               var l = reqBaseLength + 9; // '&modules='.length == 9
-                                               var r = 0;
-                                               reqs[0] = [];
-                                               for ( var i = 0; i < groups[group].length; i++ ) {
-                                                       // If the request would become too long, create a new one,
-                                                       // but don't create empty requests
-                                                       // '%7C'.length == 3
-                                                       if ( reqs[r].length > 0 && l + 3 + groups[group][i].length > limit ) {
-                                                               // This request would become too long, create a new one
-                                                               r++;
-                                                               reqs[r] = [];
-                                                               l = reqBaseLength + 9;
-                                                       }
-                                                       reqs[r][reqs[r].length] = groups[group][i];
-                                                       l += groups[group][i].length + 3;
+                                       // We may need to split up the request to honor the query string length limit
+                                       // So build it piece by piece
+                                       var l = reqBaseLength + 9; // '&modules='.length == 9
+                                       var r = 0;
+                                       reqs[0] = {}; // { prefix: [ suffixes ] }
+                                       for ( var i = 0; i < groups[group].length; i++ ) {
+                                               // Determine how many bytes this module would add to the query string
+                                               var lastDotIndex = groups[group][i].lastIndexOf( '.' );
+                                               // Note that these substr() calls work even if lastDotIndex == -1
+                                               var prefix = groups[group][i].substr( 0, lastDotIndex );
+                                               var suffix = groups[group][i].substr( lastDotIndex + 1 );
+                                               var bytesAdded = prefix in reqs[r] ?
+                                                       suffix.length + 3 : // '%2C'.length == 3
+                                                       groups[group][i].length + 3; // '%7C'.length == 3
+                                               
+                                               // If the request would become too long, create a new one,
+                                               // but don't create empty requests
+                                               if ( limit > 0 &&  reqs[r] != {} && l + bytesAdded > limit ) {
+                                                       // This request would become too long, create a new one
+                                                       r++;
+                                                       reqs[r] = {};
+                                                       l = reqBaseLength + 9;
                                                }
-                                       } else {
-                                               // No splitting needed
-                                               reqs = [ groups[group] ];
+                                               if ( !( prefix in reqs[r] ) ) {
+                                                       reqs[r][prefix] = [];
+                                               }
+                                               reqs[r][prefix].push( suffix );
+                                               l += bytesAdded;
                                        }
                                        for ( var r = 0; r < reqs.length; r++ ) {
                                                requests[requests.length] = $.extend(
-                                                       { 'modules': reqs[r].join( '|' ) }, reqBase
+                                                       { 'modules': buildModulesString( reqs[r] ) }, reqBase
                                                );
                                        }
                                }