From 2f8cd8f4d5d7238d1ede3a801cb470cd46431c0d Mon Sep 17 00:00:00 2001
From: Roan Kattouw <roan.kattouw@gmail.com>
Date: Mon, 20 Jul 2015 17:22:56 -0500
Subject: [PATCH] mw.loader: Fix late loading of CSS in certain cases

By protecting the CSS callbacks array against being
polluted by the callbacks themselves. This fixes
a bug where if A depends on B and both A and B have
styles, both A and B are executed once B's styles
have loaded (but before A's styles have loaded).
This breaks mw.loader's contract to only execute
A once its styles have loaded.

Bug: T105973
Change-Id: Ifa8fc7b3d275faa1f9a136a8c4a0e0a7cc358083
---
 resources/src/mediawiki/mediawiki.js          | 12 ++++-
 .../resources/mediawiki/mediawiki.test.js     | 54 +++++++++++++++++++
 2 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index 2c88e93654..c0b1642543 100644
--- a/resources/src/mediawiki/mediawiki.js
+++ b/resources/src/mediawiki/mediawiki.js
@@ -819,6 +819,14 @@
 			function addEmbeddedCSS( cssText, callback ) {
 				var $style, styleEl;
 
+				function fireCallbacks() {
+					var oldCallbacks = cssCallbacks;
+					// Reset cssCallbacks variable so it's not polluted by any calls to
+					// addEmbeddedCSS() from one of the callbacks (T105973)
+					cssCallbacks = $.Callbacks();
+					oldCallbacks.fire().empty();
+				}
+
 				if ( callback ) {
 					cssCallbacks.add( callback );
 				}
@@ -884,14 +892,14 @@
 						} else {
 							styleEl.appendChild( document.createTextNode( cssText ) );
 						}
-						cssCallbacks.fire().empty();
+						fireCallbacks();
 						return;
 					}
 				}
 
 				$( newStyleTag( cssText, getMarker() ) ).data( 'ResourceLoaderDynamicStyleTag', true );
 
-				cssCallbacks.fire().empty();
+				fireCallbacks();
 			}
 
 			/**
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index 77fecb3a43..c8f00115eb 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -670,6 +670,60 @@
 
 	} );
 
+	QUnit.asyncTest( 'mw.loader.implement( dependency with styles )', 4, function ( assert ) {
+		var $element = $( '<div class="mw-test-implement-e"></div>' ).appendTo( '#qunit-fixture' ),
+			$element2 = $( '<div class="mw-test-implement-e2"></div>' ).appendTo( '#qunit-fixture' );
+
+		assert.notEqual(
+			$element.css( 'float' ),
+			'right',
+			'style is clear'
+		);
+		assert.notEqual(
+			$element2.css( 'float' ),
+			'left',
+			'style is clear'
+		);
+
+		mw.loader.register( [
+			[ 'test.implement.e', '0', ['test.implement.e2']],
+			[ 'test.implement.e2', '0' ]
+		] );
+
+		mw.loader.implement(
+			'test.implement.e',
+			function () {
+				assert.equal(
+					$element.css( 'float' ),
+					'right',
+					'Depending module\'s style is applied'
+				);
+				QUnit.start();
+			},
+			{
+				'all': '.mw-test-implement-e { float: right; }'
+			}
+		);
+
+		mw.loader.implement(
+			'test.implement.e2',
+			function () {
+				assert.equal(
+					$element2.css( 'float' ),
+					'left',
+					'Dependency\'s style is applied'
+				);
+			},
+			{
+				'all': '.mw-test-implement-e2 { float: left; }'
+			}
+		);
+
+		mw.loader.load( [
+			'test.implement.e'
+		] );
+	} );
+
 	QUnit.test( 'mw.loader.implement( only scripts )', 1, function ( assert ) {
 		mw.loader.implement( 'test.onlyscripts', function () {} );
 		assert.strictEqual( mw.loader.getState( 'test.onlyscripts' ), 'ready' );
-- 
2.20.1