1635774 - Initial implementation of customizable Hotkeys

Status: RESOLVED FIXED

(Firefox :: Keyboard Navigation, enhancement)

Description

[DummieBot]

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

Expected results:

Its way too hard to customize hotkeys currently, u ether have to do this https://github.com/nilcons/firefox-hacks (modify omni.ja/compile each release from source)
or install addons which wont work on protected URLs like
https://addons.mozilla.org (because these dont allow javascript injection?).

Please implement something more user friendly

Comment 1

[08xjcec48]

This is a duplicate of https://bugzilla.mozilla.org/show_bug.cgi?id=972662

Comment 2

[Peter Magyari (Desktop QA)]

Since Bug 972662 is a very old report and the original reporter's account is disabled, I thought It might be a good idea to leave this one open.
Feel free to set it as a duplicate if you think that would be the best course of action here.

Comment 3

[Sebastian Zartner [:sebo]]

(In reply to Peter Magyari (Desktop QA) from comment #2)

Since Bug 972662 is a very old report and the original reporter's account is disabled, I thought It might be a good idea to leave this one open.
Feel free to set it as a duplicate if you think that would be the best course of action here.

I think the age of that bug is not that important. The more important part is that it focuses on the DevTools shortcuts while this one asks for a general way to customize the shortcuts throughout Firefox.

So I believe both should be kept open for now, as the DevTools might add a way to customize their shortcuts. Though if a general way like an about:hotkeys as previously suggested or something similar is introduced, it should include the DevTools shortcuts.

Sebastian

Comment 4

[Artem Manushenkov]

Seems to be quite relevant now with so many apps moving into web with their desktop shortcuts

Opera seems to be the only browser allowing browser shortcut customization, i can draw some inspiration from them. Meanwhile chromium only allows changing devtools shortcuts

I'll start working on it. I'll put them somewhere in about:preferences

This is the correct place to work on it. It'll also cover for

https://bugzilla.mozilla.org/show_bug.cgi?id=492557
https://bugzilla.mozilla.org/show_bug.cgi?id=972662
https://bugzilla.mozilla.org/show_bug.cgi?id=1116951

Comment 5

[James Teh [:Jamie]]

FWIW, I've been working on a prototype as I have time, but it's not quite ready for testing or feedback yet.

Comment 6

[Artem Manushenkov]

I'll leave it to you then, I'm glad this implementation conflict was caught early

Comment 7

[James Teh [:Jamie]]

Attachment: Bug 1635774 part 1: Add ids to some <key> elements that previously didn't have them.

Subsequent patches will implement customisation of keyboard shortcuts based on <key> elements.
However, this is only possible for <key> elements which have ids.

Comment 8

[James Teh [:Jamie]]

Attachment: Bug 1635774 part 2: Functionality to support customisation of keyboard shortcuts.

This adds a CustomKeys module which provides the functionality to customise keyboard shortcuts defined by <key> elements.
Customisations are saved to and loaded from a JSON configuration file.
This is hooked into browser-init so that it can apply to all browser windows.

Comment 9

[James Teh [:Jamie]]

Attachment: Bug 1635774 part 3: Add about:keyboard UI to customise keyboard shortcuts.

This is an unpolished UI for experimentation.
It is likely that this UI will be heavily modified or even completely replaced before it is considered official.
As such, this is only available via a new about:keyboard page with no entry point elsewhere in the UI.

This primarily obtains keyboard shortcuts from the menu bar menus.
This doesn't allow all keyboard shortcuts to be customised, since many aren't available in the menu bar menus and haven't been hard-coded here.
There is a basic framework to allow for hard-coding of additional shortcuts, though, and this has been used for a few shortcuts already.

For now, when a key combination involves a character and a modifier changes the character, the UI shows the modified character.
For example, on a US English QWERTY keyboard, shift+3 produces the # character, so the UI shows Shift+#.
While this is likely to be a little confusing for some users, this is difficult to fix in a locale independent way.
The current behaviour should allow this to work across locales, albeit with this somewhat quirky presentation.

Comment 10

[James Teh [:Jamie]]

Attachment: Bug 1635774 part 4: Ensure that DevTools is initialized when about:keyboard is opened.

This is necessary in order to ensure that DevTools items such as Browser Console are added to the menu bar menus and thus picked up by about:keyboard.

Comment 12

[James Teh [:Jamie]]

Attachment: Bug 1635774 part 5: Show a caution dialog before the main about:keyboard UI.

Since this is experimental at this stage, we want to warn users that it might not work as expected, while still giving them the chance to test it.
They can choose to disable the caution in future if they wish.

Comment 13

[James Teh [:Jamie]]

Attachment: Bug 1635774 part 5: Add telemetry for about:keyboard.

Comment 14

[James Teh [:Jamie]]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=161791

Comment 16

[Tom S. (please needinfo tschuster)]

Would about:shortcuts maybe be a better name instead of about:keyboard? (There was a patch in bug 492557 with that URL)

Comment 17

[James Teh [:Jamie]]

Personally, I'm not convinced about:shortcuts is better. At best, I think it's 50 50 either way. Shortcuts can also mean things other than keyboard shortcuts; e.g. Windows has Desktop shortcuts and Start Menu shortcuts. That said, I've asked others involved with this work (product manager, etc.) for their thoughts as well.

Comment 18

[villeneuve]

about:shortcutkeys :-)

