Bug 1520068 - Handle more system shortcuts in add-on shortcuts ui r=dao
authorMark Striemer <mstriemer@mozilla.com>
Fri, 08 Mar 2019 18:32:20 +0000
changeset 521419 4f506f386199
parent 521418 f00a5d27f417
child 521420 96e78962a053
push id10866
push usernerli@mozilla.com
push dateTue, 12 Mar 2019 18:59:09 +0000
treeherdermozilla-beta@445c24a51727 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdao
bugs1520068
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1520068 - Handle more system shortcuts in add-on shortcuts ui r=dao This centralizes the checks for system shortcuts into ShortcutUtils.jsm, so they can be checked for in the add-on shortcut assignment UI. It also introduces a mechanism for skipping the system event handler if it is already being handled by the shortcuts UI. Differential Revision: https://phabricator.services.mozilla.com/D17586
browser/base/content/browser-ctrlTab.js
browser/base/content/tabbrowser.js
toolkit/content/widgets/tabbox.js
toolkit/modules/ShortcutUtils.jsm
toolkit/mozapps/extensions/content/shortcuts.js
--- a/browser/base/content/browser-ctrlTab.js
+++ b/browser/base/content/browser-ctrlTab.js
@@ -424,20 +424,18 @@ var ctrlTab = {
     document.removeEventListener("keyup", this, true);
 
     for (let preview of this.previews) {
       this.updatePreview(preview, null);
     }
   },
 
   onKeyDown(event) {
-    if (event.keyCode != event.DOM_VK_TAB ||
-        !event.ctrlKey ||
-        event.altKey ||
-        event.metaKey) {
+    let action = ShortcutUtils.getSystemActionForEvent(event);
+    if (action != ShortcutUtils.CYCLE_TABS) {
       return;
     }
 
     event.preventDefault();
     event.stopPropagation();
 
     if (this.isOpen) {
       this.advanceFocus(!event.shiftKey);
--- a/browser/base/content/tabbrowser.js
+++ b/browser/base/content/tabbrowser.js
@@ -4222,70 +4222,64 @@ window._gBrowser = {
   },
 
   _handleKeyDownEvent(aEvent) {
     if (!aEvent.isTrusted) {
       // Don't let untrusted events mess with tabs.
       return;
     }
 
-    if (aEvent.altKey)
+    // Skip this only if something has explicitly cancelled it.
+    if (aEvent.defaultCancelled) {
       return;
+    }
 
     // Don't check if the event was already consumed because tab
     // navigation should always work for better user experience.
 
-    if (aEvent.ctrlKey && aEvent.shiftKey && !aEvent.metaKey) {
-      switch (aEvent.keyCode) {
-        case aEvent.DOM_VK_PAGE_UP:
-          this.moveTabBackward();
-          aEvent.preventDefault();
-          return;
-        case aEvent.DOM_VK_PAGE_DOWN:
-          this.moveTabForward();
-          aEvent.preventDefault();
-          return;
-      }
-    }
-
-    if (AppConstants.platform != "macosx") {
-      if (aEvent.ctrlKey && !aEvent.shiftKey && !aEvent.metaKey &&
-          aEvent.keyCode == KeyEvent.DOM_VK_F4) {
+    switch (ShortcutUtils.getSystemActionForEvent(aEvent)) {
+      case ShortcutUtils.MOVE_TAB_BACKWARD:
+        this.moveTabBackward();
+        aEvent.preventDefault();
+        return;
+      case ShortcutUtils.MOVE_TAB_FORWARD:
+        this.moveTabForward();
+        aEvent.preventDefault();
+        return;
+      case ShortcutUtils.CLOSE_TAB:
         if (gBrowser.multiSelectedTabsCount) {
           gBrowser.removeMultiSelectedTabs();
         } else if (!this.selectedTab.pinned) {
           this.removeCurrentTab({ animate: true });
         }
         aEvent.preventDefault();
-      }
     }
   },
 
   _handleKeyPressEventMac(aEvent) {
     if (!aEvent.isTrusted) {
       // Don't let untrusted events mess with tabs.
       return;
     }
 
-    if (aEvent.altKey)
+    // Skip this only if something has explicitly cancelled it.
+    if (aEvent.defaultCancelled) {
       return;
+    }
 
     if (AppConstants.platform == "macosx") {
-      if (!aEvent.metaKey)
-        return;
-
-      var offset = 1;
-      switch (aEvent.charCode) {
-        case "}".charCodeAt(0):
-          offset = -1;
-        case "{".charCodeAt(0):
-          if (!RTL_UI)
-            offset *= -1;
-          this.tabContainer.advanceSelectedTab(offset, true);
+      switch (ShortcutUtils.getSystemActionForEvent(aEvent, {rtl: RTL_UI})) {
+        case ShortcutUtils.NEXT_TAB:
+          this.tabContainer.advanceSelectedTab(1, true);
           aEvent.preventDefault();
+          break;
+        case ShortcutUtils.PREVIOUS_TAB:
+          this.tabContainer.advanceSelectedTab(-1, true);
+          aEvent.preventDefault();
+          break;
       }
     }
   },
 
   createTooltip(event) {
     event.stopPropagation();
     var tab = document.tooltipNode;
     if (!tab || tab.localName != "tab") {
--- a/toolkit/content/widgets/tabbox.js
+++ b/toolkit/content/widgets/tabbox.js
@@ -4,16 +4,20 @@
 
 "use strict";
 
 // This is loaded into chrome windows with the subscript loader. Wrap in
 // a block to prevent accidentally leaking globals onto `window`.
 {
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 
+let imports = {};
+ChromeUtils.defineModuleGetter(imports, "ShortcutUtils",
+                               "resource://gre/modules/ShortcutUtils.jsm");
+
 class MozTabbox extends MozXULElement {
   constructor() {
     super();
     this._handleMetaAltArrows = /Mac/.test(navigator.platform);
     this.disconnectedCallback = this.disconnectedCallback.bind(this);
   }
 
   connectedCallback() {
@@ -103,59 +107,45 @@ class MozTabbox extends MozXULElement {
   }
 
   handleEvent(event) {
     if (!event.isTrusted) {
       // Don't let untrusted events mess with tabs.
       return;
     }
 
+    // Skip this only if something has explicitly cancelled it.
+    if (event.defaultCancelled) {
+      return;
+    }
+
     // Don't check if the event was already consumed because tab
     // navigation should always work for better user experience.
 
-    switch (event.keyCode) {
-      case event.DOM_VK_TAB:
-        if (event.ctrlKey && !event.altKey && !event.metaKey)
-          if (this.tabs && this.handleCtrlTab) {
-            this.tabs.advanceSelectedTab(event.shiftKey ? -1 : 1, true);
-            event.preventDefault();
-          }
+    const {ShortcutUtils} = imports;
+
+    switch (ShortcutUtils.getSystemActionForEvent(event)) {
+      case ShortcutUtils.CYCLE_TABS:
+        if (this.tabs && this.handleCtrlTab) {
+          this.tabs.advanceSelectedTab(event.shiftKey ? -1 : 1, true);
+          event.preventDefault();
+        }
         break;
-      case event.DOM_VK_PAGE_UP:
-        if (event.ctrlKey && !event.shiftKey && !event.altKey && !event.metaKey &&
-            this.tabs) {
+      case ShortcutUtils.PREVIOUS_TAB:
+        if (this.tabs) {
           this.tabs.advanceSelectedTab(-1, true);
           event.preventDefault();
         }
         break;
-      case event.DOM_VK_PAGE_DOWN:
-        if (event.ctrlKey && !event.shiftKey && !event.altKey && !event.metaKey &&
-            this.tabs) {
+      case ShortcutUtils.NEXT_TAB:
+        if (this.tabs) {
           this.tabs.advanceSelectedTab(1, true);
           event.preventDefault();
         }
         break;
-      case event.DOM_VK_LEFT:
-        if (event.metaKey && event.altKey && !event.shiftKey && !event.ctrlKey)
-          if (this.tabs && this._handleMetaAltArrows) {
-            let offset = window.getComputedStyle(this)
-              .direction == "ltr" ? -1 : 1;
-            this.tabs.advanceSelectedTab(offset, true);
-            event.preventDefault();
-          }
-        break;
-      case event.DOM_VK_RIGHT:
-        if (event.metaKey && event.altKey && !event.shiftKey && !event.ctrlKey)
-          if (this.tabs && this._handleMetaAltArrows) {
-            let offset = window.getComputedStyle(this)
-              .direction == "ltr" ? 1 : -1;
-            this.tabs.advanceSelectedTab(offset, true);
-            event.preventDefault();
-          }
-        break;
     }
   }
 }
 
 customElements.define("tabbox", MozTabbox);
 
 class MozTabpanels extends MozXULElement {
   connectedCallback() {
--- a/toolkit/modules/ShortcutUtils.jsm
+++ b/toolkit/modules/ShortcutUtils.jsm
@@ -4,16 +4,20 @@
 
 "use strict";
 
 var EXPORTED_SYMBOLS = ["ShortcutUtils"];
 
 const {Services} = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const {XPCOMUtils} = ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
+XPCOMUtils.defineLazyModuleGetters(this, {
+  AppConstants: "resource://gre/modules/AppConstants.jsm",
+});
+
 XPCOMUtils.defineLazyGetter(this, "PlatformKeys", function() {
   return Services.strings.createBundle(
     "chrome://global-platform/locale/platformKeys.properties");
 });
 
 XPCOMUtils.defineLazyGetter(this, "Keys", function() {
   return Services.strings.createBundle(
     "chrome://global/locale/keys.properties");
@@ -22,16 +26,23 @@ XPCOMUtils.defineLazyGetter(this, "Keys"
 var ShortcutUtils = {
   IS_VALID: "valid",
   INVALID_KEY: "invalid_key",
   INVALID_MODIFIER: "invalid_modifier",
   INVALID_COMBINATION: "invalid_combination",
   DUPLICATE_MODIFIER: "duplicate_modifier",
   MODIFIER_REQUIRED: "modifier_required",
 
+  MOVE_TAB_FORWARD: "MOVE_TAB_FORWARD",
+  MOVE_TAB_BACKWARD: "MOVE_TAB_BACKWARD",
+  CLOSE_TAB: "CLOSE_TAB",
+  CYCLE_TABS: "CYCLE_TABS",
+  PREVIOUS_TAB: "PREVIOUS_TAB",
+  NEXT_TAB: "NEXT_TAB",
+
   /**
     * Prettifies the modifier keys for an element.
     *
     * @param Node aElemKey
     *        The key element to get the modifiers from.
     * @param boolean aNoCloverLeaf
     *        Pass true to use a descriptive string instead of the cloverleaf symbol. (OS X only)
     * @return string
@@ -266,12 +277,71 @@ var ShortcutUtils = {
 
     let keyEl = win.document.querySelector([
       `${baseSelector}[key="${chromeKey}"]`,
       `${baseSelector}[key="${chromeKey.toLowerCase()}"]`,
       `${baseSelector}[keycode="${keycode}"]`,
     ].join(","));
     return keyEl && !keyEl.closest("keyset").id.startsWith("ext-keyset-id");
   },
+
+  /**
+   * Determine what action a KeyboardEvent should perform, if any.
+   *
+   * @param {KeyboardEvent} event The event to check for a related system action.
+   * @returns {string} A string identifying the action, or null if no action is found.
+   */
+  getSystemActionForEvent(event, {rtl} = {}) {
+    switch (event.keyCode) {
+      case event.DOM_VK_TAB:
+        if (event.ctrlKey && !event.altKey && !event.metaKey)
+          return ShortcutUtils.CYCLE_TABS;
+        break;
+      case event.DOM_VK_PAGE_UP:
+        if (event.ctrlKey && !event.shiftKey && !event.altKey && !event.metaKey)
+          return ShortcutUtils.PREVIOUS_TAB;
+        if (event.ctrlKey && event.shiftKey && !event.altKey && !event.metaKey)
+          return ShortcutUtils.MOVE_TAB_BACKWARD;
+        break;
+      case event.DOM_VK_PAGE_DOWN:
+        if (event.ctrlKey && !event.shiftKey && !event.altKey && !event.metaKey)
+          return ShortcutUtils.NEXT_TAB;
+        if (event.ctrlKey && event.shiftKey && !event.altKey && !event.metaKey)
+          return ShortcutUtils.MOVE_TAB_FORWARD;
+        break;
+      case event.DOM_VK_LEFT:
+        if (event.metaKey && event.altKey && !event.shiftKey && !event.ctrlKey)
+          return ShortcutUtils.PREVIOUS_TAB;
+        break;
+      case event.DOM_VK_RIGHT:
+        if (event.metaKey && event.altKey && !event.shiftKey && !event.ctrlKey)
+          return ShortcutUtils.NEXT_TAB;
+        break;
+    }
+
+    if (AppConstants.platform == "macosx") {
+      if (!event.altKey && event.metaKey) {
+        switch (event.charCode) {
+          case "}".charCodeAt(0):
+            if (rtl)
+              return ShortcutUtils.PREVIOUS_TAB;
+            return ShortcutUtils.NEXT_TAB;
+          case "{".charCodeAt(0):
+            if (rtl)
+              return ShortcutUtils.NEXT_TAB;
+            return ShortcutUtils.PREVIOUS_TAB;
+        }
+      }
+    }
+    // Not on Mac from now on.
+    if (AppConstants.platform != "macosx") {
+      if (event.ctrlKey && !event.shiftKey && !event.metaKey &&
+          event.keyCode == KeyEvent.DOM_VK_F4) {
+        return ShortcutUtils.CLOSE_TAB;
+      }
+    }
+
+    return null;
+  },
 };
 
 Object.freeze(ShortcutUtils);
 
--- a/toolkit/mozapps/extensions/content/shortcuts.js
+++ b/toolkit/mozapps/extensions/content/shortcuts.js
@@ -212,16 +212,23 @@ function onShortcutChange(e) {
 
   if (e.key == "Tab") {
     return;
   }
 
   e.preventDefault();
   e.stopPropagation();
 
+  // Some system actions aren't in the keyset, handle them independantly.
+  if (ShortcutUtils.getSystemActionForEvent(e)) {
+    e.defaultCancelled = true;
+    setError(input, "shortcuts-system");
+    return;
+  }
+
   let shortcutString = getShortcutForEvent(e);
   input.value = getShortcutValue(shortcutString);
 
   if (e.type == "keyup" || shortcutString.length == 0) {
     return;
   }
 
   let validation = ShortcutUtils.validate(shortcutString);