From 5553be79b9ae97318dcdf816ec99c10126369f42 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <krinklemail@gmail.com>
Date: Tue, 2 Sep 2014 03:25:13 +0200
Subject: [PATCH] OutputPage: Restore ResourceLoader condition wrap for
 embedded modules

Follows-up 9272bc6c47, 03c503da22.

* In 9272bc6c47, the condition wrap was removed from
  OutputPage for no reason. This went unnnoticed as I had also
  accidentally made the cond wrap in makeModuleResponse apply
  to both only=scripts and regular (faux) responses, such as by
  OutputPage embedding private modules.

* In 03c503da22, the latter bug was fixed, thus exposing the former.

This wrapper belongs in OutputPage, not in ResourceLoader. It's
OutputPage making the loader request. And just like in other places,
it's the "client"'s responsibility to ensure the request is either not
made or wrapped appropriately.

The test for "private module (only=scripts)" could be removed but
I'll keep it so we can see how this changes in the future. It's
a case that can't ever happen, but if it would, it currently gets
a double condition wrapper, which is fine.

Change-Id: Id333e4958ed769831fabca02164c1e8505962d57
---
 includes/OutputPage.php                   |  4 +++-
 tests/phpunit/includes/OutputPageTest.php | 18 +++++++++++++-----
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index a58a79a200..af90ca6da4 100644
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2797,7 +2797,9 @@ $templates
 						);
 					} else {
 						$links['html'] .= Html::inlineScript(
-							$resourceLoader->makeModuleResponse( $context, $grpModules )
+							ResourceLoader::makeLoaderConditionalScript(
+								$resourceLoader->makeModuleResponse( $context, $grpModules )
+							)
 						);
 					}
 					$links['html'] .= "\n";
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index 5c1994b4a8..69a302d7bb 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -151,18 +151,26 @@ class OutputPageTest extends MediaWikiTestCase {
 				'<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)
+			// Load private module (only=scripts)
+			// This is asserted for completion (would get two condition wrappers),
+			// though in practice we'd never embed a module with only=scripts,
+			// that mode is reserved for hardcoded requests. Embedded modules
+			// would always be combined.
 			array(
 				array( 'test.quux', ResourceLoaderModule::TYPE_SCRIPTS ),
-				'<script>if(window.mw){mw.test.baz({token:123});mw.loader.state({"test.quux":"ready"});}
-</script>
+				'<script>if(window.mw){
+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>mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]},{});
-</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
-- 
2.20.1