Closed Bug 1768567 Opened 3 years ago Closed 2 years ago

Fix and clean up global shortcut code

Categories

(Thunderbird :: Mail Window Front End, task)

Tracking

(thunderbird_esr91 unaffected)

RESOLVED FIXED
102 Branch
Tracking Status
thunderbird_esr91 --- unaffected

People

(Reporter: aleca, Assigned: aleca)

References

Details

(Keywords: leave-open)

Attachments

(2 files)

Bug 1665511 is introducing an initial implementation of a global shortcut manager with the ability to customize each shortcut based on different contexts.

Once that lands, we need to use some of those shortcuts that currently only affect the primary mail tab and spaces toolbar, to the standalone Window and Composer.

Just a small note that I thought of.

Shortcuts will have platform and keyboard-layout dependencies. As such, a shortcut on one desktop might not function at all on another desktop or with a different keyboard because the corresponding KeyboardEvent cannot be generated. Similarly, what might look like a shortcut on one setup might be used to generate a printed character on another.

This is not likely to be an issue when using a single profile on one machine because the custom shortcuts can be set by the user. We can make the user explicitly press the sequence themselves and just save whatever KeyboardEvent details are generated from that. This should naturally handle any quirks.

However, this might break down if the user imports their profile onto a new machine. Particularly if they transfer across operating systems. I'm not sure how well importing from other systems works in general, but we can probably expect problems with shortcuts. So we might want to drop the shortcuts when importing from a different operating system.

That's a very good point.
We might experiment with an OS migration helper, which asks the user to migrate shortcuts if we detect that custom shortcuts where set for an OS different from the current one.

This will definitely be an edge case since I don't expect users to customize ALL shortcuts, but just a few to their liking, but of course I could be wrong.
Anyway, that's gonna be an interesting problem to fix.

Keywords: leave-open
Target Milestone: --- → 102 Branch

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/33f2b3229242
Remove unused leftover object property. r=#thunderbird-reviewers,john.bieling

Status: NEW → ASSIGNED
Attachment #9276947 - Attachment description: Bug 1768567 - Fix missing plus sign in the fluent strings of the global shortcuts file. r=mkmelin → Bug 1768567 - Fix missing plus sign in the fluent strings of the global shortcuts file and create a string mapping for easier update. r=mkmelin

Pushed by alessandro@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/279167f85f2f
Fix missing plus sign in the fluent strings of the global shortcuts file and create a string mapping for easier update. r=mkmelin

Something I just noticed in the shortcut manager: we compile a list of shortcuts that match the given Event.key https://searchfox.org/comm-central/rev/e7b3861a0462ee06d11436fd2d0617f0af421163/mail/modules/ShortcutsManager.jsm#228 and if more than one matches we error https://searchfox.org/comm-central/rev/e7b3861a0462ee06d11436fd2d0617f0af421163/mail/modules/ShortcutsManager.jsm#244

What this means is it would error if we have both the shortcut Ctrl+b and Ctrl+Alt+b and press "b" because both match the Event.key, even though their modifiers are different.

This doesn't make a difference right now, but it'll probably become relevant in the future, so we might want to move the logic here https://searchfox.org/comm-central/rev/e7b3861a0462ee06d11436fd2d0617f0af421163/mail/modules/ShortcutsManager.jsm#278-283 to earlier in the method where we construct found.

Indeed, good point. We will do that.

Better close this bug and retitle it for what it actually is.
We will continue the work to achieve a global shortcut after 115

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Summary: Implement new shortcut manager to the compose and standalone window → Fix and clean up global shortcut code
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: