Page MenuHomePhabricator

Debug-mode CSS loading can fail because of the IE stylesheet limit
Closed, DeclinedPublic

Description

See bug 47064 and bug 47065. In Internet Explorer 8 and 9, when the ResourceLoader tries to load a 32nd stylesheet in debug mode (as determined by document.querySelectorAll('link[rel="stylesheet"], style').length in the IE 8 debug console), this JS error is produced:

"Could not set the href property. Invalid property value."

This is consistent with hitting the 31 stylesheet limit in these IE versions. ResourceLoader should work around this limitation so debug mode can be used with these browsers, even if many extensions (each having a few stylesheets) are installed.


Version: 1.22.0
Severity: normal
URL: http://test2.wikipedia.org/w/index.php?title=Main_Page&action=edit
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=31676

Details

Reference
bz47277

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 1:40 AM
bzimport set Reference to bz47277.
bzimport added a subscriber: Unknown Object (MLST).

Debug mode is supposed to split things up so that you can see the individual files. I'm not sure we should be messing around with things in debug mode because of IE's stupidity.

I have not found "$wgResourceLoaderDebug = true;" set for test2wiki but perhaps I'm just missing it.

Is it true that extensions should not load enough style sheets to cause errors in IE regardless?

  • Bug 47064 has been marked as a duplicate of this bug. ***

Chris, it is not set globally on test2 (nor should it be). Perhaps you have the cookie set. Go to https://test2.wikipedia.org/wiki/Main_Page (no URL parameters), then run:

console.log(mw.config.get('debug'));

console.log($.cookie('resourceLoaderDebug'));

The first gives the debug mode (true is debug), and the second checks if you have that cookie set.

Is it true that extensions should not load enough style sheets to cause errors
in IE regardless?

No. This is a bug/design flaw in Internet Explorer, fixed in IE 10. It only impacts debug mode (see bug 31676 for production), and we should not hurt modularity on that account. Moreover, I think it's often more to do with the number of extensions, then just any one hogging too many.

I think the proposal of this bug is to have ResourceLoader itslef restrict the number of stylesheets to 31 by combining them when necessary. Daniel correctly points out this partly goes against the purpose of debug mode.

It should be possible to only do this for affected versions of IE based on user agent sniffing and having ResourceLoader add a parameter for this. This is kind of a hack, but it should work, and avoids impacting other browsers.

There is two things that inflate the number of stylesheets in debug mode:

  • unversioned module styles loaded via OutputPage::addModuleStyles each get their own <link rel=stylesheet>
  • regularly loaded modules constructed by ResourceLoaderFileModule (as opposed to DataModule or WikiModule) via mw.loader.load will get their .js and .css files loaded raw from disk.

An item not in this list is regular styles for the majority of modules. Both in debug mode and production mode, each module gets its own <style> tag as it arrives from the server (we don't keep appending to the same <style> tag, not even in production mode, as doing so is significantly more expensive and visibly worsens client-side performance, per bug 45810).

However in IE9 and below we do resort to recycling the same <style> tag again and again because of the IE stylesheet limit (bug 31676).

The only place where we actually concatenate stylesheets on the server side (and naturally, the only place that doesn't concatenate them in debug mode) is unversioned skin styles loaded through OutputPage::addModuleStyles.

(The regular ResourceLoader module requests are also concatenated, but as JSON, not as css text.)

It is limited to debug mode (which we plan to get rid of since it has almost become obsolete and full of bugs that don't happen in production; the few use cases left for browsers without support for source maps or developer tools "prettify" source, can be addressed in better ways).

And aside from being a use case that isn't worth upsetting stuff all over the place for (IE9 and below, only debug mode), even if we would want to fix this, we'd have no way to do so. Resorting to browser sniffing server side is not acceptable per current Wikimedia operations status quo, and conditioning it client side is not acceptable per the very nature of these requests being for basic styles before javascript.

Also let me reiterate that addModuleStyles should be used very sparingly. Often do I find uses of it that are "wrong" due to developers overusing it thinking they deserve the special priority treatment (and backfiring with a performance regression as a result). It is by design a hack and circumvents all principles of ResourceLoader. It is there for basic skin styles and should never get anywhere near 20 modules, even on large installs like Wikipedia.


So how do we debug IE9 and below? Well, for starters, don't use the "debug" mode (this needs no justification, since per this bug, debug mode is broken).

Secondly, as minified as things may be, the error message and originating request should contain enough identifiers for a MediaWiki developer to figure out the source of a problem. It will be harder to trace, but it'll have to do.

Once the problem can be reproduced outside your production site, one can reproducing it on a local MediaWiki install and, if you need the exact line number, manually disable minification there (add return $data; in ResourceLoader::filter).

Closing as WONTFIX, but if anyone has an idea of tackling, I'd be happy to hear it :)

(In reply to Krinkle from comment #5)

An item not in this list is regular styles for the majority of modules.

I don't follow. Don't "regular styles for the majority of modules" use ResourceLoaderFileModule (other than some known exceptions like data modules and EventLogging schemas)?

Specifically "Both in debug mode and production mode, each module gets its own <style> tag as it arrives" does not seem to be true for debug mode.

As far as I can tell, it creates a <link> tag in almost every case (except some obscure things like generating "a { text-decoration: underline; }" for that preference and a CentralNotice one, which is probably custom).