Page MenuHomePhabricator

Global JS module version not updating properly
Closed, ResolvedPublic

Description

I created a global.js page at
http://meta.wikimedia.beta.wmflabs.org/w/index.php?oldid=551
and it works when I visit any of these pages in debug mode
http://en.wikipedia.beta.wmflabs.org/wiki/Special:BlankPage?debug=1
http://meta.wikimedia.beta.wmflabs.org/wiki/Special:BlankPage?debug=1

BUT the global code is not loaded if I'm not in debug mode:
http://en.wikipedia.beta.wmflabs.org/wiki/Special:BlankPage
http://meta.wikimedia.beta.wmflabs.org/wiki/Special:BlankPage

I tested using Google Chrome 33.0.1750.149 and Firefox 27.0.1.

On Chrome I tried CTRL+F5, SHITFT+F5, CTRL+SHIFT+R, and also with the option "Disable cache (while DevTools is open)", and nothing worked.

On
http://en.wikipedia.beta.wmflabs.org/wiki/Special:BlankPage
Google Chrome shows the following link in the Sources panel:
http://bits.beta.wmflabs.org/meta.wikimedia.beta.wmflabs.org/load.php?debug=false&lang=en&modules=ext.globalcssjs.user&skin=vector&user=Helder.wiki&version=20140310T100305Z&*

Its content is:
mw.loader.implement("ext.globalcssjs.user",function(){},{},{});
/* cache key: metawiki:resourceloader:filter:minify-js:7:25e245bf16e0037a7248f80fd384e529 */

If I remove the "&version=20140310T100305Z&*" from the URL, the content changes to
mw.loader.implement("ext.globalcssjs.user",function(){$(function(){$('#mw-content-text').prepend('<strong style="color: #cc0000; font-size: 150%;">Your <a href="//meta.wikimedia.beta.wmflabs.org/w/index.php?title=Special:MyPage/global.js&action=edit">global scripts</a> were loaded!</strong>');});someTest1='global.js';var someTest2='global.js';window.someTest3='global.js';;},{},{});
/* cache key: metawiki:resourceloader:filter:minify-js:7:101b9d1a9e616e1315c94ace38794a15 */

Is this a problem in the extension? Or maybe it needs some configuration related to Resource Loader?


Version: unspecified
Severity: normal

Details

Reference
bz62602

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:06 AM
bzimport added projects: GlobalCssJs, JavaScript.
bzimport set Reference to bz62602.
bzimport added a subscriber: Unknown Object (MLST).
He7d3r created this task.Mar 13 2014, 2:04 PM

Ops... I was doing some other tests, so the last example has a few someTest* variables which were not in the version of global.js I linked. The correct example is this: If I remove the "&version=20140310T100305Z&*" from the URL, the content changes to
mw.loader.implement("ext.globalcssjs.user",function(){$(function(){$('#mw-content-text').prepend('<strong style="color: #cc0000; font-size: 150%;">Your <a href="//meta.wikimedia.beta.wmflabs.org/w/index.php?title=Special:MyPage/global.js&action=edit">global scripts</a> were loaded!</strong>');});;},{},{});
/* cache key: metawiki:resourceloader:filter:minify-js:7:94d6fa5683df6cab0cdc82575eb686ba */

Lego: thoughts on this bug?

He7d3r added a comment.Apr 9 2014, 1:35 PM

So, I haven't touched that JS page for ~4 weeks and now, when I access any of
http://meta.wikimedia.beta.wmflabs.org/wiki/Special:BlankPage
http://en.wikipedia.beta.wmflabs.org/wiki/Special:BlankPage
the global JS is loaded.

But then I decided to check how it is dealing with updates: I made a small change in the message my global code was displaying
http://meta.wikimedia.beta.wmflabs.org/w/index.php?diff=584

  • After the page was saved and finished reloading, the old message was still displayed
  • CTRL+SHIFT+R, SHIFT+Reload and CTRL+F5 were not enough to force the message to be updated (both on meta.wikimedia.beta.wmflabs.org and on en.wikipedia.beta.wmflabs.org).
  • The new message was immediately visible in debug mode, on these pages:

http://meta.wikimedia.beta.wmflabs.org/wiki/Special:BlankPage?debug=1
http://en.wikipedia.beta.wmflabs.org/wiki/Special:BlankPage?debug=1
but the old one would still show up when I removed the "?debug=1" from these URLs.

