ResourceLoader should not load CSS again if it was already included with only=styles
Closed, DeclinedPublic
Actions

Assigned To

Authored By

	Subfader
	Jul 3 2011, 1:37 PM

Description

ResourceLoader loads user CSS and MediaWiki:Common.css 3 times.
Example source code of the head ouput:

<head>
<meta charset="utf-8" />
<title>Upload file - MixesDB</title>
<meta name="robots" content="noindex,nofollow" />
<link rel="shortcut icon" href="http://www.mixesdb.com/favicon.ico" />
<link rel="search" type="application/opensearchdescription+xml" href="/db17/opensearch_desc.php" title="MixesDB (en)" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=mediawiki.legacy.commonPrint&only=styles&skin=monobook&*" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=mediawiki.legacy.shared&only=styles&skin=monobook&*" />
<link rel="stylesheet" href="/db17/skins/monobook/main.css?301" media="screen" />



<meta name="ResourceLoaderDynamicStyles" content="" /><link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=site&only=styles&skin=monobook&*" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=user&only=styles&skin=monobook&user=MDB&version=20110702T150017Z&*" />

</head>

Version: 1.17.x
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=61577

Details

Reference: bz29693

Related Objects

Mentioned In: T147667: Site and Gadget CSS should apply after core and skin CSS

Event Timeline

• bzimport raised the priority of this task from to Low.Nov 21 2014, 11:28 PM

• bzimport added a project: MediaWiki-ResourceLoader.

• bzimport set Reference to bz29693.

Subfader created this task.Jul 3 2011, 1:37 PM

Looks like you use a customised version of Monobook. Was it updated for 1.17? Is the same behaviour reproduceable with vanilla skins?

I have the same with default monobook.php, logged in or logged out and in various skins. It must be OutputPage.php creating the headelement.

Can you reproduce it? Would like to see other 1.17.0 wikis in action.

Vector:
<head>
<meta charset="utf-8" />
<title>MixesDB - The Database for Mixes</title>
<link rel="alternate" type="application/x-wiki" title="Edit" href="/db17/index.php?title=Main_Page&action=edit" />
<link rel="edit" title="Edit" href="/db17/index.php?title=Main_Page&action=edit" />
<link rel="shortcut icon" href="http://www.mixesdb.com/favicon.ico" />
<link rel="search" type="application/opensearchdescription+xml" href="/db17/opensearch_desc.php" title="MixesDB (en)" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=mediawiki.legacy.commonPrint&only=styles&skin=vector&*" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=mediawiki.legacy.shared&only=styles&skin=vector&*" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=skins.vector&only=styles&skin=vector&*" />
<meta name="ResourceLoaderDynamicStyles" content="" /><link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=site&only=styles&skin=vector&*" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=site&only=styles&skin=vector&*" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=site&only=styles&skin=vector&*" />

</head>

Modern:
<head>
<meta charset="utf-8" />
<title>Preferences - MixesDB</title>
<meta name="robots" content="noindex,nofollow" />
<link rel="shortcut icon" href="http://www.mixesdb.com/favicon.ico" />
<link rel="search" type="application/opensearchdescription+xml" href="/db17/opensearch_desc.php" title="MixesDB (en)" />
<link rel="stylesheet" href="/db17/skins/common/shared.css?301" media="screen" />
<link rel="stylesheet" href="/db17/skins/modern/main.css?301" media="screen" />
<link rel="stylesheet" href="/db17/skins/modern/print.css?301" media="print" /><meta name="ResourceLoaderDynamicStyles" content="" /><link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=site&only=styles&skin=modern&*" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=site&only=styles&skin=modern&*" />
<link rel="stylesheet" href="/db17/load.php?debug=true&lang=en&modules=site&only=styles&skin=modern&*" />

</head>

It doesn't happen in MW 1.18a though.

