Page MenuHomePhabricator

ResourceLoaderWikiModule::isKnownEmpty does not check whether the page is empty
Closed, ResolvedPublic

Description

I noticed that even if I blank my common.js, a <script> tag is still added for it.

ResourceLoaderWikiModule::isKnownEmpty() is just checking for page existence, not rev_len > 0.


Version: 1.24rc
Severity: enhancement

Details

Reference
bz68488

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 3:28 AM
bzimport set Reference to bz68488.
Legoktm created this task.Jul 23 2014, 11:18 PM

This is by design.

The main use (and currently only use) of this method is to determine whether to actually load a module that is specified to be hardcoded in the page without a version number / cache buster in the url. E.g. the 'site' module which is hardcoded in the html (dedicated request, not mw.loader.load).

If those modules are not configured (e.g. the wiki has these disabled via wg config), or not yet initialised (the wiki or user has no such page), then we exclude it from the html to save a useless http request.

Emptying the page means the structure is still there but (temporarily?) unused or disabled by users. In those cases we don't want to generate html with no <script src=".. modules=site"> in it as that would be be cached for 30 days (e.g. 'site' module in html for anonymous users). When the page is no longer empty, it would be missing from all cached pages.

That seems reasonable for the site module and other modules in the MediaWiki: namespace, but I don't think it applies to the user module where we don't need to really worry about the 30 day cache. The use-case I'm imagining is where a user creates a common.js page, decides they don't need the JS anymore and then blanks it since they aren't a sysop and can't delete it. (There are about 6850 such pages on enwp).

Maybe the user module (and the ext.globalCssJs.user module) should override isKnownEmpty?

(In reply to Kunal Mehta (Legoktm) from comment #2)

That seems reasonable for the site module and other modules in the
MediaWiki: namespace, but I don't think it applies to the user module where
we don't need to really worry about the 30 day cache. The use-case I'm
imagining is where a user creates a common.js page, decides they don't need
the JS anymore and then blanks it since they aren't a sysop and can't delete
it. (There are about 6850 such pages on enwp).

Maybe the user module (and the ext.globalCssJs.user module) should override
isKnownEmpty?

For purposes of clarity, does empty mean page.page_len == 0 or does empty mean there's no functional JavaScript/CSS (it's all comments or commented out code)?

(In reply to MZMcBride from comment #3)

For purposes of clarity, does empty mean page.page_len == 0 or does empty
mean there's no functional JavaScript/CSS (it's all comments or commented
out code)?

page.page_len == 0

(In reply to Kunal Mehta (Legoktm) from comment #2)

That seems reasonable for the site module and other modules in the
MediaWiki: namespace, but I don't think it applies to the user module where
we don't need to really worry about the 30 day cache. The use-case I'm
imagining is where a user creates a common.js page, decides they don't need
the JS anymore and then blanks it since they aren't a sysop and can't delete
it. (There are about 6850 such pages on enwp).

Maybe the user module (and the ext.globalCssJs.user module) should override
isKnownEmpty?

Yes. In that case we could do that. Plus, in that change, document in the main WikiModule class why that one only checks page existence.

+performance; this'll save one or two http requests for logged-in users who created user scripts but blanked them.

Change 157043 had a related patch set uploaded by Legoktm:
Check page_len in ResourceLoaderWikiModule::isKnownEmpty() for 'user' modules

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

Change 157043 merged by jenkins-bot:
Check page_len in ResourceLoaderWikiModule::isKnownEmpty() for 'user' modules

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

bd808 moved this task from Tag to Archive on the Performance Issue board.Mar 11 2015, 7:29 PM