Move tree code away from system group keypress event listeners and make them use keydown listeners instead
Categories
(Core :: XUL, defect, P5)
Tracking
()
Tracking | Status | |
---|---|---|
firefox77 | --- | affected |
People
(Reporter: Gijs, Unassigned)
References
(Blocks 1 open bug)
Details
Quoting Emilio:
The benefits are not having to use
mozSystemGroup
and thus not interacting with the system event listeners, behaving more like the rest of the web.
There's some keypress listeners at https://searchfox.org/mozilla-central/rev/158bac3df3a1890da55bdb6ffdaf9a7ffc0bfb0a/toolkit/content/widgets/tree.js#997-1026 .
Then there are system group listeners in browser-places.js at https://searchfox.org/mozilla-central/rev/158bac3df3a1890da55bdb6ffdaf9a7ffc0bfb0a/browser/base/content/browser-places.js#50 .
I think we can just switch to keydown, as long as we update all the consumers that also interact with these listeners / where the value of defaultPrevented
ends up mattering, to match. But having read bug 1449018 (which switched us to system group) I'm not 100% sure anymore.
Masayuki-san, can you confirm if it makes sense to switch away from system group and to use keydown
listeners instead?
Comment 1•5 years ago
|
||
Ideally, we should move it unless keydown
event is followed by one or more keypress
events on web contents.
However, there are some issues:
- Using
keydown
event to handle something means that it gets higher priority than otherkeypress
event listeners. I.e., it may change priority between event handlers. - Currently, there is no API to distinguish whether a
keydown
event is followed by one or morekeypress
events. I suggested it on UI Events WG, but it was rejected. But if our chrome code needs it, we could implement the API with chrome-only accessible one. - There are blacklists to keep the legacy behavior on specific web apps,
dom.keyboardevent.keypress.hack.dispatch_non_printable_keys
anddom.keyboardevent.keypress.hack.dispatch_non_printable_keys.addl
. Therefore, switching event listeners cause breaking something only on the listed web apps.
So, it might be difficult to do it right now, but I guess that the blocklisted apps won't be updated for current behavior though...
Reporter | ||
Comment 2•5 years ago
|
||
(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(sick, wait for a couple of days for ni?) from comment #1)
Ideally, we should move it unless
keydown
event is followed by one or morekeypress
events on web contents.However, there are some issues:
- Using
keydown
event to handle something means that it gets higher priority than otherkeypress
event listeners. I.e., it may change priority between event handlers.- Currently, there is no API to distinguish whether a
keydown
event is followed by one or morekeypress
events. I suggested it on UI Events WG, but it was rejected. But if our chrome code needs it, we could implement the API with chrome-only accessible one.- There are blacklists to keep the legacy behavior on specific web apps,
dom.keyboardevent.keypress.hack.dispatch_non_printable_keys
anddom.keyboardevent.keypress.hack.dispatch_non_printable_keys.addl
. Therefore, switching event listeners cause breaking something only on the listed web apps.
OK, but this bug is about <tree>
, which isn't web-exposed, so I guess (2) or (3) shouldn't matter here? I guess this means we should check whether there were repercussions for the changes in bug 1627520 though...
Comment 3•5 years ago
|
||
Ah, right. So, the issue of here is, the priority changes between <xul:key>
vs. <tree>
.
Currently, there is no API to check this line:
https://searchfox.org/mozilla-central/rev/158bac3df3a1890da55bdb6ffdaf9a7ffc0bfb0a/toolkit/content/widgets/tree.js#1013
However, this bans any key presses with modifiers. So, I guess that this can be replaced with KeyboardEvent.key
value check. In strictly speaking, JS cannot distinguish whether the key
value indicates whether specific key name or inputting string, but the former is at least 2 characters (e.g., F1
, Fn
). So, the following check must be valid for checking same as KeyboardEvent.charCode > 0
:
if (event.key.length > 1 || event.key.charCodeAt(0) > 0x7F)
Comment 4•4 years ago
|
||
The priority flag is not set for this bug.
:enndeakin, could you have a look please?
For more information, please visit auto_nag documentation.
Updated•4 years ago
|
Comment 5•3 years ago
|
||
Ah, sorry, I misunderstood the word, "tree". I'll file a meta bug for these changes.
Updated•3 years ago
|
Description
•