(In reply to comment #3)

It doesn't happen in MW 1.18a though.

So it doesn't happen in trunk, and I don't believe it happens in stock 1.17.0 either. Perhaps you have modules depending on user or site?

Same with nothing custom in LocalSettings.php (no settings, no extensions)

It's so buggy MW 1.17.0 is NOT stable!

plaz wrote:

I had the same issue on my heavily customized wiki with my custom skin. (http://theplaz.com)

I hacked a fix by commenting out the loop in OutputPage::buildCssLinks

foreach ( array( 'site', 'private', 'user' ) as $group ) {
$ret .= $this->makeResourceLoaderLink($sk, array_merge( $styles['site'], $styles['user'] ), 'styles');
}

The sites and users scripts are all loaded at once, so commenting out the foreach does not remove functionality.

Thanks, that works.

Why does RL load gadget's CSS twice? One time it ships CSS with JavaScript and writing this into the DOM and one time CSS is delivered in a separate CSS file duplicating the style definitions. You can easily see it opening Firebug on Wikimedia Commons and inspecting an element that was created by one of our Gadgets that both consist of a JavaScript and CSS (e.g. the slideshow start button)

Is this intended behaviour?
Is this documented (except in your source files that I can't search any more since Krinkle's SVN search is down)?
Are there intentions to optimise this issue?

(In reply to comment #9)

Why does RL load gadget's CSS twice? One time it ships CSS with JavaScript and
writing this into the DOM and one time CSS is delivered in a separate CSS file
duplicating the style definitions. You can easily see it opening Firebug on
Wikimedia Commons and inspecting an element that was created by one of our
Gadgets that both consist of a JavaScript and CSS (e.g. the slideshow start
button)

This behavior was apparently introduced in November 2011 to make Gadget CSS load on time.

Is this intended behaviour?

Ideally the CSS wouldn't be loaded twice, no. The reason it's currently done is to work around the fact that you can't currently set the position=top property on a Gadget to make it load before the page renders.

Is this documented (except in your source files that I can't search any more

since Krinkle's SVN search is down)?

No.

Are there intentions to optimise this issue?

The rewrite of the Gadgets extension in the RL2 branch fixes this by only bundling the CSS with the JS, rather than loading it separately. This does mean that, unless position=top is set in the Gadget properties, the CSS will load after the page renders, right before the JS executes. This is fine if you're only styling things that your JS creates, but it's ugly if you're styling things that MediaWiki puts on the page.

(In reply to comment #10)

(In reply to comment #9)

Why does RL load gadget's CSS twice? One time it ships CSS with JavaScript and
writing this into the DOM and one time CSS is delivered in a separate CSS file
duplicating the style definitions. You can easily see it opening Firebug on
Wikimedia Commons and inspecting an element that was created by one of our
Gadgets that both consist of a JavaScript and CSS (e.g. the slideshow start
button)

This behavior was apparently introduced in November 2011 to make Gadget CSS
load on time.

bug 40284 has been filed for this, linking this response from there.

The fucked up Ressource Loader (sorry for the strong language) is the reason why I still cannot upgrade from MW 1.16. I wait for RL 2.

(In reply to comment #12)

The fucked up Ressource Loader (sorry for the strong language) is the reason
why I still cannot upgrade from MW 1.16. I wait for RL 2.

1.17 was buggy because some fixes that made it into production accidentally didn't make it into the tarball, but have you tried using 1.19? Did you encounter any RL-related problems with 1.19?

No I havent't but reading that CSS is adding CSS twice to fix a problem RL created itself makes RL look a bad and not very trustworthy.

My wiki is extremely customized. Upgrading means lot of manual adjustment to core code and testing.

I won't upgrade now when RL will be fixed in RL 2 soon after I upgraded.

(In reply to comment #12)

The fucked up Ressource Loader (sorry for the strong language) is the reason
why I still cannot upgrade from MW 1.16. I wait for RL 2.

RL2 is Gadgets 2.0, this has nothing to do with ResourceLoader in MediaWiki core. There were a few bugs in MediaWiki core that blocked Gadgets 2.0, but those have already been resolved and were released in MediaWiki 1.19 and are now in MediaWiki 1.20alpha as well.

So in a way you could say that as far as MediaWiki core is concerned RL2 is ready and released.

Can you be more specific what you were waiting for RL-related?

(In reply to comment #14)

No I havent't but reading that CSS is adding CSS twice to fix a problem RL
created itself makes RL look a bad and not very trustworthy.

It doesn't just load it twice, that's non-sense. The way this happens is by design but there are a few modules that have not been converted correctly and as such get their css applied twice. Its minimal.

As for stability, speed and efficiency for the client side (users using a web browser) I believe, ResourceLoader is most definitely an improvement over how it was in MediaWiki 1.16.

So unless you actually tried upgrading and have a tangible problem, I'd say, upgrade and enjoy.

(In reply to comment #4)

(In reply to comment #3)

It doesn't happen in MW 1.18a though.

So it doesn't happen in trunk, and I don't believe it happens in stock 1.17.0
either. Perhaps you have modules depending on user or site?

So there is currently no evidence at all that this problem still happens in a currently supported version of MediaWiki?

(Note that CSS of gadgets is bug 40284 and still open.)

This bug is either fixed (if the reporter saw css being loaded twice by a bug).

Or it is a wontfix (if the reporter saw css being loaded once by only=styles and once by a regular load request), because that's only possible if the code using resourceloader is incorrectly adding the module to the load queue.

e.g. by calling addModuleStyles() and then addModules() for the same module.

Closing as worksforme, feel free to change to fixed or wontfix respectively.

(In reply to comment #18)

Or it is a wontfix (if the reporter saw css being loaded once by only=styles
and once by a regular load request), because that's only possible if the code
using resourceloader is incorrectly adding the module to the load queue.

e.g. by calling addModuleStyles() and then addModules() for the same module.

I wouldn't call that a wontfix. Making the server side loader smarter and not adding css that's already in the page will probably actually be a part of fixing other bugs we have related to RL that we need to fix. This bug would just end up depending on them instead of being WONTFIXED.

(In reply to comment #19)

(In reply to comment #18)

Or it is a wontfix (if the reporter saw css being loaded once by only=styles
and once by a regular load request), because that's only possible if the code
using resourceloader is incorrectly adding the module to the load queue.

e.g. by calling addModuleStyles() and then addModules() for the same module.

I wouldn't call that a wontfix. Making the server side loader smarter and not
adding css that's already in the page will probably actually be a part of
fixing other bugs we have related to RL that we need to fix. This bug would
just end up depending on them instead of being WONTFIXED.

I doubt that this will fit into any refactoring (planned or imaginary).

I can't discover more than one style definition applied to some elements targeted by Commons' Common.js (so it seems to be fixed?); however gadgets are still loading their CSS trough both, JS ( mw.loader.implement( ... {"css":[" /* CSS here */ "]}) ) and a separate CSS.

Dispenser mentioned this in T147667: Site and Gadget CSS should apply after core and skin CSS.Feb 14 2017, 1:06 PM

ResourceLoader should not load CSS again if it was already included with only=stylesClosed, DeclinedPublicActions

Description

Details

Related Objects

Event Timeline

ResourceLoader should not load CSS again if it was already included with only=styles
Closed, DeclinedPublic
Actions