Comment 19

[kbryant]

I agree with Jamie's feedback that "shortcuts" can also mean other functionality.

In addition:
Our competitors all call what we've built "keyboard shortcuts":
Apple
Google
Microsoft

For v2, we plan to redirect to about:keyboard when users search for synonymous terms and phrases (e.g., hotkeys, shortcuts, custom hotkeys) -- starting with the URL bar, but probably other locations, too (e.g., Settings).

tl;dr: from a Product perspective, we should keep about:keyboard for MVP.

Comment 20

[villeneuve]

What about if someone wants to create shortcuts on his gamecontroller etc. for browsing on TV etc.? The term "keyboard" doesn't really fit then.

Comment 21

[Romain Chossart]

I don't really have skin in the game here, but I if that one person ever exists, I'm sure they can live with the fact it's called about:keyboard.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cf3d3685c671
https://hg.mozilla.org/integration/autoland/rev/07b781dad2a6
part 1: Add ids to some <key> elements that previously didn't have them. r=Gijs
https://github.com/mozilla-firefox/firefox/commit/3e3e1240b321
https://hg.mozilla.org/integration/autoland/rev/61c373f653db
part 2: Functionality to support customisation of keyboard shortcuts. r=mossop
https://github.com/mozilla-firefox/firefox/commit/5eb494e0efb4
https://hg.mozilla.org/integration/autoland/rev/6d5ca7a51397
part 3: Add about:keyboard UI to customise keyboard shortcuts. r=fluent-reviewers,mossop,bolsson,toolkit-telemetry-reviewers,frontend-codestyle-reviewers
https://github.com/mozilla-firefox/firefox/commit/42a0e13bf50e
https://hg.mozilla.org/integration/autoland/rev/52e3b6d393fe
part 4: Ensure that DevTools is initialized when about:keyboard is opened. r=devtools-reviewers,mossop,ochameau

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d277e7f5b98c
https://hg.mozilla.org/integration/autoland/rev/889b8879c2c7
Revert "Bug 1635774 part 4: Ensure that DevTools is initialized when about:keyboard is opened. r=devtools-reviewers,mossop,ochameau" for causing failures on browser_CustomKeys.js

Comment 24

[Cosmin Sabou [:CosminS]]

Backed out for causing failures on browser_CustomKeys.js

Push with failures

Failure log

Backout link

Comment 25

[James Teh [:Jamie]]

https://treeherder.mozilla.org/jobs?repo=try&landoCommitID=162517

Comment 26

[James Teh [:Jamie]]

It's taken me forever to debug the failure in comment 24, largely because it only seems to occur on very specific builds: ASan and macOS 15 AArch64 Shippable opt Mochitests with networking on socket process enabled test-macosx1500-aarch64-shippable/opt-mochitest-browser-chrome-spi-nw. I eventually realised I was able to reproduce it reliably on my local mac opt build with --verify. For reference, here are some notes from that investigation:

