Last modified: 2014-08-26 03:07:04 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 31511 - Sporadic and unpredictable updating of (classic) toolbar via JS
Sporadic and unpredictable updating of (classic) toolbar via JS
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
ResourceLoader (Other open bugs)
1.17.x
All All
: Normal normal with 2 votes (vote)
: 1.18.0 release
Assigned To: Nobody - You can work on this!
: javascript, need-integration-test
: 35077 (view as bug list)
Depends on: 33952
Blocks: edit-toolbar
  Show dependency treegraph
 
Reported: 2011-10-08 00:40 UTC by Helder
Modified: 2014-08-26 03:07 UTC (History)
14 users (show)

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


Attachments
Current gadget, WHITHOUT ResourceLoader: fully functional (but will break once RL is enabled by default) (219.80 KB, image/png)
2011-10-08 14:52 UTC, Helder
Details
Current gadget + [ResourceLoader]: not functional and causes some errors (204.76 KB, image/png)
2011-10-08 14:55 UTC, Helder
Details
Current gadget + [ResourceLoader|dependencies=mediawiki.action.edit]: no errors, but not fully functional (213.91 KB, image/png)
2011-10-08 15:01 UTC, Helder
Details
Moving some JS code to $(document).ready (2.28 KB, patch)
2011-10-10 09:39 UTC, Helder
Details
Toolbar after moving some JS code to $(document).ready (81.55 KB, image/png)
2011-10-10 09:45 UTC, Helder
Details
Current gadget + new config + [ResourceLoader|dependencies=mediawiki.action.edit]: no errors, but not fully functional (105.94 KB, image/png)
2011-10-10 09:52 UTC, Helder
Details

Description Helder 2011-10-08 00:40:25 UTC
Since the recent MediaWiki update, the user script [[User:MarkS/extraeditbuttons.js]] (and its translation [[pt:MediaWiki:Gadget-Extra-Editbuttons.js]]) is not working anymore.[1][2]

I was trying to fix the Portuguese version and make it into a ResourceLoader-compatible gadget, but I wasn't capable of restoring its previous behaviour because it is currently impossible (AFAIK) to force its function "initButtons()" which depends on "mw.toolbar.addButton()" (from module "mediawiki.action.edit"[3]) to be executed BEFORE the execution of "mw.toolbar.init()"[4] BUT AFTER the default buttons[5] are added to the array "mw.toolbar.buttons"[6] (a replacemente to the legacy "window.mwCustomEditButtons") by the inline <script> tag in the botton of the page.

This order is necessary because "initButtons()" needs to be capable to redefine the content of the array "mw.toolbar.buttons" (adding and/or removing buttons) before it is used by "mw.toolbar.init()" to insert the listed buttons in the edit toolbar.

In its current state[7], the extra buttons are being added before the ones from MediaWiki, and the user can't change the order of (neither remove) the default MW buttons. This seems to be because those "mw.toolbar.addButton" from MediaWiki is appending the default elements to the array AFTER it has already being filled with user defined buttons (how can the user remove something which is not there yet?).

If I change "initButtons()" to "$(document).ready(initButtons)", then MediaWiki default buttons are added to the begining of the array and the gadget could edit the array, but in this case the gadget is executed too late, after "mw.toolbar.init()" happened, and so any changes made to "mw.toolbar.buttons" will have no efect in the resulting toolbar.

Please provide some alternative to fix this kind of user scripts which worked fine previously.

