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

diff --git a/resources/src/mediawiki/mediawiki.js b/resources/src/mediawiki/mediawiki.js
index 2c88e93..c0b1642 100644 (file)
--- 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 77fecb3..c8f0011 100644 (file)
--- 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' );