Sporadic and unpredictable updating of (classic) toolbar via JS
Closed, ResolvedPublic

Description

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


Version: 1.17.x
Severity: normal

bzimport added a subscriber: Unknown Object (MLST).
bzimport set Reference to bz31511.
He7d3r created this task.Via LegacyOct 8 2011, 12:40 AM
He7d3r added a comment.Via ConduitOct 8 2011, 12:48 AM

(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

Schnark added a comment.Via ConduitOct 8 2011, 8:16 AM

See http://de.wikipedia.org/wiki/MediaWiki:Gadget-Extra-Editbuttons.js for a working version of the script (rewritten by [[User:✓]])

He7d3r added a comment.Via ConduitOct 8 2011, 2:52 PM

Created attachment 9191
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.

Attached:

He7d3r added a comment.Via ConduitOct 8 2011, 2:55 PM

Created attachment 9192
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.

Attached:

He7d3r added a comment.Via ConduitOct 8 2011, 3:01 PM

Created attachment 9193
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...

Attached:

MarkAHershberger added a comment.Via ConduitOct 8 2011, 8:59 PM

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.

He7d3r added a comment.Via ConduitOct 9 2011, 12:50 PM

Since the problem happens after enabling ResourceLoader, it seems more correct to put this bug into Bugzilla's ResourceLoader component.

Schnark added a comment.Via ConduitOct 10 2011, 8:15 AM

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.

He7d3r added a comment.Via ConduitOct 10 2011, 9:37 AM

(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?

He7d3r added a comment.Via ConduitOct 10 2011, 9:39 AM

Created attachment 9206
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).

Attached: Moving_some_code_to_(document).ready

He7d3r added a comment.Via ConduitOct 10 2011, 9:45 AM

Created attachment 9207
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}} ","",'']

};

Attached:

He7d3r added a comment.Via ConduitOct 10 2011, 9:52 AM

Created attachment 9208
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)

Attached:

MarkAHershberger added a comment.Via ConduitOct 15 2011, 10:03 PM

tagging bugs for Marcus to look at

He7d3r added a comment.Via ConduitOct 21 2011, 11:51 PM

(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

Catrope added a comment.Via ConduitOct 25 2011, 2:07 PM

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.

MarkAHershberger added a comment.Via ConduitOct 28 2011, 2:19 AM

(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?

He7d3r added a comment.Via ConduitNov 8 2011, 8:07 PM

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.

bzimport added a comment.Via ConduitNov 9 2011, 9:21 PM

sumanah wrote:

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"?

He7d3r added a comment.Via ConduitNov 9 2011, 10:09 PM

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?

RobLa-WMF added a comment.Via ConduitNov 14 2011, 10:26 PM

Removing as 1.18 tarball blocker.

bzimport added a comment.Via ConduitDec 22 2011, 12:59 AM

sumanah wrote:

(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.

MarkAHershberger added a comment.Via ConduitJan 25 2012, 5:07 PM

This looks fixed on enwiki.beta

He7d3r added a comment.Via ConduitJan 25 2012, 5:32 PM

(In reply to comment #22)

This looks fixed on enwiki.beta

Could you try the small example from comment 17?

MarkAHershberger added a comment.Via ConduitJan 26 2012, 3:24 AM

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

He7d3r added a comment.Via ConduitJan 26 2012, 11:12 AM

(In reply to comment #24)

Without it, it didn't.

I suppose you mean "With [ResourceLoader], it didn't work."

MarkAHershberger added a comment.Via ConduitJan 26 2012, 2:45 PM

(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.

MarkAHershberger added a comment.Via ConduitJan 30 2012, 7:23 PM

(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?

Krinkle added a comment.Via ConduitJan 30 2012, 7:33 PM

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).

He7d3r added a comment.Via ConduitJan 30 2012, 8:02 PM

(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.

He7d3r added a comment.Via ConduitJan 30 2012, 8:09 PM

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

MarkAHershberger added a comment.Via ConduitFeb 21 2012, 9:47 PM

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.

MarkAHershberger added a comment.Via ConduitFeb 21 2012, 9:54 PM

Note, also, that eowiki isn't doing this with gadgets. Should this be a new bug?

MarkAHershberger added a comment.Via ConduitFeb 21 2012, 10:12 PM

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

MarkAHershberger added a comment.Via ConduitFeb 21 2012, 10:36 PM

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.

Krinkle added a comment.Via ConduitFeb 21 2012, 11:18 PM

(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.

MarkAHershberger added a comment.Via ConduitFeb 21 2012, 11:48 PM

(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.

Krinkle added a comment.Via ConduitMar 9 2012, 5:03 AM

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).

Krinkle added a comment.Via ConduitMar 9 2012, 5:04 AM
  • Bug 35077 has been marked as a duplicate of this bug. ***

Add Comment