Closed
Bug 1441144
Opened 6 years ago
Closed 6 years ago
browser/extensions/onboarding/content/onboarding.js should handle non-printable keys with keydown event listener rather than keypress event listener
Categories
(Firefox :: General, enhancement)
Tracking
()
RESOLVED
FIXED
Firefox 60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: masayuki, Assigned: masayuki)
References
Details
Attachments
(1 file)
We'll stop dispatching keypress event in the default event group of web content (bug 968056). So, onboarding.js needs to handle non-printable keys with keydown event listener.
Assignee | ||
Comment 1•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a077ff93667f0197faa2277e025555697d762dc6
Comment hidden (mozreview-request) |
Assignee | ||
Comment 3•6 years ago
|
||
Although not scope of this bug, checking KeyboardEvent.key with " " must be wrong. IIRC, spacebar of some keyboard layouts produces other Unicode whitespace character instead of ASCII whitespace. Perhaps, KeyboardEvent.code should be checked instead.
Comment 4•6 years ago
|
||
mozreview-review |
Comment on attachment 8954027 [details] Bug 1441144 - Make browser/extensions/onboarding/content/onboarding.js handle non-printable keys with "keydown" event listener rather than "keypress" event listener https://reviewboard.mozilla.org/r/223176/#review229114 Code analysis found 2 defects in this patch: - 2 defects found by mozlint You can run this analysis locally with: - `./mach lint path/to/file` (JS/Python) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: browser/extensions/onboarding/content/onboarding.js:785 (Diff revision 1) > } > event.stopPropagation(); > } > > + handleKeypress(event) { > + let { target, key, shiftKey } = event; Error: 'shiftkey' is assigned a value but never used. allowed unused vars must match /^cc|ci|cu|cr|exported_symbols/. [eslint: no-unused-vars] ::: browser/extensions/onboarding/content/onboarding.js:802 (Diff revision 1) > + // Currently focused item could be tab container if previous navigation was done > + // via mouse. > + if (target.classList.contains("onboarding-tour-item-container")) { > + target = target.firstChild; > + } > + let targetIndex; Error: 'targetindex' is defined but never used. allowed unused vars must match /^cc|ci|cu|cr|exported_symbols/. [eslint: no-unused-vars]
Assignee | ||
Comment 5•6 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c71a249619d4a4a9a32540a08e51bada657d0325
Comment hidden (mozreview-request) |
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8954027 [details] Bug 1441144 - Make browser/extensions/onboarding/content/onboarding.js handle non-printable keys with "keydown" event listener rather than "keypress" event listener https://reviewboard.mozilla.org/r/223176/#review229640
Attachment #8954027 -
Flags: review?(dtownsend) → review+
Pushed by masayuki@d-toybox.com: https://hg.mozilla.org/integration/autoland/rev/1fa874194a00 Make browser/extensions/onboarding/content/onboarding.js handle non-printable keys with "keydown" event listener rather than "keypress" event listener r=mossop
Comment 9•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1fa874194a00
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
You need to log in
before you can comment on or make changes to this bug.
Description
•