Page MenuHomePhabricator

mw.loader.load() fails in IE8 when loading external stylesheet
Closed, ResolvedPublic

Description

External stylesheet are not applied in IE 8 (and 7) when that stylesheet is loaded through mw.loader.load(). The link does not appear inside <head> until after the DOM is refreshed in the console (after which, it is still not applied). importStylesheetURI does not exhibit this problem.


Version: unspecified
Severity: normal
URL: http://en.wikipedia.org/wiki/User:Edokter/MenuTabsToggle.js

Details

Reference
bz41331

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 1:11 AM
bzimport set Reference to bz41331.

Reproduce:

Execute the following in WinXP/IE8:

mw.loader.load('//example.org/some-styles.css', 'text/css');

As a result, it does add the <link> tag but the styles aren't applied.

Also, I noticed IE8/WinXP seems to be throwing security warnings when dynamically inserting <link> from the console with a protocol relative url when on HTTPS. Not sure if that is related.

Lowering priority as ResourceLoader doesn't actually use this syntax anywhere (not core, not extensions, not gadgets).

It was added as a convenience method for migrating away from the deprecated importStylesheetURI.

However now that I think about it, it seems that was a mistake. As proper migration to ResourceLoader involves letting ResourceLoader manage the stylesheet in the first place, so it seems kinda useless to change a script from importStylesheetURI to mw.loader.load, it doesn't really change the fact that it is still a globally-executing-raw-legacy-loading script.

importScriptURI and importStylesheetURI are officially deprecated since 1.17 according to http://www.mediawiki.org/wiki/ResourceLoader/JavaScript_Deprecations#wikibits.js. Until ResourceLoader can load inter-wiki (user)scripts/sheets, I don't consider mw.loader.load loading external scripts a 'convenience'.

The difference I notice between mw.loader.load and importStylesheetURI is the method used to add the script; mw.loader.load uses jQuery to select the head and create the link, and only does so when loading an external stylesheet. May I suggest trying:

if ( type === 'text/css' ) {
var link = document.createElement( 'link' );
link.type = 'text/css';
link.rel = 'stylesheet';
link.href = modules;
document.getElementsByTagName( 'head' )[0].appendChild( link );
return;
}

...instead to see if that eliviates the problem?

Ooh, this is interesting: using 'http://' instead of just '' does work. Either jQuery or IE8 has a problem with protocol relative URLs. As importStylesheetURI does work with '', my bet is on jQuery.

(In reply to comment #3)

importScriptURI and importStylesheetURI are officially deprecated since 1.17
according to
http://www.mediawiki.org/wiki/ResourceLoader/JavaScript_Deprecations#wikibits.js.
Until ResourceLoader can load inter-wiki (user)scripts/sheets, I don't consider
mw.loader.load loading external scripts a 'convenience'.

These deprecated methods work fine, there is no need to use something else (other than migrating away from the legacy loading system entirely).

ResourceLoader will never provide a way to load interwiki files because that's not what it should be doing. ResourceLoader loads modules. The proper migration path is to convert the soup of raw files into a module and load that.

Right now there is no way to register a module on the user-level. Until there is a way to create modules on the user level (which is the only use left for the legacy system), all the methods that support this system will stay too.

And when that feature exists[1], user scripts can be converted to modules at which point importStylesheetURI and mw.loader.load are both irrelevant.

Hence the recommendation to stick to the deprecated methods instead of making a invisible change by using differently named methods.

May I suggest trying:
[..]

Indeed, that seems to improve the situation. I was already on that in a local branch (the branch I use to reduce usage of jQuery in the ResourceLoader client to, eventually, make it standalone separate from mediawiki.js).

I'll extract that bit and commit it and see if that makes IE8 happy.

  • Krinkle

[1] For the record, don't confuse user-level modules with what Gadgets 2.0 is bringing, Gadgets 2.0 is "just" bringing existing gadgets to the next level by making them globally available, proper localisation and full ResourceLoader support. It doesn't provide a way for user-level modules, which is a whole other story that will require a very different system (which may or may not be done as part of a future version of the Gadgets extension).

(In reply to comment #4)

Ooh, this is interesting: using 'http://' instead of just '' does work.
Either jQuery or IE8 has a problem with protocol relative URLs. As
importStylesheetURI does work with '
', my bet is on jQuery.

I mentioned that already:

(Cited from comment #1)

mw.loader.load('//example.org/some-styles.css', 'text/css');

[..] I noticed IE8/WinXP seems to be throwing security warnings when
dynamically inserting <link> from the console with a protocol relative url when
on HTTPS. Not sure if that is related.

Note though that jQuery is just javascript. Whatever it is, is a bug in IE8, not jQuery. Except for a few rare cases there is no such thing as a "jQuery bug" in most cases.

Essentially what jQuery is doing is just this:

var el = document.createElement('link');
el.setAttribute('type', ..);
el.setAttribute('rel', ..);
el.setAttribute('href', ..);
var target = document.getElementsByTagName( 'head' )[0];
target.appendChild( el );

So the only difference I see is that it uses attributes instead of properties.

IE7 and IE8 have problems with protocol-relative URLs. We learned this the hard way back in September 2011:

https://commons.wikimedia.org/wiki/Commons:Administrators%27_noticeboard/Archive_30#Helpdesk_and_Villagepump_flooded_by_users_who_cannot_upload_using_upload_wizard

https://commons.wikimedia.org/wiki/MediaWiki_talk:Gadget-HotCat.js/Archive02#HotCat_is_not_working

Avoid using them in JavaScript. As a work-around, make sure any protocol-relative URL in your JS code is prefixed by document.location.protocol before using it in Ajax calls or script or CSS inclusions.

In this case, it seems to be a combination of protocol-relative URLs and setAttribute (used in jQuery) that bugs IE. So avoiding jQuery is the sensible thing to do here.

(In reply to comment #7)

IE7 and IE8 have problems with protocol-relative URLs. We learned this the hard
way back in September 2011:

https://commons.wikimedia.org/wiki/Commons:Administrators%27_noticeboard/Archive_30#Helpdesk_and_Villagepump_flooded_by_users_who_cannot_upload_using_upload_wizard

https://commons.wikimedia.org/wiki/MediaWiki_talk:Gadget-HotCat.js/Archive02#HotCat_is_not_working

Avoid using them in JavaScript. As a work-around, make sure any
protocol-relative URL in your JS code is prefixed by document.location.protocol
before using it in Ajax calls or script or CSS inclusions.

No need to use document.location.prototocol. Protocol-relative urls on themselves work fine. We're hitting an IE bug here that can be fixed in our code.

Change-Id: I04f1775213633e6822b4f982fd43a35b19d19531