Page MenuHomePhabricator

MediaWiki:Common.css not applied if it uses '@import' rules and user has any style-only gadgets enabled (works when using ?debug=true and when disabling all gadgets)
Closed, ResolvedPublic

Description

It looks like styles contained in https://it.wikivoyage.org/wiki/MediaWiki:Common.css are not anymore applied.
Homepage (https://it.wikivoyage.org) is a disaster, while all the other pages are affected but in a minor way (Infobox has a different layout).

Event Timeline

Andyrom75 renamed this task from https://it.wikivoyage.org homepage has a broken layout to MediaWiki:Common.css doesn't work.May 11 2017, 1:38 PM
Andyrom75 updated the task description. (Show Details)
Andyrom75 updated the task description. (Show Details)

@Zppix: Not a site request as this is not about changing settings.

Aklapper renamed this task from MediaWiki:Common.css doesn't work to MediaWiki:Common.css not applied on it.wikivoyage (works when using ?debug=true).May 11 2017, 1:44 PM

Can reproduce with https://it.wikivoyage.org/wiki/Pagina_principale
Cannot reproduce with https://it.wikivoyage.org/wiki/Pagina_principale?debug=true

(Probably unrelated: https://it.wikivoyage.org/wiki/MediaWiki:Gadget-Sandbox.js is broken and needs to get fixed by an it.wikivoyage admin: TypeError: mw.util is undefined. Needs wrapping with mw.loader.using. See T164262.)

@Andyrom75: I cannot reproduce the problem on https://it.wikivoyage.org/wiki/Pagina_principale after disabling all gadgets. Can you?

(Also unrelated, but more brokenness in gadgets: https://commons.wikimedia.org/w/index.php?title=MediaWiki:Gadget-HotCat.js is also loaded on it.wikivoyage and triggers TypeError: span.firstChild is null for line if (span.firstChild.nodeType !== Node.ELEMENT_NODE) return null;)

Aklapper renamed this task from MediaWiki:Common.css not applied on it.wikivoyage (works when using ?debug=true) to MediaWiki:Common.css not applied on it.wikivoyage (works when using ?debug=true and when disabling all gadgets).May 11 2017, 1:55 PM

In debug mode, the styles are loaded from https://it.wikivoyage.org/w/load.php?debug=true&lang=it&modules=site.styles&only=styles&skin=vector , which works fine.

In the normal mode, the styles are loaded from https://it.wikivoyage.org/w/load.php?debug=false&lang=it&modules=ext.gadget.IconeLinkEsterni%2CIndentazioneColorata%2CInterWikiLinkIcons%7Csite.styles&only=styles&skin=vector – note how the stylesheets for some gadgets are added. This doesn't work anymore, because the @import statements must appear at the beginning of a file. (This also explains why the problem disappears if you disable all gadgets.)

There's definitely a MediaWiki bug here.

The immediate cause of this bug is rEGAD86905f8d78af: Move gadget styles from main stylesheet request to site request, and we could probably resolve it for now by reverting that. That will un-resolve T147667 though.

But it seems that we never handled the situation where a CSS module using @import is loaded in a single request with other modules… we were just lucky this never broke anything before.

The current problem on Wikivoyage can also be resolved by putting all of the site styles into MediaWiki:Common.css, rather than loading them with @import.

@matmarex regrouping block of styles in separated files, helps their maintenance, hence I would avoid to reverse the whole set of instruction inside the main file.

@Andyrom75 Then I would recommend turning the entire MediaWiki:Common.css into a gadget that is enabled by default. This lets you use multiple CSS pages without annoying @import rules. We have the site styles on https://www.mediawiki.org/ set up that way, for example, as the "Site (styles)" gadget (https://www.mediawiki.org/wiki/Special:Gadgets). Compared to @import, this also lets the styles load faster, since they go through the ResourceLoader minification and they all load in a single request.

There is still a MediaWiki bug here, and we should still fix it, but regardless @import rules are a pain and are best avoided.

Change 353302 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] [WIP] Allow modules with @import rules to work with only=styles

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

@matmarex in MediaWiki:Common.css we have style that must be active for the correct functioning of the wiki. As far as you know, is possible to have a gadget that cannot be disabled by a user?

@Aklapper although unrelated, could you suggest me in detail how to modify https://it.wikivoyage.org/wiki/MediaWiki:Gadget-Sandbox.js ?
I've applyed the following but I'm not sure if this is what you meant:

mw.loader.using( 'mediawiki.util' ).then(function() {
mw.util.addPortletLink('p-navigation', '/wiki/Utente:' + mw.config.get('wgUserName') + '/Sandbox', 'Sandbox', '');
});

By the way, I received no error message from Chrome.

https://commons.wikimedia.org/w/index.php?title=MediaWiki:Gadget-HotCat.js is just an import from commons, so I've left a message on the talk page where the code is.

I don't think it's possible. Users could still technically disable this gadget. But you can advise them in its description that it's a really bad idea to do so.

Another example: styles for the main page of https://pl.wikipedia.org/ are also a gadget (https://pl.wikipedia.org/wiki/MediaWiki:Gadget-main-page.css). Over the years I had exactly one user accidentally turn them off and complain. I honestly think putting site styles in a gadget is a reasonable thing to do.

@matmarex do you think that in the (very?) long term can MediaWiki:Common.css/js be managed through resourceLoader having the benefit of compression and speed without giving the possibility to any user to disable it?
Or maybe there is a technical constraint that preclude this possibility?

Note that it's already loaded through ResourceLoader (as part of the 'site'/'site.styles' modules), but ResourceLoader doesn't know how to "follow" the @import rules, so they are not followed and loaded in separate requests.

We seem to have T146733 filed asking for MediaWiki:Common.css etc. to be "modularized".

matmarex renamed this task from MediaWiki:Common.css not applied on it.wikivoyage (works when using ?debug=true and when disabling all gadgets) to MediaWiki:Common.css not applied if it uses '@import' rules and user has any style-only gadgets enabled (works when using ?debug=true and when disabling all gadgets).May 11 2017, 4:57 PM

Change 353333 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/extensions/Gadgets@master] Revert "Move gadget styles from main stylesheet request to site request"

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

@matmarex in MediaWiki:Common.css we have style that must be active for the correct functioning of the wiki. As far as you know, is possible to have a gadget that cannot be disabled by a user?

Yes, simply set the gadget as [default] and [hidden]. This setting was added exactly for cases such as yours.

Change 353335 had a related patch set uploaded (by Krinkle; owner: Legoktm):
[mediawiki/extensions/Gadgets@wmf/1.30.0-wmf.1] Revert "Move gadget styles from main stylesheet request to site request"

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

Change 353333 merged by jenkins-bot:
[mediawiki/extensions/Gadgets@master] Revert "Move gadget styles from main stylesheet request to site request"

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

Krinkle claimed this task.

Regarding use of @import see also T37562#387622, which was declined 2014.

  • @import rules for proper ResourceLoader modules (e.g. not legacy modules 'site' or 'user') is only supported if they are loaded dynamically through mw.loader.
  • @import rules for site and user are only supported if they are placed in the first file of that module (e.g. Common.css, not <Skinname>.css). @import rules in <Skinname.css> also wasn't supported before ResourceLoader in 2011.

Continue at T147667#3256182.

Change 353335 merged by jenkins-bot:
[mediawiki/extensions/Gadgets@wmf/1.30.0-wmf.1] Revert "Move gadget styles from main stylesheet request to site request"

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

Mentioned in SAL (#wikimedia-operations) [2017-05-11T18:53:05Z] <thcipriani@tin> Synchronized php-1.30.0-wmf.1/extensions/Gadgets/includes/GadgetResourceLoaderModule.php: SWAT: [[gerrit:353335|Revert "Move gadget styles from main stylesheet request to site request"]] T165040 T165031 (duration: 00m 42s)

Change 353302 abandoned by Bartosz Dziewoński:
[WIP] Allow modules with @import rules to work with only=styles

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

@Krinkle, could you explain me better this point: @import rules for proper ResourceLoader modules (e.g. not legacy modules 'site' or 'user') is only supported if they are loaded dynamically through mw.loader.?

If this is not the right place, please let's switch on my talk page.
Thanks

@Krinkle, could you explain me better this point: @import rules for proper ResourceLoader modules (e.g. not legacy modules 'site' or 'user') is only supported if they are loaded dynamically through mw.loader.?

If this is not the right place, please let's switch on my talk page.
Thanks

It means that our support for @import mainly focusses on stylesheets that come from a wiki module, such as your common.css.

For code maintained as part of the MediaWiki software itself ("proper RL modules"), @import should never be used there because such software modules can simply include files directly from the server hard drive in a more efficient way. However, if such modules do use @import (for any reason), then they can only do this if the module is loaded dynamically through mw.loader mechanism in JavaScript. (Because our JavaScript code has special logic to give each each module file its own <style> tag if needed). If software modules are loaded as part of a link-stylesheet, then @import will not work. This is because we have well over 1000 modules, and we cannot load them all separately since that would make your web browser very slow. That's why we combine all modules in 1 large compact request. That means if any of the modules use @import it may end up in the middle, like this:

Combined Gadget A + B + Common.css
/* From Gadget-A.css */
.some-styles {
  ...
}

/* From Gadget-B.css */
.some-styles {
  ...
}

/* From Common.css */
@import '/w/index.php?title=MediaWiki:Common.css/subpage.css&action=raw&ctype=text/css'

.some-styles {
  ...
}

/* From Vector.css */
.some-styles {
  ...
}

When this happens, the import is broken because web browsers do not allow this. @import is only allowed at the very top of a file.

This is why we recommend @import is never used. But, because it is common for users to want this, we still allow it, but only in Common.css. Not in Gadgets, and not in Vector.css or Monobook.css.

That is why we cannot combine Gadgets and Site module in the same request. My commit accidentally did the above. It is now changed back:

1. Combined software and gadgets
/* From MediaWiki core */ ..
/* From Cite extension */ ..
/* From Wikidata extension */ ..
/* From ULS extension */ .., etc.
/* From Gadgets ... */ .., etc.
2. Site styles
/* From MediaWiki:Common.css */ ..
/* From MediaWiki:Vector.css */ ..

Thanks for your excellent clarification!