Bug 1437838 - Narrate module should listen to keydown events instead of keypress events r?eeejay draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 13 Feb 2018 22:11:01 +0900
changeset 754770 389c0609547bc2e8e7679cf870cebb6c526a3c80
parent 754769 fc9dd66f58bbc28dd781fe232a16d5c124f96056
child 754771 04ee9af5e06b2000d15dc84c8b539f43d9ce816a
push id98989
push usermasayuki@d-toybox.com
push dateWed, 14 Feb 2018 07:43:07 +0000
reviewerseeejay
bugs1437838
milestone60.0a1
Bug 1437838 - Narrate module should listen to keydown events instead of keypress events r?eeejay Looks like that any script of web pages don't run in reader mode. If so, it's safe narrate module to listen to keydown events of non-printable keys because nobody listens to following keypress events of them. So, it's OK to stop dispatching following keypress event of keys handled by keydown event handlers of narrate module. MozReview-Commit-ID: Agn8H8j8opz
toolkit/components/narrate/VoiceSelect.jsm
toolkit/components/narrate/test/browser_voiceselect.js
--- a/toolkit/components/narrate/VoiceSelect.jsm
+++ b/toolkit/components/narrate/VoiceSelect.jsm
@@ -17,22 +17,22 @@ function VoiceSelect(win, label) {
       <span class="label">${label}</span> <span class="current-voice"></span>
     </button>
     <div class="options" id="voice-options" role="listbox"></div>`;
 
   this._elementRef = Cu.getWeakReference(element);
 
   let button = this.selectToggle;
   button.addEventListener("click", this);
-  button.addEventListener("keypress", this);
+  button.addEventListener("keydown", this);
 
   let listbox = this.listbox;
   listbox.addEventListener("click", this);
   listbox.addEventListener("mousemove", this);
-  listbox.addEventListener("keypress", this);
+  listbox.addEventListener("keydown", this);
   listbox.addEventListener("wheel", this, true);
 
   win.addEventListener("resize", () => {
     this._updateDropdownHeight();
   });
 }
 
 VoiceSelect.prototype = {
@@ -98,26 +98,26 @@ VoiceSelect.prototype = {
           this.toggleList();
         }
         break;
 
       case "mousemove":
         this.listbox.classList.add("hovering");
         break;
 
-      case "keypress":
+      case "keydown":
         if (target.classList.contains("select-toggle")) {
           if (evt.altKey) {
             this.toggleList(true);
           } else {
-            this._keyPressedButton(evt);
+            this._keyDownedButton(evt);
           }
         } else {
           this.listbox.classList.remove("hovering");
-          this._keyPressedInBox(evt);
+          this._keyDownedInBox(evt);
         }
         break;
 
       case "wheel":
         // Don't let wheel events bubble to document. It will scroll the page
         // and close the entire narrate dialog.
         evt.stopPropagation();
         break;
@@ -142,17 +142,17 @@ VoiceSelect.prototype = {
       }
 
       next = sibling;
     }
 
     return next;
   },
 
-  _keyPressedButton(evt) {
+  _keyDownedButton(evt) {
     if (evt.altKey && (evt.key === "ArrowUp" || evt.key === "ArrowUp")) {
       this.toggleList(true);
       return;
     }
 
     let toSelect;
     switch (evt.key) {
       case "PageUp":
@@ -172,17 +172,17 @@ VoiceSelect.prototype = {
     }
 
     if (toSelect && toSelect.classList.contains("option")) {
       evt.preventDefault();
       this.selected = toSelect;
     }
   },
 
-  _keyPressedInBox(evt) {
+  _keyDownedInBox(evt) {
     let toFocus;
     let cur = this._doc.activeElement;
 
     switch (evt.key) {
       case "ArrowUp":
         toFocus = cur.previousElementSibling || this.listbox.lastElementChild;
         break;
       case "ArrowDown":
--- a/toolkit/components/narrate/test/browser_voiceselect.js
+++ b/toolkit/components/narrate/test/browser_voiceselect.js
@@ -82,31 +82,31 @@ add_task(async function testVoiceselectK
 
     let firstValue = $(NarrateTestUtils.VOICE_SELECTED).dataset.value;
 
     ok(!NarrateTestUtils.isVisible($(NarrateTestUtils.VOICE_OPTIONS)),
       "voice options initially are hidden");
 
     $(NarrateTestUtils.VOICE_SELECT).focus();
 
-    eventUtils.sendKey("DOWN", content);
+    eventUtils.synthesizeKey("KEY_ArrowDown", {}, content);
 
     await ContentTaskUtils.waitForCondition(
       () => $(NarrateTestUtils.VOICE_SELECTED).dataset.value != firstValue,
-      "value changed after pressing DOWN key");
+      "value changed after pressing ArrowDown key");
 
-    eventUtils.sendKey("RETURN", content);
+    eventUtils.synthesizeKey("KEY_Enter", {}, content);
 
     ok(NarrateTestUtils.isVisible($(NarrateTestUtils.VOICE_OPTIONS)),
-      "voice options showing after pressing RETURN");
+      "voice options showing after pressing Enter");
 
-    eventUtils.sendKey("UP", content);
+    eventUtils.synthesizeKey("KEY_ArrowUp", {}, content);
 
-    eventUtils.sendKey("RETURN", content);
+    eventUtils.synthesizeKey("KEY_Enter", {}, content);
 
     ok(!NarrateTestUtils.isVisible($(NarrateTestUtils.VOICE_OPTIONS)),
-      "voice options hidden after pressing RETURN");
+      "voice options hidden after pressing Enter");
 
     await ContentTaskUtils.waitForCondition(
       () => $(NarrateTestUtils.VOICE_SELECTED).dataset.value == firstValue,
-      "value changed back to original after pressing RETURN");
+      "value changed back to original after pressing Enter");
   });
 });