557987 - Toolbar buttons with drop-down menus are busted

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[IU]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100407 Firefox/3.6 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a5pre) Gecko/20100407 Minefield/3.7a5pre

Toolbar buttons that have a drop-down menu no longer work.  Only the drop-down component is accessible -- even when the button itself is clicked.

Regression range:

http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f292d7bd2e93&tochange=a1d1e59ac1de


Reproducible: Always

Steps to Reproduce:
1. Install "Reload Every" extension: http://reloadevery.mozdev.org/#installation
2. After restarting Minefield, notice that whenever you click the refresh button, it is only the button's drop-down menu that works.
3. Reproducible also with the following extensions:
a) Video DownloadHelper: https://addons.mozilla.org/en-US/firefox/addon/3006
b) Print Preview: https://addons.mozilla.org/en-US/firefox/addon/1778

Comment 1

[Alice0775 White]

Yes, toolbarbutton with type="menu-button"does not work properly.

Comment 2

[:Felipe Gomes (needinfo for replies!)]

Attachment: fix

Thanks for catching that.

originalLength was to be used to check if new elements were added, not to list the local list of frames.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We need a mochitest here to catch this regression.

Comment 4

[Robert Kaiser]

(In reply to comment #3)
> We need a mochitest here to catch this regression.

I hope the fix can still land without the test being already there, though, as functionality is broken right now...

Comment 5

[Philip Chee]

Affects the Flashblock toolbar button as well. See: <https://www.mozdev.org/bugs/show_bug.cgi?id=22698>

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Yes, the fix can land as-is.

Comment 7

[:Felipe Gomes (needinfo for replies!)]

Attachment: fix with tests

Merged the fix here, with tests included in this patch.

For the tests, it's not possible to get anonymous nodes from elementFromPoint, so what I did is to dispatch mouseclicks on the toolbarbutton and make sure that the button and the menu dropmarker correctly receive the events. roc, does this seem reasonable for the test? I verified and it does catch the regression if the fix is not applied.

Also included in the test a check for <menulist editable="true"/> (reported as bug 558162) which is the same bug.

Comment 8

[:Felipe Gomes (needinfo for replies!)]

(anyone interested feel free to land and I can watch the tree)

Comment 11

[IU]

Is there anyone monitoring this bug, who could land this?  Thanks.

Comment 12

[:Felipe Gomes (needinfo for replies!)]

Attachment: fix with tests - for checkin

turns out the button inside menubuttons have a rounded style on mac and dispatching a click on the edge is not ideal. I made a minor change in this version to send the click to the center.
Sent to tryserver, everything ran green.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/aa360768a319

Comment 14

[Jim Jeffery not reading bug-mail 1/2/11]

Verifying fixed using an hourly build with the cset:
http://hg.mozilla.org/mozilla-central/rev/aa360768a319

Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.3a5pre) Gecko/20100410 Minefield/3.7a5pre ID:20100410132025

Comment 15

[Ruediger Lahl]

(In reply to comment #14)
> Verifying fixed using an hourly build with the cset:

Patch seems not solve the bug in 'Modern Theme' from SM. Right-clicking on that Icons in 'Modern' don't show a menue. It always choose the default of this button.

Please set 'Status' to 'REOPENED'

Comment 16

[:Felipe Gomes (needinfo for replies!)]

(In reply to comment #15)
> (In reply to comment #14)
> > Verifying fixed using an hourly build with the cset:
> 
> Patch seems not solve the bug in 'Modern Theme' from SM. Right-clicking on that
> Icons in 'Modern' don't show a menue. It always choose the default of this
> button.
> 
> Please set 'Status' to 'REOPENED'

I'm not sure that it is the same bug, since this one was tracking a specific regression. Can you file a new bug and CC me on it, and I'll help investigate. If it is indeed the same bug I'll mark it as duplicate and reopen this.

Comment 17

[Philip Chee]

Filed Bug 562554 for the issue in Comment 16