1333781 - Allow existing (built-in) menu shortcuts to be reassigned as custom shortcuts via System Preferences

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P3)

Description

[Stephen A Pohl [:spohl]]

+++ This bug was initially created as a clone of Bug #429824 +++

Bug 429824 added handling for custom shortcuts by the native menu system if the shortcut was not an existing, built-in shortcut. We should expand this to allow for reassignment of existing shortcuts. There seem to be two possible approaches:

1. Use the existing mechanics to set the built-in shortcuts, but remove the internal handling of the shortcuts via XBL prototype handlers[1] and let the native menu system handle them instead. This is scary because of the code we currently use to walk and execute our handlers[2]. It is unknown if this special handling is necessary and whether we can achieve the same via the native menu system.
2. Revert the change in bug 429824 and instead keep our XUL representation of keyboard shortcuts in sync with any custom shortcuts set via System Preferences. We would continue to use XBL prototype handlers to execute commands associated with a menu item. This would require that we check if the system has a custom shortcut set for every menu item that we create during startup. If yes, we need to translate and update the shortcuts in XUL. We would also need to listen for com.apple.userkeyequivalentsdidchange notifications from the system. This notification is sent if the user sets a custom shortcut for a menu item while Firefox is running. It may be easiest to set the menu to be rebuilt at this point. We don't have to listen for this notification if we go with approach 1 above as the native menu system will take care of this for us.

Also note that at present, existing and custom shortcuts in the application menu are already handled by the native menu system. We should get the behavior of the application menu in sync with the main menu, either as part of this bug or a separate bug.

[1] https://dxr.mozilla.org/mozilla-central/rev/6dccae211ae5fec6a1c1244b878ce0b93860154f/widget/cocoa/nsChildView.mm#5474
[2] https://dxr.mozilla.org/mozilla-central/rev/6dccae211ae5fec6a1c1244b878ce0b93860154f/dom/xbl/nsXBLWindowKeyHandler.cpp#637

Comment 1

[Stephen A Pohl [:spohl]]

Attachment: debug patch for com.apple.userkeyequivalentsdidchange notification

This is a debug patch that I put together for solution 2. It was used to observe the com.apple.userkeyequivalentsdidchange notification and to set the menu items to be rebuilt. It does not attempt to update our XUL representation of shortcuts yet. There are a number of printf statements left in the patch. I'm uploading this in case it helps jumpstart an actual patch.

Comment 6

[klambour]

Any progress with this? It's truly annoying on macOS. Even Excel, Word, Powerpoint now handle this correctly

Comment 11

[hongsy2006]

What is XUL and XBL?

Comment 12

[klambour]

What do you precisely mean with "wontfix"?

Comment 13

[Stephen A Pohl [:spohl]]

