Bug 1008772 part.1 tabs should handle non-printable keys with keydown event handlers in the system group rather than keypress event handlers in the normal group r=smaug+enndeakin
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 05 Jun 2014 11:57:53 +0900
changeset 207044 c142ccc5bb25c3188978a2c3f44bee7d3aa79744
parent 207043 62c85e7fc6adcc7da216f5e65c573569305a800d
child 207045 721046203db098453b3d7dd09cf5bcb585ca90c7
push id494
push userraliiev@mozilla.com
push dateMon, 25 Aug 2014 18:42:16 +0000
treeherdermozilla-release@a3cc3e46b571 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1008772
milestone32.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 1008772 part.1 tabs should handle non-printable keys with keydown event handlers in the system group rather than keypress event handlers in the normal group r=smaug+enndeakin
browser/base/content/tabbrowser.xml
toolkit/content/widgets/tabbox.xml
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -2880,42 +2880,65 @@
       <property name="textZoom"
                 onget="return this.mCurrentBrowser.textZoom;"
                 onset="this.mCurrentBrowser.textZoom = val;"/>
 
       <property name="isSyntheticDocument"
                 onget="return this.mCurrentBrowser.isSyntheticDocument;"
                 readonly="true"/>
 
-      <method name="_handleKeyEvent">
+      <method name="_handleKeyDownEvent">
         <parameter name="aEvent"/>
         <body><![CDATA[
           if (!aEvent.isTrusted) {
             // Don't let untrusted events mess with tabs.
             return;
           }
 
           if (aEvent.altKey)
             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.stopPropagation();
                 aEvent.preventDefault();
                 return;
               case aEvent.DOM_VK_PAGE_DOWN:
                 this.moveTabForward();
-                aEvent.stopPropagation();
                 aEvent.preventDefault();
                 return;
             }
           }
 
+#ifndef XP_MACOSX
+          if (aEvent.ctrlKey && !aEvent.shiftKey && !aEvent.metaKey &&
+              aEvent.keyCode == KeyEvent.DOM_VK_F4 &&
+              !this.mCurrentTab.pinned) {
+            this.removeCurrentTab({animate: true});
+            aEvent.preventDefault();
+          }
+#endif
+        ]]></body>
+      </method>
+
+      <method name="_handleKeyPressEvent">
+        <parameter name="aEvent"/>
+        <body><![CDATA[
+          if (!aEvent.isTrusted) {
+            // Don't let untrusted events mess with tabs.
+            return;
+          }
+
+          if (aEvent.altKey)
+            return;
+
           // We need to take care of FAYT-watching as long as the findbar
           // isn't initialized.  The checks on aEvent are copied from
           // _shouldFastFind (see findbar.xml).
           if (!gFindBarInitialized &&
               !(aEvent.ctrlKey || aEvent.metaKey) &&
               !aEvent.defaultPrevented) {
             let charCode = aEvent.charCode;
             if (charCode) {
@@ -2935,27 +2958,18 @@
           var offset = 1;
           switch (aEvent.charCode) {
             case '}'.charCodeAt(0):
               offset = -1;
             case '{'.charCodeAt(0):
               if (window.getComputedStyle(this, null).direction == "ltr")
                 offset *= -1;
               this.tabContainer.advanceSelectedTab(offset, true);
-              aEvent.stopPropagation();
               aEvent.preventDefault();
           }
-#else
-          if (aEvent.ctrlKey && !aEvent.shiftKey && !aEvent.metaKey &&
-              aEvent.keyCode == KeyEvent.DOM_VK_F4 &&
-              !this.mCurrentTab.pinned) {
-            this.removeCurrentTab({animate: true});
-            aEvent.stopPropagation();
-            aEvent.preventDefault();
-          }
 #endif
         ]]></body>
       </method>
 
       <property name="userTypedClear"
                 onget="return this.mCurrentBrowser.userTypedClear;"
                 onset="return this.mCurrentBrowser.userTypedClear = val;"/>
 
@@ -2977,18 +2991,21 @@
                                              tab.getAttribute("label"));
         ]]></body>
       </method>
 
       <method name="handleEvent">
         <parameter name="aEvent"/>
         <body><![CDATA[
           switch (aEvent.type) {
+            case "keydown":
+              this._handleKeyDownEvent(aEvent);
+              break;
             case "keypress":
-              this._handleKeyEvent(aEvent);
+              this._handleKeyPressEvent(aEvent);
               break;
             case "sizemodechange":
               if (aEvent.target == window) {
                 this.mCurrentBrowser.docShellIsActive =
                   (window.windowState != window.STATE_MINIMIZED);
               }
               break;
           }
@@ -3045,17 +3062,19 @@
       </method>
 
       <constructor>
         <![CDATA[
           let browserStack = document.getAnonymousElementByAttribute(this, "anonid", "browserStack");
           this.mCurrentBrowser = document.getAnonymousElementByAttribute(this, "anonid", "initialBrowser");
 
           this.mCurrentTab = this.tabContainer.firstChild;
-          document.addEventListener("keypress", this, false);
+          let els = Cc["@mozilla.org/eventlistenerservice;1"].getService(Ci.nsIEventListenerService);
+          els.addSystemEventListener(document, "keydown", this, false);
+          els.addSystemEventListener(document, "keypress", this, false);
           window.addEventListener("sizemodechange", this, false);
 
           var uniqueId = this._generateUniquePanelID();
           this.mPanelContainer.childNodes[0].id = uniqueId;
           this.mCurrentTab.linkedPanel = uniqueId;
           this.mCurrentTab._tPos = 0;
           this.mCurrentTab._fullyOpen = true;
           this.mCurrentTab.linkedBrowser = this.mCurrentBrowser;
@@ -3123,17 +3142,19 @@
               delete browser.registeredOpenURI;
             }
             browser.webProgress.removeProgressListener(this.mTabFilters[i]);
             this.mTabFilters[i].removeProgressListener(this.mTabListeners[i]);
             this.mTabFilters[i] = null;
             this.mTabListeners[i].destroy();
             this.mTabListeners[i] = null;
           }
-          document.removeEventListener("keypress", this, false);
+          let els = Cc["@mozilla.org/eventlistenerservice;1"].getService(Ci.nsIEventListenerService);
+          els.removeSystemEventListener(document, "keydown", this, false);
+          els.removeSystemEventListener(document, "keypress", this, false);
           window.removeEventListener("sizemodechange", this, false);
 
           if (gMultiProcessBrowser) {
             messageManager.removeMessageListener("DOMTitleChanged", this);
             messageManager.removeMessageListener("contextmenu", this);
           }
         ]]>
       </destructor>
