(bug 40834) mw.loader: Fix comma-joined 404 error.
authorTimo Tijhof <ttijhof@wikimedia.org>
Sat, 6 Oct 2012 09:05:36 +0000 (11:05 +0200)
committerTimo Tijhof <ttijhof@wikimedia.org>
Sun, 7 Oct 2012 07:27:28 +0000 (09:27 +0200)
It passed an array instead of a url to addLink so internally
Array.prototype.toString happened which joins by comma.

In debug mode modules with more than 1 stylesheet were 404 error
as it was trying to load 'http://.../foo.css,http://.../bar.css'.

Unit tests didn't catch it because they all only tested an array
with 1 url. Updated the test to assert this from now on.

Follows-up I3e8227ddb87fd9441071ca935439fc6467751dab.

Change-Id: I32a79af637acb782e07bbae413b0f007350096c3

resources/mediawiki/mediawiki.js
tests/qunit/suites/resources/mediawiki/mediawiki.test.js

index b484629..1a72ed1 100644 (file)
@@ -828,7 +828,7 @@ var mw = ( function ( $, undefined ) {
                         * @param module string module name to execute
                         */
                        function execute( module ) {
-                               var key, value, media, i, script, markModuleReady, nestedAddScript;
+                               var key, value, media, i, urls, script, markModuleReady, nestedAddScript;
 
                                if ( registry[module] === undefined ) {
                                        throw new Error( 'Module has not been registered yet: ' + module );
@@ -896,7 +896,12 @@ var mw = ( function ( $, undefined ) {
                                                // Array of urls inside media-type key
                                                } else if ( typeof value === 'object' ) {
                                                        // { "url": { <media>: [url, ..] } }
-                                                       $.each( value, addLink );
+                                                       for ( media in value ) {
+                                                               urls = value[media];
+                                                               for ( i = 0; i < urls.length; i += 1 ) {
+                                                                       addLink( media, urls[i] );
+                                                               }
+                                                       }
                                                }
                                        }
                                }
index 806058d..be59f1a 100644 (file)
@@ -170,7 +170,7 @@ function assertStyleAsync( assert, $element, prop, val, fn ) {
                // If it is passing or if we timed out, run the real test and stop the loop
                if ( isCssImportApplied() || styleTestSince > styleTestTimeout ) {
                        assert.equal( $element.css( prop ), val,
-                               'style from url is applied (after ' + styleTestSince + 'ms)'
+                               'style "' + prop + ': ' + val + '" from url is applied (after ' + styleTestSince + 'ms)'
                        );
 
                        if ( fn ) {
@@ -253,35 +253,49 @@ QUnit.test( 'mw.loader.implement( styles={ "css": [text, ..] } )', 2, function (
        ]);
 } );
 
-QUnit.asyncTest( 'mw.loader.implement( styles={ "url": { <media>: [url, ..] } } )', 4, function ( assert ) {
-       var $element = $( '<div class="mw-test-implement-b"></div>' ).appendTo( '#qunit-fixture' ),
-               $element2 = $( '<div class="mw-test-implement-b2"></div>' ).appendTo( '#qunit-fixture' );
+QUnit.asyncTest( 'mw.loader.implement( styles={ "url": { <media>: [url, ..] } } )', 7, function ( assert ) {
+       var $element1 = $( '<div class="mw-test-implement-b1"></div>' ).appendTo( '#qunit-fixture' ),
+               $element2 = $( '<div class="mw-test-implement-b2"></div>' ).appendTo( '#qunit-fixture' ),
+               $element3 = $( '<div class="mw-test-implement-b3"></div>' ).appendTo( '#qunit-fixture' );
 
        assert.notEqual(
-               $element.css( 'float' ),
-               'right',
+               $element1.css( 'text-align' ),
+               'center',
                'style is clear'
        );
        assert.notEqual(
-               $element2.css( 'text-align' ),
-               'center',
+               $element2.css( 'float' ),
+               'left',
+               'style is clear'
+       );
+       assert.notEqual(
+               $element3.css( 'text-align' ),
+               'right',
                'style is clear'
        );
 
        mw.loader.implement(
                'test.implement.b',
                function () {
-                       assertStyleAsync( assert, $element, 'float', 'right', function () {
+                       assertStyleAsync( assert, $element2, 'float', 'left', function () {
+                               assert.notEqual( $element1.css( 'text-align' ), 'center', 'print style is not applied' );
 
-                               assert.notEqual( $element2.css( 'text-align' ), 'center', 'print style is not applied' );
+                               QUnit.start();
+                       } );
+                       assertStyleAsync( assert, $element3, 'float', 'right', function () {
+                               assert.notEqual( $element1.css( 'text-align' ), 'center', 'print style is not applied' );
 
                                QUnit.start();
                        } );
                },
                {
                        'url': {
-                               'screen': [urlStyleTest( '.mw-test-implement-b', 'float', 'right' )],
-                               'print': [urlStyleTest( '.mw-test-implement-b2', 'text-align', 'center' )]
+                               'print': [urlStyleTest( '.mw-test-implement-b1', 'text-align', 'center' )],
+                               'screen': [
+                                       // bug 40834: Make sure it actually works with more than 1 stylesheet reference
+                                       urlStyleTest( '.mw-test-implement-b2', 'float', 'left' ),
+                                       urlStyleTest( '.mw-test-implement-b3', 'float', 'right' )
+                               ]
                        }
                },
                {}