Last modified: 2012-11-12 13:00:03 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T31308, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 29308 - Implement ParserOutput::addModuleStyles
Implement ParserOutput::addModuleStyles
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Parser (Other open bugs)
1.20.x
All All
: Normal enhancement (vote)
: 1.20.0 release
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-06-07 21:46 UTC by s7eph4n
Modified: 2012-11-12 13:00 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description s7eph4n 2011-06-07 21:46:53 UTC
Currently to load a CSS file for an extension there are three options:
* use OutputPage::addModuleStyles - does not cache, so has to be included on each page, if needed or not
* use ParserOutput::addModules - does not work on JS-disabled browsers
* use ParserOutput::addHeadItem - does not use ResourceLoader (i.e. no minifying, no batching), requires the full html text of the head item instead of just the module name

To improve this it should be possible to add style-only modules to the ParserOutput using a method ParserOutput::addModuleStyles.
The list of style-only modules should be cached.
The modules should be added to the output in OutputPage::addParserOutputNoText using OutputPage::addModuleStyles (not OutputPage::addModules).
Comment 1 Brion Vibber 2011-06-08 18:34:18 UTC
Hmm...

Roan, is there a reason that styles for modules requested at page output time don't have their styles loaded up via <link rel="stylesheet">? Is this to force load order, or perhaps because we don't have a good way to distinguish that the style's already been grabbed but JS still has to be loaded?

There's nothing terribly clear in the doc comments on addModules() vs addModuleScripts() vs addModuleStyles() on OutputPage.
Comment 2 Roan Kattouw 2011-06-08 19:24:22 UTC
(In reply to comment #1)
> Hmm...
> 
> Roan, is there a reason that styles for modules requested at page output time
> don't have their styles loaded up via <link rel="stylesheet">? Is this to force
> load order, or perhaps because we don't have a good way to distinguish that the
> style's already been grabbed but JS still has to be loaded?
> 
> There's nothing terribly clear in the doc comments on addModules() vs
> addModuleScripts() vs addModuleStyles() on OutputPage.
Yeah this isn't really documented, but there's a very good reason.

We serve CSS together with JS and load it dynamically (we insert it into the DOM in a <style> tag). So that kind of precludes the <link> tag thing, but even if we would load them as separate requests (which we don't because fewer requests == win) we'd still have another problem. We want to cache requests forever whenever possible. The way we accomplish this is by having one startup module which contains the last-modified timestamps of all other modules, and use a short cache timeout for that (5 minutes). The other modules have cache timeouts of 30 days, and we use a cache-busting version parameter that is set to the last modified timestamp. However, in order to be able to append the version parameter and put the right timestamp in it, we have to do the request from JS. We can't do this in the HTML output, because the timestamp would get Squid-cached and that would take us straight back to the pre-ResourceLoader $wgStyleVersion hell.

For that reason, we only load styles as a <link> tag if they absolutely must be present in a no-JS environment (skin styles are a good example), and we put them in short cache timeout mode because that's the worst of the two evils.

In the not-too-distant future, we'll be able to put this link tag in a <noscript> tag and load it with JS if JS is available. The only reason we're not doing that yet is because the 11.0x versions of Opera have a bug causing <noscript><link ... ></noscript> not to work. All other major browsers, including newer and older versions of Opera, support this. However, this regression is fixed in Opera 11.10, which came out a while ago. We're just waiting for the buggy versions to go extinct (Opera 11.00 is at like 0.25% right now, IIRC), which should happen pretty quickly. I'll look at our browser stats again in mid-July, when we'll be doing a ResourceLoader sprint, and hopefully they'll be low enough to allow doing this then.
Comment 3 Umherirrender 2012-04-14 18:21:32 UTC
Method from bug title was added with r93758
Comment 4 Krinkle 2012-11-12 12:59:39 UTC
So, fixed?

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links