Closed
Bug 1434837
Opened 7 years ago
Closed 7 years ago
autocomplete should listen to keypress events at the system event group
Categories
(Toolkit :: Autocomplete, enhancement, P1)
Toolkit
Autocomplete
Tracking
()
RESOLVED
FIXED
mozilla60
| Tracking | Status | |
|---|---|---|
| firefox60 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
(Depends on 1 open bug)
Details
Attachments
(1 file)
We'll stop dispatching keypress events for non-printable keys and non-printable key combinations. Therefore, autocomplete should stop using keypress events to handle such keys and key combinations.
|
Assignee | |
Comment 1•7 years ago
|
||
|
||
Updated•7 years ago
|
Priority: -- → P1
|
|
Assignee | |
Comment 2•7 years ago
|
||
|
|
Assignee | |
Comment 3•7 years ago
|
||
|
|
Assignee | |
Comment 4•7 years ago
|
||
Hmm, if we change some part of keypress event listeners to keydown event listeners, the order of key event listeners are changed between other modules. That causes a lot of oranges which are difficult to understand. Without event listener order debugging system, we cannot use such ideal approach...
So, perhaps, we should just use keypress event listeners at the system event group for now, unfortunately.
OS: Unspecified → All
Hardware: Unspecified → All
Summary: autocomplete should handle keydown event if it needs to handle non-printable key or key combination → autocomplete should listen to keypress events at the system event group
Version: unspecified → Trunk
|
|
Assignee | |
Comment 5•7 years ago
|
||
|
|
Assignee | |
Comment 6•7 years ago
|
||
|
|
Assignee | |
Comment 7•7 years ago
|
||
|
|
Assignee | |
Comment 8•7 years ago
|
||
|
|
Assignee | |
Comment 9•7 years ago
|
||
| Comment hidden (mozreview-request) |
|
|
||
Comment 11•7 years ago
|
||
| mozreview-review | ||
Comment on attachment 8950533 [details]
Bug 1434837 - Make autocomplete and satchel listen to keypress event at the system event group
https://reviewboard.mozilla.org/r/219806/#review227218
LGTM, thanks!
::: toolkit/content/tests/chrome/test_autocomplete_mac_caret.xul:63
(Diff revision 1)
> + }, {once: true});
> + synthesizeKey(key, {});
> + if (result) {
> + ok(keypressFired, testid + " keypress event should be fired");
> + } else {
> + ok(!keypressFired, testid + " keypress event shouldn't be fired");
nit: you could probably unify these:
is(keypressFired, result,
`${testid} keypress event should${result ? "" : " not"} be fired`);
Attachment #8950533 -
Flags: review?(mak77) → review+
|
|
Assignee | |
Comment 12•7 years ago
|
||
| Comment hidden (mozreview-request) |
|
||
Comment 14•7 years ago
|
||
Pushed by masayuki@d-toybox.com:
https://hg.mozilla.org/integration/autoland/rev/c0b4ca69376c
Make autocomplete and satchel listen to keypress event at the system event group r=mak
|
||
Comment 15•7 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in
before you can comment on or make changes to this bug.
Description
•