From 7f817ae7342beafd35c5e48ac69f8eb346014c54 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <ttijhof@wikimedia.org>
Date: Sat, 6 Oct 2012 11:05:36 +0200
Subject: [PATCH] (bug 40834) mw.loader: Fix comma-joined 404 error.

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              |  9 ++++-
 .../resources/mediawiki/mediawiki.test.js     | 38 +++++++++++++------
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/resources/mediawiki/mediawiki.js b/resources/mediawiki/mediawiki.js
index b48462965f..1a72ed138f 100644
--- a/resources/mediawiki/mediawiki.js
+++ b/resources/mediawiki/mediawiki.js
@@ -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] );
+								}
+							}
 						}
 					}
 				}
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index 806058d2e7..be59f1adb1 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -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' )
+				]
 			}
 		},
 		{}
-- 
2.20.1