Page MenuHomePhabricator

OutputPage: Don't add <script> for load.php?modules=site&only=scripts if there is no site js
Closed, DeclinedPublic

Description

The OutputPage::makeResourceLoaderLink call for "user.groups" outputs:

mw.loader.state({"user.groups":"ready"});

instead of a <script src="..."> if the module is empty (saves an http request).

However the OutputPage::makeResourceLoaderLink call for "site" does not, that one always results in an http request even if Vector.js/Common.js are nonexistent.


Version: unspecified
Severity: enhancement

Details

Reference
bz50037

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 1:53 AM
bzimport set Reference to bz50037.
bzimport added a subscriber: Unknown Object (MLST).

I thought perhaps it did so because of caching reasons (the user ones are user specific, whereas the site one should be able to change without needing to purge the page) however it looks like that isn't the reason, at least not explicitly so.

The reason is that ResourceLoaderWikiModule::isKnownEmpty (which OutputPage:: makeResourceLoaderLink uses to decide whether to output <script src> or mw.loader.state) isn't being told that it should only check for the script pages when checking for only=scripts.

As such, if your wiki has Common.css but not Common.js it will still create an http request for modules=site&only=scripts.

Once I deleted Common.css as well, it outputted 'mw.loader.state({"site":"ready"});'.

In a way I suppose this is nice in that if you have one of them, you'll always have both in the HTML output. Since these unversioned urls are only cached for 30 minutes it means that aside from being able to change one of them and have it be live on cached pages in 30 minutes, you can also add a script (which may or may not depend on the new css and the css depend on the js) and it'll show up.

Though this may or may not be an intentional feature-ish side-effect, it isn't entirely waterproof either.

If neither exist, and you create both, it'll still fail since only the css request is in the html by default, the one for js is not.

@Roan: What do you think? Should we output 'mw.loader.state({"site":"ready"});' in place of load.php?modules=site&only=scripts from OutputPage when we know this will be an empty request when there is content in the css ones?

btw, this happens because ResourceLoaderWikiModule::isKnownEmpty uses ResourceLoaderWikiModule::getTitleMtimes which uses both "script" and "style" pages.

e.g. the isKnownEmpty call from OutputPage::makeResourceLoaderLink for modules=site&only=scripts on a wiki with Common.css but no Common.js ends up being non-empty because it returns:

[Sun Jun 23 01:24:29 2013] [client 127.0.0.1] RLWM::isKnownEmpty> {"MediaWiki:Common.css":"1368646445"}

Continuing the monolog, this is a WONTFIX per the reason I presumed earlier:

(In reply to Krinkle from comment #1)

[if] you have one of them, you'll always have both in the HTML output. Since these unversioned urls are only cached for 30 minutes it means that aside from being able to change one of them and have it be live on cached pages in 30 minutes, you can also add a script (which may or may not depend on the new css and the css depend on the js) and it'll show up.