Wikimedia Bugzilla is closed!

(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

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

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.

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.

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

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.

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

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.

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

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

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}} ","",'']
};

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)

tagging bugs for Marcus to look at

(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

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.

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

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.

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

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?

Removing as 1.18 tarball blocker.

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

This looks fixed on enwiki.beta

(In reply to comment #22)
> This looks fixed on enwiki.beta
Could you try the small example from comment 17?

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

(In reply to comment #24)
> Without it, it didn't.

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

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

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

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

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

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

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.

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

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

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.

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

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

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

*** Bug 35077 has been marked as a duplicate of this bug. ***