mw.loader: Fixes for <script> and <link> loading.
authorTimo Tijhof <ttijhof@wikimedia.org>
Wed, 24 Oct 2012 18:13:26 +0000 (20:13 +0200)
committerGerrit Code Review <gerrit@wikimedia.org>
Mon, 5 Nov 2012 04:16:08 +0000 (04:16 +0000)
addScript:
* Integrate fixes from jQuery where this function is based on.
  * Set properties instead of attributes (faster, shorter and
    less potential bugs)
  * Set async property (why not!)
  * Remove redundant try/catch. Whatever this was for,
    apparently not needed anymore. Follows-up 41d636d3.

mw.loader.load( , text/css ):
* (bug 41331) Security warnings in IE7 for protocol-relative
  urls for stylesheets in mw.loader.load.
  Fixed by using properties instead of attributes. Not sure why.
  Probably related to how IE "normalises" urls when set as
  attribute, which would not happen as much when setting a
  property.

importStylesheetURI:
* Remove redundant .type property, fix space.

Change-Id: I04f1775213633e6822b4f982fd43a35b19d19531

resources/mediawiki/mediawiki.js
skins/common/wikibits.js

index 1a72ed1..690138c 100644 (file)
@@ -758,16 +758,20 @@ var mw = ( function ( $, undefined ) {
                                // Using isReady directly instead of storing it locally from
                                // a $.fn.ready callback (bug 31895).
                                if ( $.isReady || async ) {
-                                       // jQuery's getScript method is NOT better than doing this the old-fashioned way
-                                       // because jQuery will eval the script's code, and errors will not have sane
-                                       // line numbers.
+                                       // Can't use jQuery.getScript because that only uses <script> for cross-domain,
+                                       // it uses XHR and eval for same-domain scripts, which we don't want because it
+                                       // messes up line numbers.
+                                       // The below is based on jQuery ([jquery@1.8.2]/src/ajax/script.js)
+
+                                       // IE-safe way of getting the <head>. document.head isn't supported
+                                       // in old IE, and doesn't work when in the <head>.
+                                       head = document.getElementsByTagName( 'head' )[0] || document.body;
+
                                        script = document.createElement( 'script' );
-                                       script.setAttribute( 'src', src );
-                                       script.setAttribute( 'type', 'text/javascript' );
+                                       script.async = true;
+                                       script.src = src;
                                        if ( $.isFunction( callback ) ) {
-                                               // Attach handlers for all browsers (based on jQuery.ajax)
                                                script.onload = script.onreadystatechange = function () {
-
                                                        if (
                                                                !done
                                                                && (
@@ -775,24 +779,20 @@ var mw = ( function ( $, undefined ) {
                                                                        || /loaded|complete/.test( script.readyState )
                                                                )
                                                        ) {
-
                                                                done = true;
 
-                                                               callback();
+                                                               // Handle memory leak in IE
+                                                               script.onload = script.onreadystatechange = null;
 
-                                                               // Handle memory leak in IE. This seems to fail in
-                                                               // IE7 sometimes (Permission Denied error when
-                                                               // accessing script.parentNode) so wrap it in
-                                                               // a try catch.
-                                                               try {
-                                                                       script.onload = script.onreadystatechange = null;
-                                                                       if ( script.parentNode ) {
-                                                                               script.parentNode.removeChild( script );
-                                                                       }
-
-                                                                       // Dereference the script
-                                                                       script = undefined;
-                                                               } catch ( e ) { }
+                                                               // Remove the script
+                                                               if ( script.parentNode ) {
+                                                                       script.parentNode.removeChild( script );
+                                                               }
+
+                                                               // Dereference the script
+                                                               script = undefined;
+
+                                                               callback();
                                                        }
                                                };
                                        }
@@ -800,20 +800,17 @@ var mw = ( function ( $, undefined ) {
                                        if ( window.opera ) {
                                                // Appending to the <head> blocks rendering completely in Opera,
                                                // so append to the <body> after document ready. This means the
-                                               // scripts only start loading after  the document has been rendered,
+                                               // scripts only start loading after the document has been rendered,
                                                // but so be it. Opera users don't deserve faster web pages if their
-                                               // browser makes it impossible
-                                               $( function () { document.body.appendChild( script ); } );
+                                               // browser makes it impossible.
+                                               $( function () {
+                                                       document.body.appendChild( script );
+                                               } );
                                        } else {
-                                               // IE-safe way of getting the <head> . document.documentElement.head doesn't
-                                               // work in scripts that run in the <head>
-                                               head = document.getElementsByTagName( 'head' )[0];
-                                               ( document.body || head ).appendChild( script );
+                                               head.appendChild( script );
                                        }
                                } else {
-                                       document.write( mw.html.element(
-                                               'script', { 'type': 'text/javascript', 'src': src }, ''
-                                       ) );
+                                       document.write( mw.html.element( 'script', { 'src': src }, '' ) );
                                        if ( $.isFunction( callback ) ) {
                                                // Document.write is synchronous, so this is called when it's done
                                                // FIXME: that's a lie. doc.write isn't actually synchronous
@@ -1127,7 +1124,7 @@ var mw = ( function ( $, undefined ) {
                                                                }
                                                        }
 
-                                                       currReqBase = $.extend( { 'version': formatVersionNumber( maxVersion ) }, reqBase );
+                                                       currReqBase = $.extend( { version: formatVersionNumber( maxVersion ) }, reqBase );
                                                        // For user modules append a user name to the request.
                                                        if ( group === "user" && mw.config.get( 'wgUserName' ) !== null ) {
                                                                currReqBase.user = mw.config.get( 'wgUserName' );
@@ -1242,15 +1239,15 @@ var mw = ( function ( $, undefined ) {
                                        }
                                        // List the module as registered
                                        registry[module] = {
-                                               'version': version !== undefined ? parseInt( version, 10 ) : 0,
-                                               'dependencies': [],
-                                               'group': typeof group === 'string' ? group : null,
-                                               'source': typeof source === 'string' ? source: 'local',
-                                               'state': 'registered'
+                                               version: version !== undefined ? parseInt( version, 10 ) : 0,
+                                               dependencies: [],
+                                               group: typeof group === 'string' ? group : null,
+                                               source: typeof source === 'string' ? source: 'local',
+                                               state: 'registered'
                                        };
                                        if ( typeof dependencies === 'string' ) {
                                                // Allow dependencies to be given as a single module name
-                                               registry[module].dependencies = [dependencies];
+                                               registry[module].dependencies = [ dependencies ];
                                        } else if ( typeof dependencies === 'object' || $.isFunction( dependencies ) ) {
                                                // Allow dependencies to be given as an array of module names
                                                // or a function which returns an array
@@ -1331,7 +1328,7 @@ var mw = ( function ( $, undefined ) {
                                        }
                                        // Allow calling with a single dependency as a string
                                        if ( tod === 'string' ) {
-                                               dependencies = [dependencies];
+                                               dependencies = [ dependencies ];
                                        }
                                        // Resolve entire dependency map
                                        dependencies = resolve( dependencies );
@@ -1366,7 +1363,7 @@ var mw = ( function ( $, undefined ) {
                                 *  be assumed if loading a URL, and false will be assumed otherwise.
                                 */
                                load: function ( modules, type, async ) {
-                                       var filtered, m, module;
+                                       var filtered, m, module, l;
 
                                        // Validate input
                                        if ( typeof modules !== 'object' && typeof modules !== 'string' ) {
@@ -1381,11 +1378,13 @@ var mw = ( function ( $, undefined ) {
                                                                async = true;
                                                        }
                                                        if ( type === 'text/css' ) {
-                                                               $( 'head' ).append( $( '<link>', {
-                                                                       rel: 'stylesheet',
-                                                                       type: 'text/css',
-                                                                       href: modules
-                                                               } ) );
+                                                               // IE7-8 throws security warnings when inserting a <link> tag
+                                                               // with a protocol-relative URL set though attributes (instead of
+                                                               // properties) - when on HTTPS. See also bug #.
+                                                               l = document.createElement( 'link' );
+                                                               l.rel = 'stylesheet';
+                                                               l.href = modules;
+                                                               $( 'head' ).append( l );
                                                                return;
                                                        }
                                                        if ( type === 'text/javascript' || type === undefined ) {
@@ -1396,7 +1395,7 @@ var mw = ( function ( $, undefined ) {
                                                        throw new Error( 'invalid type for external url, must be text/css or text/javascript. not ' + type );
                                                }
                                                // Called with single module
-                                               modules = [modules];
+                                               modules = [ modules ];
                                        }
 
                                        // Filter out undefined modules, otherwise resolve() will throw
@@ -1448,7 +1447,7 @@ var mw = ( function ( $, undefined ) {
                                        if ( registry[module] === undefined ) {
                                                mw.loader.register( module );
                                        }
-                                       if ( $.inArray(state, ['ready', 'error', 'missing']) !== -1
+                                       if ( $.inArray( state, ['ready', 'error', 'missing'] ) !== -1
                                                && registry[module].state !== state ) {
                                                // Make sure pending modules depending on this one get executed if their
                                                // dependencies are now fulfilled!
index cae08af..c2c00db 100644 (file)
@@ -91,10 +91,9 @@ window.importStylesheet = function( page ) {
 
 window.importStylesheetURI = function( url, media ) {
        var l = document.createElement( 'link' );
-       l.type = 'text/css';
        l.rel = 'stylesheet';
        l.href = url;
-       if( media ) {
+       if ( media ) {
                l.media = media;
        }
        document.getElementsByTagName('head')[0].appendChild( l );