r112370 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r112369‎ | r112370 | r112371 >
Date:23:22, 24 February 2012
Author:krinkle
Status:ok
Tags:
Comment:
[mw.loader] Refactor addInlineCSS's logic, fixing various bugs

* Using mw.util.addCSS as base to instance of bug 33305 automatically
* Expose as mw.loader.addStyleTag
* Re-use the code in mw.util.addCSS
* Drop the "text > Cdata > element > jQuery object > innerHTML", in favor of setting cssText or appending a text node (html escapement isn't a problem when working with text nodes directly).
* Appending this way also works in IE, no need for the dispose/re-create style-tag logic in the try-catch().

* Follows-up r110988 (fixme)
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.util.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.util.js
@@ -149,23 +149,24 @@
150150 },
151151
152152 /**
153 - * Append a new style block to the head
 153+ * Append a new style block to the head and return the CSSStyleSheet object.
 154+ * Use .ownerNode to access the <style> element, or use mw.loader.addStyleTag.
 155+ * This function returns the styleSheet object for convience (due to cross-browsers
 156+ * difference as to where it is located).
 157+ * @example
 158+ * <code>
 159+ * var sheet = mw.util.addCSS('.foobar { display: none; }');
 160+ * $(foo).click(function () {
 161+ * // Toggle the sheet on and off
 162+ * sheet.disabled = !sheet.disabled;
 163+ * });
 164+ * </code>
154165 *
155166 * @param text string CSS to be appended
156 - * @return CSSStyleSheet
 167+ * @return CSSStyleSheet (use .ownerNode to get to the <style> element)
157168 */
158169 addCSS: function ( text ) {
159 - var s = document.createElement( 'style' );
160 - s.type = 'text/css';
161 - s.rel = 'stylesheet';
162 - // Insert into document before setting cssText (bug 33305)
163 - document.getElementsByTagName('head')[0].appendChild( s );
164 - if ( s.styleSheet ) {
165 - s.styleSheet.cssText = text; // IE
166 - } else {
167 - // Safari sometimes borks on null
168 - s.appendChild( document.createTextNode( String( text ) ) );
169 - }
 170+ var s = mw.loader.addStyleTag( text );
170171 return s.sheet || s;
171172 },
172173
Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -389,41 +389,57 @@
390390 return $marker;
391391 }
392392 }
393 -
 393+
 394+ /**
 395+ * Create a new style tag and add it to the DOM.
 396+ *
 397+ * @param text String: CSS text
 398+ * @param $nextnode mixed: [optional] An Element or jQuery object for an element where
 399+ * the style tag should be inserted before. Otherwise appended to the <head>.
 400+ * @return HTMLStyleElement
 401+ */
 402+ function addStyleTag( text, $nextnode ) {
 403+ var s = document.createElement( 'style' );
 404+ s.type = 'text/css';
 405+ s.rel = 'stylesheet';
 406+ // Insert into document before setting cssText (bug 33305)
 407+ if ( $nextnode ) {
 408+ // If a raw element, create a jQuery object, otherwise use directly
 409+ if ( $nextnode.nodeType ) {
 410+ $nextnode = $( $nextnode );
 411+ }
 412+ $nextnode.before( s );
 413+ } else {
 414+ document.getElementsByTagName('head')[0].appendChild( s );
 415+ }
 416+ if ( s.styleSheet ) {
 417+ s.styleSheet.cssText = text; // IE
 418+ } else {
 419+ // Safari sometimes borks on null
 420+ s.appendChild( document.createTextNode( String( text ) ) );
 421+ }
 422+ return s;
 423+ }
 424+
394425 function addInlineCSS( css, media ) {
395 - var $style = getMarker().prev(),
396 - $newStyle,
397 - attrs = { 'type': 'text/css', 'media': media };
 426+ var $style, style, $newStyle;
 427+ $style = getMarker().prev();
398428 if ( $style.is( 'style' ) && $style.data( 'ResourceLoaderDynamicStyleTag' ) === true ) {
399 - // There's already a dynamic <style> tag present, append to it
400 - // This recycling of <style> tags is for bug 31676 (can't have
401 - // more than 32 <style> tags in IE)
402 -
403 - // Also, calling .append() on a <style> tag explodes with a JS error in IE,
404 - // so if the .append() fails we fall back to building a new <style> tag and
405 - // replacing the existing one
406 - try {
407 - // Do cdata sanitization on the provided CSS, and prepend a double newline
408 - css = $( mw.html.element( 'style', {}, new mw.html.Cdata( "\n\n" + css ) ) ).html();
409 - $style.append( css );
410 - } catch ( e ) {
411 - // Generate a new tag with the combined CSS
412 - css = $style.html() + "\n\n" + css;
413 - $newStyle = $( mw.html.element( 'style', attrs, new mw.html.Cdata( css ) ) )
414 - .data( 'ResourceLoaderDynamicStyleTag', true );
415 - // Prevent a flash of unstyled content by inserting the new tag
416 - // before removing the old one
417 - $style.after( $newStyle );
418 - $style.remove();
 429+ // There's already a dynamic <style> tag present, append to it. This recycling of
 430+ // <style> tags is for bug 31676 (can't have more than 32 <style> tags in IE)
 431+ style = $style.get( 0 );
 432+ if ( style.styleSheet ) {
 433+ style.styleSheet.cssText += css; // IE
 434+ } else {
 435+ style.appendChild( document.createTextNode( String( css ) ) );
419436 }
420437 } else {
421 - // Create a new <style> tag and insert it
422 - $style = $( mw.html.element( 'style', attrs, new mw.html.Cdata( css ) ) );
423 - $style.data( 'ResourceLoaderDynamicStyleTag', true );
424 - getMarker().before( $style );
 438+ $newStyle = $( addStyleTag( css, getMarker() ) )
 439+ .attr( 'media', media )
 440+ .data( 'ResourceLoaderDynamicStyleTag', true );
425441 }
426442 }
427 -
 443+
428444 function compare( a, b ) {
429445 var i;
430446 if ( a.length !== b.length ) {
@@ -874,6 +890,8 @@
875891
876892 /* Public Methods */
877893 return {
 894+ addStyleTag: addStyleTag,
 895+
878896 /**
879897 * Requests dependencies from server, loading and executing when things when ready.
880898 */

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r106992[Core JS] mw.util.addCSS: Insert style tag into dom before setting cssText...krinkle22:08, 21 December 2011
r107043Use mw.util.addCSS for adding css. Based on bug 33305, and fixed in r106992.santhosh08:49, 22 December 2011
r110988(bug 31676) Group dynamically inserted CSS into a single <style> tag, because...catrope00:10, 9 February 2012

Status & tagging log