(In reply to klambour from comment #12)

What do you precisely mean with "wontfix"?

This will not get fixed in Firefox 54.

Comment 14

[klambour]

Thanks for clarification, I thus conclude that newer Firefox versions are still tageted to finally work with the macOS built-in custom shortcuts functionality in System Preferences.

Comment 15

[Stephen A Pohl [:spohl]]

(In reply to klambour from comment #14)

Thanks for clarification, I thus conclude that newer Firefox versions are still tageted to finally work with the macOS built-in custom shortcuts functionality in System Preferences.

Correct. If it wasn't, this entire bug would be closed as RESOLVED > WONTFIX. Also note that you can already set custom shortcuts as long as they do not try to override built-in (already assigned) shortcuts.

Comment 16

[hongsy2006]

(In reply to Stephen A Pohl [:spohl] from comment #0)

1. Use the existing mechanics to set the built-in shortcuts, but remove the
   internal handling of the shortcuts via XBL prototype handlers[1] and let the
   native menu system handle them instead. This is scary because of the code we
   currently use to walk and execute our handlers[2]. It is unknown if this
   special handling is necessary and whether we can achieve the same via the
   native menu system.

I'd like to try to fix this bug. Could you point me to the resources on what "XBL prototype handlers" are?

Comment 17

[hongsy2006]

[Tracking Requested - why for this release]:

Comment 18

[Stephen A Pohl [:spohl]]

(In reply to hongsy2006 from comment #16)

(In reply to Stephen A Pohl [:spohl] from comment #0)

1. Use the existing mechanics to set the built-in shortcuts, but remove the
   internal handling of the shortcuts via XBL prototype handlers[1] and let the
   native menu system handle them instead. This is scary because of the code we
   currently use to walk and execute our handlers[2]. It is unknown if this
   special handling is necessary and whether we can achieve the same via the
   native menu system.

I'd like to try to fix this bug. Could you point me to the resources on what "XBL prototype handlers" are?

This is referenced in comment 0, in particular link [2]. You should expect to have to do a lot of debugging in the actual code to get a better understanding of the mechanisms here. Other resources will be limited at best and I wouldn't know of any off-hand.

Please do not change tracking flags.

Comment 20

[melekcrescent]

(In reply to hongsy2006 from comment #16)

I'd like to try to fix this bug. Could you point me to the resources on what "XBL prototype handlers" are?

This has been driving me crazy lately. Did you make any headway on this that can be shared?

Comment 22

[Nicolas Le Cam]

I'd like to add that Bug #429824 didn't fix everything.

I've set up custom shortcuts for "Move Window to Left Side of Screen" and "Move Window to Right Side of Screen" menu items (as explained for example on https://apple.stackexchange.com/questions/50330/how-to-arrange-two-windows-easily-to-left-and-right-side) and it doesn't work in Firefox when it works in any other apps

It's really unfortunate as it is mostly with Firefox that I need that functionality

Comment 25

[vitali.brodetski]

Team, any news on this bug? This one has been raised 5 years ago, any option to have it fixed soon?
Thanks

Comment 28

[BugBot [:suhaib / :marco]]

The severity field for this bug is relatively low, S3. However, the bug has 13 duplicates and 11 votes.
:spohl, could you consider increasing the bug severity?

For more information, please visit auto_nag documentation.

Comment 29

[BugBot (nomail) [:suhaib / :marco/ :calixte]]

The last needinfo from me was triggered in error by recent activity on the bug. I'm clearing the needinfo since this is a very old bug and I don't know if it's still relevant.

Comment 30

[Stephen A Pohl [:spohl]]

This bug is still relevant and the severity of S3 is correct.

Comment 31

[Ralf Jung]

Thunderbird has quite a lot of default shortcuts, and some of them are outright dangerous since they require no modifier keys: it happens regularly that I think I am typing in the 'filter' field, but Thunderbird actually interprets my keystrokes as shortcuts. Then I spend the next 10min trying to figure out what my 10 keystrokes did and un-doing that. (In particular un-doing an accidental "ignore thread" is really hard.)

I think the severity should be raised, at least to provide a way to disable/remap the potentially destructive modifier-free shortcuts (J, K -- maybe there are others but these are the most painful).

Comment 32

[5silentrain]

As a left-hander, it is extremely difficult for me to use Firefox when I need to cut, copy or paste something. The standard keyboard shortcuts Cmd + X, Cmd + C and Cmd + V do not suit me and are very inconvenient, which is why I assigned these functions to the F13, F14 and F15 keys on the Apple Magic Keyboard with Touch ID and Numeric Keypad. And these keys work great in Safari, Chrome, and the entire system, including Spotlight. But they don't work in Firefox, and that's terribly annoying! 😠

Comment 33

[5silentrain]

Attachment: Screenshot

Comment 34

[Noir-Fox]

Encountering the same problem.

Comment 35

[René W.]

I remapped the copy/paste/cut/undo/redo.

only undo works with both shortcuts command+z / ctrl+z , which is curios.

Name 	Firefox
Version 	108.0.1
Build ID 	20221215175817
Update Channel 	release
User Agent 	Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:108.0) Gecko/20100101 Firefox/108.0
OS 	Darwin 22.2.0 Darwin Kernel Version 22.2.0: Fri Nov 11 02:03:51 PST 2022; root:xnu-8792.61.2~4/RELEASE_ARM64_T6000

Comment 36

[5silentrain]

(In reply to René W. from comment #35)

I remapped the copy/paste/cut/undo/redo.

only undo works with both shortcuts command+z / ctrl+z , which is curios.

Name 	Firefox
Version 	108.0.1
Build ID 	20221215175817
Update Channel 	release
User Agent 	Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:108.0) Gecko/20100101 Firefox/108.0
OS 	Darwin 22.2.0 Darwin Kernel Version 22.2.0: Fri Nov 11 02:03:51 PST 2022; root:xnu-8792.61.2~4/RELEASE_ARM64_T6000

Will this change appear in the next version of Firefox? Or do you need permission from the developers?

Comment 37

[me]

My understanding is that S3 means "Blocks non-critical functionality and a work around exists".

From what I can see, there is no workaround available here. If a workaround does exist, I'd appreciate it if someone would let me know.

Comment 38

[Stephen A Pohl [:spohl]]

(In reply to me from comment #37)

From what I can see, there is no workaround available here. If a workaround does exist, I'd appreciate it if someone would let me know.

All menu items are accessible via the menu bar, clicking on the respective menu and selecting the desired menu item.

Comment 39

[jirtahupsu]

Firefox allows to re-define shortcuts of extensions, so I suppose devs understand that this functionality is important.
Should be the same for FF shortcuts, too.

Comment 40

[Denis]

Hello,

I am wondering, is there any progress with this issue, or any workaround? It is so annoying, that I cannot use ctrl+c/v for copy/paste in firefox :(

Comment 41

[strogge]

(In reply to Stephen A Pohl [:spohl] from comment #38)

(In reply to me from comment #37)

From what I can see, there is no workaround available here. If a workaround does exist, I'd appreciate it if someone would let me know.

All menu items are accessible via the menu bar, clicking on the respective menu and selecting the desired menu item.

The problem is not that you can't copy/paste but that the modified keyboard shortcuts are not working. Asking users to use something other than the keyboard for copy/paste is not a "work-around". Please prioritize this issue more seriously

Comment 42

[starbrk]

I couldn't believe my eyes that there was 7 years old issue about a missing essential feature: not being able to paste copied text using CTRL+V. Simply not able to use Firefox 117.0 ARM64 on my new fancy shiny Mac M2, have to stick with Safari just because there is no way to paste text in Firefox. Great :(

Comment 43

[5silentrain]

It would be great if Mozilla developers caught up with Google developers and everyone else in this regard. This would allow us to finally remove the Opt + Cmd + M key combination, which in Firefox is responsible for switching to responsive design mode. Because in other applications, the keyboard shortcut Opt + Cmd + M results in minimizing all application windows. This is spelled out in Apple's guidelines, so it's a shame that Firefox developers don't adhere to these guidelines 🤦‍♂️

By the way, here is a link to my bug report on this matter: https://bugzilla.mozilla.org/show_bug.cgi?id=1750155

Comment 44

[Jonathan]

This bug has been bothering me for a long time, so I decided to fix it:

https://gist.github.com/Wowfunhappy/b09900c5420767651338a57692348e31

Compile this into a dylib (you'll need ZKSwizzle) and inject it into Firefox via DYLD_INSERT_LIBRARIES or similar. Unfortunately, modern macOS makes code injection kind of annoying, but it is still possible. I'm pretty happy with it >160 lines of code.

The code works by intercepting key events, checking if they match any user-defined key equivalents in NSUserDefaults, and if they do, executing the matching menu item instead of passing the event to Firefox.

I'm not sure if this helps, since I imagine you wouldn't want to do it this way in the actual Firefox codebase—but you could and it does work, so I wanted to share. If you have any questions, please ask!

Comment 47

[Stephen A Pohl [:spohl]]

Attachment: Bug 1333781: Rebuild XUL keyset handlers when its key elements change. r?#dom-core-reviewers

A keyset builds its handler chain once, so changing a key element's key, modifiers or similar attributes has no effect until the keyset is re-inserted into the document. The keyset now marks its handlers stale on such a change and rebuilds them at the next key event. The rebuild waits while a walk of the handlers is running script.

Comment 48

[Stephen A Pohl [:spohl]]

Attachment: Bug 1333781: Highlight a menu bar item when its shortcut runs, not before the window handles the key. r?#mac-reviewers!

The menu bar ran a highlight-only pass for every Command shortcut before the page saw the key, so menus flashed for keys the page consumed and for disabled commands, sometimes twice. That pass is gone. When a key element runs its command, the menu item showing that shortcut is highlighted through performActionForItemAtIndex: with its action switched off, which is the same AppKit path a key equivalent takes. Keys without Command still stay with a native key window such as a sheet, as bug 2060306 requires, and for Command keys the menu bar now reports whether it handled the key instead of always returning NO.

Comment 49

[Stephen A Pohl [:spohl]]

Attachment: Bug 1333781: Let a shortcut assigned to a menu item in System Settings replace the built-in one. r?#mac-reviewers!

AppKit honors a shortcut the user assigns to a menu item in System Settings, but Gecko kept running the built-in shortcut first. The menu bar now runs an item whose assigned shortcut matches the key and consumes the event, so neither Gecko nor web content sees it. The XUL key element is rewritten to the assigned shortcut and keeps its own values in data-builtin-* attributes, so tooltips and about:keyboard show the shortcut that actually works. AppKit applies assignments when an item is inserted into a menu and announces later changes through NSMenuDidChangeItemNotification, so the sync runs at those points.

Comment 50

[Stephen A Pohl [:spohl]]

Attachment: Bug 1333781: Make about:keyboard work underneath a shortcut assigned in System Settings. r?Jamie,#firefox-desktop-core-reviewers,#fluent-reviewers

While a System Settings assignment is in effect, a customization made in about:keyboard goes into the key element's data-builtin-* copies, so it applies once the assignment is removed instead of fighting it. about:keyboard shows where such a shortcut comes from and doesn't offer to change it. The keyset re-append workaround is gone, since key elements now take attribute changes in place.

Comment 51

[Stephen A Pohl [:spohl]]

I'm setting a needinfo on myself to land this on 10/22, once the Firefox 161 Nightly cycle starts. QA coverage has been requested in https://mozilla-hub.atlassian.net/browse/QA-5854, and the plan is for QA to test it in Nightly 161.

Comment 52

[Stephen A Pohl [:spohl]]

Rebased patches have been sent to try before landing on m-c: https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod-2025&landoCommitID=109175

Comment 53

[Stephen A Pohl [:spohl]]

I have been coordinating with QA and this is ready to be tested as soon as it lands. Queued for landing.

Comment 54

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cfb6458b99bb
https://hg.mozilla.org/integration/autoland/rev/4b8b450fbb5a
Rebuild XUL keyset handlers when its key elements change. r=dom-core-reviewers,smaug
https://github.com/mozilla-firefox/firefox/commit/df17f7ba110e
https://hg.mozilla.org/integration/autoland/rev/aee194e4fc59
Highlight a menu bar item when its shortcut runs, not before the window handles the key. r=mac-reviewers,bradwerth
https://github.com/mozilla-firefox/firefox/commit/8e8903c1cd3e
https://hg.mozilla.org/integration/autoland/rev/1096293273e9
Let a shortcut assigned to a menu item in System Settings replace the built-in one. r=mac-reviewers,bradwerth
https://github.com/mozilla-firefox/firefox/commit/2140b2d6746e
https://hg.mozilla.org/integration/autoland/rev/9e5021a79c94
Make about:keyboard work underneath a shortcut assigned in System Settings. r=Jamie,fluent-reviewers,bolsson

Comment 55

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4b8b450fbb5a
https://hg.mozilla.org/mozilla-central/rev/aee194e4fc59
https://hg.mozilla.org/mozilla-central/rev/1096293273e9
https://hg.mozilla.org/mozilla-central/rev/9e5021a79c94

Comment 56

[Pascal Chevrel (relman team) -> :pascalc]

Did you want to nominate this for the Fx161 relnotes? If so, set the relnote-firefox flag to "?"
https://wiki.mozilla.org/Release_Management/Release_Notes_Nomination

Possible wording:

On macOS, keyboard shortcuts assigned to Firefox menu items in System Settings now replace the built-in shortcuts for those items.

Comment 57

[Stephen A Pohl [:spohl]]

Thank you for the reminder!

Release Note Request (optional, but appreciated)
[Why is this notable]: As far back as 2008, we had reports that user-defined shortcuts didn't work as intended. While this was addressed for shortcuts that were not already used as a built-in shortcut in 2017 (bug 429824), the much more complex problem of allowing built-in shortcuts to be reassigned dragged on. After over 18 years, 15 duplicate bug reports and 50 people in CC, this appears to finally be fully fixed.
[Affects Firefox for Android]: no
[Suggested wording]: On macOS, keyboard shortcuts assigned to Firefox menu items in System Settings now replace the built-in shortcuts for those items.
[Links (documentation, blog post, etc)]: n/a

Comment 58

[Pascal Chevrel (relman team) -> :pascalc]

Added to nightly release notes