Bug 1441144 - Make browser/extensions/onboarding/content/onboarding.js handle non-printable keys with "keydown" event listener rather than "keypress" event listener r?mossop draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 26 Feb 2018 22:37:11 +0900
changeset 760230 e1370c3757046aef80559755c5a44ee9be61adbb
parent 760229 3278f7f58e8ba99a05dbdb27bb8ed9f236c2633d
push id100576
push usermasayuki@d-toybox.com
push dateTue, 27 Feb 2018 02:50:51 +0000
reviewersmossop
bugs1441144
milestone60.0a1
Bug 1441144 - Make browser/extensions/onboarding/content/onboarding.js handle non-printable keys with "keydown" event listener rather than "keypress" event listener r?mossop We'll stop dispatching "keypress" event for non-printable keys and key combinations in the default event group of web content. Therefore, onbording.js needs to stop handling some keys with "keypress" event listener. This patch makes add a "keydown" event listener, handleKeydown(), and make it handle only non-printable keys. Note that we'll keep dispatching "keypress" event for " " and "Enter". So, we can keep handling them in "keypress" event listener. MozReview-Commit-ID: 1jFIv9N03fa
browser/extensions/onboarding/content/onboarding.js
--- a/browser/extensions/onboarding/content/onboarding.js
+++ b/browser/extensions/onboarding/content/onboarding.js
@@ -460,16 +460,17 @@ class Onboarding {
     let { body } = this._window.document;
     this._overlayIcon = this._renderOverlayButton();
     this._overlayIcon.addEventListener("click", this);
     this._overlayIcon.addEventListener("keypress", this);
     body.insertBefore(this._overlayIcon, body.firstChild);
 
     this._overlay = this._renderOverlay();
     this._overlay.addEventListener("click", this);
+    this._overlay.addEventListener("keydown", this);
     this._overlay.addEventListener("keypress", this);
     body.appendChild(this._overlay);
 
     this._loadJS(TOUR_AGENT_JS_URI);
 
     this._initPrefObserver();
     this._onIconStateChange(Services.prefs.getStringPref("browser.onboarding.state", ICON_STATE_DEFAULT));
 
@@ -729,45 +730,26 @@ class Onboarding {
       }
     } else if (elms.indexOf(current) === elms.length - 1) {
       next = elms[0];
       next.focus();
     }
     return next;
   }
 
-  handleKeypress(event) {
+  handleKeydown(event) {
     let { target, key, shiftKey } = event;
 
-    if (target === this._overlayIcon) {
-      if ([" ", "Enter"].includes(key)) {
-        // Remember that the dialog was opened with a keyboard.
-        this._overlayIcon.dataset.keyboardFocus = true;
-        this.handleClick(target);
-        event.preventDefault();
-      }
-      return;
-    }
-
     // 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;
     switch (key) {
-      case " ":
-      case "Enter":
-        // Assume that the handle function should be identical for keyboard
-        // activation if there is a click handler for the target.
-        if (target.classList.contains("onboarding-tour-item")) {
-          this.handleClick(target);
-          target.focus();
-        }
-        break;
       case "ArrowUp":
         // Go to and focus on the previous tab if it's available.
         targetIndex = this._tourItems.indexOf(target);
         if (targetIndex > 0) {
           let previous = this._tourItems[targetIndex - 1];
           this.handleClick(previous);
           previous.focus();
         }
@@ -794,16 +776,50 @@ class Onboarding {
         }
         break;
       default:
         break;
     }
     event.stopPropagation();
   }
 
+  handleKeypress(event) {
+    let { target, key } = event;
+
+    if (target === this._overlayIcon) {
+      if ([" ", "Enter"].includes(key)) {
+        // Remember that the dialog was opened with a keyboard.
+        this._overlayIcon.dataset.keyboardFocus = true;
+        this.handleClick(target);
+        event.preventDefault();
+      }
+      return;
+    }
+
+    // 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;
+    }
+    switch (key) {
+      case " ":
+      case "Enter":
+        // Assume that the handle function should be identical for keyboard
+        // activation if there is a click handler for the target.
+        if (target.classList.contains("onboarding-tour-item")) {
+          this.handleClick(target);
+          target.focus();
+        }
+        break;
+      default:
+        break;
+    }
+    event.stopPropagation();
+  }
+
   handleEvent(evt) {
     switch (evt.type) {
       case "beforeunload":
         // To make sure the telemetry pings are sent,
         // we send "onboarding-session-end" ping as well as
         // "overlay-session-end" and "notification-session-end" ping
         // (by hiding the overlay and notificaiton) on beforeunload.
         this.hideOverlay();
@@ -819,16 +835,19 @@ class Onboarding {
         // See Bug 1413830#c190 and Bug 1429652 for details.
         this.destroy();
         break;
       case "resize":
         this._window.cancelIdleCallback(this._resizeTimerId);
         this._resizeTimerId =
           this._window.requestIdleCallback(() => this._resizeUI());
         break;
+      case "keydown":
+        this.handleKeydown(evt);
+        break;
       case "keypress":
         this.handleKeypress(evt);
         break;
       case "click":
         this.handleClick(evt.target);
         break;
       default:
         break;