1. This is a race condition. If the test synthesises a key too early, the command is never executed.
2. For keys that aren't reserved, we must first send them to the content process to see if the content document wants to intercept them. After that, the parent process stops processing the event. The content process then replies, and if the event wasn't prevented by content, it then goes through the parent process event handlers again.
3. In this case, the content document didn't handle the key. Even so, when the failure occurred, the event never went through the parent process event handlers again.
4. I'm not entirely sure whether the remote process never received the event, chose to prevent default (even though it shouldn't normally do that) or never replied to the event. I didn't dig that far.
5. Originally, I was just calling add_task and running the tests; I never waited for the content document to load. My guess, then, is that the content document wasn't quite ready in the failure case. Maybe the remote document wasn't created yet, maybe it can't handle events at that point in its setup, but I'm really not sure.
6. I've fixed this by just creating a new foreground tab, waiting until it is definitely loaded, running the tests and then removing the tab. This seems to stabilise things locally. Hopefully, it will also behave on CI.

Here's the debugging code I used to track this down.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/85c86acf1425
https://hg.mozilla.org/integration/autoland/rev/a12435929874
part 1: Add ids to some <key> elements that previously didn't have them. r=Gijs
https://github.com/mozilla-firefox/firefox/commit/6f4719b83cd8
https://hg.mozilla.org/integration/autoland/rev/329cecc9af62
part 2: Functionality to support customisation of keyboard shortcuts. r=mossop
https://github.com/mozilla-firefox/firefox/commit/287985a9b3a9
https://hg.mozilla.org/integration/autoland/rev/0db5991a518a
part 3: Add about:keyboard UI to customise keyboard shortcuts. r=fluent-reviewers,mossop,bolsson,toolkit-telemetry-reviewers,frontend-codestyle-reviewers
https://github.com/mozilla-firefox/firefox/commit/84f8b56d85d0
https://hg.mozilla.org/integration/autoland/rev/7bdfbeb48bca
part 4: Ensure that DevTools is initialized when about:keyboard is opened. r=devtools-reviewers,mossop,ochameau

Comment 29

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/a12435929874
https://hg.mozilla.org/mozilla-central/rev/329cecc9af62
https://hg.mozilla.org/mozilla-central/rev/0db5991a518a
https://hg.mozilla.org/mozilla-central/rev/7bdfbeb48bca

Comment 30

[Ryan VanderMeulen [:RyanVM]]

Did you want to nominate this for the Fx147 relnotes?

Comment 31

[James Teh [:Jamie]]

Redirecting to Kim. There's some nuance here: this is somewhat of an early experiment, so it's not quite ready for "everyone should use this". I think we want to get this into Nightly release notes but maybe not official release, but I'm not quite sure of the specifics from a product perspective.

Comment 32

[:Gijs (he/him)]

(In reply to James Teh [:Jamie] from comment #31)

Redirecting to Kim. There's some nuance here: this is somewhat of an early experiment, so it's not quite ready for "everyone should use this". I think we want to get this into Nightly release notes but maybe not official release, but I'm not quite sure of the specifics from a product perspective.

We should get some QA in place. Is that already happening?

Comment 33

[James Teh [:Jamie]]

Yes, we have a QA request open and QA are actively working on test cases.

Comment 34

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Hi James. This is a nice new feature. Do you know if there are plans to also add new shortcuts to eg menu items which do not yet have a shortcut set? Maybe a meta bug would be nice to collect all the bugs?

Comment 35

[Sylvestre Ledru [:Sylvestre]]

Release Note Request (optional, but appreciated)
[Why is this notable]: requested 25 years ago!
[Affects Firefox for Android]: no?
[Suggested wording]: Customize keyboard shortcuts through a configuration screen
[Links (documentation, blog post, etc)]:

Comment 36

[Sylvestre Ledru [:Sylvestre]]

Submitting the relnote flag to make sure we don't forget

Comment 37

[James Teh [:Jamie]]

(In reply to Henrik Skupin [:whimboo][⌚️UTC+2] from comment #34)

This is a nice new feature. Do you know if there are plans to also add new shortcuts to eg menu items which do not yet have a shortcut set?

It's been discussed, but at this stage, there are no clear plans, since this work currently has very minimal resourcing.

Maybe a meta bug would be nice to collect all the bugs?

Fair. I filed bug 2000731 and moved the other dependent bugs there.

Comment 38

[Ryan VanderMeulen [:RyanVM]]

Note that per comment 31, I was waiting on a reply from Kim as to how we wanted to proceed with a release note here.

Comment 39

[kbryant]

Sorry I missed this. Jamie and I have been discussing in the last week. It's an experimental feature, and we do NOT want it included in a release note at this time.