resourceloader: Implement support for 'site' into mw.loader
authorTimo Tijhof <krinklemail@gmail.com>
Fri, 24 Jul 2015 04:00:39 +0000 (21:00 -0700)
committerOri.livneh <ori@wikimedia.org>
Wed, 29 Jul 2015 20:12:01 +0000 (20:12 +0000)
* No longer a dedicated <script> with only=scripts.
  This means it creates no extra script request and becomes a versioned
  request using data from the startup module.

* No longer in group=site.
  This means it collapses into the existing bottom queue.
  Not even one dedicated script request, but zero.

* No longer exclude from module storage. This can be cached like any other module.
  It was previously excluded because it was already loaded separately.

* Change mw.loader#execute to special-case the 'site' module with $.globalEval.

* Add hack to ensure the styles of the 'site' module still load without
  JavaScript, in the top, and after the ResourceLoaderDynamicStyles marker.
  This unfortunately stays its own request. Not sure how to avoid that.

Bug: T32358
Bug: T106736
Bug: T102077
Change-Id: I291a8c3aae1a71760bec58161891c1bd77c9b724

includes/OutputPage.php
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderSiteModule.php
resources/src/mediawiki/mediawiki.js

index a551fe1..1c76f0b 100644 (file)
@@ -2286,9 +2286,10 @@ class OutputPage extends ContextSource {
                        // add skin specific modules
                        $modules = $sk->getDefaultModules();
 
-                       // enforce various default modules for all skins
+                       // Enforce various default modules for all skins
                        $coreModules = array(
-                               // keep this list as small as possible
+                               // Keep this list as small as possible
+                               'site',
                                'mediawiki.page.startup',
                                'mediawiki.user',
                        );
@@ -3004,7 +3005,8 @@ class OutputPage extends ContextSource {
                // Separate user.tokens as otherwise caching will be allowed (T84960)
                $links[] = $this->makeResourceLoaderLink( 'user.tokens', ResourceLoaderModule::TYPE_COMBINED );
 
-               // Scripts and messages "only" requests marked for top inclusion
+               // "Scripts only" modules marked for top inclusion
+               $styleModules = $this->getModuleScripts( true, 'top' );
                $links[] = $this->makeResourceLoaderLink(
                        $this->getModuleScripts( true, 'top' ),
                        ResourceLoaderModule::TYPE_SCRIPTS
@@ -3064,11 +3066,6 @@ class OutputPage extends ContextSource {
                // Legacy Scripts
                $links[] = "\n" . $this->mScripts;
 
-               // Add site JS if enabled
-               $links[] = $this->makeResourceLoaderLink( 'site', ResourceLoaderModule::TYPE_SCRIPTS,
-                       /* $useESI = */ false, /* $extraQuery = */ array(), /* $loadCall = */ $inHead
-               );
-
                // Add user JS if enabled
                if ( $this->getConfig()->get( 'AllowUserJs' )
                        && $this->getUser()->isLoggedIn()
@@ -3683,9 +3680,17 @@ class OutputPage extends ContextSource {
                        if ( !$module ) {
                                continue;
                        }
+                       if ( $name === 'site' ) {
+                               // HACK: The site module shouldn't be fragmented with a cache group and
+                               // http request. But in order to ensure its styles are separated and after the
+                               // ResourceLoaderDynamicStyles marker, pretend it is in a group called 'site'.
+                               // The scripts remain ungrouped and rides the bottom queue.
+                               $styles['site'][] = $name;
+                               continue;
+                       }
                        $group = $module->getGroup();
-                       // Modules in groups different than the ones listed on top (see $styles assignment)
-                       // will be placed in the "other" group
+                       // Modules in groups other than the ones needing special treatment (see $styles assignment)
+                       // will be placed in the "other" style category.
                        $styles[isset( $styles[$group] ) ? $group : 'other'][] = $name;
                }
 
@@ -3704,7 +3709,7 @@ class OutputPage extends ContextSource {
                        array( 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' )
                ) . "\n";
 
-               // Add site, private and user styles
+               // Add site-specific and user-specific styles
                // 'private' at present only contains user.options, so put that before 'user'
                // Any future private modules will likely have a similar user-specific character
                foreach ( array( 'site', 'noscript', 'private', 'user' ) as $group ) {
index bf34c22..305b197 100644 (file)
@@ -1105,7 +1105,11 @@ MESSAGE;
                $name, $scripts, $styles, $messages, $templates
        ) {
                if ( is_string( $scripts ) ) {
-                       $scripts = new XmlJsCode( "function ( $, jQuery ) {\n{$scripts}\n}" );
+                       // Site module is a legacy script that runs in the global scope (no closure).
+                       // Transportation as string instructs mw.loader.implement to use globalEval.
+                       if ( $name !== 'site' ) {
+                               $scripts = new XmlJsCode( "function ( $, jQuery ) {\n{$scripts}\n}" );
+                       }
                } elseif ( !is_array( $scripts ) ) {
                        throw new MWException( 'Invalid scripts error. Array of URLs or string of code expected.' );
                }
index 19e0bae..380b7a5 100644 (file)
@@ -47,13 +47,4 @@ class ResourceLoaderSiteModule extends ResourceLoaderWikiModule {
                }
                return $pages;
        }
-
-       /**
-        * Get group name
-        *
-        * @return string
-        */
-       public function getGroup() {
-               return 'site';
-       }
 }
index c0b1642..700513e 100644 (file)
 
                                if ( !hasOwn.call( registry, module ) ) {
                                        throw new Error( 'Module has not been registered yet: ' + module );
-                               } else if ( registry[module].state === 'registered' ) {
+                               }
+                               if ( registry[module].state === 'registered' ) {
                                        throw new Error( 'Module has not been requested from the server yet: ' + module );
-                               } else if ( registry[module].state === 'loading' ) {
+                               }
+                               if ( registry[module].state === 'loading' ) {
                                        throw new Error( 'Module has not completed loading yet: ' + module );
-                               } else if ( registry[module].state === 'ready' ) {
+                               }
+                               if ( registry[module].state === 'ready' ) {
                                        throw new Error( 'Module has already been executed: ' + module );
                                }
 
                                                if ( $.isArray( script ) ) {
                                                        nestedAddScript( script, markModuleReady, registry[module].async, 0 );
                                                } else if ( $.isFunction( script ) ) {
-                                                       registry[module].state = 'ready';
                                                        // Pass jQuery twice so that the signature of the closure which wraps
                                                        // the script can bind both '$' and 'jQuery'.
+                                                       registry[module].state = 'ready';
                                                        script( $, $ );
                                                        handlePending( module );
+                                               } else if ( typeof script === 'string' ) {
+                                                       // Site module is a legacy script that runs in the global scope. This is transported
+                                                       // as a string instead of a function to avoid needing to use string manipulation to
+                                                       // undo the function wrapper.
+                                                       registry[module].state = 'ready';
+                                                       $.globalEval( script );
+                                                       handlePending( module );
                                                }
                                        } catch ( e ) {
                                                // This needs to NOT use mw.log because these errors are common in production mode
                                        if ( typeof module !== 'string' ) {
                                                throw new Error( 'module must be of type string, not ' + typeof module );
                                        }
-                                       if ( script && !$.isFunction( script ) && !$.isArray( script ) ) {
-                                               throw new Error( 'script must be of type function or array, not ' + typeof script );
+                                       if ( script && !$.isFunction( script ) && !$.isArray( script ) && typeof script !== 'string' ) {
+                                               throw new Error( 'script must be of type function, array, or script; not ' + typeof script );
                                        }
                                        if ( style && !$.isPlainObject( style ) ) {
                                                throw new Error( 'style must be of type object, not ' + typeof style );
                                                        // Module failed to load
                                                        descriptor.state !== 'ready' ||
                                                        // Unversioned, private, or site-/user-specific
-                                                       ( !descriptor.version || $.inArray( descriptor.group, [ 'private', 'user', 'site' ] ) !== -1 ) ||
+                                                       ( !descriptor.version || $.inArray( descriptor.group, [ 'private', 'user' ] ) !== -1 ) ||
                                                        // Partial descriptor
                                                        $.inArray( undefined, [ descriptor.script, descriptor.style,
                                                                        descriptor.messages, descriptor.templates ] ) !== -1