Page MenuHomePhabricator
Create Task
Maniphest T108590

User and site CSS loaded twice
Closed, ResolvedPublic
Actions

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.

x2.png (383×931 px, 37 KB)

In T102077#1495896, @Krinkle wrote:
In T102077#1495849, @Edokter wrote:

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.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
resourceloader: Separate 'user.styles' module from 'user'mediawiki/corewmf/1.28.0-wmf.13+102 -26
resourceloader: Separate 'user.styles' module from 'user'mediawiki/coremaster+102 -26
resourceloader: Remove styles from 'site', depend on 'site.styles'mediawiki/coremaster+7 -6
Customize query in gerrit

Related Objects
Search...

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 project: MediaWiki-ResourceLoader.
Schnark subscribed.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 10 2015, 3:52 PM
Legoktm subscribed.Aug 10 2015, 4:21 PM

See T102077#1495896.

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.
Zebulon84 subscribed.Dec 7 2015, 8:20 AM

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

Aklapper added a project: Performance Issue.Dec 7 2015, 9:30 AM
Krinkle added a subtask: T87871: Loading with addModuleStyles() should set module state to "ready".Dec 7 2015, 6:02 PM
He7d3r subscribed.Dec 8 2015, 10:31 AM
Krinkle merged a task: T122645: Styles defined in MediaWiki:common.css are applied twice.Dec 30 2015, 6:04 PM
Krinkle added subscribers: Krinkle, Nikerabbit, StudiesWorld.
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.
Nemo_bis subscribed.Dec 31 2015, 7:46 AM

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?

Krinkle added a comment.Dec 31 2015, 11:46 PM
In T108590#1911399, @Nemo_bis wrote:

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 subscribed.Jan 23 2016, 5:55 PM
Agabi10 subscribed.Jan 23 2016, 9:11 PM
Krinkle mentioned this in T127328: Optimise critical rendering path.Feb 18 2016, 4:31 PM
Danny_B subscribed.Feb 18 2016, 4:54 PM
• MZMcBride subscribed.Apr 25 2016, 1:52 PM
Nirmos mentioned this in T131203: On svwiki, cursor disappears when all content is deleted (due to gadget/config?).Apr 27 2016, 11:03 PM
• Elitre subscribed.Apr 28 2016, 9:03 AM
Krinkle mentioned this in T92459: ResourceLoader should restrict addModuleStyles() to modules that only provide styles.May 10 2016, 5:49 PM
Majr subscribed.May 26 2016, 5:55 AM
gerritbot subscribed.Jun 6 2016, 6:26 PM

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

gerritbot added a project: Patch-For-Review.Jun 6 2016, 6:26 PM
gerritbot added a comment.Jul 27 2016, 12:12 AM

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

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

gerritbot added a comment.Aug 2 2016, 9:18 AM

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

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

ReleaseTaggerBot added projects: MW-1.28-release (WMF-deploy-2016-08-02_(1.28.0-wmf.13)), MW-1.28-release-notes.Aug 2 2016, 10:00 AM
gerritbot added a comment.Aug 2 2016, 10:17 PM

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

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

ReleaseTaggerBot added a project: MW-1.28-release (WMF-deploy-2016-08-09_(1.28.0-wmf.14)).Aug 2 2016, 11:00 PM
gerritbot added a comment.Aug 3 2016, 12:07 AM

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

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

gerritbot added a comment.Aug 3 2016, 5:29 AM

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

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

Krinkle closed this task as Resolved.Aug 3 2016, 5:38 AM
Krinkle claimed this task.
Krinkle removed projects: MW-1.28-release (WMF-deploy-2016-08-09_(1.28.0-wmf.14)), Patch-For-Review.
Krinkle closed subtask T87871: Loading with addModuleStyles() should set module state to "ready" as Resolved.Aug 17 2016, 4:20 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits