Page MenuHomePhabricator

Extension stylesheets should be loaded before any others
Closed, DeclinedPublic

Description

Styles are currently loaded in improper order, where user's style is not loaded as last (concretely eg. GeSHi syntax highlight styles are loaded after).

The proper order should be:

Built-in styles (such as /skins/.../main.css)
MediaWiki:Common.css
MediaWiki:Skin.css
MediaWiki:Extension(s).css (such as syntax highlighting etc.)
User:Common.css (see bug 10183 for details)
User:Skin.css


Version: unspecified
Severity: trivial

Details

Reference
bz10184

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 9:52 PM
bzimport set Reference to bz10184.
bzimport added a subscriber: Unknown Object (MLST).

I think extension stylesheets should also be loaded before site-wide styles like MediaWiki:Common.css and MediaWiki:*Skin*.css, because it would be useful to be able to override them site-wide.

ayg wrote:

Yes, there's no reason whatsoever for anything built in the software to load before customizations. Specifically, it should follow the order

  • CSS and JS in core MediaWiki
  • CSS and JS in extensions
  • Site-specific CSS and JS
  • User-specific CSS and JS

I'd be inclined to just move everything around to fit this. Since none of the higher rungs on that ladder should even know about the lower ones, for the most part, I find it unlikely that anything bad will happen. So concretely, for Monobook it would look like

  1. Global JS variables (moved from above wikibits)
  2. Core files: a) main.css b) commonPrint.css c) handheld.css d) IE fixes e) wikibits.js
  3. Extension files (moved from bottom!)
  4. Site files: a) CSS files (moved above site JS) b) JS files
  5. User files: a) CSS files b) JS files c) JS preview

Whereas now it looks like

  1. Core files/global JS: a) main.css b) commonPrint.css c) handheld.css d) IE fixes e) global JS variables f) wikibits.js
  2. Site files: a) JS files b) CSS files
  3. User files: a) CSS files b) JS files c) JS preview
  4. Extension files

So global JS variables moved above the CSS and IE fixes and site JS/CSS order swapped for consistency, neither of which make any difference. But extension files get moved up, which is the main point. Any objections?

I'd also suggest including of extension CSS files always via

either

<link href="..." />

or

<style>
@import ...
</style>

but never directly like

<style>
selector {

property: value;
property: value;

}
...
</style>

ayg wrote:

That's unrelated. Please create a specific bug for the extension in question if you have an issue with it.

Reverted as this breaks AJAX watch and god knows what else.

wknight8111 wrote:

What's the status of this bug? Is there another way to fix this issue without breaking the AJAX watch? instead of moving the extension JS/CSS code up, would it be possible to move the User JS/CSS code *down*? Small difference, i suppose.

Some of the GeSHi colors conflict with my skin's background color. As a temporary fix, i'm loading an additional stylesheet after the onLoad event to alter the colors. It would seem a waste if many people started doing a hack-job like this just to modify the syntax highlighting.

darklama wrote:

patch for Monobook.php r27436 to put head scripts before site js

Looking at r23541, I noticed why it breaks AJAX watch and other things. It was slightly misplaced inside an "if condition" that it shouldn't of been placed inside. Wanting to see this fixed myself, I've made attached a patch against [http://svn.wikimedia.org/viewvc/mediawiki?view=rev&revision=r27436 r27436] do put it outside the if condition.

Attached:

wknight8111 wrote:

i've looked over this patch, it appears to be a simple reordering of the previous patch, and it also appears to not break the AJAX watch feature. Of course, this is only on my local test wiki. Can somebody please take a look at this again?

ayg wrote:

(In reply to comment #6)

Reverted as this breaks AJAX watch and god knows what else.

Possibly because, as darkcode on IRC points out, I moved the head scripts up inside a conditional, which was quite possibly returning false.

daniel wrote:

I'm not convinced that the current JS loading order is good the way it is. It would be helpful, if the Mediawiki:Common.js contents were loaded _before_ the extension JS. Common.js defines common JS functions to be used by other scripts on the site. Specifically when the "Gadgets"-Extension is used the gadget-scripts are currently loaded before common.js and cannot use its functionality.

With regards to the Gadgets extension, I agree with #12: loading extension JS before commons.js introduces new problems, namely, that gadgets can not use functionality defined globally in common.js. Several scripts where broken by this change. The old order was bad, too, because loading extension's JS last means it can not be overwritten by per-user settings.

What we really need is a way for extensions to control the load order. Perhaps we could introduce numbered slots into the addScript function? The point is that most extensions just have static JS and CSS code, which should be loaded before commons.js etc so it can be overwritten and customized. But extensions like Gadgets load *custom* JS code - and should be loaded after common.js, but before user js. There are probably special cases that would need a still different load order. So a more flexible system is really called for.

ayg wrote:

Yeah, I was assuming extensions were injecting static code, not user-defined site-wide code. We should have an extra insertion point after site-wide core stuff, for site-wide customizable extension stuff.

michaeldaly wrote:

What we really need is a way for extensions to control the load order.

That opens up the possibility that two extensions will generate conflicting sequences. It would be better to simply force an order and oblige the extensions to fit into their slot(s). "Common then extensions then user" seems reasonable. However, it may be worthwhile to have "Common 1 then extensions then common 2 then user" to allow the extensions to have both a before and an after common set of functions - this, of course, increases complexity and requires a new common page; unless there is a clear need, this should not be used.

ayg wrote:

(In reply to comment #15)

What we really need is a way for extensions to control the load order.

That opens up the possibility that two extensions will generate conflicting
sequences.

The load order for their *own* head items, obviously, not the load order for *all* extensions' head items.

darklama wrote:

In general I think its a good idea for extensions to not rely on site-wide js code. However
short of allowing extensions decide what order they would like to be loaded in, it might be
a good idea to allow both a before page and an after page, because extensions can benefit
from both. Extensions like Gadgets can benefit from common routines that can be used in
other pages and from common settings that can be overridden site-wide after extension's
default settings are loaded.

One possibility might be to add MediaWiki:Extensions.js or something similar that is loaded
before extensions on pages that load an extension's js. Another possibility might be to
include MediaWiki:Common.js twice, once before and once after, however I am not sure if that
would work at all, or if it would work with all browsers. If it were to work with all browsers
I think that would probably be the best option as browsers could cache the first request
without needing to make a second request and there would be no need to add another MediaWiki
page.

ayg wrote:

This isn't a difficult problem. Add a HeadScriptsBeforeSiteFiles hook and allow extensions to add stuff there as well as wherever they currently add it. Someone just has to do it.

darklama wrote:

(In reply to comment #18)

This isn't a difficult problem. Add a HeadScriptsBeforeSiteFiles hook and
allow extensions to add stuff there as well as wherever they currently add it.
Someone just has to do it.

In the current setup, extension files are added before site files, so this
isn't a problem. I suppose a third option I hadn't thought of with the current
setup is that the Gadgets extension could be changed to include a new page such
as MediaWiki:Gadgets-common.js that is always loaded before any gadgets. Which
would allow for gadgets to share some common functions like before, while
allowing extensions to benefit from the site-wide js coming after there default
settings as it is now. So maybe close this bug again and get the Gadgets
extension patched instead?

ayg wrote:

(In reply to comment #19)

(In reply to comment #18)

This isn't a difficult problem. Add a HeadScriptsBeforeSiteFiles hook and
allow extensions to add stuff there as well as wherever they currently add it.
Someone just has to do it.

In the current setup, extension files are added before site files, so this
isn't a problem.

It is if the extension wants to access the site files. That's the point.

I suppose a third option I hadn't thought of with the current
setup is that the Gadgets extension could be changed to include a new page such
as MediaWiki:Gadgets-common.js that is always loaded before any gadgets. Which
would allow for gadgets to share some common functions like before, while
allowing extensions to benefit from the site-wide js coming after there default
settings as it is now. So maybe close this bug again and get the Gadgets
extension patched instead?

No. There's no good reason to maintain multiple admin-modifiable CSS or JS files that all get loaded on every page view for everyone. We only need one of each. The need for [[MediaWiki:Geshi.css]] is what spawned this whole bug report, actually.

CSS styles need to be loaded in a predictable order because this controls what overrides what. Eg, a user's custom styles should override the site's custom styles, which override an extension's styles, which override the default skin styles.

For JS, though things should be different. Immediately-executed JS should be mostly limited to function definitions, variable definitions, and some event hooking.

Nothing should really be *running* until all scripts are loaded, so there shouldn't be much issue of functions not yet being defined.

ayg wrote:

. . . hmm, yeah. Since you point it out, just tell the users to make sure their gadgets are clean. Is there any reason any given piece of JS has to be loaded in a particular order? If not, may as well reclose. Any objections?

darklama wrote:

(In reply to comment #23)

. . . hmm, yeah. Since you point it out, just tell the users to make sure
their gadgets are clean. Is there any reason any given piece of JS has to be
loaded in a particular order? If not, may as well reclose. Any objections?

No objections here. I think the loading of additional styles and scripts could
be considered one case in which functions might be called outside a load event.
Another is calling a function to unload events loaded through addOnloadHook.
These cases would be fixed though by adding functions to do these things to
wikibits.js.

alexsm333 wrote:

Nothing should really be *running* until all scripts are loaded, so
there shouldn't be much issue of functions not yet being defined.

Mostly true, except for importScript() function. Right now each gaget has to define it again.

daniel wrote:

Well, as the reopener I do have an objection. The reason I reopened is a Javascript configuration framework which is deployed on commons.wikimedia.org
It allows user scripts and gadgets to define configuration options (assign default values), automatically creates widgets on the preference page to configure values and stores (and recalls) values to/from cookies completely transparent to user and script programmer.
Currently new configuration options are declared before the addOnLoadEvent is hooked, this allows the config framework to retrieve stored config values from cookies before the user script accesses them.
With the current load order there is no way to keep the current clean interface.
A second JS insertion hook (after common.js) seems to be a very small price to pay for that functionality.

That sounds like something which should be integrated upstream, not done as a site customization.

arnomane wrote:

For the same reasons as Dschwen (Gadgets extension) I really need another load order of Javascripts. Furthermore in MediaWiki:Common.js there are often useful functions that help you making your own Javascript (loaded as Gadget) shorter. So the proposed order of the initial comment is the best one and it is needed *desperately* (and you can acutally do it without brakeing anything - even the contrary some things might even start to work for the first time ;-). So can someone please fix this upstream?

ayg wrote:

Why isn't the tactic suggested by comment 21 sufficient? If your JavaScript inclusion consists only of function definitions, and the functions are run at page load time, why can't you define your own functions, and use the Common.js functions safe in the knowledge that they'll be loaded before the function is executed?

This might still be useful for CSS at the very least, and I guess for JavaScript as well if people don't adhere to the function-definitions-only system. It's fairly trivial, someone just needs to bother to write up a patch.

ayg wrote:

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

The JavaScript inclusion does other things other than JavaScript inclusion. A good example of it is commons:MediaWiki:Gadget-QuickDelete.js which used a variable defined on Common.js, conditionally including subscripts, using the includePage() defined on Common.js (this should be moved to wikibits.js)
Or as i found today, adding "malicious links" (which eg. would make me delete a page if i followed) before my user script checked for its existance.

Either Common.js is loaded before the extensions js, or extensions can choose where to be added.

They should be loaded after the other site css but before custom MediaWiki:/user css. There is an addExtenstionStyle() function for this.