[1] https://en.wikipedia.org/w/index.php?title=Wikipedia:Village_pump_%28technical%29&diff=454228426&oldid=454227274&diffonly=1
[2] https://secure.wikimedia.org/wikipedia/pt/w/index.php?title=Wikip%C3%A9dia%3ACaf%C3%A9_dos_programadores&action=historysubmit&diff=27182215&oldid=27174834
[3] http://svn.wikimedia.org/viewvc/mediawiki/branches/REL1_18/phase3/resources/mediawiki.action/mediawiki.action.edit.js?view=markup#l8
[4] http://svn.wikimedia.org/viewvc/mediawiki/branches/REL1_18/phase3/resources/mediawiki.action/mediawiki.action.edit.js?view=markup#l85
[5] http://svn.wikimedia.org/viewvc/mediawiki/branches/REL1_18/phase3/includes/EditPage.php?view=markup#l2493
[6] http://svn.wikimedia.org/viewvc/mediawiki/branches/REL1_18/phase3/resources/mediawiki.action/mediawiki.action.edit.js?view=markup#l41
Comment 1 Helder 2011-10-08 00:48:12 UTC
(In reply to comment #0)
> [[User:MarkS/extraeditbuttons.js]] (and its translation
> [[pt:MediaWiki:Gadget-Extra-Editbuttons.js]]) is not working anymore.[1][2]

FYI: The Portuguese version was updated[8] to pass on JSHint before trying to make it compatible with RL.

PS: I missed this link in the first comment:
[7] https://pt.wikipedia.org/w/index.php?title=MediaWiki:Gadget-Extra-Editbuttons.js&oldid=27182587

[8] https://pt.wikipedia.org/w/index.php?diff=27164126&oldid=27158987
Comment 2 Michael M. 2011-10-08 08:16:45 UTC
See http://de.wikipedia.org/wiki/MediaWiki:Gadget-Extra-Editbuttons.js for a working version of the script (rewritten by [[User:✓]])
Comment 3 Helder 2011-10-08 14:52:07 UTC
Created attachment 9191 [details]
Current gadget, WHITHOUT ResourceLoader: fully functional (but will break once RL is enabled by default)

Thanks for pointing us to that script. It works fine on my local Mw1.18 instalation, but only if we keep ResourceLoader DISABLED.
Comment 4 Helder 2011-10-08 14:55:22 UTC
Created attachment 9192 [details]
Current gadget + [ResourceLoader]: not functional and  causes some errors

Unfortunately, that version doesn't works with ResourceLoader either.

If you go to [[de:MediaWiki:Gadgets-definition]] and replace the line
 * Extra-Editbuttons|Extra-Editbuttons.js
by
 * Extra-Editbuttons[ResourceLoader]|Extra-Editbuttons.js
(i.e., if you activate ResourceLoader for this gadget - see documentation at [[mw:Extension:Gadgets#Options]]) then your browser will give some error messages such as "mw.toolbar is undefined" on its error console.
Comment 5 Helder 2011-10-08 15:01:15 UTC
Created attachment 9193 [details]
Current gadget + [ResourceLoader|dependencies=mediawiki.action.edit]: no errors, but not fully functional

If you inspect the gadget code, you'll notice that it uses "mw.toolbar" from module "mediawiki.action.edit". So, it seems necessary to add this module as a dependency for that gadget. This is made by changing its definition to

 * Extra-Editbuttons[ResourceLoader|dependencies=mediawiki.action.edit]|Extra-Editbuttons.js

Althought this fix the errors, the gadget is now unable to remove the default buttons from MediaWiki classic toolbar.

This is something which I don't see how could be fixed by the local sysops themselves. The JS code from classic toolbar kind of has the gadget as a dependency in order to work as the user expects, but this isn't something we (sysops) can do, I think. This is why I opened this bug. Maybe it is already possible to fix it directly on our wikis, but I wasn't able to find out how...
Comment 6 Mark A. Hershberger 2011-10-08 20:59:34 UTC
This sounds similar to these two reports from WP:VPT.  I'll link this bug in so that maybe they'll be able to see what you've done.

http://en.wikipedia.org/w/index.php?diff=454405866

I have had about half the toolbar vanish on a number of occasions today. No big deal, but a singular occurrence.

http://en.wikipedia.org/w/index.php?diff=454588601&oldid=454587788

Not sure if this is the right place to report this, but I've noticed since the recent upgrade that not all the tools are displayed whenever a new article is created or some articles are edited. This is particularly true with the redirect and ref tools. I thought at first it might be because I'd recently changed my username, but this doesn't appear to be the case as it works with some long-established articles. Apologies if this has already been reported.
Comment 7 Helder 2011-10-09 12:50:59 UTC
Since the problem happens after enabling ResourceLoader, it seems more correct to put this bug into Bugzilla's ResourceLoader component.
Comment 8 Michael M. 2011-10-10 08:15:30 UTC
I can't test it at the moment, but I think the gadget could work in ResourceLoader mode if you
* move the normal = mw.toolbar.buttons.splice(0); and the mw.toolbar.generateTable = function into the $(document).ready bit and
* remove the dependency mw.action.edit

This is a bit hackish, but I have some hope that it works.
Comment 9 Helder 2011-10-10 09:37:40 UTC
(In reply to comment #8)
> * remove the dependency mw.action.edit

Roan or Krinkle may confirm/correct this, but I think if we remove a dependency from a gadget, we will have no guarantee that its functions (in this case mw.toolbar) will be available when needed. If there was that guarantee even without indicating the dependency, then what would be the purpose of having dependencies on ResourceLoader?
Comment 10 Helder 2011-10-10 09:39:39 UTC
Created attachment 9206 [details]
Moving some JS code to $(document).ready

(In reply to comment #8)
> I can't test it at the moment, but I think the gadget could work in
> ResourceLoader mode if you
> * move the normal = mw.toolbar.buttons.splice(0); and the
> mw.toolbar.generateTable = function into the $(document).ready bit and

Anyway, I did this change and it doesn't works as expected either (see next screenshot).
Comment 11 Helder 2011-10-10 09:45:56 UTC
Created attachment 9207 [details]
Toolbar after moving some JS code to $(document).ready

It is odd, but the proposed change actually adds the buttons two times!


PS: On this version I've changed my /common.js to remove all default buttons and include the first two again, but in the middle of the extra buttons. The code is the following (and works in the dewiki version without enabling ResourceLoader):

var customEditButtons = "SM,0,1,MY1,MY2";
var rmEditButtons = ['all'];
var myButtons = {
    'MY1':['//upload.wikimedia.org/wikipedia/commons/b/ba/Button_conserver.png','Vote *pro*',"# {{pro}} ","",''],
    'MY2':['//upload.wikimedia.org/wikipedia/commons/f/fc/Button_supp.png','Vote *contra*',"# {{contra}} ","",'']
};
Comment 12 Helder 2011-10-10 09:52:57 UTC
Created attachment 9208 [details]
Current gadget + new config + [ResourceLoader|dependencies=mediawiki.action.edit]: no errors, but not fully functional

BTW: Using this new personal common.js with the current gadget from dewiki, the normal buttons are not removed, the extra buttons are added to the end, and the selected default buttons are added in the middle of the extra buttons but without any of the expected HTML attributes (such as the icon)
Comment 13 Mark A. Hershberger 2011-10-15 22:03:07 UTC
tagging bugs for Marcus to look at
Comment 14 Helder 2011-10-21 23:51:03 UTC
(In reply to comment #11)
> It is odd, but the proposed change actually adds the buttons two times!

FYI: If I remove the line
 t.init();
from $(document).ready(function(){...}), the gadget will add the buttons only once, as expected.

I just don't understand if this is the safe/right way to do this, because of the following:
(In reply to comment #9)
> (In reply to comment #8)
> > * remove the dependency mw.action.edit
> 
> Roan or Krinkle may confirm/correct this, but I think if we remove a dependency
> from a gadget, we will have no guarantee that its functions (in this case
> mw.toolbar) will be available when needed. If there was that guarantee even
> without indicating the dependency, then what would be the purpose of having
> dependencies on ResourceLoader?

Could some Resource Loader expert clarify this? Is it safe to remove the dependency "mw.action.edit" even if we use "mw.toolbar" and "mw.toolbar.buttons" in the gadget? (and if so, why?)

Besides, if the variable "window.mwCustomEditButtons" is supposed to be customized by user scripts, shouldn't the module "mediawiki.action.edit" have the module "user" as a dependency[1], so that the custom edit buttons (defined on [[User:Example/common.js]]) are granted to be available to function "mw.toolbar.init" when it merges[2] mwCustomEditButtons and "mw.toolbar.buttons" (which will be used to add buttons to the DOM)?

[1] On http://svn.wikimedia.org/viewvc/mediawiki/branches/REL1_18/phase3/resources/Resources.php?revision=99054&view=markup#l502
[2] On http://svn.wikimedia.org/viewvc/mediawiki/branches/REL1_18/phase3/resources/mediawiki.action/mediawiki.action.edit.js?view=markup#l41
Comment 15 Roan Kattouw 2011-10-25 14:07:24 UTC
It seems the old toolbar's interface for adding buttons depends on loading order too much. The WikiEditor toolbar's interface is a bit better in this regard, but not much. These interfaces should be redesigned such that the loading order either doesn't matter or must be toolbar-first-gadget-later (so the gadget can depend on the toolbar and force the order to be that way), and such that adding, removing and modifying buttons at any location in the toolbar is supported.
Comment 16 Mark A. Hershberger 2011-10-28 02:19:45 UTC
(In reply to comment #15)
> These interfaces should be redesigned such that the
> loading order either doesn't matter

Obviously not going to happen in time for the tarball, but perhaps we can put it in the release notes?
Comment 17 Helder 2011-11-08 20:07:34 UTC
Here is a simpler test case, after seeing this post to wikitech-l:
http://lists.wikimedia.org/pipermail/wikitech-l/2011-November/056221.html

Consider a gadget having the code
----
mwCustomEditButtons[ mwCustomEditButtons.length ] = {
     "imageFile": "//upload.wikimedia.org/wikipedia/commons/b/ba/Button_clipboard_category.gif",
     "speedTip": "Test",
     "tagOpen": "{{Foo|",
     "tagClose": "|Bar}}",
     "sampleText": 'Baz...'
};
----

If it is added to [[MediaWiki:Gadgets-definition]] without [ResourceLoader], there will be a new button in the (classic) toolbar when in edit mode. If we add [ResourceLoader] to the definition, the button disappears.

I've confirmed this behavior on MW 1.17 and MW 1.18. On MW 1.18, the addition of [ResourceLoader] also causes the following message to appear in error console:
----
mw.loader::execute> Exception thrown by ext.gadget.Extra-Editbuttons: mwCustomEditButtons is not defined
----

Replacing mwCustomEditButtons by window.mwCustomEditButtons in the gadget's code doesn't fix it. And adding
----
window.mwCustomEditButtons = window.mwCustomEditButtons || [];
----

before the code only removes the error console message, but the button is still missing.

The tests above were done on Google Chrome 15.0.874.106 and on Firefox 7.0.1.
Comment 18 Sumana Harihareswara 2011-11-09 21:21:08 UTC
mybugs, am I right in inferring that you are still awaiting review for your patch in comment 10, "Moving some JS code to $(document).ready"?
Comment 19 Helder 2011-11-09 22:09:31 UTC
Er... not really. I uploaded that patch just to attach to this bug the "diff" of how I implemented the suggestion from comment 8.

Should the "patch" keyword be removed in this case?
Comment 20 Rob Lanphier 2011-11-14 22:26:31 UTC
Removing as 1.18 tarball blocker.
Comment 21 Sumana Harihareswara 2011-12-22 00:59:45 UTC
(In reply to comment #19)
> Er... not really. I uploaded that patch just to attach to this bug the "diff"
> of how I implemented the suggestion from comment 8.
> 
> Should the "patch" keyword be removed in this case?

OK, removed "patch" and "need-review" keywords.
Comment 22 Mark A. Hershberger 2012-01-25 17:07:27 UTC
This looks fixed on enwiki.beta
Comment 23 Helder 2012-01-25 17:32:37 UTC
(In reply to comment #22)
> This looks fixed on enwiki.beta
Could you try the small example from comment 17?
Comment 24 Mark A. Hershberger 2012-01-26 03:24:06 UTC
Both mybugs and I tried it.  No JS errors, but w/o [ResourceLoader] in MW:Gadgets-definition, it worked.  Without it, it didn't.

http://en.wikipedia.beta.wmflabs.org/wiki/MediaWiki:Gadgets-definition
Comment 25 Helder 2012-01-26 11:12:55 UTC
(In reply to comment #24)
> Without it, it didn't.

I suppose you mean "With [ResourceLoader], it didn't work."
Comment 26 Mark A. Hershberger 2012-01-26 14:45:57 UTC
(In reply to comment #25)
> (In reply to comment #24)
> > Without it, it didn't.
> 
> I suppose you mean "With [ResourceLoader], it didn't work."

right.
Comment 27 Mark A. Hershberger 2012-01-30 19:23:34 UTC
(In reply to comment #23)
> (In reply to comment #22)
> > This looks fixed on enwiki.beta
> Could you try the small example from comment 17?

Could you test this again since Krinkle fixed bug 33746?
Comment 28 Krinkle 2012-01-30 19:33:35 UTC
This bug can't be fixed without bug 33952.

Any script that adds buttons to this classic toolbar is either in luck or not, and this has been so even in the old days before 1.17. The classic bar has been this way forever. It accepts extra buttons until it initializes at document ready and then it's immutable.

Reports about being unable to add things have been floating around for years. i.e. sometimes it would only work from script A and not from script B. Sometimes only when executed directly and not when loaded from another wiki, in order words: unstable race condition based on a dice roll.

This is not new in 1.19, 1.18 or 1.17. Also not affected by the load speed improvement in 1.19.
It does hoever get affected when a gadget is marked with [ResourceLoader] because that puts the module in the module load queue instead of the legacy scripts queue. The legacy queue has better odds of being in time for the legacy toolbar.

Your best bet is to either:
* not use [ResourceLoader] when manipulating the classic toolbar
* upgrade the script to instead manipulate the new WikiEditor. The new WikiEditor has a much more stable API and has fixed many many of these kind of bugs. And it also has more features for button types, group and user interaction. And it allows adding buttons at any time, including after document ready and from any point in the execution flow. Also, when your gadget provides buttons for WikiEditor instead of the classic toolbar, more users will see it, since there are more WikiEditor users than classic toolbar users.

When bug 33952 is fixed, the classic toolbar will basically get WikiEditor-like features (adding buttons at any time).
Comment 29 Helder 2012-01-30 20:02:17 UTC
(In reply to comment #27)
> Could you test this again since Krinkle fixed bug 33746?

Using [ResourceLoader] I get:
> Uncaught TypeError: Cannot call method 'anonymous' of undefined
> Uncaught ReferenceError: mwCustomEditButtons is not defined
and the button is not added.

Removing [ResourceLoader] the button is added to the toolbar.
Comment 30 Helder 2012-01-30 20:09:44 UTC
BTW: On debug mode, on
http://en.wikipedia.beta.wmflabs.org/w/index.php?title=Testing&action=edit&debug=1
the gadget with RL causes this error to be displayed:
Uncaught ReferenceError: mwCustomEditButtons is not defined

but on normal mode,
http://en.wikipedia.beta.wmflabs.org/w/index.php?title=Testing&action=edit
the error do not happens. Instead, I get the error
Uncaught TypeError: Cannot call method 'anonymous' of undefined
mentioned on
http://labs.wikimedia.beta.wmflabs.org/wiki/Problem_reports#Special:BannerController
Comment 31 Mark A. Hershberger 2012-02-21 21:47:17 UTC
eowiki is seeing this now.  https://eo.wikipedia.org/wiki/MediaWiki:Common.js tries to add buttons to mwCustomEditButtons and some people are reporting they don't see the buttons after they hit preview.  I'm not seeing them until I hit preview.
Comment 32 Mark A. Hershberger 2012-02-21 21:54:24 UTC
Note, also, that eowiki isn't doing this with gadgets.  Should this be a new bug?
Comment 33 Mark A. Hershberger 2012-02-21 22:12:12 UTC
Commons has something similar in https://commons.wikimedia.org/wiki/MediaWiki:Monobook.js and a lot of people doing similar things in their user scripts: https://commons.wikimedia.org/wiki/Special:Search?search=mwCustomEditButtons&ns2=1
Comment 34 Mark A. Hershberger 2012-02-21 22:36:41 UTC
Note that I've updated the summary of this bug to remove the reference to gadgets, which is what I was talking about in Comment 32.

I think this will render much of what Krinkle has written at https://commons.wikimedia.org/wiki/Commons:Counter_Vandalism_Unit useless.  Testing now.
Comment 35 Krinkle 2012-02-21 23:18:43 UTC
(In reply to comment #34)
> Note that I've updated the summary of this bug to remove the reference to
> gadgets, which is what I was talking about in Comment 32.
> 
> I think this will render much of what Krinkle has written at
> https://commons.wikimedia.org/wiki/Commons:Counter_Vandalism_Unit useless. 
> Testing now.

Actually most of that Commons page section on 'Buttons' is about Vector/WikiEditor. Not about the classic toolbar. That section is and will stay correct as it is, and works fine without race conditions or bugs like these.

The classic toolbar utilities (listed there under "monobook"), do indeed fail right now for some people due to the race condition between the building of the toolbar and the execution of the module that registers them. This can be fixed if we implement bug 33952.

Bug 33952 proposes to make the classic toolbar more like the WikiEditor. By offering a simple-to-use API with a addButton-function that will either add it to a queue if the toolbar hasn't been build yet, or it will change the toolbar that is already build and add an extra button.

Doing that isn't trivial, but not very complicated either. However note that if we would create such an API for the classic toolbar, that will still require existing code to be updated to use that API instead.

I think if we require user scripts to update, we might as do it right and let them migrate to the WikiEditor API instead. That way the button will also show up for users using Vector/WikiEditor. Right now the classic toolbar button stuff is very in minority as those buttons only show up for users using Monobook/classic-toolbar. And if we implement bug 33952, we'd only be restoring it for monobook/classic-toolbar.
Comment 36 Mark A. Hershberger 2012-02-21 23:48:42 UTC
(In reply to comment #35)
> Actually most of that Commons page section on 'Buttons' is about
> Vector/WikiEditor. Not about the classic toolbar. That section is and will stay
> correct as it is, and works fine without race conditions or bugs like these.
> 
> The classic toolbar utilities (listed there under "monobook"), do indeed fail
> right now for some people due to the race condition between the building of the
> toolbar and the execution of the module that registers them. This can be fixed
> if we implement bug 33952.

I should said "the part under monobook" instead of "much of" since I knew that was the case.  Apologies.
Comment 37 Krinkle 2012-03-09 05:03:02 UTC
mw.toolbar.addButton has been further enhanced to allow button insertion at any time. Wether it is called before or after initialization, it will insert it in the toolbar (before init it will memorize it until init, and after init it inserts directly) - this was done by feature ticket bug 33952.

Marking this as FIXED as such. 

Code that uses mwCustomEditButtons may continue to do so if it works, however when items are added to this array after the toolbar has initialized, the buttons can no longer be detected. This has always been the case (even before 1.17 and ResourceLoader).

mw.toolbar.addButton does exactly the same (fully compatible) but does not have this issue (due to it being a function instead of an array it can execute logic at any time).
Comment 38 Krinkle 2012-03-09 05:04:02 UTC
*** Bug 35077 has been marked as a duplicate of this bug. ***

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


Navigation
Links