resourceloader: Restore "blocking" legacy modules

This ensures 'wikibits' and 'mediawiki.util' (if so configured)
finish loading before executing other modules.

This was previously implicitly provided for the bottom queue by
being in the top-queue, but since this is now asynchronous that
is no lower guaranteed. Fix this by making this explicit instead
of implicit.

Keep them in the top queue as-is to ensure consistency with cached
pages and it allows them to preload and batch in the same request
instead of being discovered later at run-time as a separate request.

Bug: T108124
Change-Id: I74e0cbe616404da927ea46d06308a7bae930eb69

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 53137a6..19d4b89 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -3524,28 +3524,27 @@ $wgResourceLoaderMinifierStatementsOnOwnLine = false;
 $wgResourceLoaderMinifierMaxLineLength = 1000;
 
 /**
- * Whether to include the mediawiki.legacy JS library (old wikibits.js), and its
- * dependencies.
+ * Whether to ensure the mediawiki.legacy library is loaded before other modules.
+ *
+ * @deprecated since 1.26: Always declare dependencies.
  */
 $wgIncludeLegacyJavaScript = true;
 
 /**
- * Whether to preload the mediawiki.util module as blocking module in the top
- * queue.
+ * Whether to ensure the mediawiki.util is loaded before other modules.
  *
- * Before MediaWiki 1.19, modules used to load slower/less asynchronous which
- * allowed modules to lack dependencies on 'popular' modules that were likely
- * loaded already.
+ * Before MediaWiki 1.19, modules used to load less asynchronous which allowed
+ * modules to lack dependencies on 'popular' modules that were likely loaded already.
  *
  * This setting is to aid scripts during migration by providing mediawiki.util
- * unconditionally (which was the most commonly missed dependency).
- * It doesn't cover all missing dependencies obviously but should fix most of
- * them.
+ * unconditionally (which was the most commonly missed dependency). It doesn't
+ * cover all missing dependencies obviously but should fix most of them.
  *
  * This should be removed at some point after site/user scripts have been fixed.
  * Enable this if your wiki has a large amount of user/site scripts that are
  * lacking dependencies.
- * @todo Deprecate
+ *
+ * @deprecated since 1.26: Always declare dependencies.
  */
 $wgPreloadJavaScriptMwUtil = false;
 

diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php
index 144107c..64678f4 100644 (file)
--- a/includes/resourceloader/ResourceLoaderStartUpModule.php
+++ b/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -101,6 +101,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                        'wgLegalTitleChars' => Title::convertByteClassToUnicodeClass( Title::legalChars() ),
                        'wgResourceLoaderStorageVersion' => $conf->get( 'ResourceLoaderStorageVersion' ),
                        'wgResourceLoaderStorageEnabled' => $conf->get( 'ResourceLoaderStorageEnabled' ),
+                       'wgResourceLoaderLegacyModules' => self::getLegacyModules(),
                );
 
                Hooks::run( 'ResourceLoaderGetConfigVars', array( &$vars ) );
@@ -293,6 +294,20 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                return array( 'jquery', 'mediawiki' );
        }
 
+       public static function getLegacyModules() {
+               global $wgIncludeLegacyJavaScript, $wgPreloadJavaScriptMwUtil;
+
+               $legacyModules = array();
+               if ( $wgIncludeLegacyJavaScript ) {
+                       $legacyModules[] = 'mediawiki.legacy.wikibits';
+               }
+               if ( $wgPreloadJavaScriptMwUtil ) {
+                       $legacyModules[] = 'mediawiki.util';
+               }
+
+               return $legacyModules;
+       }
+
        /**
         * Get the load URL of the startup modules.
         *

diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index 2a1b0a3..53af3e7 100644 (file)
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -180,8 +180,7 @@ abstract class Skin extends ContextSource {
         * @return array Array of modules with helper keys for easy overriding
         */
        public function getDefaultModules() {
-               global $wgIncludeLegacyJavaScript, $wgPreloadJavaScriptMwUtil, $wgUseAjax,
-                       $wgAjaxWatch, $wgEnableAPI, $wgEnableWriteAPI;
+               global $wgUseAjax, $wgAjaxWatch, $wgEnableAPI, $wgEnableWriteAPI;
 
                $out = $this->getOutput();
                $user = $out->getUser();
@@ -191,7 +190,7 @@ abstract class Skin extends ContextSource {
                                'mediawiki.page.ready',
                        ),
                        // modules that exist for legacy reasons
-                       'legacy' => array(),
+                       'legacy' => ResourceLoaderStartUpModule::getLegacyModules(),
                        // modules relating to search functionality
                        'search' => array(),
                        // modules relating to functionality relating to watching an article
@@ -199,13 +198,6 @@ abstract class Skin extends ContextSource {
                        // modules which relate to the current users preferences
                        'user' => array(),
                );
-               if ( $wgIncludeLegacyJavaScript ) {
-                       $modules['legacy'][] = 'mediawiki.legacy.wikibits';
-               }
-
-               if ( $wgPreloadJavaScriptMwUtil ) {
-                       $modules['legacy'][] = 'mediawiki.util';
-               }
 
                // Add various resources if required
                if ( $wgUseAjax ) {

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index 3c5a5f5..6548896 100644 (file)
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -1174,7 +1174,10 @@
                                }
 
                                function runScript() {
-                                       var script, markModuleReady, nestedAddScript;
+                                       var script, markModuleReady, nestedAddScript, legacyWait,
+                                               // Expand to include dependencies since we have to exclude both legacy modules
+                                               // and their dependencies from the legacyWait (to prevent a circular dependency).
+                                               legacyModules = resolve( mw.config.get( 'wgResourceLoaderLegacyModules', [] ) );
                                        try {
                                                script = registry[module].script;
                                                markModuleReady = function () {
@@ -1195,32 +1198,38 @@
                                                        } );
                                                };
 
-                                               if ( $.isArray( script ) ) {
-                                                       nestedAddScript( script, markModuleReady, 0 );
-                                               } else if ( $.isFunction( script ) ) {
-                                                       // 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 and user modules are a legacy scripts that run 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.
-                                                       if ( module === 'user' ) {
-                                                               // Implicit dependency on the site module. Not real dependency because
-                                                               // it should run after 'site' regardless of whether it succeeds or fails.
-                                                               mw.loader.using( 'site' ).always( function () {
+                                               legacyWait = ( $.inArray( module, legacyModules ) !== -1 )
+                                                       ? $.Deferred().resolve()
+                                                       : mw.loader.using( legacyModules );
+
+                                               legacyWait.always( function () {
+                                                       if ( $.isArray( script ) ) {
+                                                               nestedAddScript( script, markModuleReady, 0 );
+                                                       } else if ( $.isFunction( script ) ) {
+                                                               // 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 and user modules are a legacy scripts that run 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.
+                                                               if ( module === 'user' ) {
+                                                                       // Implicit dependency on the site module. Not real dependency because
+                                                                       // it should run after 'site' regardless of whether it succeeds or fails.
+                                                                       mw.loader.using( 'site' ).always( function () {
+                                                                               registry[module].state = 'ready';
+                                                                               $.globalEval( script );
+                                                                               handlePending( module );
+                                                                       } );
+                                                               } else {
                                                                        registry[module].state = 'ready';
                                                                        $.globalEval( script );
                                                                        handlePending( module );
-                                                               } );
-                                                       } else {
-                                                               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
                                                // and not in debug mode, such as when a symbol that should be global isn't exported