Page MenuHomePhabricator

User and site CSS loaded twice
Closed, ResolvedPublic

Description

The personal CSS is loaded twice, both in an inline <style> tag together with other CSS, and as <link rel="stylesheet" href="https://de.wikipedia.org/w/load.php?debug=false&amp;lang=de&amp;modules=user%7Cuser.groups&amp;only=styles&amp;skin=vector&amp;user=Schnark&amp;version=CLAHufrP&amp;*">.

This seems to be introduced with the latest ResourceLoader changes.

In T122645, @Nikerabbit wrote:
Since some time when using for example Chrome's element inspector, styles defined in MediaWiki:Common.css are shown twice, see screenshot below. This makes it more difficult to try out things by just disabling rules, because now you have to do it twice.

Now site and user styles are loaded twice. @Krinkle calls this 'harmless', but it is extremely annoying having to dig through duplicate properties, not knowing where they come from. I also experience occasional load order issues and discrepancies between debug and live loads.
Is this double-loading absolutely necessary, or is it a by-product of something else? If the latter, how hard is it to remove this redundant loading?

It is not done on purpose and is the same as T42284 (for Gadgets). The proper solution (which will be enforced by T92459 soon) is to not have a single module used as both addModules() and addModuleStyles().
A module is either a stylesheet module essential at load time, or it is a dynamic module with js and css that is loaded at run time. Modules trying to be both at the same time are not supported.
This is already enforced by code-review and convention in core and extensions. The only exception right now (for legacy reasons) is gadgets and user/site scripts. Until now, the site worked around this by loading the scripts in a dedicated HTTP request (one that can't be cached due to not having a version number in the url, and having to be embedded in the page html). This is now solved and significantly improves caching and run-time performance for end-users like you and me – at the cost of loading the styles twice (which is cancelled out by the gains the first time, and the second page visit it's entirely absent because it's coming from local cache).
In due time we'll either deprecate 'site' in favour of an enabled-hidden gadget (with automatic conversion for existing ones) – or, we'll have to split 'site' into 'site.styles' and 'site.scripts' – or something else we come up with.

Event Timeline

Schnark created this task.Aug 10 2015, 3:52 PM
Schnark raised the priority of this task from to Needs Triage.
Schnark updated the task description. (Show Details)
Schnark added a subscriber: Schnark.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 10 2015, 3:52 PM
Krinkle triaged this task as Low priority.Aug 20 2015, 12:07 PM
Krinkle added a project: Technical-Debt.
Krinkle set Security to None.
Krinkle moved this task from Inbox to Backlog on the MediaWiki-ResourceLoader board.

Mediawiki:Common.css and all gadgets css are also loaded twice.

He7d3r added a subscriber: He7d3r.Dec 8 2015, 10:31 AM
Nemo_bis renamed this task from User CSS loaded twice to User and site CSS loaded twice.Dec 31 2015, 7:44 AM
Nemo_bis updated the task description. (Show Details)
Nemo_bis added a subscriber: Edokter.

The proper solution (which will be enforced by T92459 soon) is to not have a single module used as both addModules() and addModuleStyles().

Are you saying this is blocked on T92459 and T87871?

The proper solution (which will be enforced by T92459 soon) is to not have a single module used as both addModules() and addModuleStyles().

Are you saying this is blocked on T92459 and T87871?

Yes. I marked them as blocking two weeks ago:

@Krinkle added a blocking task: T87871: Loading with addModuleStyles() should set module state to "ready" · Mon, Dec 7

However that's merely a conceptual blocker.

The user and site styles load twice now because the code currently explicitly loads them twice. It's working exactly as can be expected. They're very generic modules applied to a wide range of use cases, none of which are declared anywhere. We load the module in two ways because users want these three properties to be true:

  • The module also contains scripts to execute (not just styles).
  • The styles are not always related to the scripts and should be be available to non-js environments as well (e.g. MediaWiki:Common.css).
  • Styles may apply to server-generated page content and should apply in the first browser render of any page. Styles may be unrelated to elements created by the javascript code.

These are loosely implied guarantees are inherited from the legacy system. It's hard for the software to make any reasonable decision.

The blocking tasks (T92459 and T87871) will adapt the state machine to account for style modules. Meaning, if a module is loaded via addModuleStyles, it will be marked as "loaded". Modules only load once. This means modules can either serve universal styles (not limited to js), or, provide scripts+styles loaded dynamically. Not both. Either addModules (mw.loader.load) or addModuleStyles (<link rel=stylesheet>).

In order to fulfil the user requirements listed above, we should change the module structure of "user" and "site" to better match reality. We should probably split them up to have a separate styles module. (Thanks to T87871, we can have the scripts module depend on the styles module for back-compat, e.g. "user.css" and "user").

This split up can already happen today and is not blocked.

GOIII added a subscriber: GOIII.Jan 23 2016, 5:55 PM
Elitre added a subscriber: Elitre.Apr 28 2016, 9:03 AM
Majr added a subscriber: Majr.May 26 2016, 5:55 AM

Change 292973 had a related patch set uploaded (by Krinkle):
resourceloader: Remove styles from 'site', depend on 'site.styles'

https://gerrit.wikimedia.org/r/292973

Change 301314 had a related patch set uploaded (by Krinkle):
resourceloader: Separate 'user.styles' module from 'user'

https://gerrit.wikimedia.org/r/301314

Change 292973 merged by jenkins-bot:
resourceloader: Remove styles from 'site', depend on 'site.styles'

https://gerrit.wikimedia.org/r/292973

Change 301314 merged by jenkins-bot:
resourceloader: Separate 'user.styles' module from 'user'

https://gerrit.wikimedia.org/r/301314

Change 302632 had a related patch set uploaded (by Krinkle):
resourceloader: Separate 'user.styles' module from 'user'

https://gerrit.wikimedia.org/r/302632

Change 302632 merged by jenkins-bot:
resourceloader: Separate 'user.styles' module from 'user'

https://gerrit.wikimedia.org/r/302632