@@ -4242,25 +4263,28 @@
           BrowserOpenTab();
         } else {
           return;
         }
 
         event.stopPropagation();
       ]]></handler>
 
-      <handler event="keypress"><![CDATA[
+      <handler event="keydown" group="system"><![CDATA[
         if (event.altKey || event.shiftKey ||
 #ifdef XP_MACOSX
             !event.metaKey)
 #else
             !event.ctrlKey || event.metaKey)
 #endif
           return;
 
+        // Don't check if the event was already consumed because tab navigation
+        // should work always for better user experience.
+
         switch (event.keyCode) {
           case KeyEvent.DOM_VK_UP:
             this.tabbrowser.moveTabBackward();
             break;
           case KeyEvent.DOM_VK_DOWN:
             this.tabbrowser.moveTabForward();
             break;
           case KeyEvent.DOM_VK_RIGHT:
@@ -4269,21 +4293,20 @@
             break;
           case KeyEvent.DOM_VK_HOME:
             this.tabbrowser.moveTabToStart();
             break;
           case KeyEvent.DOM_VK_END:
             this.tabbrowser.moveTabToEnd();
             break;
           default:
-            // Stop the keypress event for the above keyboard
+            // Consume the keydown event for the above keyboard
             // shortcuts only.
             return;
         }
-        event.stopPropagation();
         event.preventDefault();
       ]]></handler>
 
       <handler event="dragstart"><![CDATA[
         var tab = this._getDragTargetTab(event);
         if (!tab || this._isCustomizing)
           return;
 
--- a/toolkit/content/widgets/tabbox.xml
+++ b/toolkit/content/widgets/tabbox.xml
@@ -142,92 +142,102 @@
         <parameter name="event"/>
         <body>
         <![CDATA[
           if (!event.isTrusted) {
             // Don't let untrusted events mess with tabs.
             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.stopPropagation();
                   event.preventDefault();
                 }
               break;
             case event.DOM_VK_PAGE_UP:
               if (event.ctrlKey && !event.shiftKey && !event.altKey && !event.metaKey)
                 if (this.tabs && this.handleCtrlPageUpDown) {
                   this.tabs.advanceSelectedTab(-1, true);
-                  event.stopPropagation();
                   event.preventDefault();
                 }
               break;
             case event.DOM_VK_PAGE_DOWN:
               if (event.ctrlKey && !event.shiftKey && !event.altKey && !event.metaKey)
                 if (this.tabs && this.handleCtrlPageUpDown) {
                   this.tabs.advanceSelectedTab(1, true);
-                  event.stopPropagation();
                   event.preventDefault();
                 }
               break;
             case event.DOM_VK_LEFT:
               if (event.metaKey && event.altKey && !event.shiftKey && !event.ctrlKey)
                 if (this.tabs && this._handleMetaAltArrows) {
                   var offset = window.getComputedStyle(this, "")
                                      .direction == "ltr" ? -1 : 1;
                   this.tabs.advanceSelectedTab(offset, true);
-                  event.stopPropagation();
                   event.preventDefault();
                 }
               break;
             case event.DOM_VK_RIGHT:
               if (event.metaKey && event.altKey && !event.shiftKey && !event.ctrlKey)
                 if (this.tabs && this._handleMetaAltArrows) {
                   var offset = window.getComputedStyle(this, "")
                                      .direction == "ltr" ? 1 : -1;
                   this.tabs.advanceSelectedTab(offset, true);
-                  event.stopPropagation();
                   event.preventDefault();
                 }
               break;
           }
         ]]>
         </body>
       </method>
 
       <field name="_eventNode">this</field>
 
       <property name="eventNode" onget="return this._eventNode;">
         <setter>
           <![CDATA[
             if (val != this._eventNode) {
-              val.addEventListener("keypress", this, false);
-              this._eventNode.removeEventListener("keypress", this, false);
+              const nsIEventListenerService =
+                Components.interfaces.nsIEventListenerService;
+              let els = Components.classes["@mozilla.org/eventlistenerservice;1"]
+                                  .getService(nsIEventListenerService);
+              els.addSystemEventListener(val, "keydown", this, false);
+              els.removeSystemEventListener(this._eventNode, "keydown", this, false);
               this._eventNode = val;
             }
             return val;
           ]]>
         </setter>
       </property>
 
       <constructor>
         switch (this.getAttribute("eventnode")) {
           case "parent": this._eventNode = this.parentNode; break;
           case "window": this._eventNode = window; break;
           case "document": this._eventNode = document; break;
         }
-        this._eventNode.addEventListener("keypress", this, false);
+        const nsIEventListenerService =
+          Components.interfaces.nsIEventListenerService;
+        let els = Components.classes["@mozilla.org/eventlistenerservice;1"]
+                            .getService(nsIEventListenerService);
+        els.addSystemEventListener(this._eventNode, "keydown", this, false);
       </constructor>
 
       <destructor>
-        this._eventNode.removeEventListener("keypress", this, false);
+        const nsIEventListenerService =
+          Components.interfaces.nsIEventListenerService;
+        let els = Components.classes["@mozilla.org/eventlistenerservice;1"]
+                            .getService(nsIEventListenerService);
+        els.removeSystemEventListener(this._eventNode, "keydown", this, false);
       </destructor>
     </implementation>
   </binding>
 
   <binding id="tabs" role="xul:tabs"
            extends="chrome://global/content/bindings/general.xml#basecontrol">
     <resources>
       <stylesheet src="chrome://global/skin/tabbox.css"/>
@@ -801,49 +811,49 @@
           }
         }
         // Otherwise this tab is already selected and we will fall
         // through to mousedown behavior which sets focus on the current tab,
         // Only a click on an already selected tab should focus the tab itself.
       ]]>
       </handler>
 
