From 03c503da222360a3f8065078907da65b6435721a Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Wed, 13 Aug 2014 19:06:10 +0100
Subject: [PATCH] resourceloader: Only conditional-wrap script responses with
 only=scripts

Follows-up 9272bc6c4721225. The shouldIncludeScripts method returns
true for only=scripts, but also for other responses (except for
only=styles, naturally).

Regular load.php responses that load both scripts and styles don't
need the conditional wrap because those requests should only be
made from the mw.loader client which inherently means the
environment has been provided already.

It's merely unnecessary and shouldn't have caused any issues since
it simply evaluates to true always. It was already correctly being
excluded from raw modules that run before the environment is
provided (such as startup, jquery and mediawiki).

Change-Id: I375a7a8039f9b3ad4909e76005725f7d975d8a5e
---
 includes/resourceloader/ResourceLoader.php | 14 ++++++++------
 tests/phpunit/includes/OutputPageTest.php  |  9 ++++++++-
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index c2004080a5..5f72d8d2f4 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1007,12 +1007,14 @@ class ResourceLoader {
 				$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 );
+			if ( $context->getOnly() === 'scripts' ) {
+				// 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/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index 3eb58fcb40..5c1994b4a8 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -149,12 +149,19 @@ class OutputPageTest extends MediaWikiTestCase {
 			array(
 				array( array( 'test.baz', 'test.foo', 'test.bar' ), ResourceLoaderModule::TYPE_STYLES ),
 				'<link rel=stylesheet href="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.bar%2Cbaz%2Cfoo&amp;only=styles&amp;skin=fallback&amp;*">
+'
+			),
+			// Load private module (scripts)
+			array(
+				array( 'test.quux', ResourceLoaderModule::TYPE_SCRIPTS ),
+				'<script>if(window.mw){mw.test.baz({token:123});mw.loader.state({"test.quux":"ready"});}
+</script>
 '
 			),
 			// 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"]},{});}
+				'<script>mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]},{});
 </script>
 '
 			),
-- 
2.20.1