Bug 1476852 - Implement keyboard selection for multiselect tabs. r=Gijs,Jamie
authorJared Wein <jwein@mozilla.com>
Mon, 24 Sep 2018 20:36:59 +0000
changeset 437942 110f70038099ac777b4c394b109c0100ac11e4b0
parent 437941 fbc27c2a3b88e1a66b7936cc7b0d59d9e9aee0ed
child 437943 fd4c5546dfa32a0af1643da5e209bcee4230a196
push id34707
push userebalazs@mozilla.com
push dateTue, 25 Sep 2018 09:18:36 +0000
treeherdermozilla-central@8db8a228536d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, Jamie
bugs1476852
milestone64.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 1476852 - Implement keyboard selection for multiselect tabs. r=Gijs,Jamie To use this (Windows/Linux instructions), press Ctrl+L to give focus to the location bar. Shift+Tab to move focus backwards to the tab. Ctrl+Left/Ctrl+Right to change which tab is focused Ctrl+Space to add/remove a tab from the multiselection Moving a tab has been changed from Ctrl+Left/Ctrl+Right to Ctrl+Shift+Left/Ctrl+Shift+Right, respectively. Differential Revision: https://phabricator.services.mozilla.com/D4670
browser/base/content/tabbrowser.xml
browser/base/content/test/general/browser_bug563588.js
browser/base/content/test/tabs/browser.ini
browser/base/content/test/tabs/browser_multiselect_tabs_using_keyboard.js
browser/themes/linux/browser.css
browser/themes/osx/browser.css
browser/themes/windows/browser.css
toolkit/content/widgets/tabbox.xml
--- a/browser/base/content/tabbrowser.xml
+++ b/browser/base/content/tabbrowser.xml
@@ -1161,54 +1161,106 @@
         } else {
           return;
         }
 
         event.stopPropagation();
       ]]></handler>
 
       <handler event="keydown" group="system"><![CDATA[
-        if (event.altKey || event.shiftKey)
-          return;
+        let {altKey, shiftKey} = event;
+        let [accel, nonAccel] = AppConstants.platform == "macosx" ? [event.metaKey, event.ctrlKey] : [event.ctrlKey, event.metaKey];
 
-        let wrongModifiers;
-        if (AppConstants.platform == "macosx") {
-          wrongModifiers = !event.metaKey;
-        } else {
-          wrongModifiers = !event.ctrlKey || event.metaKey;
+        let keyComboForMove = accel && shiftKey && !altKey && !nonAccel;
+        let keyComboForFocus = accel && !shiftKey && !altKey && !nonAccel;
+
+        if (!keyComboForMove && !keyComboForFocus) {
+          return;
         }
 
-        if (wrongModifiers)
-          return;
-
         // Don't check if the event was already consumed because tab navigation
         // should work always for better user experience.
-
+        let {visibleTabs, selectedTab} = gBrowser;
+        let {arrowKeysShouldWrap} = this;
+        let focusedTabIndex = this.ariaFocusedIndex;
+        if (focusedTabIndex == -1) {
+          focusedTabIndex = visibleTabs.indexOf(selectedTab);
+        }
+        let lastFocusedTabIndex = focusedTabIndex;
         switch (event.keyCode) {
           case KeyEvent.DOM_VK_UP:
-            gBrowser.moveTabBackward();
+            if (keyComboForMove) {
+              gBrowser.moveTabBackward();
+            } else {
+              focusedTabIndex--;
+            }
             break;
           case KeyEvent.DOM_VK_DOWN:
-            gBrowser.moveTabForward();
+            if (keyComboForMove) {
+              gBrowser.moveTabForward();
+            } else {
+              focusedTabIndex++;
+            }
             break;
           case KeyEvent.DOM_VK_RIGHT:
           case KeyEvent.DOM_VK_LEFT:
-            gBrowser.moveTabOver(event);
+            if (keyComboForMove) {
+              gBrowser.moveTabOver(event);
+            } else {
+              let isRTL = Services.locale.isAppLocaleRTL;
+              if ((!isRTL && event.keyCode == KeyEvent.DOM_VK_RIGHT) ||
+                  (isRTL && event.keyCode == KeyEvent.DOM_VK_LEFT)) {
+                focusedTabIndex++;
+              } else {
+                focusedTabIndex--;
+              }
+            }
             break;
           case KeyEvent.DOM_VK_HOME:
-            gBrowser.moveTabToStart();
+            if (keyComboForMove) {
+              gBrowser.moveTabToStart();
+            } else {
+              focusedTabIndex = 0;
+            }
             break;
           case KeyEvent.DOM_VK_END:
-            gBrowser.moveTabToEnd();
+            if (keyComboForMove) {
+              gBrowser.moveTabToEnd();
+            } else {
+              focusedTabIndex = visibleTabs.length - 1;
+            }
+            break;
+          case KeyEvent.DOM_VK_SPACE:
+            if (visibleTabs[lastFocusedTabIndex].multiselected) {
+              gBrowser.removeFromMultiSelectedTabs(visibleTabs[lastFocusedTabIndex]);
+            } else {
+              gBrowser.addToMultiSelectedTabs(visibleTabs[lastFocusedTabIndex], false);
+            }
             break;
           default:
             // Consume the keydown event for the above keyboard
             // shortcuts only.
             return;
         }
+
+        if (arrowKeysShouldWrap) {
+          if (focusedTabIndex >= visibleTabs.length) {
+            focusedTabIndex = 0;
+          } else if (focusedTabIndex < 0) {
+            focusedTabIndex = visibleTabs.length - 1;
+          }
+        } else {
+          focusedTabIndex = Math.min(visibleTabs.length - 1, Math.max(0, focusedTabIndex));
+        }
+
+        if (keyComboForFocus &&
+            focusedTabIndex != lastFocusedTabIndex) {
+          this.ariaFocusedItem = visibleTabs[focusedTabIndex];
+        }
+
         event.preventDefault();
       ]]></handler>
 
       <handler event="dragstart"><![CDATA[
         var tab = this._getDragTargetTab(event, false);
         if (!tab || this._isCustomizing)
           return;
 
--- a/browser/base/content/test/general/browser_bug563588.js
+++ b/browser/base/content/test/general/browser_bug563588.js
@@ -1,17 +1,17 @@
 function press(key, expectedPos) {
   var originalSelectedTab = gBrowser.selectedTab;
-  EventUtils.synthesizeKey("VK_" + key.toUpperCase(), { accelKey: true });
+  EventUtils.synthesizeKey("VK_" + key.toUpperCase(), { accelKey: true, shiftKey: true });
   is(gBrowser.selectedTab, originalSelectedTab,
-     "accel+" + key + " doesn't change which tab is selected");
+     "shift+accel+" + key + " doesn't change which tab is selected");
   is(gBrowser.tabContainer.selectedIndex, expectedPos,
-     "accel+" + key + " moves the tab to the expected position");
+     "shift+accel+" + key + " moves the tab to the expected position");
   is(document.activeElement, gBrowser.selectedTab,
-     "accel+" + key + " leaves the selected tab focused");
+     "shift+accel+" + key + " leaves the selected tab focused");
 }
 
 function test() {
   BrowserTestUtils.addTab(gBrowser);
   BrowserTestUtils.addTab(gBrowser);
   is(gBrowser.tabs.length, 3, "got three tabs");
   is(gBrowser.tabs[0], gBrowser.selectedTab, "first tab is selected");
 
--- a/browser/base/content/test/tabs/browser.ini
+++ b/browser/base/content/test/tabs/browser.ini
@@ -33,16 +33,18 @@ support-files =
 [browser_multiselect_tabs_move_to_another_window_drag.js]
 [browser_multiselect_tabs_move_to_new_window_contextmenu.js]
 [browser_multiselect_tabs_mute_unmute.js]
 [browser_multiselect_tabs_pin_unpin.js]
 [browser_multiselect_tabs_positional_attrs.js]
 [browser_multiselect_tabs_reload.js]
 [browser_multiselect_tabs_reorder.js]
 [browser_multiselect_tabs_using_Ctrl.js]
+[browser_multiselect_tabs_using_keyboard.js]
+skip-if = os == 'mac' # Skipped because macOS keyboard support requires changing system settings
 [browser_multiselect_tabs_using_selectedTabs.js]
 [browser_multiselect_tabs_using_Shift_and_Ctrl.js]
 [browser_multiselect_tabs_using_Shift.js]
 [browser_navigatePinnedTab.js]
 [browser_new_file_whitelisted_http_tab.js]
 skip-if = !e10s # Test only relevant for e10s.
 [browser_new_tab_insert_position.js]
 skip-if = (debug && os == 'linux' && bits == 32) #Bug 1455882, disabled on Linux32 for almost permafailing
new file mode 100644
--- /dev/null
+++ b/browser/base/content/test/tabs/browser_multiselect_tabs_using_keyboard.js
@@ -0,0 +1,101 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const PREF_MULTISELECT_TABS = "browser.tabs.multiselect";
+
+function synthesizeKeyAndWaitForFocus(element, keyCode, options) {
+  let focused = BrowserTestUtils.waitForEvent(element, "focus");
+  EventUtils.synthesizeKey(keyCode, options);
+  return focused;
+}
+
+function synthesizeKeyAndWaitForTabToGetKeyboardFocus(tab, keyCode, options) {
+  let focused = TestUtils.waitForCondition(() => {
+    return tab.classList.contains("keyboard-focused-tab");
+  }, "Waiting for tab to get keyboard focus");
+  EventUtils.synthesizeKey(keyCode, options);
+  return focused;
+}
+
+add_task(async function setup() {
+  await SpecialPowers.pushPrefEnv({
+    set: [[PREF_MULTISELECT_TABS, true]],
+  });
+
+  let prevActiveElement = document.activeElement;
+  registerCleanupFunction(() => {
+    prevActiveElement.focus();
+  });
+});
+
+add_task(async function changeSelectionUsingKeyboard() {
+  await SpecialPowers.pushPrefEnv({
+    set: [[PREF_MULTISELECT_TABS, true]],
+  });
+
+  const tab1 = await addTab("http://mochi.test:8888/1");
+  const tab2 = await addTab("http://mochi.test:8888/2");
+  const tab3 = await addTab("http://mochi.test:8888/3");
+  const tab4 = await addTab("http://mochi.test:8888/4");
+  const tab5 = await addTab("http://mochi.test:8888/5");
+
+  await BrowserTestUtils.switchTab(gBrowser, tab3);
+  info("Move focus to location bar using the keyboard");
+  await synthesizeKeyAndWaitForFocus(gURLBar, "l", {accelKey: true});
+  ok(document.activeElement, "urlbar should be focused");
+
+  info("Move focus to the selected tab using the keyboard");
+  let identityBox = document.querySelector("#identity-box");
+  await synthesizeKeyAndWaitForFocus(identityBox, "VK_TAB", {shiftKey: true});
+  await synthesizeKeyAndWaitForFocus(tab3, "VK_TAB", {shiftKey: true});
+  is(document.activeElement, tab3, "Tab3 should be focused");
+
+  info("Move focus to tab 1 using the keyboard");
+  await synthesizeKeyAndWaitForTabToGetKeyboardFocus(tab2, "KEY_ArrowLeft", {accelKey: true});
+  await synthesizeKeyAndWaitForTabToGetKeyboardFocus(tab1, "KEY_ArrowLeft", {accelKey: true});
+  is(gBrowser.tabContainer.ariaFocusedItem, tab1, "Tab1 should be the ariaFocusedItem");
+
+  ok(!tab1.multiselected, "Tab1 shouldn't be multiselected");
+  info("Select tab1 using keyboard");
+  EventUtils.synthesizeKey("VK_SPACE", { accelKey: true });
+  ok(tab1.multiselected, "Tab1 should be multiselected");
+
+  info("Move focus to tab 5 using the keyboard");
+  await synthesizeKeyAndWaitForTabToGetKeyboardFocus(tab2, "KEY_ArrowRight", { accelKey: true });
+  await synthesizeKeyAndWaitForTabToGetKeyboardFocus(tab3, "KEY_ArrowRight", { accelKey: true });
+  await synthesizeKeyAndWaitForTabToGetKeyboardFocus(tab4, "KEY_ArrowRight", { accelKey: true });
+  await synthesizeKeyAndWaitForTabToGetKeyboardFocus(tab5, "KEY_ArrowRight", { accelKey: true });
+
+  ok(!tab5.multiselected, "Tab5 shouldn't be multiselected");
+  info("Select tab5 using keyboard");
+  EventUtils.synthesizeKey("VK_SPACE", { accelKey: true });
+  ok(tab5.multiselected, "Tab5 should be multiselected");
+
+  ok(tab1.multiselected && gBrowser._multiSelectedTabsSet.has(tab1), "Tab1 is (multi) selected");
+  ok(tab3.multiselected && gBrowser._multiSelectedTabsSet.has(tab3), "Tab3 is (multi) selected");
+  ok(tab5.multiselected && gBrowser._multiSelectedTabsSet.has(tab5), "Tab5 is (multi) selected");
+  is(gBrowser.multiSelectedTabsCount, 3, "Three tabs (multi) selected");
+  is(tab3, gBrowser.selectedTab, "Tab3 is still the selected tab");
+
+  await synthesizeKeyAndWaitForFocus(tab4, "KEY_ArrowLeft", {});
+  is(tab4, gBrowser.selectedTab, "Tab4 is now selected tab since tab5 had keyboard focus");
+
+  is(tab4.previousElementSibling, tab3, "tab4 should be after tab3");
+  is(tab4.nextElementSibling, tab5, "tab4 should be before tab5");
+
+  let tabsReordered = BrowserTestUtils.waitForCondition(() => {
+    return tab4.previousElementSibling == tab2 &&
+           tab4.nextElementSibling == tab3;
+  }, "tab4 should now be after tab2 and before tab3");
+  EventUtils.synthesizeKey("KEY_ArrowLeft", {accelKey: true, shiftKey: true});
+  await tabsReordered;
+
+  is(tab4.previousElementSibling, tab2, "tab4 should be after tab2");
+  is(tab4.nextElementSibling, tab3, "tab4 should be before tab3");
+
+  BrowserTestUtils.removeTab(tab1);
+  BrowserTestUtils.removeTab(tab2);
+  BrowserTestUtils.removeTab(tab3);
+  BrowserTestUtils.removeTab(tab4);
+  BrowserTestUtils.removeTab(tab5);
+});
--- a/browser/themes/linux/browser.css
+++ b/browser/themes/linux/browser.css
@@ -492,17 +492,18 @@ notification[value="translation"] menuli
 
 @media (-moz-menubar-drag) {
   #toolbar-menubar:not([autohide="true"]),
   #TabsToolbar {
     -moz-binding: url("chrome://browser/content/customizableui/toolbar.xml#toolbar-drag");
   }
 }
 