-      <handler event="keypress" keycode="VK_LEFT">
+      <handler event="keydown" keycode="VK_LEFT" group="system" preventdefault="true">
       <![CDATA[
         var direction = window.getComputedStyle(this.parentNode, null).direction;
         this.parentNode.advanceSelectedTab(direction == 'ltr' ? -1 : 1, this.arrowKeysShouldWrap);
       ]]>
       </handler>
 
-      <handler event="keypress" keycode="VK_RIGHT">
+      <handler event="keydown" keycode="VK_RIGHT" group="system" preventdefault="true">
       <![CDATA[
         var direction = window.getComputedStyle(this.parentNode, null).direction;
         this.parentNode.advanceSelectedTab(direction == 'ltr' ? 1 : -1, this.arrowKeysShouldWrap);
       ]]>
       </handler>
 
-      <handler event="keypress" keycode="VK_UP">
+      <handler event="keydown" keycode="VK_UP" group="system" preventdefault="true">
       <![CDATA[
         this.parentNode.advanceSelectedTab(-1, this.arrowKeysShouldWrap);
       ]]>
       </handler>
 
-      <handler event="keypress" keycode="VK_DOWN">
+      <handler event="keydown" keycode="VK_DOWN" group="system" preventdefault="true">
       <![CDATA[
         this.parentNode.advanceSelectedTab(1, this.arrowKeysShouldWrap);
       ]]>
       </handler>
 
-      <handler event="keypress" keycode="VK_HOME">
+      <handler event="keydown" keycode="VK_HOME" group="system" preventdefault="true">
       <![CDATA[
         this.parentNode._selectNewTab(this.parentNode.childNodes[0]);
       ]]>
       </handler>
 
-      <handler event="keypress" keycode="VK_END">
+      <handler event="keydown" keycode="VK_END" group="system" preventdefault="true">
       <![CDATA[
         var tabs = this.parentNode.childNodes;
         this.parentNode._selectNewTab(tabs[tabs.length - 1], -1);
       ]]>
       </handler>
     </handlers>
   </binding>