From ff594d3b0ae88009120fcde48c91c24c1e3d0445 Mon Sep 17 00:00:00 2001
From: Timo Tijhof <ttijhof@wikimedia.org>
Date: Wed, 25 Jul 2012 14:20:21 -0700
Subject: [PATCH] ResourceLoader: Refactor style loading

Fixes:
* bug 31676: Work around IE stylesheet limit.
* bug 35562: @import styles broken in modules that combine
  multiple stylesheets.
* bug 40498: Don't output empty "@media print { }" blocks.
* bug 40500: Don't ignore media-type for urls in debug mode.

Approach:
* Re-use the same <style> tag so that we stay under the 31
  stylesheet limit in IE. Unless the to-be-added css text from
  the being-loaded module contains @import, in which case we do
  create a new <style> tag and then re-use that one from that
  point on (bug 31676).

* Return stylesheets as arrays, instead of a concatenated string.
  This fixes bug 35562, because @import only works when at the
  top of a stylesheet. By not unconditionally concatenating files
  within a module on the server side already, @import will work
  in e.g. module 'site' that contains 2 wiki pages.

  This is normalized in ResourceLoader::makeCombinedStyles(),
  so far only ResourceLoaderWikiModule makes use of this.

Misc. clean up and bug fixes:
* Reducing usage of jQuery() and mw.html.element() where
  native DOM would be very simple and faster. Aside from
  simplicity and speed, this is also working towards a more
  stand-alone ResourceLoader.
* Trim server output a little bit more
  - Redundant new line after minify-css (it is now an array, so
    no need to keep space afterwards)
  - Redundant semi-colon after minify-js if it ends in a colon
* Allow space in styleTest.css.php
* Clean up and extend unit tests to cover for these features
  and bug fixes.
* Don't set styleEl.rel = 'stylesheet'; that has no business
  on a <style> tag.
* Fix bug in mw.loader's addStyleTag(). It turns out IE6
  has an odd security measure that does not allow manipulation
  of elements (at least style tags) that are created by a
  different script (even if that script was served from the same
  domain/origin etc.). We didn't ran into this before because
  we only created new style tags, never appended to them. Now
  that we do, this came up. Took a while to figure out because
  it was created by mediawiki.js but it calls jQuery which did
  the actual dom insertion. Odd thing is, we load jquery.js and
  mediawiki.js in the same request even...
  Without this all css-url related mw.loader tests would fail
  in IE6.
* mediawiki.js and mediawiki.test.js now pass jshint again.

Tested (and passing qunit/?module=mediawiki; 123 of 123):
* Chrome 14, 21
* Firefox 3.0, 3.6, 4, 7, 14, 15, 16beta
* IE 6, 7, 8, 9
* Safari 4.0, 5.0, 5.1
* Opera 10.0, 11.1, 11.5, 11.6, 12.0, 12.5beta
* iPhone 3GS / iOS 3.0 / Mobile Safari 4.0
  iPhone 4 / iOS 4.0.1 / Mobile Safari 4.0.5
  iPhone 4S / iOS 6.0 Beta / Mobile Safari 6.0

Change-Id: I3e8227ddb87fd9441071ca935439fc6467751dab
---
 RELEASE-NOTES-1.20                            |   3 +
 includes/resourceloader/ResourceLoader.php    |  75 +++--
 .../ResourceLoaderFileModule.php              |   3 +-
 .../resourceloader/ResourceLoaderModule.php   |   4 +-
 .../ResourceLoaderWikiModule.php              |   6 +-
 resources/mediawiki/mediawiki.js              | 179 +++++++---
 tests/qunit/data/styleTest.css.php            |   9 +-
 .../resources/mediawiki/mediawiki.test.js     | 309 +++++++++++++-----
 8 files changed, 426 insertions(+), 162 deletions(-)

diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20
index d313865c7a..66cca2ecc4 100644
--- a/RELEASE-NOTES-1.20
+++ b/RELEASE-NOTES-1.20
@@ -256,6 +256,9 @@ upgrade PHP if you have not done so prior to upgrading MediaWiki.
   basic per-char typing.
 * (bug 34495) patrol log now credit the user patrolling (instead of patrolled
   user).
+* (bug 31676) ResourceLoader should work around IE stylesheet limit.
+* (bug 40498) ResourceLoader should not output an empty "@media print { }" block.
+* (bug 40500) ResourceLoader should not ignore media-type for urls in debug mode.
 
 === API changes in 1.20 ===
 * (bug 34316) Add ability to retrieve maximum upload size from MediaWiki API.
diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 99fe7ed5eb..3b48a26689 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -163,11 +163,11 @@ class ResourceLoader {
 						$wgResourceLoaderMinifierStatementsOnOwnLine,
 						$wgResourceLoaderMinifierMaxLineLength
 					);
-					$result .= "\n\n/* cache key: $key */\n";
+					$result .= "\n/* cache key: $key */";
 					break;
 				case 'minify-css':
 					$result = CSSMin::minify( $data );
-					$result .= "\n\n/* cache key: $key */\n";
+					$result .= "\n/* cache key: $key */";
 					break;
 			}
 
@@ -703,7 +703,7 @@ class ResourceLoader {
 						$scripts = $module->getScriptURLsForDebug( $context );
 					} else {
 						$scripts = $module->getScript( $context );
-						if ( is_string( $scripts ) ) {
+						if ( is_string( $scripts ) && strlen( $scripts ) && substr( $scripts, -1 ) !== ';' ) {
 							// bug 27054: Append semicolon to prevent weird bugs
 							// caused by files not terminating their statements right
 							$scripts .= ";\n";
@@ -720,19 +720,31 @@ class ResourceLoader {
 						// If we are in debug mode without &only= set, we'll want to return an array of URLs
 						// See comment near shouldIncludeScripts() for more details
 						if ( $context->getDebug() && !$context->getOnly() && $module->supportsURLLoading() ) {
-							$styles = $module->getStyleURLsForDebug( $context );
+							$styles = array(
+								'url' => $module->getStyleURLsForDebug( $context )
+							);
 						} else {
 							// Minify CSS before embedding in mw.loader.implement call
 							// (unless in debug mode)
 							if ( !$context->getDebug() ) {
 								foreach ( $stylePairs as $media => $style ) {
-									if ( is_string( $style ) ) {
+									// Can be either a string or an array of strings.
+									if ( is_array( $style ) ) {
+										$stylePairs[$media] = array();
+										foreach ( $style as $cssText ) {
+											if ( is_string( $cssText ) ) {
+												$stylePairs[$media][] = $this->filter( 'minify-css', $cssText );
+											}
+										}
+									} elseif ( is_string( $style ) ) {
 										$stylePairs[$media] = $this->filter( 'minify-css', $style );
 									}
 								}
 							}
-							// Combine styles into @media groups as one big string
-							$styles = array( '' => self::makeCombinedStyles( $stylePairs ) );
+							// Wrap styles into @media groups as needed and flatten into a numerical array
+							$styles = array(
+								'css' => self::makeCombinedStyles( $stylePairs )
+							);
 						}
 					}
 				}
@@ -752,11 +764,10 @@ class ResourceLoader {
 						}
 						break;
 					case 'styles':
-						// We no longer seperate into media, they are all concatenated now with
-						// custom media type groups into @media .. {} sections.
-						// Module returns either an empty array or an array with '' (no media type) as
-						// only key.
-						$out .= isset( $styles[''] ) ? $styles[''] : '';
+						// We no longer seperate into media, they are all combined now with
+						// custom media type groups into @media .. {} sections as part of the css string.
+						// Module returns either an empty array or a numerical array with css strings.
+						$out .= isset( $styles['css'] ) ? implode( '', $styles['css'] ) : '';
 						break;
 					case 'messages':
 						$out .= self::makeMessageSetScript( new XmlJsCode( $messagesBlob ) );
@@ -860,24 +871,32 @@ class ResourceLoader {
 	 * Combines an associative array mapping media type to CSS into a
 	 * single stylesheet with "@media" blocks.
 	 *
-	 * @param $styles Array: List of CSS strings keyed by media type
+	 * @param $styles Array: Array keyed by media type containing (arrays of) CSS strings.
 	 *
-	 * @return string
+	 * @return Array
 	 */
-	public static function makeCombinedStyles( array $styles ) {
-		$out = '';
-		foreach ( $styles as $media => $style ) {
-			// Transform the media type based on request params and config
-			// The way that this relies on $wgRequest to propagate request params is slightly evil
-			$media = OutputPage::transformCssMedia( $media );
-
-			if ( $media === null ) {
-				// Skip
-			} elseif ( $media === '' || $media == 'all' ) {
-				// Don't output invalid or frivolous @media statements
-				$out .= "$style\n";
-			} else {
-				$out .= "@media $media {\n" . str_replace( "\n", "\n\t", "\t" . $style ) . "\n}\n";
+	private static function makeCombinedStyles( array $stylePairs ) {
+		$out = array();
+		foreach ( $stylePairs as $media => $styles ) {
+			// ResourceLoaderFileModule::getStyle can return the styles
+			// as a string or an array of strings. This is to allow separation in
+			// the front-end.
+			$styles = (array) $styles;
+			foreach ( $styles as $style ) {
+				$style = trim( $style );
+				// Don't output an empty "@media print { }" block (bug 40498)
+				if ( $style !== '' ) {
+					// Transform the media type based on request params and config
+					// The way that this relies on $wgRequest to propagate request params is slightly evil
+					$media = OutputPage::transformCssMedia( $media );
+
+					if ( $media === '' || $media == 'all' ) {
+						$out[] = $style;
+					} else if ( is_string( $media ) ) {
+						$out[] = "@media $media {\n" . str_replace( "\n", "\n\t", "\t" . $style ) . "}";
+					}
+					// else: skip
+				}
 			}
 		}
 		return $out;
diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index f0892ec2a8..d0c56ae818 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -634,7 +634,8 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
 		// Get and register local file references
 		$this->localFileRefs = array_merge(
 			$this->localFileRefs,
-			CSSMin::getLocalFileReferences( $style, $dir ) );
+			CSSMin::getLocalFileReferences( $style, $dir )
+		);
 		return CSSMin::remap(
 			$style, $dir, $remoteDir, true
 		);
diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php
index 68f4a282ec..a1030bd77c 100644
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -172,7 +172,9 @@ abstract class ResourceLoaderModule {
 	 * Get all CSS for this module for a given skin.
 	 *
 	 * @param $context ResourceLoaderContext: Context object
-	 * @return Array: List of CSS strings keyed by media type
+	 * @return Array: List of CSS strings or array of CSS strings keyed by media type.
+	 *  like array( 'screen' => '.foo { width: 0 }' );
+	 *  or array( 'screen' => array( '.foo { width: 0 }' ) );
 	 */
 	public function getStyles( ResourceLoaderContext $context ) {
 		// Stub, override expected
diff --git a/includes/resourceloader/ResourceLoaderWikiModule.php b/includes/resourceloader/ResourceLoaderWikiModule.php
index 623a26924e..0f09fc3435 100644
--- a/includes/resourceloader/ResourceLoaderWikiModule.php
+++ b/includes/resourceloader/ResourceLoaderWikiModule.php
@@ -138,12 +138,12 @@ abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
 			}
 			$style = CSSMin::remap( $style, false, $wgScriptPath, true );
 			if ( !isset( $styles[$media] ) ) {
-				$styles[$media] = '';
+				$styles[$media] = array();
 			}
 			if ( strpos( $titleText, '*/' ) === false ) {
-				$styles[$media] .=  "/* $titleText */\n";
+				$style =  "/* $titleText */\n" . $style;
 			}
-			$styles[$media] .= $style . "\n";
+			$styles[$media][] = $style;
 		}
 		return $styles;
 	}
diff --git a/resources/mediawiki/mediawiki.js b/resources/mediawiki/mediawiki.js
index f0e464dd65..b48462965f 100644
--- a/resources/mediawiki/mediawiki.js
+++ b/resources/mediawiki/mediawiki.js
@@ -393,48 +393,82 @@ var mw = ( function ( $, undefined ) {
 			 * Create a new style tag and add it to the DOM.
 			 *
 			 * @param text String: CSS text
-			 * @param $nextnode mixed: [optional] An Element or jQuery object for an element where
+			 * @param nextnode mixed: [optional] An Element or jQuery object for an element where
 			 * the style tag should be inserted before. Otherwise appended to the <head>.
 			 * @return HTMLStyleElement
 			 */
-			function addStyleTag( text, $nextnode ) {
+			function addStyleTag( text, nextnode ) {
 				var s = document.createElement( 'style' );
-				s.type = 'text/css';
-				s.rel = 'stylesheet';
 				// Insert into document before setting cssText (bug 33305)
-				if ( $nextnode ) {
-					// If a raw element, create a jQuery object, otherwise use directly
-					if ( $nextnode.nodeType ) {
-						$nextnode = $( $nextnode );
+				if ( nextnode ) {
+					// Must be inserted with native insertBefore, not $.fn.before.
+					// When using jQuery to insert it, like $nextnode.before( s ),
+					// then IE6 will throw "Access is denied" when trying to append
+					// to .cssText later. Some kind of weird security measure.
+					// http://stackoverflow.com/q/12586482/319266
+					// Works: jsfiddle.net/zJzMy/1
+					// Fails: jsfiddle.net/uJTQz
+					// Works again: http://jsfiddle.net/Azr4w/ (diff: the next 3 lines)
+					if ( nextnode.jquery ) {
+						nextnode = nextnode.get( 0 );
 					}
-					$nextnode.before( s );
+					nextnode.parentNode.insertBefore( s, nextnode );
 				} else {
-					document.getElementsByTagName('head')[0].appendChild( s );
+					document.getElementsByTagName( 'head' )[0].appendChild( s );
 				}
 				if ( s.styleSheet ) {
-					s.styleSheet.cssText = text; // IE
+					// IE
+					s.styleSheet.cssText = text;
 				} else {
-					// Safari sometimes borks on null
+					// Other browsers.
+					// (Safari sometimes borks on non-string values,
+					// play safe by casting to a string, just in case.)
 					s.appendChild( document.createTextNode( String( text ) ) );
 				}
 				return s;
 			}
 
-			function addInlineCSS( css ) {
-				var $style, style, $newStyle;
+			/**
+			 * Checks if certain cssText is safe to append to
+			 * a stylesheet.
+			 *
+			 * Right now it only makes sure that cssText containing @import
+			 * rules will end up in a new stylesheet (as those only work when
+			 * placed at the start of a stylesheet; bug 35562).
+			 * This could later be extended to take care of other bugs, such as
+			 * the IE cssRules limit - not the same as the IE styleSheets limit).
+			 */
+			function canExpandStylesheetWith( $style, cssText ) {
+				return cssText.indexOf( '@import' ) === -1;
+			}
+
+			function addEmbeddedCSS( cssText ) {
+				var $style, styleEl;
 				$style = getMarker().prev();
-				// Disable <style> tag recycling/concatenation because of bug 34669
-				if ( false && $style.is( 'style' ) && $style.data( 'ResourceLoaderDynamicStyleTag' ) === true ) {
-					// There's already a dynamic <style> tag present, append to it. This recycling of
-					// <style> tags is for bug 31676 (can't have more than 32 <style> tags in IE)
-					style = $style.get( 0 );
-					if ( style.styleSheet ) {
-						style.styleSheet.cssText += css; // IE
+				// Re-use <style> tags if possible, this to try to stay
+				// under the IE stylesheet limit (bug 31676).
+				// Also verify that the the element before Marker actually is one
+				// that came from ResourceLoader, and not a style tag that some
+				// other script inserted before our marker, or, more importantly,
+				// it may not be a style tag at all (could be <meta> or <script>).
+				if (
+					$style.data( 'ResourceLoaderDynamicStyleTag' ) === true &&
+					canExpandStylesheetWith( $style, cssText )
+				) {
+					// There's already a dynamic <style> tag present and
+					// canExpandStylesheetWith() gave a green light to append more to it.
+					styleEl = $style.get( 0 );
+					if ( styleEl.styleSheet ) {
+						try {
+							styleEl.styleSheet.cssText += cssText; // IE
+						} catch ( e ) {
+							log( 'addEmbeddedCSS fail\ne.message: ' + e.message, e );
+						}
 					} else {
-						style.appendChild( document.createTextNode( String( css ) ) );
+						styleEl.appendChild( document.createTextNode( String( cssText ) ) );
 					}
 				} else {
-					$newStyle = $( addStyleTag( css, getMarker() ) )
+					$( addStyleTag( cssText, getMarker() ) )
 						.data( 'ResourceLoaderDynamicStyleTag', true );
 				}
 			}
@@ -794,7 +828,7 @@ var mw = ( function ( $, undefined ) {
 			 * @param module string module name to execute
 			 */
 			function execute( module ) {
-				var style, media, i, script, markModuleReady, nestedAddScript;
+				var key, value, media, i, script, markModuleReady, nestedAddScript;
 
 				if ( registry[module] === undefined ) {
 					throw new Error( 'Module has not been registered yet: ' + module );
@@ -806,28 +840,72 @@ var mw = ( function ( $, undefined ) {
 					throw new Error( 'Module has already been loaded: ' + module );
 				}
 
-				// Add styles
+				/**
+				 * Define loop-function here for efficiency
+				 * and to avoid re-using badly scoped variables.
+				 */
+				function addLink( media, url ) {
+					var el = document.createElement( 'link' );
+					getMarker().before( el ); // IE: Insert in dom before setting href
+					el.rel = 'stylesheet';
+					if ( media && media !== 'all' ) {
+						el.media = media;
+					}
+					el.href = url;
+				}
+
+				// Process styles (see also mw.loader.implement)
+				// * back-compat: { <media>: css }
+				// * back-compat: { <media>: [url, ..] }
+				// * { "css": [css, ..] }
+				// * { "url": { <media>: [url, ..] } }
 				if ( $.isPlainObject( registry[module].style ) ) {
-					// 'media' type ignored, see documentation of mw.loader.implement
-					for ( media in registry[module].style ) {
-						style = registry[module].style[media];
-						if ( $.isArray( style ) ) {
-							for ( i = 0; i < style.length; i += 1 ) {
-								getMarker().before( mw.html.element( 'link', {
-									'type': 'text/css',
-									'rel': 'stylesheet',
-									'href': style[i]
-								} ) );
+					for ( key in registry[module].style ) {
+						value = registry[module].style[key];
+						media = undefined;
+
+						if ( key !== 'url' && key !== 'css' ) {
+							// Backwards compatibility, key is a media-type
+							if ( typeof value === 'string' ) {
+								// back-compat: { <media>: css }
+								// Ignore 'media' because it isn't supported (nor was it used).
+								// Strings are pre-wrapped in "@media". The media-type was just ""
+								// (because it had to be set to something).
+								// This is one of the reasons why this format is no longer used.
+								addEmbeddedCSS( value );
+							} else {
+								// back-compat: { <media>: [url, ..] }
+								media = key;
+								key = 'bc-url';
 							}
-						} else if ( typeof style === 'string' ) {
-							addInlineCSS( style );
+						}
+
+						// Array of css strings in key 'css',
+						// or back-compat array of urls from media-type
+						if ( $.isArray( value ) ) {
+							for ( i = 0; i < value.length; i += 1 ) {
+								if ( key === 'bc-url' ) {
+									// back-compat: { <media>: [url, ..] }
+									addLink( media, value[i] );
+								} else if ( key === 'css' ) {
+									// { "css": [css, ..] }
+									addEmbeddedCSS( value[i] );
+								}
+							}
+						// Not an array, but a regular object
+						// Array of urls inside media-type key
+						} else if ( typeof value === 'object' ) {
+							// { "url": { <media>: [url, ..] } }
+							$.each( value, addLink );
 						}
 					}
 				}
+
 				// Add localizations to message system
 				if ( $.isPlainObject( registry[module].messages ) ) {
 					mw.messages.set( registry[module].messages );
 				}
+
 				// Execute script
 				try {
 					script = registry[module].script;
@@ -860,7 +938,7 @@ var mw = ( function ( $, undefined ) {
 				} catch ( e ) {
 					// This needs to NOT use mw.log because these errors are common in production mode
 					// and not in debug mode, such as when a symbol that should be global isn't exported
-					log('mw.loader::execute> Exception thrown by ' + module + ': ' + e.message, e);
+					log( 'Exception thrown by ' + module + ': ' + e.message, e );
 					registry[module].state = 'error';
 					handlePending( module );
 				}
@@ -1182,17 +1260,20 @@ var mw = ( function ( $, undefined ) {
 				 *
 				 * All arguments are required.
 				 *
-				 * @param module String: Name of module
-				 * @param script Mixed: Function of module code or String of URL to be used as the src
-				 *  attribute when adding a script element to the body
-				 * @param style Object: Object of CSS strings keyed by media-type or Object of lists of URLs
-				 *  keyed by media-type. Media-type should be "all" or "", actual types are not supported
-				 *  right now due to the way execute() processes the stylesheets (they are concatenated
-				 *  into a single <style> tag). In the past these weren't concatenated together (which is
-				 *  these are keyed by media-type),  but bug 31676 forces us to. In practice this is not a
-				 *  problem because ResourceLoader only generates stylesheets for media-type all (e.g. print
-				 *  stylesheets are wrapped in @media print {} and concatenated with the others).
-				 * @param msgs Object: List of key/value pairs to be passed through mw.messages.set
+				 * @param {String} module Name of module
+				 * @param {Function|Array} script Function with module code or Array of URLs to
+				 *  be used as the src attribute of a new <script> tag.
+				 * @param {Object} style Should follow one of the following patterns:
+				 *  { "css": [css, ..] }
+				 *  { "url": { <media>: [url, ..] } }
+				 *  And for backwards compatibility (needs to be supported forever due to caching):
+				 *  { <media>: css }
+				 *  { <media>: [url, ..] }
+				 *
+				 *  The reason css strings are not concatenated anymore is bug 31676. We now check
+				 *  whether it's safe to extend the stylesheet (see canExpandStylesheetWith).
+				 *
+				 * @param {Object} msgs List of key/value pairs to be passed through mw.messages.set
 				 */
 				implement: function ( module, script, style, msgs ) {
 					// Validate input
diff --git a/tests/qunit/data/styleTest.css.php b/tests/qunit/data/styleTest.css.php
index 093c71f150..1870d5a38d 100644
--- a/tests/qunit/data/styleTest.css.php
+++ b/tests/qunit/data/styleTest.css.php
@@ -25,14 +25,15 @@
 header( 'Content-Type: text/css; charset=utf-8' );
 
 /**
- * Allows characters in ranges [a-z], [A-Z], [0-9] as well as a dot ("."), dash ("-") and hash ("#").
+ * Allows characters in ranges [a-z], [A-Z] and [0-9],
+ * in addition to a dot ("."), dash ("-"), space (" ") and hash ("#").
  * @since 1.20
  *
- * @param $val string
- * @return string: input with illigal characters removed 
+ * @param string $val
+ * @return string Value with any illegal characters removed.
  */
 function cssfilter( $val ) {
-	return preg_replace( '/[^A-Za-z0-9\.\-#]/', '', $val );
+	return preg_replace( '/[^A-Za-z0-9\.\- #]/', '', $val );
 }
 
 // Do basic sanitization
diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
index 58e56adf4b..806058d2e7 100644
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.test.js
@@ -1,6 +1,8 @@
+( function ( mw ) {
+
 QUnit.module( 'mediawiki', QUnit.newMwEnvironment() );
 
-QUnit.test( '-- Initial check', 8, function ( assert ) {
+QUnit.test( 'Initial check', 8, function ( assert ) {
 	assert.ok( window.jQuery, 'jQuery defined' );
 	assert.ok( window.$, '$j defined' );
 	assert.ok( window.$j, '$j defined' );
@@ -137,6 +139,67 @@ QUnit.test( 'mw.msg', 11, function ( assert ) {
 
 });
 
+/**
+ * The sync style load test (for @import). This is, in a way, also an open bug for
+ * ResourceLoader ("execute js after styles are loaded"), but browsers don't offer a
+ * way to get a callback from when a stylesheet is loaded (that is, including any
+ * @import rules inside). To work around this, we'll have a little time loop to check
+ * if the styles apply.
+ * Note: This test originally used new Image() and onerror to get a callback
+ * when the url is loaded, but that is fragile since it doesn't monitor the
+ * same request as the css @import, and Safari 4 has issues with
+ * onerror/onload not being fired at all in weird cases like this.
+ */
+function assertStyleAsync( assert, $element, prop, val, fn ) {
+	var styleTestStart,
+		el = $element.get( 0 ),
+		styleTestTimeout = ( QUnit.config.testTimeout - 200 ) || 5000;
+
+	function isCssImportApplied() {
+		// Trigger reflow, repaint, redraw, whatever (cross-browser)
+		var x = $element.css( 'height' );
+		x = el.innerHTML;
+		el.className = el.className;
+		x = document.documentElement.clientHeight;
+
+		return $element.css( prop ) === val;
+	}
+
+	function styleTestLoop() {
+		var styleTestSince = new Date().getTime() - styleTestStart;
+		// 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)'
+			);
+
+			if ( fn ) {
+				fn();
+			}
+
+			return;
+		}
+		// Otherwise, keep polling
+		setTimeout( styleTestLoop, 150 );
+	}
+
+	// Start the loop
+	styleTestStart = new Date().getTime();
+	styleTestLoop();
+}
+
+function urlStyleTest( selector, prop, val ) {
+	return QUnit.fixurl(
+		mw.config.get( 'wgScriptPath' ) +
+			'/tests/qunit/data/styleTest.css.php?' +
+			$.param( {
+				selector: selector,
+				prop: prop,
+				val: val
+			} )
+	);
+}
+
 QUnit.asyncTest( 'mw.loader', 2, function ( assert ) {
 	var isAwesomeDone;
 
@@ -161,83 +224,176 @@ QUnit.asyncTest( 'mw.loader', 2, function ( assert ) {
 	});
 });
 
-QUnit.asyncTest( 'mw.loader.implement', 5, function ( assert ) {
-	var isJsExecuted, $element, styleTestUrl;
-
-	styleTestUrl = QUnit.fixurl(
-		mw.config.get( 'wgScriptPath' )
-		+ '/tests/qunit/data/styleTest.css.php?'
-		+ $.param({
-			selector: '.mw-test-loaderimplement',
-			prop: 'float',
-			val: 'right'
-		})
+QUnit.test( 'mw.loader.implement( styles={ "css": [text, ..] } )', 2, function ( assert ) {
+	var $element = $( '<div class="mw-test-implement-a"></div>' ).appendTo( '#qunit-fixture' );
+
+	assert.notEqual(
+		$element.css( 'float' ),
+		'right',
+		'style is clear'
 	);
 
 	mw.loader.implement(
-		'test.implement',
+		'test.implement.a',
 		function () {
-			var styleTestTimeout, styleTestStart, styleTestSince;
+			assert.equal(
+				$element.css( 'float' ),
+				'right',
+				'style is applied'
+			);
+		},
+		{
+			'all': '.mw-test-implement-a { float: right; }'
+		},
+		{}
+	);
+
+	mw.loader.load([
+		'test.implement.a'
+	]);
+} );
 
+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' );
+
+	assert.notEqual(
+		$element.css( 'float' ),
+		'right',
+		'style is clear'
+	);
+	assert.notEqual(
+		$element2.css( 'text-align' ),
+		'center',
+		'style is clear'
+	);
+
+	mw.loader.implement(
+		'test.implement.b',
+		function () {
+			assertStyleAsync( assert, $element, 'float', 'right', function () {
+
+				assert.notEqual( $element2.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' )]
+			}
+		},
+		{}
+	);
+
+	mw.loader.load([
+		'test.implement.b'
+	]);
+} );
+
+// Backwards compatibility
+QUnit.test( 'mw.loader.implement( styles={ <media>: text } ) (back-compat)', 2, function ( assert ) {
+	var $element = $( '<div class="mw-test-implement-c"></div>' ).appendTo( '#qunit-fixture' );
+
+	assert.notEqual(
+		$element.css( 'float' ),
+		'right',
+		'style is clear'
+	);
+
+	mw.loader.implement(
+		'test.implement.c',
+		function () {
+			assert.equal(
+				$element.css( 'float' ),
+				'right',
+				'style is applied'
+			);
+		},
+		{
+			'all': '.mw-test-implement-c { float: right; }'
+		},
+		{}
+	);
+
+	mw.loader.load([
+		'test.implement.c'
+	]);
+} );
+
+// Backwards compatibility
+QUnit.asyncTest( 'mw.loader.implement( styles={ <media>: [url, ..] } ) (back-compat)', 4, function ( assert ) {
+	var $element = $( '<div class="mw-test-implement-d"></div>' ).appendTo( '#qunit-fixture' ),
+		$element2 = $( '<div class="mw-test-implement-d2"></div>' ).appendTo( '#qunit-fixture' );
+
+	assert.notEqual(
+		$element.css( 'float' ),
+		'right',
+		'style is clear'
+	);
+	assert.notEqual(
+		$element2.css( 'text-align' ),
+		'center',
+		'style is clear'
+	);
+
+	mw.loader.implement(
+		'test.implement.d',
+		function () {
+			assertStyleAsync( assert, $element, 'float', 'right', function () {
+
+				assert.notEqual( $element2.css( 'text-align' ), 'center', 'print style is not applied (bug 40500)' );
+
+				QUnit.start();
+			} );
+		},
+		{
+			'all': [urlStyleTest( '.mw-test-implement-d', 'float', 'right' )],
+			'print': [urlStyleTest( '.mw-test-implement-d2', 'text-align', 'center' )]
+		},
+		{}
+	);
+
+	mw.loader.load([
+		'test.implement.d'
+	]);
+} );
+
+// @import (bug 31676)
+QUnit.asyncTest( 'mw.loader.implement( styles has @import)', 5, function ( assert ) {
+	var isJsExecuted, $element;
+
+	mw.loader.implement(
+		'test.implement.import',
+		function () {
 			assert.strictEqual( isJsExecuted, undefined, 'javascript not executed multiple times' );
 			isJsExecuted = true;
 
-			assert.equal( mw.loader.getState( 'test.implement' ), 'ready', 'module state is "ready" while implement() is executing javascript' );
+			assert.equal( mw.loader.getState( 'test.implement.import' ), 'ready', 'module state is "ready" while implement() is executing javascript' );
 
-			$element = $( '<div class="mw-test-loaderimplement">Foo bar</div>' ).appendTo( '#qunit-fixture' );
+			$element = $( '<div class="mw-test-implement-import">Foo bar</div>' ).appendTo( '#qunit-fixture' );
 
 			assert.equal( mw.msg( 'test-foobar' ), 'Hello Foobar, $1!', 'Messages are loaded before javascript execution' );
 
-			// The @import test. This is, in a way, also an open bug for ResourceLoader
-			// ("execute js after styles are loaded"), but browsers don't offer a way to
-			// get a callback from when a stylesheet is loaded (that is, including any
-			// @import rules inside).
-			// To work around this, we'll have a little time loop to check if the styles
-			// apply.
-			// Note: This test originally used new Image() and onerror to get a callback
-			// when the url is loaded, but that is fragile since it doesn't monitor the
-			// same request as the css @import, and Safari 4 has issues with
-			// onerror/onload not being fired at all in weird cases like this.
-
-			styleTestTimeout = QUnit.config.testTimeout || 5000; // milliseconds
-
-			function isCssImportApplied() {
-				return $element.css( 'float' ) === 'right';
-			}
-
-			function styleTestLoop() {
-				styleTestSince = new Date().getTime() - styleTestStart;
-				// 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( 'float' ), 'right',
-						'CSS stylesheet via @import was applied (after ' + styleTestSince + 'ms) (bug 34669). ("float: right")'
-					);
-
-					assert.equal( $element.css( 'text-align' ),'center',
-						'CSS styles after the @import are working ("text-align: center")'
-					);
-
-					// Async done
-					QUnit.start();
-
-					return;
-				}
-				// Otherwise, keep polling
-				setTimeout( styleTestLoop, 100 );
-			}
+			assertStyleAsync( assert, $element, 'float', 'right', function () {
+				assert.equal( $element.css( 'text-align' ),'center',
+					'CSS styles after the @import rule are working'
+				);
 
-			// Start the loop
-			styleTestStart = new Date().getTime();
-			styleTestLoop();
+				QUnit.start();
+			} );
 		},
 		{
-			"all": "@import url('"
-				+ styleTestUrl
-				+ "');\n"
-				+ '.mw-test-loaderimplement { text-align: center; }'
+			'css': [
+				'@import url(\''
+				+ urlStyleTest( '.mw-test-implement-import', 'float', 'right' )
+				+ '\');\n'
+				+ '.mw-test-implement-import { text-align: center; }'
+			]
 		},
 		{
-			"test-foobar": "Hello Foobar, $1!"
+			'test-foobar': 'Hello Foobar, $1!'
 		}
 	);
 
@@ -245,6 +401,19 @@ QUnit.asyncTest( 'mw.loader.implement', 5, function ( assert ) {
 
 });
 
+QUnit.asyncTest( 'mw.loader.implement( only messages )' , 2, function ( assert ) {
+	assert.assertFalse( mw.messages.exists( 'bug_29107' ), 'Verify that the test message doesn\'t exist yet' );
+
+	mw.loader.implement( 'test.implement.msgs', [], {}, { 'bug_29107': 'loaded' } );
+	mw.loader.using( 'test.implement.msgs', function() {
+		QUnit.start();
+		assert.ok( mw.messages.exists( 'bug_29107' ), 'Bug 29107: messages-only module should implement ok' );
+	}, function() {
+		QUnit.start();
+		assert.ok( false, 'Error callback fired while implementing "test.implement.msgs" module' );
+	});
+});
+
 QUnit.test( 'mw.loader erroneous indirect dependency', 3, function ( assert ) {
 	mw.loader.register( [
 		['test.module1', '0'],
@@ -368,21 +537,7 @@ QUnit.asyncTest( 'mw.loader dependency handling', 5, function ( assert ) {
 	);
 } );
 
-QUnit.asyncTest( 'mw.loader bug29107' , 2, function ( assert ) {
-	// Message doesn't exist already
-	assert.ok( !mw.messages.exists( 'bug29107' ) );
-
-	mw.loader.implement( 'bug29107.messages-only', [], {}, {'bug29107': 'loaded'} );
-	mw.loader.using( 'bug29107.messages-only', function() {
-		QUnit.start();
-		assert.ok( mw.messages.exists( 'bug29107' ), 'Bug 29107: messages-only module should implement ok' );
-	}, function() {
-		QUnit.start();
-		assert.ok( false, 'Error callback fired while implementing "bug29107.messages-only" module' );
-	});
-});
-
-QUnit.asyncTest( 'mw.loader.bug30825', 2, function ( assert ) {
+QUnit.asyncTest( 'mw.loader( "//protocol-relative" ) (bug 30825)', 2, function ( assert ) {
 	// This bug was actually already fixed in 1.18 and later when discovered in 1.17.
 	// Test is for regressions!
 
@@ -476,3 +631,5 @@ QUnit.test( 'mw.html', 13, function ( assert ) {
 		'html.element DIV (attribs + content)' );
 
 });
+
+}( mediaWiki ) );
-- 
2.20.1