1945032 - Perform visibility checks on HTML elements for accesskeys in the chrome

Status: NEW

(Core :: DOM: Events, defect)

Description

[hecerinc]

Attachment: about:support export

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:136.0) Gecko/20100101 Firefox/136.0

Steps to reproduce:

I have a consistent repro in build 20250118091314. The previous build 20250117215754 does not exhibit the behavior.

To repro:

1. Install build 20250118091314 of the nightly channel.
2. Run firefox.exe -p and create a new profile.
3. Open https://addons.mozilla.org/en-US/firefox/addon/temporary-containers/
4. Install the "Temporary Containers" extension
5. Press Alt+C on the keyboard

(By default, Alt+C is the shortcut registered by the extension to open a new temporary container tab. )

See attached about:support.

Other info

I tested the Alt+X shortcut for the extension (which opens a new tab of the same container number), and this works as expected. This shortcut is not enabled by default, but can be enabled through the Advanced options in the Options of the extension.

The extension has not been updated recently.

Actual results:

Pressing Alt+C once does not do anything.

Additionally, if you long press Alt+C, then after some number of seconds a suite of new temporary container tabs are opened all at once.

Expected results:

A new temporary container tab should have opened (within approx. the same response time as Ctrl+T for opening a non-containerized tab).

Comment 1

[hecerinc]

Additionally, the bug is still present in today's build (20250130092924)

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Alex Cornestean]

Hello,

I reproduced the issue on the latest Nightly (136.0a1/20250202210625) under Windows 10 and Ubuntu 24.04 LTS.

Pressing Alt+C does nothing i.e. it does not open a temporary container tab as expected. Holding Alt+C pressed for a short time will open, with several seconds delay, a suite of new temporary container tabs at once.

The issue is not reproducible on the latest Beta (135.0/20250130195129) or Release (134.0.2/20250120135430).

Regression range:

2025-02-03T12:10:43.292000: DEBUG : Found commit message:
Bug 1942186 - Set automatic fluent support for accesskey in moz-button r=reusable-components-reviewers,tgiles

Differential Revision: https://phabricator.services.mozilla.com/D234610

2025-02-03T12:10:43.292000: DEBUG : Did not find a branch, checking all integration branches
2025-02-03T12:10:43.294000: INFO : The bisection is done.
2025-02-03T12:10:43.301000: INFO : Stopped

Pushlog:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=4f1775f4e14575b7734764c4c38a16d54c2c8618&tochange=44e482539c57ba8486619c4f8f84f8e07401a5ef

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

:akulyk, since you are the author of the regressor, bug 1942186, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1942186

Comment 6

[akulyk]

As my patch is set as a regressor for this bug, I did some research.
First of all, the issue with Alt+C shortcut is relevant not only for extension "Temporary Containers", for example Match Case checkbox in the findbar has the same accesskey and it has the same issue.
My patch did not create this issue, but it revealed the issue that we have with accesskeys at the moment.
Alt+C shortcut isn't working, because there's a button in the tabgroup menu panel with the same accesskey. When we check if the element is an accesskey target, we do visibility checks only for XUL elements. But have to do the same check for the chrome elements (like that button in the tabgroup menu panel).

Comment 7

[Mark Striemer [:mstriemer]]

Attachment: WIP: Bug 1945032 - Perform visibility checks for accesskey in chrome documents

Comment 8

[Mark Striemer [:mstriemer]]

Here's a sample patch (and try build) that enables the visibility checks for chrome documents, this resolved the issue with the extension for me (Alt+C opens the temporary container tab and if you've opened the tab group panel then Alt+C cancels the operation)

This change does not affect Alt+C toggling the Match Case option if the findbar is open, as Anna pointed out.

As we begin to add more HTML elements to the chrome document in panels that are always present but hidden this seems like a situation that will continue to show up. Fortunately in this case it's just the Cancel button that was being triggered, but I could imagine a similar issue happening with a control that performs some action in the future, which probably isn't ideal. For example putting an accesskey on the ETP toggle would trigger click handlers on it even if its panel is closed. Fortunately it appears as though the listeners aren't active at that point, but that might not always be the case. In the inspector I can see that the accesskey changes the toggle's pressed state, but no actions are performed and its state is reset when the panel opens.

@masayuki does this seem like a reasonable solution? If so is there some testing I should add for this change?

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yeah, looks like it's reasonable, although I'm not 100% sure because I've not touched the case yet. And yes, it's really reasonable to check whether the accesskey works and does not work as you expected. I guess using the "Match Case" checkbox in the findbar within a mochitest-browser-chrome is reasonable.

Comment 10

[Rob Wu [:robwu]]

(Moving to Core :: DOM: Events because the patch touches dom/events/EventStateManager.cpp)

Comment 11

[Donal Meehan [:dmeehan]]

:peterv could this be triaged for severity? (pinging as triage owner)

Comment 12

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think this should be S3 or lower because Alt - foo should not be used as shortcut keys for avoiding to conflict with access keys.

Comment 13

[Donal Meehan [:dmeehan]]

:masayuki, there are only two beta builds left in the cycle for Fx136
Is there a plan to land a fix and request an uplift this week? Or, will it be in a later release?

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that later is fine because it's unusual key combination for shortcut keys.

Comment 16

[akulyk]

Looks like Alt+C shortcut issue brought up concerns in [https://bugzilla.mozilla.org/show_bug.cgi?id=1952611](Bug 1952611 - Alt+C no longer toggles the “Match Case” checkbox, only focuses it).
Is it better to back out [https://bugzilla.mozilla.org/show_bug.cgi?id=1942186](Set automatic fluent support for accesskey in moz-button) patch that revealed this issue until it's fixed?

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I'm not sure what's changed in the patch because I don't have enough knowledge around the code.

Comment 19

[Mark Striemer [:mstriemer]]

Since the regressor was backed out I'm going to repurpose this bug for what the underlying issue we need solved is. We just ran into the problem that the regressor was intended to fix in bug 1987682.