Closed Bug 1713794 Opened 4 years ago Closed 3 years ago

Webextension keyboard shortcuts overriden by page

Categories

(WebExtensions :: Untriaged, defect)

Firefox 91
defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: code, Unassigned)

References

Details

Attachments

(1 file)

10.00 KB, application/x-tar
Details
Attached file reproducer.tar

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:88.0) Gecko/20100101 Firefox/88.0

Steps to reproduce:

Assign Ctrl+E to a webextension keyboard shortcut and attempt to use it while a Github textarea (e.g. a "new issue" textarea) is focused.

Actual results:

Github overrides the keyboard shortcut.

Expected results:

The webextension keyboard shortcut should have been used instead.

Additional information:

Can reproduce on a nightly build from Mozilla, and a stable one from ubuntu. My keyboard layout is latin9 (azerty). I've attached a minimal reproducer that exhibits the bug. I've had multiple reports from people who use my extension.

The Bugbug bot thinks this bug should belong to the 'Core::DOM: UI Events & Focus Handling' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Component: Untriaged → DOM: UI Events & Focus Handling
Product: Firefox → Core

Forgot to mention: this used to work fine until a few weeks/months ago.

(In reply to glacambre from comment #2)

Forgot to mention: this used to work fine until a few weeks/months ago.

Masayuki, do you have ideas of what might have happened here?

Flags: needinfo?(masayuki)

I guess that it's caused by a change of Github.

Currently, we don't expose keypress event for Ctrl + E because of non-text-input event. Therefore, web apps need to handle it at keydown event. On the other hand, WebExtensions does not use keydown event for handling shortcut key with modifier(s) + a printable key.
https://searchfox.org/mozilla-central/rev/c1ec9ecbbc7eac698923ffd18c8594aa3e2e9da0/toolkit/components/extensions/ExtensionShortcuts.jsm#499-502
https://searchfox.org/mozilla-central/rev/c1ec9ecbbc7eac698923ffd18c8594aa3e2e9da0/toolkit/modules/ShortcutUtils.jsm#150-153

If there is no event attr, keypress is used by default.
https://searchfox.org/mozilla-central/rev/c1ec9ecbbc7eac698923ffd18c8594aa3e2e9da0/dom/events/KeyEventHandler.cpp#557,560

So, such shortcut key is always handled later than web content. Perhaps, this is not easy to fix because our default action handlers still use keypress events. I.e., setting event attribute to key may break some event orders which currently work.

Component: DOM: UI Events & Focus Handling → Untriaged
Flags: needinfo?(masayuki)
Product: Core → WebExtensions

Web pages can intercept key events and prevent the default behavior. This is sometimes useful, e.g. if web pages want to intercept Ctrl-S to offer the Save functionality. Preventing default behavior also includes the ability to override the extension shortcut.

Status: UNCONFIRMED → RESOLVED
Closed: 3 years ago
Resolution: --- → WONTFIX

Web pages can intercept key events and prevent the default behavior. This is sometimes useful, e.g. if web pages want to intercept Ctrl-S to offer the Save functionality. Preventing default behavior also includes the ability to override the extension shortcut.

This is inconsistent for two reasons. First, Firefox provides a default set of keyboard shortcuts that can't be overriden by the page (e.g. the ones with required=true in browser/base/content/browser-sets.inc). Pages could provide useful behavior by overriding them but Firefox considers that it's better for the user to not let the page do that, despite these keyboard shortcuts not being choosen by the user.
Second, Webextension keyboard shortcuts are choosen by the user. The user is clearly expressing their intent to use a webextension feature rather than a page feature by doing that. Letting the page override these shortcuts breaks user expectations.

If the reason you closed this bug is that it is very hard to fix architecturally and that Mozilla doesn't have the engineering resources required to fix it, that's fine with me. But saying that Firefox is acting as desired when that's clearly not the case seems very wrong to me.

One other thing that in my opinion makes this bug worth fixing: Chromium correctly triggers webextension shortcuts instead of page shortcuts on Github. With the recent move to turn webextensions into a standard it would make sense if webextensions behaved in a standard way in all browsers.

:masayuki is there a reason webextensions can't be triggered on keydown rather than keypress? FWIW I've worked on two webextensions that make heavy use of custom keyboard shortcuts and using keydown there worked fine.

:masayuki is there a reason webextensions can't be triggered on keydown rather than keypress? FWIW I've worked on two webextensions that make heavy use of custom keyboard shortcuts and using keydown there worked fine.

By the historical reasons, default event handlers in Gecko and Firefox still use keypress events in various places. Ideally, we should make them handle at keydown. However, doing it partially means that we may change some handlers' order. Then, we'll have regressions. Actually, I tried to fix some of them, but they caused regressions. So, we need to sort out the order of event listeners before touching them, but I have no idea. (If we could change all of them once, it'd be the safest fix. However, it conflicts with a lot of work by the other developers...)

(In reply to glacambre from comment #2)

Forgot to mention: this used to work fine until a few weeks/months ago.

If this ussed to work, could you run mozregression to identify the commit in hat changed the behavior?

The extension implementation doesn't do anything special: it just registers shortcuts and relies on the platform to handle the shortcuts. If it is feasible to support the keydown behavior for extensions, then this bug can be reopened. WDYT Masayuki?

See Also: → 1555620
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: