From: Timo Tijhof <krinklemail@gmail.com>
Date: Thu, 21 Jan 2016 17:25:14 +0000 (+0000)
Subject: resourceloader: Simplify and clean up RLQ wrap
X-Git-Tag: 1.31.0-rc.0~8192
X-Git-Url: http://git.cyclocoop.org/data/Fool?a=commitdiff_plain;h=0c8101570c3c1c9df01efc6a4fac1441ff080389;p=lhc%2Fweb%2Fwiklou.git

resourceloader: Simplify and clean up RLQ wrap

startup.js:
* Don't assign to "window.RLQ" and read from "RLQ". Use the same
  identifier in both places instead of relying on global scope.
* Create the global window.RLQ only once. The temporary [] fallback
  doesn't need to be exposed globally, so use a local variable for
  the loop that processes the pre-existing queue built up while
  the startup module was loading.

OutputPage:
* Simplify the RLQ wrap to be more idiomatic and less repetitive.
  Matches the pattern used in Google and Facebook open-source projects.

Change-Id: I9176377bc05432e51add841696a356428feec8ce
---

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 74ee6ab7bd..d7b51b864d 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1365,7 +1365,7 @@ MESSAGE;
 	 * @return string
 	 */
 	public static function makeLoaderConditionalScript( $script ) {
-		return "window.RLQ = window.RLQ || []; window.RLQ.push( function () {\n" .
+		return "(window.RLQ = window.RLQ || []).push(function () {\n" .
 			trim( $script ) . "\n} );";
 	}
 
@@ -1382,7 +1382,7 @@ MESSAGE;
 		$js = self::makeLoaderConditionalScript( $script );
 		return new WrappedString(
 			Html::inlineScript( $js ),
-			"<script>window.RLQ = window.RLQ || []; window.RLQ.push( function () {\n",
+			"<script>(window.RLQ = window.RLQ || []).push(function () {\n",
 			"\n} );</script>"
 		);
 	}
diff --git a/resources/src/startup.js b/resources/src/startup.js
index 0f51a35332..0c2d6d69ec 100644
--- a/resources/src/startup.js
+++ b/resources/src/startup.js
@@ -87,7 +87,7 @@ function isCompatible( ua ) {
 
 		// Must be after mw.config.set because these callbacks may use mw.loader which
 		// needs to have values 'skin', 'debug' etc. from mw.config.
-		window.RLQ = window.RLQ || [];
+		var RLQ = window.RLQ || [];
 		while ( RLQ.length ) {
 			RLQ.shift()();
 		}
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index f5ef0162d0..cfc416b5d6 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -142,7 +142,7 @@ class OutputPageTest extends MediaWikiTestCase {
 			// Load module script only
 			array(
 				array( 'test.foo', ResourceLoaderModule::TYPE_SCRIPTS ),
-				"<script>window.RLQ = window.RLQ || []; window.RLQ.push( function () {\n"
+				"<script>(window.RLQ = window.RLQ || []).push(function () {\n"
 					. 'mw.loader.load("http://127.0.0.1:8080/w/load.php?debug=false\u0026lang=en\u0026modules=test.foo\u0026only=scripts\u0026skin=fallback");'
 					. "\n} );</script>"
 			),
@@ -161,14 +161,14 @@ class OutputPageTest extends MediaWikiTestCase {
 			// Load private module (only=scripts)
 			array(
 				array( 'test.quux', ResourceLoaderModule::TYPE_SCRIPTS ),
-				"<script>window.RLQ = window.RLQ || []; window.RLQ.push( function () {\n"
+				"<script>(window.RLQ = window.RLQ || []).push(function () {\n"
 					. "mw.test.baz({token:123});mw.loader.state({\"test.quux\":\"ready\"});\n"
 					. "} );</script>"
 			),
 			// Load private module (combined)
 			array(
 				array( 'test.quux', ResourceLoaderModule::TYPE_COMBINED ),
-				"<script>window.RLQ = window.RLQ || []; window.RLQ.push( function () {\n"
+				"<script>(window.RLQ = window.RLQ || []).push(function () {\n"
 					. "mw.loader.implement(\"test.quux\",function($,jQuery){"
 					. "mw.test.baz({token:123});},{\"css\":[\".mw-icon{transition:none}"
 					. "\"]});\n} );</script>"
@@ -186,10 +186,10 @@ class OutputPageTest extends MediaWikiTestCase {
 			// Load two modules in separate groups
 			array(
 				array( array( 'test.group.foo', 'test.group.bar' ), ResourceLoaderModule::TYPE_COMBINED ),
-				"<script>window.RLQ = window.RLQ || []; window.RLQ.push( function () {\n"
+				"<script>(window.RLQ = window.RLQ || []).push(function () {\n"
 					. 'mw.loader.load("http://127.0.0.1:8080/w/load.php?debug=false\u0026lang=en\u0026modules=test.group.bar\u0026skin=fallback");'
 					. "\n} );</script>\n"
-					. "<script>window.RLQ = window.RLQ || []; window.RLQ.push( function () {\n"
+					. "<script>(window.RLQ = window.RLQ || []).push(function () {\n"
 					. 'mw.loader.load("http://127.0.0.1:8080/w/load.php?debug=false\u0026lang=en\u0026modules=test.group.foo\u0026skin=fallback");'
 					. "\n} );</script>"
 			),