Keyboard shortcuts should not propagate to the normal UI while customizing Unified Toolbar
Categories
(Thunderbird :: Toolbars and Tabs, defect, P1)
Tracking
(thunderbird111 fixed, thunderbird112 fixed)
People
(Reporter: freaktechnik, Assigned: freaktechnik)
References
(Blocks 1 open bug)
Details
(Keywords: dataloss, ux-error-prevention, Whiteboard: [Supernova3p])
Attachments
(2 files)
(deleted),
text/x-phabricator-request
|
wsmwk
:
approval-comm-beta+
|
Details |
(deleted),
text/x-phabricator-request
|
Details |
STR:
- Open unified toolbar customization (right click on the toolbar and select customize)
- Execute a global command shortcut like Ctrl + K
Expected result:
Nothing actually happened
Actual result:
The keyboard handler for the global Ctrl + K shortcut is executed and opens a gloda facet.
Importantly we shouldn't just check if Ctrl + K doesn't propagate, this should apply to all keyboard interactions.
Updated•2 years ago
|
Comment 1•2 years ago
|
||
Imho this is extremely dangerous. Users might think they are already in the customization search box and start typing. Any A
in their typing will covertly archive messages away in the background. If for any reason they press Del
after entering customization, messages get deleted behind user's back. It's just unpredictable what will happen when keypresses end up on the main UI layer while that's hidden.
Updated•2 years ago
|
Assignee | ||
Comment 2•2 years ago
|
||
A couple of observations:
- We should move focus into the customization when it opens
- Apparently preventingDefault on all (document level) keyboard events makes problems go away (at least partially, I did not check every keyboard shortcut type we have was fixed by it), however, it also makes it so typing doesn't type, so that's not great.
Comment 3•2 years ago
|
||
I added this to the unified-toolbar-customization.mjs
as a quick test and it seems to work:
this.addEventListener("keydown", event => {
event.stopPropagation();
});
Comment 4•2 years ago
|
||
(In reply to Martin Giger [:freaktechnik] from comment #2)
A couple of observations:
- We should move focus into the customization when it opens
If the customization search field could have initial focus, that would be great, and also help to reduce the problem a little bit (not entirely at all), as "a" and Del
key get absorbed by the text input (but other shortcuts like Ctrl+K are not absorbed because the text input doesn't use them). Of course, that's still not enough because whenever something outside the search box gets focus, we're back to the problem. Alex' proposal of comment 3 looks promising!
Assignee | ||
Comment 5•2 years ago
|
||
(In reply to Alessandro Castellani [:aleca] from comment #3)
I added this to the
unified-toolbar-customization.mjs
as a quick test and it seems to work:this.addEventListener("keydown", event => { event.stopPropagation(); });
That's not enough in my tests. Make sure you have a selected message before opening the customization.
Comment 6•2 years ago
|
||
Yeah, what I suggested seem to work only if the focus is on the search input.
Comment 7•2 years ago
|
||
(In reply to Martin Giger [:freaktechnik] from comment #5)
(In reply to Alessandro Castellani [:aleca] from comment #3)
I added this to the
unified-toolbar-customization.mjs
as a quick test and it seems to work:this.addEventListener("keydown", event => { event.stopPropagation(); });
That's not enough in my tests. Make sure you have a selected message before opening the customization.
In my tests, this keydown listener didn't even trigger on the <unified-toolbar-customization> when you press a key like "Del" after opening customization. Is that correct?
Because I used a click listener for comparison which triggered, but the keydown listener never triggered here.
Assignee | ||
Updated•2 years ago
|
Assignee | ||
Comment 8•2 years ago
|
||
Comment 9•2 years ago
|
||
Comment on attachment 9319485 [details]
Bug 1815210 - Prevent commands from being executed in tabs while unified toolbar customization is shown. r=#thunderbird-front-end-reviewers
[Triage Comment]
Optimistically approving for beta.
If testing comes back negative we will back out.
Comment 10•2 years ago
|
||
bugherder uplift |
Thunderbird 111.0b1:
https://hg.mozilla.org/releases/comm-beta/rev/701c87942186
Assignee | ||
Updated•2 years ago
|
Comment 11•2 years ago
|
||
Tests successfull using Windows build from treeherder, per Martin's steps at https://phabricator.services.mozilla.com/D170766#5618238
Comment 12•2 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/a00d2f33bfd6
Prevent commands from being executed in tabs while unified toolbar customization is shown. r=aleca
Comment 13•2 years ago
|
||
(In reply to Wayne Mery (:wsmwk) from comment #11)
Tests successfull using Windows build from treeherder, per Martin's steps at https://phabricator.services.mozilla.com/D170766#5618238
I have also just tested Martin's steps and they work as expected. Also Del
/Shift+Del
or A
does no longer leak, that's good!
111.0b1 (64-bit) cand build3, en-US, Win10
Assignee | ||
Comment 14•2 years ago
|
||
Assignee | ||
Updated•2 years ago
|
Comment 15•2 years ago
|
||
Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/1955f6c5a729
Prevent various global commands from executing while customization is open. r=aleca
Updated•1 year ago
|
Description
•