From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 8 Aug 2018 21:58:33 +0000 (+0100)
Subject: resourceloader: Add to debug mode the same 'jquery' clause as for prod
X-Git-Tag: 1.34.0-rc.0~4516^2
X-Git-Url: http://git.cyclocoop.org/?a=commitdiff_plain;h=2873d65d9f2ba6a5f5db141d6c7326c8a8566da9;p=lhc%2Fweb%2Fwiklou.git

resourceloader: Add to debug mode the same 'jquery' clause as for prod

=== module.exports

Follows-up dec800968e, which added a clause for `window.$` inside runScript()
that ensures require/module.exports is not given to 'jquery'.

This commit adds the same clause to debug mode handling, which follows a
different code path. Without this, jquery.migrate.js execution throws from
`require('jquery')`, when viewing a page in debug mode.

=== mediawiki.legacy.wikibits

Before dec800968e, 'jquery' was a raw module and not allowed as dependency,
and before that commit base modules did not follow debug mode. Instead,
they were always combined in the same request (even in debug mode), with
only the order in the query string dictating their execution order.

After that commit, it is mandatory for base modules to express their
execution order through dependency links. This was done for 'mediawiki.base',
but forgotten for 'mediawiki.legacy.wikibits'. That module isn't used by
default, but becomes used when enabling $wgIncludeLegacyJavaScript, which
is off by default, but on for Wikimedia wikis.

Bug: T192623
Change-Id: Id4fbfee71deeb9528e8a622604d4cd972dd25d3b
---

diff --git a/resources/Resources.php b/resources/Resources.php
index a06a25f266..c518399223 100644
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -136,7 +136,9 @@ return [
 		],
 	],
 
-	/* jQuery */
+	/* Base modules */
+	// These modules' dependencies MUST also be included in StartUpModule::getBaseModules().
+	// These modules' dependencies MUST be dependency-free (having dependencies would cause recursion).
 
 	'jquery' => [
 		'scripts' => [
@@ -145,6 +147,20 @@ return [
 		],
 		'targets' => [ 'desktop', 'mobile' ],
 	],
+	'mediawiki.base' => [
+		'scripts' => [
+			// This MUST be kept in sync with maintenance/jsduck/eg-iframe.html
+			'resources/src/mediawiki.base/mediawiki.errorLogger.js',
+			'resources/src/mediawiki.base/mediawiki.base.js',
+		],
+		'dependencies' => 'jquery',
+		'targets' => [ 'desktop', 'mobile' ],
+	],
+	'mediawiki.legacy.wikibits' => [
+		'scripts' => 'resources/src/mediawiki.legacy/wikibits.js',
+		'dependencies' => 'jquery',
+		'targets' => [ 'desktop', 'mobile' ],
+	],
 
 	/* jQuery Plugins */
 
@@ -831,18 +847,6 @@ return [
 	],
 
 	/* MediaWiki */
-
-	'mediawiki.base' => [
-		'scripts' => [
-			// This MUST be kept in sync with maintenance/jsduck/eg-iframe.html
-			'resources/src/mediawiki.base/mediawiki.errorLogger.js',
-			'resources/src/mediawiki.base/mediawiki.base.js',
-		],
-		// - These dependencies MUST NOT also have dependencies (would cause recursion).
-		// - These dependencies MUST also be returned from StartUpModule::getBaseModules().
-		'dependencies' => 'jquery',
-		'targets' => [ 'desktop', 'mobile' ],
-	],
 	'mediawiki.apihelp' => [
 		'styles' => 'resources/src/mediawiki.apihelp.css',
 		'targets' => [ 'desktop' ],
@@ -2391,10 +2395,6 @@ return [
 			'resources/src/mediawiki.legacy/oldshared.css' => [ 'media' => 'screen' ]
 		],
 	],
-	'mediawiki.legacy.wikibits' => [
-		'scripts' => 'resources/src/mediawiki.legacy/wikibits.js',
-		'targets' => [ 'desktop', 'mobile' ],
-	],
 
 	/* MediaWiki UI */
 
diff --git a/resources/src/startup/mediawiki.js b/resources/src/startup/mediawiki.js
index 351acc321a..a2ba85fb7a 100644
--- a/resources/src/startup/mediawiki.js
+++ b/resources/src/startup/mediawiki.js
@@ -1004,6 +1004,8 @@
 			/**
 			 * Queue the loading and execution of a script for a particular module.
 			 *
+			 * This does for debug mode what runScript() does for production.
+			 *
 			 * @private
 			 * @param {string} src URL of the script
 			 * @param {string} moduleName Name of currently executing module
@@ -1011,8 +1013,8 @@
 			 */
 			function queueModuleScript( src, moduleName, callback ) {
 				pendingRequests.push( function () {
-					if ( hasOwn.call( registry, moduleName ) ) {
-						// Emulate runScript() part of execute()
+					// Keep in sync with execute()/runScript().
+					if ( moduleName !== 'jquery' && hasOwn.call( registry, moduleName ) ) {
 						window.require = mw.loader.require;
 						window.module = registry[ moduleName ].module;
 					}
@@ -1191,11 +1193,8 @@
 						if ( Array.isArray( script ) ) {
 							nestedAddScript( script, markModuleReady, 0 );
 						} else if ( typeof script === 'function' ) {
-							if ( window.$ ) {
-								// Pass jQuery twice so that the signature of the closure which wraps
-								// the script can bind both '$' and 'jQuery'.
-								script( window.$, window.$, mw.loader.require, registry[ module ].module );
-							} else {
+							// Keep in sync with queueModuleScript() for debug mode
+							if ( module === 'jquery' ) {
 								// This is a special case for when 'jquery' itself is being loaded.
 								// - The standard jquery.js distribution does not set `window.jQuery`
 								//   in CommonJS-compatible environments (Node.js, AMD, RequireJS, etc.).
@@ -1203,6 +1202,10 @@
 								//   in a CommonJS-compatible environment, will use require('jquery'),
 								//   but that can't work when we're still inside that module.
 								script();
+							} else {
+								// Pass jQuery twice so that the signature of the closure which wraps
+								// the script can bind both '$' and 'jQuery'.
+								script( window.$, window.$, mw.loader.require, registry[ module ].module );
 							}
 							markModuleReady();