From: Timo Tijhof <ttijhof@wikimedia.org>
Date: Wed, 6 Mar 2013 23:02:51 +0000 (+0100)
Subject: mw.loader: Create new style tags instead of appending text
X-Git-Tag: 1.31.0-rc.0~20429^2
X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/operations/recherche.php?a=commitdiff_plain;h=e32786c86b776dacae84cd425f7ba3ee76907c3a;p=lhc%2Fweb%2Fwiklou.git

mw.loader: Create new style tags instead of appending text

As brought to our attention by Chromium/WebKit developers at Google,
when we fixed bug 31676 (IE stylesheet limit) by appending to
<style> whenever we can (i.e. whenever the received stylesheet
does not involve @import which only works on top of the a
stylesheet) – this is causing several slowdowns.

Which build up on mobile to several dozen seconds in pages
with a lot of content and many modules being loaded.

Appending a text node to a <style> tag, while it doesn't require
the DOM to do anything fancy (no need to reparse the contents).
The stylesheet handler of the browser has to re-parse the css text
after each modification as the css syntax is too tolerant to be
able to just pick up parsing again (at least, as of writing
neither Gecko or WebKit are able to do so).

As a result, the stylesheet is invalidated, re-parsed and
re-applied to the page.

So instead default the other way around and only re-use a <style>
tag if we have to do so for IE.

Bug: 45810
Change-Id: I52252e699a518dc1c1327ee598a9e023cc2555e2
---

diff --git a/resources/mediawiki/mediawiki.js b/resources/mediawiki/mediawiki.js
index 4dbf04cc73..af1204710e 100644
--- a/resources/mediawiki/mediawiki.js
+++ b/resources/mediawiki/mediawiki.js
@@ -452,52 +452,51 @@ var mw = ( function ( $, undefined ) {
 			}
 
 			/**
-			 * Checks if certain cssText is safe to append to
-			 * a stylesheet.
+			 * Checks whether it is safe to add this css 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).
 			 * @private
-			 * @param {jQuery} $style
 			 * @param {string} cssText
-			 * @return {boolean}
+			 * @return {boolean} False if a new one must be created.
 			 */
-			function canExpandStylesheetWith( $style, cssText ) {
+			function canExpandStylesheetWith( cssText ) {
+				// 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).
 				return cssText.indexOf( '@import' ) === -1;
 			}
 
 			function addEmbeddedCSS( cssText ) {
 				var $style, styleEl;
-				$style = getMarker().prev();
-				// 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 );
+
+				// By default, always create a new <style>. Appending text
+				// to a <style> tag means the contents have to be re-parsed (bug 45810).
+				// Except, of course, in IE below 9, in there we default to
+				// re-using and appending to a <style> tag due to the
+				// IE stylesheet limit (bug 31676).
+				if ( ( 'documentMode' in document && document.documentMode <= 9 ) && canExpandStylesheetWith( cssText ) ) {
+
+					$style = getMarker().prev();
+					// Verify that the the element before Marker actually is a
+					// <style> tag and one that came from ResourceLoader
+					// (not some other style tag or even a `<meta>` or `<script>`).
+					if ( $style.data( 'ResourceLoaderDynamicStyleTag' ) === true ) {
+						// 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 {
+							styleEl.appendChild( document.createTextNode( String( cssText ) ) );
 						}
-					} else {
-						styleEl.appendChild( document.createTextNode( String( cssText ) ) );
+						return;
 					}
-				} else {
-					$( addStyleTag( cssText, getMarker() ) )
-						.data( 'ResourceLoaderDynamicStyleTag', true );
 				}
+
+				$( addStyleTag( cssText, getMarker() ) ).data( 'ResourceLoaderDynamicStyleTag', true );
 			}
 
 			/**