-.tabbrowser-tab:focus > .tab-stack > .tab-content {
+.keyboard-focused-tab > .tab-stack > .tab-content,
+.tabbrowser-tab:focus:not([aria-activedescendant]) > .tab-stack > .tab-content {
   outline: 1px dotted;
   outline-offset: -6px;
 }
 
 #context_reloadTab {
   list-style-image: url("moz-icon://stock/gtk-refresh?size=menu");
 }
 
--- a/browser/themes/osx/browser.css
+++ b/browser/themes/osx/browser.css
@@ -656,19 +656,19 @@ html|input.urlbar-input {
   color: hsl(240, 5%, 5%);
 }
 
 .tabbrowser-tab[visuallyselected=true] {
   /* overriding tabbox.css */
   text-shadow: inherit;
 }
 
-.tabbrowser-tab:focus > .tab-stack > .tab-content > .tab-label-container:not([pinned]),
-.tabbrowser-tab:focus > .tab-stack > .tab-content > .tab-icon-image[pinned],
-.tabbrowser-tab:focus > .tab-stack > .tab-content > .tab-throbber[pinned] {
+:-moz-any(.keyboard-focused-tab, .tabbrowser-tab:focus:not([aria-activedescendant])) > .tab-stack > .tab-content > .tab-label-container:not([pinned]),
+:-moz-any(.keyboard-focused-tab, .tabbrowser-tab:focus:not([aria-activedescendant])) > .tab-stack > .tab-content > .tab-icon-image[pinned],
+:-moz-any(.keyboard-focused-tab, .tabbrowser-tab:focus:not([aria-activedescendant])) > .tab-stack > .tab-content > .tab-throbber[pinned] {
   box-shadow: var(--focus-ring-box-shadow);
 }
 
 #TabsToolbar {
   -moz-appearance: none;
   padding-top: var(--space-above-tabbar);
 }
 
