From 9272bc6c472122548199865c5175185bdaa469c3 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 6 Aug 2014 15:54:22 +0000
Subject: [PATCH] resourceloader: Wrap only=script responses in "if(window.mw)"
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

We currently have a few legacy requests to the load.php end point
that bypass the ResourceLoader client by coding a request to
load.php via a "<script src>" directly. Most prominently the
request for the 'site' wiki module (aka MediaWiki:Common.js).

Remove the manual wrapping of embedded private modules as this
is now taken are of by ResourceLoader::makeModuleResponse itself.

Misc:
* Mark "jquery" and "mediawiki" as Raw modules. While the startup
  module had this already, these didn't. Without this, they'd
  get the conditional wrap – which would be a problem since mediawiki.js
  can't be conditional on 'window.mw' for that file defines that
  namespace itself.
* Strip the cache-key comment in the unit tests because the hash
  no longer matches and using the generic 'wiki' dbname was breaking
  DB queries.
* Relates to bug 63587.
* See also 05d0f6fefdcc959d which expands the reach of the non-JS
  environment to IE6 and helped expose this bug.

Change-Id: Icf6ede09b51ce212aa70ff6be4b341762ec75b4d
---
 includes/OutputPage.php                            |  4 +---
 includes/resourceloader/ResourceLoader.php         |  7 +++++++
 .../resourceloader/ResourceLoaderStartUpModule.php | 12 ++++++++++--
 resources/Resources.php                            |  4 ++--
 tests/phpunit/includes/OutputPageTest.php          | 14 +++++---------
 5 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 87a0809257..e91f2afe49 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2797,9 +2797,7 @@ $templates
 						);
 					} else {
 						$links['html'] .= Html::inlineScript(
-							ResourceLoader::makeLoaderConditionalScript(
-								$resourceLoader->makeModuleResponse( $context, $grpModules )
-							)
+							$resourceLoader->makeModuleResponse( $context, $grpModules )
 						);
 					}
 					$links['html'] .= "\n";
diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 21e64359b4..53b6d8141d 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -990,6 +990,13 @@ class ResourceLoader {
 			if ( count( $states ) ) {
 				$out .= self::makeLoaderStateScript( $states );
 			}
+
+			// In only=script requests for modules that are not raw (e.g. not the startup module)
+			// ensure the execution is conditional to avoid situations where browsers with an
+			// unsupported environment do unconditionally execute a module's scripts. Otherwise users
+			// will get things like "ReferenceError: mw is undefined" or "jQuery is undefined" from
+			// legacy scripts loaded with only=scripts (such as the 'site' module).
+			$out = self::makeLoaderConditionalScript( $out );
 		} else {
 			if ( count( $states ) ) {
 				$exceptions .= self::makeComment(
diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php
index 2c0f8df785..65d09b0721 100644
--- a/includes/resourceloader/ResourceLoaderStartUpModule.php
+++ b/includes/resourceloader/ResourceLoaderStartUpModule.php
@@ -333,6 +333,15 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
 		return true;
 	}
 
+	/**
+	 * Base modules required for the the base environment of ResourceLoader
+	 *
+	 * @return array
+	 */
+	public static function getStartupModules() {
+		return array( 'jquery', 'mediawiki' );
+	}
+
 	/**
 	 * Get the load URL of the startup modules.
 	 *
@@ -343,8 +352,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
 	 * @return string
 	 */
 	public static function getStartupModulesUrl( ResourceLoaderContext $context ) {
-		// The core modules:
-		$moduleNames = array( 'jquery', 'mediawiki' );
+		$moduleNames = self::getStartupModules();
 
 		// Get the latest version
 		$loader = $context->getResourceLoader();
diff --git a/resources/Resources.php b/resources/Resources.php
index b588cf24a3..bd85f6eab4 100644
--- a/resources/Resources.php
+++ b/resources/Resources.php
@@ -128,7 +128,7 @@ return array(
 			array(
 				'resources/lib/jquery/jquery.js'
 			) ),
-		'debugRaw' => false,
+		'raw' => true,
 		'targets' => array( 'desktop', 'mobile' ),
 	),
 
@@ -771,7 +771,7 @@ return array(
 	'mediawiki' => array(
 		'scripts' => 'resources/src/mediawiki/mediawiki.js',
 		'debugScripts' => 'resources/src/mediawiki/mediawiki.log.js',
-		'debugRaw' => false,
+		'raw' => true,
 		'targets' => array( 'desktop', 'mobile' ),
 	),
 	'mediawiki.api' => array(
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index 2cfdfcd6ef..50fd3ef598 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -154,10 +154,8 @@ class OutputPageTest extends MediaWikiTestCase {
 			// Load private module (combined)
 			array(
 				array( 'test.quux', ResourceLoaderModule::TYPE_COMBINED ),
-				'<script>if(window.mw){
-mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n/* cache key: wiki:resourceloader:filter:minify-css:7:fd8ea20b3336b2bfb230c789d430067a */"]},{});
-/* cache key: wiki:resourceloader:filter:minify-js:7:274ccee17be73cd5f4dda5dc2a819188 */
-}</script>
+				'<script>if(window.mw){mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]},{});}
+</script>
 '
 			),
 			// Load module script with with ESI
@@ -186,10 +184,6 @@ mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"
 			'wgLoadScript' => 'http://127.0.0.1:8080/w/load.php',
 			// Affects whether CDATA is inserted
 			'wgWellFormedXml' => false,
-			// Cache key is based on database name, and affects output;
-			// this test should not touch the database anyways.
-			'wgDBname' => 'wiki',
-			'wgDBprefix' => '',
 		) );
 		$class = new ReflectionClass( 'OutputPage' );
 		$method = $class->getMethod( 'makeResourceLoaderLink' );
@@ -219,6 +213,8 @@ mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"
 			)),
 		) );
 		$links = $method->invokeArgs( $out, $args );
-		$this->assertEquals( $expectedHtml, $links['html'] );
+		// Strip comments to avoid variation due to wgDBname in WikiID and cache key
+		$actualHtml = preg_replace( '#/\*[^*]+\*/#', '', $links['html'] );
+		$this->assertEquals( $expectedHtml, $actualHtml );
 	}
 }
-- 
2.20.1