977217 - Clicking API-widget borders in the menu panel does nothing

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[u428464]

Similar to bug 946873 except it does nothing to click on the borders.

Comment 1

[:Gijs (he/him)]

This doesn't sound nearly as important, though.

Comment 2

[u428464]

I don't know if it's just a false idea, but it seems that the bug click area is bigger than the previous bug.

Comment 3

[:Gijs (he/him)]

(In reply to Guillaume C. [:ge3k0s] from comment #2)
> I don't know if it's just a false idea, but it seems that the bug click area
> is bigger than the previous bug.

The 'previous' bug was about a different widget. It didn't affect this issue; this has (apparently) always been like this.

On beta on OS X, at least, if I click the right border of a button in a toolbar, nothing happens there, either. I'm not even sure this is an Australis issue.

Comment 4

[u428464]

Sorry if I wasn't clear but I was talking about the widget in the menu panel not in toolbars.

Comment 5

[:Gijs (he/him)]

(In reply to Guillaume C. [:ge3k0s] from comment #4)
> Sorry if I wasn't clear but I was talking about the widget in the menu panel
> not in toolbars.

I realize that, but the nodes we use are the same.


In any case, from some quick checking, it seems that while we get a click event, we don't get a command event, and we use the latter for opening a view - and really, we use them for most actions from API buttons. I don't know why we don't get a command event, but it's quite unfortunate. Neil, can you enlighten us?

Comment 6

[[e-mail]]

You only get command events from clicking on UI elements, as opposed to background elements. At the frame level that means autorepeatbuttons, buttons, menuitems, resizers, scrollbarbuttons, splitters and titlebars. At the constructor level that includes checkboxes and radiobuttons. At the XBL level that includes columnpicker, dropmarker, grippy, listheader, scalethumb, statusbarpanel, tab, thumb, and toolbarpaletteitem.

Note that UI elements also capture events that would normally target their children, although you can use allowevents to work around this. (For instance, dual menubuttons do this.)

Comment 7

[:Gijs (he/him)]

(In reply to [e-mail] from comment #6)
> You only get command events from clicking on UI elements, as opposed to
> background elements. At the frame level that means autorepeatbuttons,
> buttons, menuitems, resizers, scrollbarbuttons, splitters and titlebars. At
> the constructor level that includes checkboxes and radiobuttons. At the XBL
> level that includes columnpicker, dropmarker, grippy, listheader,
> scalethumb, statusbarpanel, tab, thumb, and toolbarpaletteitem.
> 
> Note that UI elements also capture events that would normally target their
> children, although you can use allowevents to work around this. (For
> instance, dual menubuttons do this.)

Sure, but this doesn't explain what's going on here. All of these are toolbarbuttons. How is it possible that clicking them produces a command event in some places, but not in other places?

Comment 8

[[e-mail]]

OK, it looks like I'm missing something here, what exactly are you clicking on the borders of? (Also, is it possible to get there using the keyboard, so that I can use DOM Inspector to see what I'm actually clicking?)

Comment 9

[:Gijs (he/him)]

(In reply to [e-mail] from comment #8)
> OK, it looks like I'm missing something here, what exactly are you clicking
> on the borders of? (Also, is it possible to get there using the keyboard, so
> that I can use DOM Inspector to see what I'm actually clicking?)

No, we removed keyboard support because keyboard support for what are essentially menus is hard.

Here are some steps:

1) open DOMI, point to browser window
2) find by attr, name: id, value: PanelUI
3) on PanelUI-popup, set attribute "noautohide" to "true"
4) open the main menu panel (using the hamburger button, what-have-you, on the right side of the navbar)
5) click the border (I find the right is easiest for testing, but it happens with all sides) of the 'History', 'Developer', 'Add-ons', or 'Full screen' button (any of these will work)