Helder: What happens when you update a non-global CSS/JS page (such as /monobook.js or /vector.css or whatever)? The caching issue you're having may be specific to Beta Labs, not the GlobalCssJs extension.

(For the record, right now I see the most recent version of the script I mentioned in the test from comment 3. So it doesn't need ~1 month to be updated)

A made a similar test in my common.js and my vector.js
http://meta.wikimedia.beta.wmflabs.org/w/index.php?diff=587
http://meta.wikimedia.beta.wmflabs.org/w/index.php?diff=588
and the new code was executed as soon as the page finished reloading, after pressing the save button.

But on global.js it still doesn't works:
http://meta.wikimedia.beta.wmflabs.org/w/index.php?diff=589

(In reply to Helder from comment #3)

  • After the page was saved and finished reloading, the old message was still

displayed

  • CTRL+SHIFT+R, SHIFT+Reload and CTRL+F5 were not enough to force the

message to be updated (both on meta.wikimedia.beta.wmflabs.org and on
en.wikipedia.beta.wmflabs.org).

  • The new message was immediately visible in debug mode, on these pages:

http://meta.wikimedia.beta.wmflabs.org/wiki/Special:BlankPage?debug=1
http://en.wikipedia.beta.wmflabs.org/wiki/Special:BlankPage?debug=1
but the old one would still show up when I removed the "?debug=1" from these
URLs.

Doing all that cache clearing in your browser won't do anything because it's cached server-side. The urls are unique by version, and those are permanently cached. Then, once a content update is detected (within 5 minutes using the startup module) you get a new url which naturally bypasses cache because it's a new url.

I'd recommend you try making another edit, then don't clear anything, wait for upto 2*5 minutes and verify it does indeed not automatically update.

If after 10 minutes it is still giving the old version, then we've found a bug in GlobalJsCss related to cache versioning, presumably its logic to build a timestamp is not working.

But why would it update instantaneously for common.js and vector.js (which is what I expect) but take more than 10 minutes for global.js?

It has been more than 10 minutes since my last edit to global.js and the JS is not updated.

Ok, so I've tested this a bit, and can easily reproduce this on beta labs, but the version (and cache) updates properly on my local vagrant instance.

(In reply to Krinkle from comment #6)

If after 10 minutes it is still giving the old version, then we've found a
bug in GlobalJsCss related to cache versioning, presumably its logic to
build a timestamp is not working.

The code path it uses to come up with the version number is the same as ResourceLoaderWikiModule, except it makes a call to a foreign db since we override $this->getDB.

I played around with eval.php on beta labs and traced that code path through, and everything worked as expected even with the remote database, so I'm really not sure what isn't working.

ori added a comment.Apr 18 2014, 5:23 AM

The URLs for loading the user and user.group modules is constructed in PHP, in lines 2906 - 2929 of OutputPage.php (@829886b10a). (The modules are not versioned via the startup module and are not fetched by the JavaScript-based module loader.) You'll most likely need to find a way to include the global user scripts in that URL. (You could declare them as dependencies of the user module, though that'd be a bit evil.)

Change 127457 had a related patch set uploaded by Legoktm:
Add OutputPageScriptsForBottomQueue hook

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

I looked into turning it into a dependency, but I don't think that would be possible since the 'user' module is registered before GlobalCssJs would have a chance to alter it.

Ica631fd0639c13b937be2d338b9a3868d0b8e3f7 adds a hook at the same point where the user and site modules are loaded, allowing extensions to add their own modules at that point. The downside of doing it this way is that it would require an additional request client-side to load the global modules.

Change 127457 abandoned by Legoktm:
Add OutputPageScriptsForBottomQueue hook

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

From https://gerrit.wikimedia.org/r/127457:


As discussed on IRC, setting group => user and position => top in the resource definition will have the same effect without requiring a hook.

Change 127461 had a related patch set uploaded by Legoktm:
Set group=>user and position=> top for ext.globalCssJs.user

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

ori added a comment.Apr 25 2014, 5:58 PM

From https://gerrit.wikimedia.org/r/127457 again:


Timo, the reason for position => top is to be found in OutputPage::getHeadScripts. To ensure the module version is always up-to-date, the load.php link must be generated in PHP (rather than JavaScript, which depends on the version set in the startup module). And getHeadScripts will only generate links for modules in the bottom load queue if $wgResourceLoaderExperimentalAsyncLoading is set.
IMO, the proper way to fix this is to make getHeadScripts generate an inline mw.loader#register script in <head> just below the startup module for all private / user modules. It would override the version set in the startup module, which would allow us to let JavaScript code construct the load.php URLs.
(This is already kinda happening: if you update the user JS you'll notice that the version for the user module in the startup module takes the usual time to update but that doesn't matter because the load.php URL changes instantly to reference the correct version. Though we might simply want to modify ResourceLoaderStartupModule so that it doesn't register private / user modules at all.)
@Legoktm: I thought I would be OK with merging this because I figured that the only issue posed by position => top would be a performance hit, and I am confident we can mitigate it using the strategy I described above. But there's a bigger worry, which is that when we switch these modules to the bottom queue a whole bunch of global scripts could break.
I think we can use MassMessage as a test-case before modifying core. So what I would suggest is this: add a new ResourceLoaderModule subclass called something like 'ResourceLoaderUncachedRegistrationModule'. Set its group to 'private' and its position to 'top', so that it gets inlined. The module's getScript function should consist of a call to ResourceLoader::makeLoaderRegisterScript( 'ext.globalCssJs.user' ).

This would mean that even if the version of the ext.globalCssJs.user module specified in the startup module is stale, it would get overridden by the correct version in the inline script just below. You would still be setting group => user on ext.globalCssJs.user, but not position => top.

Okay, there's tons of messages back and forth and implementation has changed many times.

A fresh brain dump:

  1. The startup module can't be relied upon for user specific modules. We already don't do this for the (local) 'site' and 'user' modules either for they are called directly from the page html and (for 'user') *with* a version timestamp:

view-source:https://www.mediawiki.org/

<script src="bits.wikimedia.org/www.mediawiki.org/load.php?debug=false&amp;lang=en&amp;modules=site&amp;only=scripts&amp;skin=vector&amp;*"></script>
<script src="
bits.wikimedia.org/www.mediawiki.org/load.php?debug=false&amp;lang=en&amp;modules=user&amp;only=scripts&amp;skin=vector&amp;user=Krinkle&amp;version=20140617T190748Z&amp;*"></script>
</body></html>

  1. I'm not sure where this inline registration comes from. It looks like a hack and I forgot what that is supposed to work around. Whatever the reason is, the way it is proposed in patch set 3 of Ieb6cfe2bf9a86a4 looks wrong because it doesn't compute a module version (it passes null), which means that either it is broken or makes a request without a version in the url query, which means the request will be cached between 5 and 30 minutes and not invalidated when you want it to.
  1. I'd recommend simply going for the same approach as for local 'user'. Make the <script> and <link> directly. Have a ResourceLoaderModule class that retrives the style/scrript from the foreign database and compute a valid module version (happens automatically when extending ResourceLoaderWikiModule), and generate a load.php request inside the page html to the foreign wiki including &version= in the url. That way it is cached for 30 days and yet immediately invalidated when the module changes (for the 'version' value will no longer be the same then).

Change 141282 had a related patch set uploaded by Legoktm:
Add OutputPageScriptsForBottomQueue hook

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

Change 141284 had a related patch set uploaded by Legoktm:
Use new OutputPageScriptsForBottomQueue hook

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

(In reply to Krinkle from comment #16)

  1. I'd recommend simply going for the same approach as for local 'user'.

Make the <script> and <link> directly. Have a ResourceLoaderModule class
that retrives the style/scrript from the foreign database and compute a
valid module version (happens automatically when extending
ResourceLoaderWikiModule), and generate a load.php request inside the page
html to the foreign wiki including &version= in the url. That way it is
cached for 30 days and yet immediately invalidated when the module changes
(for the 'version' value will no longer be the same then).

Ok, that sounds good. I uploaded a patch to add a core hook to let us add a module there, and another updating GlobalCssJs to use it.

Change 141282 merged by jenkins-bot:
Add OutputPageScriptsForBottomQueue hook

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

Change 127461 abandoned by Legoktm:
Create ResourceLoaderUncachedRegistrationModule

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

(In reply to Gerrit Notification Bot from comment #18)

Change 141284 had a related patch set uploaded by Legoktm:
Use new OutputPageScriptsForBottomQueue hook
https://gerrit.wikimedia.org/r/141284

For anyone following the bug, I believe this is the remaining hurdle to clear.

Change 141284 merged by jenkins-bot:
Use new OutputPageScriptsForBottomQueue hook

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

Fixed now, thanks to everyone who helped with this! Can be tested on beta labs (in a few minutes probably) or http://legoktm.wmflabs.org/.