1794850 - Menu items in toolbar panels start blinking when you right-click them, **only after** they've been tabbed across

Status: VERIFIED FIXED

(Firefox :: Toolbars and Customization, defect, P1)

Description

[Daniel Holbert [:dholbert]]

(not entirely sure whether this is a DOM focus-handling bug, vs. a frontend bug, vs. a style system bug, vs. something else. Starting off in Toolbars&Customization, and CC'ing some folks who might have thoughts)

STR:

1. Use "Customize Toolbar" to place the History menu-button (looks like a little clock) on your toolbar.

2. Click the History icon on your toolbar (the one you just placed) to open its menu, and mouse over the bottom option, called "Manage History". Right click it and verify that there's no visual effect from that right-click action.

3. Now, with the menu still open, press "Tab" to cycle through that menu's options. Make sure that "Tab" traverses across that final "Manage History" option (though you can leave it focusing some other menu item).

4. Close and reopen the menu (e.g. by clicking elsewhere and clicking the toolbar icon again.)

5. Once again, mouse over "Manage HIstory" and right-click. What happens?

EXPECTED RESULTS:
No visual change for the right-click action, just as in step 2.

ACTUAL RESULTS:
The menu item flashes (showing its :active styling I think), just as it would if I left-clicked it.

It seems that tabbing across this menu item causes some persistent change that makes it flash its active style when right-clicked, despite the fact that the right-click action doesn't actually activate it in any way.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

https://searchfox.org/mozilla-central/rev/e94c6cb9649bfe4e6a3888460f41bcd4fe30a6ca/browser/themes/shared/customizableui/panelUI-shared.css#1227-1240 is the relevant rule, fwiw. I don't think this is a style system bug, off-hand.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I can dig, but it's the DOM state which doesn't change before tabbing, but changes after (verified via setInterval(() => console.log(toolbarbutton.matches(":active")), ...)...

Comment 4

[Emilio Cobos Álvarez (:emilio)]

So this is kind of a XUL / front-end interaction quirk. What sets the content state is this line. But why is it different if you've tabbed already or not? That's because of this code which lazily sets tabindex="-1" on the element, making it focusable.

So:

• Maybe all buttons should be focusable like HTML buttons?
• Maybe we should just not set the :active state?

I don't know why the context menu event would focus the closest ancestor. Olli this goes back to the focus manager and bug 605242 etc... Do you have opinions?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1794850 - Remove nsXULPopupListener::FireFocusOnTargetContent. r=smaug

Doesn't match HTML behavior, and our UI is inconsistent on whether some
elements triggering context menus are focusable or not.

Remove one semi-random source of :active state changes.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dbb66b295ab2
Remove nsXULPopupListener::FireFocusOnTargetContent. r=smaug

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/dbb66b295ab2

Comment 8

[Ardelean Oana, Desktop QA [:oardelean]]

Reproduced on a 2022-10-12 Nightly build on Ubuntu 22 with the STR from Comment 0. Verified as fixed on Firefox 108.0b4(build ID: 20221120185746) and Nightly 109.0a1(build ID: 20221120214001) on Ubuntu 22. Could not reproduce on macOS nor Windows.

Comment 9

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

Clearing this old leftover needinfo.