Last modified: 2012-07-25 00:48:48 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T39331, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 37331 - mw.loader sometimes executes the module script twice
mw.loader sometimes executes the module script twice
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
1.20.x
All All
: Normal critical (vote)
: 1.20.0 release
Assigned To: Roan Kattouw
:
: 37609 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-05 00:01 UTC by Ryan Kaldari
Modified: 2012-07-25 00:48 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Ryan Kaldari 2012-06-05 00:01:55 UTC
In PageTriage, we have a JS file that loads a separate ResourceLoader module if needed. It uses the syntax:
mw.loader.load( 'ext.pageTriage.views.toolbar' )

In some rare circumstances, it loads the ext.pageTriage.views.toolbar module twice (although the load command is only called once), causing the interface to break badly. This seems to only happen in Firefox.

I suspect this is a similar issue to Bug 34696. The steps to reproduce are a bit complicated, so I'm not going to post them unless you need them. Just let me know.
Comment 1 Krinkle 2012-06-05 17:10:32 UTC
(In reply to comment #0)
> [..] The steps to reproduce are a
> bit complicated, so I'm not going to post them unless you need them. Just let
> me know.

Please do post the steps to reproduce this bug :)
Comment 2 Krinkle 2012-06-05 17:11:22 UTC
Also, is this new in 1.20alpha? (I guess once the bug is reproducible, git bisect will tell us when it was introduced, or otherwise that it a bug since the beginning).
Comment 3 Ryan Kaldari 2012-06-05 20:20:58 UTC
Yeah, I'm seeing it in 1.20alpha. I first noticed the bug a couple weeks ago, but assumed it was something on our end. It only shows up in conjunction with a new feature that we haven't deployed anywhere, so the fact that I only noticed it recently doesn't rule out the bug existing earlier.

I'll post the repro steps in a bit...
Comment 4 Ryan Kaldari 2012-06-14 22:00:41 UTC
*** Bug 37609 has been marked as a duplicate of this bug. ***
Comment 5 Ryan Kaldari 2012-06-14 22:09:01 UTC
The bug is appearing on en.wiki now, so it should be fairly easy to reproduce...

Steps to reproduce in Firefox:
1. Log into en.wiki with an autoconfirmed account
2. Go to http://en.wikipedia.org/wiki/Special:NewPagesFeed
3. Click 'Review' for one of the unreviewed articles (in red)
4. On that page add '?curationtoolbar=true' to the URL
The items in the curationtoolbar should load twice if the bug occurs (you'll see duplicate icons)
Comment 6 Ryan Kaldari 2012-06-15 17:22:37 UTC
*** Bug 37605 has been marked as a duplicate of this bug. ***
Comment 7 Ryan Kaldari 2012-07-24 01:14:14 UTC
Bumping to critical since this is blocking turning on the curation toolbar on en.wiki.
Comment 8 Roan Kattouw 2012-07-24 18:40:29 UTC
Investigating this now.
Comment 9 Krinkle 2012-07-24 20:38:45 UTC
Roan and I have been debugging this. It turns out we're hitting a race condition.

When a module is ready to be executed, the module package is expanded (messages loaded, styles inserted, script executed) and then we set 'state' to 'ready'.

However in this case the script itself also has calls to mw.loader.load, which at some point calls handlePending (which executes modules that were waiting for execution due to unresolved dependencies). But this corrupted the other execution flow because it happened during execution. And the state of that module wasn't set to 'ready' until after the module is ready executing.

Fixed by setting the state right before execution begins.
Comment 10 Roan Kattouw 2012-07-24 20:39:39 UTC
(In reply to comment #9)
> Roan and I have been debugging this. It turns out we're hitting a race
> condition.
> 
> When a module is ready to be executed, the module package is expanded (messages
> loaded, styles inserted, script executed) and then we set 'state' to 'ready'.
> 
> However in this case the script itself also has calls to mw.loader.load, which
> at some point calls handlePending (which executes modules that were waiting for
> execution due to unresolved dependencies). But this corrupted the other
> execution flow because it happened during execution. And the state of that
> module wasn't set to 'ready' until after the module is ready executing.
> 
> Fixed by setting the state right before execution begins.
https://gerrit.wikimedia.org/r/16550

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links