--- a/browser/themes/windows/browser.css
+++ b/browser/themes/windows/browser.css
@@ -685,17 +685,18 @@ html|*.urlbar-input:-moz-lwtheme::placeh
   /* For high contrast themes. */
   #tabbrowser-tabpanels,
   :root[privatebrowsingmode=temporary] #tabbrowser-tabpanels {
     background-color: -moz-default-background-color;
   }
 }
 
 /* tabbrowser-tab focus ring */
-.tabbrowser-tab:focus > .tab-stack > .tab-content {
+.keyboard-focused-tab > .tab-stack > .tab-content,
+.tabbrowser-tab:focus:not([aria-activedescendant]) > .tab-stack > .tab-content {
   outline: 1px dotted;
   outline-offset: -6px;
 }
 
 /* Tab DnD indicator */
 .tab-drop-indicator {
   list-style-image: url(chrome://browser/skin/tabbrowser/tabDragIndicator.png);
   margin-bottom: -9px;
--- a/toolkit/content/widgets/tabbox.xml
+++ b/toolkit/content/widgets/tabbox.xml
@@ -194,16 +194,63 @@
             // The selectedIndex setter ignores invalid values
             // such as -1 if |val| isn't one of our child nodes.
             this.selectedIndex = this.getIndexOfItem(val);
           return val;
         ]]>
         </setter>
       </property>
 
+      <field name="ACTIVE_DESCENDANT_ID" readonly="true"><![CDATA[
+        "keyboard-focused-tab-" + Math.trunc(Math.random() * 1000000);
+      ]]></field>
+
+      <property name="ariaFocusedIndex" readonly="true">
+        <getter>
+        <![CDATA[
+          const tabs = this.children;
+          for (var i = 0; i < tabs.length; i++) {
+            if (tabs[i].id == this.ACTIVE_DESCENDANT_ID)
+              return i;
+          }
+          return -1;
+        ]]>
+        </getter>
+      </property>
+
+      <property name="ariaFocusedItem">
+        <getter>
+        <![CDATA[
+          return document.getElementById(this.ACTIVE_DESCENDANT_ID);
+        ]]>
+        </getter>
+
+        <setter>
+        <![CDATA[
+          let setNewItem = val && this.getIndexOfItem(val) != -1;
+          let clearExistingItem = this.ariaFocusedItem && (!val || setNewItem);
+          if (clearExistingItem) {
+            let ariaFocusedItem = this.ariaFocusedItem;
+            ariaFocusedItem.classList.remove("keyboard-focused-tab");
+            ariaFocusedItem.id = "";
+            this.selectedItem.removeAttribute("aria-activedescendant");
+          }
+
+          if (setNewItem) {
+            this.ariaFocusedItem = null;
+            val.id = this.ACTIVE_DESCENDANT_ID;
+            val.classList.add("keyboard-focused-tab");
+            this.selectedItem.setAttribute("aria-activedescendant", this.ACTIVE_DESCENDANT_ID);
+          }
+
+          return val;
+        ]]>
+        </setter>
+      </property>
+
       <method name="getIndexOfItem">
         <parameter name="item"/>
         <body>
         <![CDATA[
           return Array.indexOf(this.children, item);
         ]]>
         </body>
       </method>
@@ -218,16 +265,18 @@
       </method>
 
       <method name="_selectNewTab">
         <parameter name="aNewTab"/>
         <parameter name="aFallbackDir"/>
         <parameter name="aWrap"/>
         <body>
         <![CDATA[
+          this.ariaFocusedItem = null;
+
           var requestedTab = aNewTab;
           while (aNewTab.hidden || aNewTab.disabled || !this._canAdvanceToTab(aNewTab)) {
             aNewTab = aFallbackDir == -1 ? aNewTab.previousElementSibling : aNewTab.nextElementSibling;
             if (!aNewTab && aWrap)
               aNewTab = aFallbackDir == -1 ? this.children[this.children.length - 1] :
                                              this.children[0];
             if (!aNewTab || aNewTab == requestedTab)
               return;
@@ -272,17 +321,17 @@
         </body>
       </method>
 
       <method name="advanceSelectedTab">
         <parameter name="aDir"/>
         <parameter name="aWrap"/>
         <body>
         <![CDATA[
-          var startTab = this.selectedItem;
+          var startTab = this.ariaFocusedItem || this.selectedItem;
           var next = startTab[(aDir == -1 ? "previous" : "next") + "ElementSibling"];
           if (!next && aWrap) {
             next = aDir == -1 ? this.children[this.children.length - 1] :
                                 this.children[0];
           }
           if (next && next != startTab) {
             this._selectNewTab(next, aDir, aWrap);
           }
@@ -503,16 +552,18 @@
     </implementation>
 
     <handlers>
       <handler event="mousedown" button="0">
       <![CDATA[
         if (this.disabled)
           return;
 
+        this.parentNode.ariaFocusedItem = null;
+
         if (this != this.parentNode.selectedItem) { // Not selected yet
           let stopwatchid = this.parentNode.getAttribute("stopwatchid");
           if (stopwatchid) {
             this.TelemetryStopwatch.start(stopwatchid);
           }
 
           // Call this before setting the 'ignorefocus' attribute because this
           // will pass on focus if the formerly selected tab was focused as well.