Notice how nothing happens (although the panel might close, actually - it won't if you use the history button to test with).

All of these items have command handlers. None of them are activated. Clicking in the middle of the button works fine.

Comment 10

[[e-mail]]

(In reply to Gijs Kruitbosch from comment #9)
> 2) find by attr, name: id, value: PanelUI
[Huh, why doesn't find by id work? It seems to find ids in SeaMonkey's chrome just fine.]

> 3) on PanelUI-popup, set attribute "noautohide" to "true"
Nice trick!

> 4) open the main menu panel (using the hamburger button, what-have-you, on
> the right side of the navbar)
> 5) click the border (I find the right is easiest for testing, but it happens
> with all sides) of the 'History', 'Developer', 'Add-ons', or 'Full screen'
> button (any of these will work)
> 
> Notice how nothing happens
Indeed. Nothing obvious sticks out, but maybe I'll spot something in the debugger tomorrow.

Comment 11

[:Gijs (he/him)]

(In reply to [e-mail] from comment #10)
> (In reply to Gijs Kruitbosch from comment #9)
> > 2) find by attr, name: id, value: PanelUI
> [Huh, why doesn't find by id work? It seems to find ids in SeaMonkey's
> chrome just fine.]

Because nobody from the seamonkey council has updated DOMI on AMO, and so the one you have in Firefox has bugs that the one in SeaMonkey doesn't.

Comment 12

[[e-mail]]

OK, so what's going on here is that you've set overflow: hidden; on the toolbarbutton. This wraps the button in a scrollable frame. For whatever reason, the border applies to the frame, not the button, so the border is no longer a UI element, and ignores clicks.

Comment 13

[[e-mail]]

(In reply to Gijs Kruitbosch from comment #11)
> (In reply to comment #10)
> > (In reply to Gijs Kruitbosch from comment #9)
> > > 2) find by attr, name: id, value: PanelUI
> > [Huh, why doesn't find by id work? It seems to find ids in SeaMonkey's
> > chrome just fine.]
> 
> Because nobody from the seamonkey council has updated DOMI on AMO, and so
> the one you have in Firefox has bugs that the one in SeaMonkey doesn't.

Ah right, bug 865008.

Comment 14

[:Gijs (he/him)]

(In reply to [e-mail] from comment #12)
> OK, so what's going on here is that you've set overflow: hidden; on the
> toolbarbutton. This wraps the button in a scrollable frame. For whatever
> reason, the border applies to the frame, not the button, so the border is no
> longer a UI element, and ignores clicks.

Blergh. :-(

Comment 15

[Jared Wein [:jaws] (please needinfo? me)]

Attachment: Patch

This patch always applies the clip property to all buttons because we won't get overflow events since overflow:hidden was removed.

Pushed to try to see what perf hits this will cause us:
https://tbpl.mozilla.org/?tree=Try&rev=56c8db948e2b

Baseline:
https://tbpl.mozilla.org/?tree=Try&rev=25a2b5089d70

Comment 16

[:Gijs (he/him)]

Comment on attachment 8387833 [details] [diff] [review]
Patch

Review of attachment 8387833 [details] [diff] [review]:
-----------------------------------------------------------------

Code-wise, this looks excellent. Tested on OS X. WFM!

Comment 17

[Jared Wein [:jaws] (please needinfo? me)]

Try results showed an improvement on TART and CART.

https://hg.mozilla.org/integration/fx-team/rev/4dab55a994e5

Comment 18

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4dab55a994e5

Comment 19

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8387833 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 897496 
User impact if declined: some add-on icons don't handle clicks well
Testing completed (on m-c, etc.): locally and on m-c
Risk to taking this patch (and alternatives if risky): none expected
String or IDL/UUID changes made by this patch: none

Comment 20

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8a8af6cb3912

Comment 21

[Petruta Horea [:phorea], Desktop QA]

Verified as fixed on Firefox 29 RC (20140421221237) and latest Aurora 30.0a2 2014-04-24 under Win 7 64-bit, Ubuntu 32-bit and Mac OS X 10.7.5.