Bug 1366207 - remember previous view's selection when keyboard navigating panels, r?mikedeboer draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Tue, 15 Aug 2017 15:21:15 +0100
changeset 648148 223436f3e768ca171db782f2390486f294de894a
parent 648046 932388b8c22c9775264e543697ce918415db9e23
child 726724 1a8598c0ccc2ded37cebd19e56bc3103529d8e06
push id74644
push usergijskruitbosch@gmail.com
push dateThu, 17 Aug 2017 09:28:20 +0000
reviewersmikedeboer
bugs1366207
milestone57.0a1
Bug 1366207 - remember previous view's selection when keyboard navigating panels, r?mikedeboer MozReview-Commit-ID: 14U7hkrBbJq
browser/components/customizableui/PanelMultiView.jsm
browser/components/customizableui/test/browser_panel_keyboard_navigation.js
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -610,17 +610,19 @@ this.PanelMultiView = class {
               // transition-end, because that is guaranteed to happen.
               if (ev.target != nodeToAnimate || ev.propertyName != "transform")
                 return;
 
               this._viewContainer.removeEventListener("transitionend", this._transitionEndListener);
               this._transitionEndListener = null;
               onTransitionEnd();
               this._transitioning = false;
-              this._resetKeyNavigation(previousViewNode);
+              if (reverse) {
+                this._resetKeyNavigation(previousViewNode);
+              }
 
               // Myeah, panel layout auto-resizing is a funky thing. We'll wait
               // another few milliseconds to remove the width and height 'fixtures',
               // to be sure we don't flicker annoyingly.
               // NB: HACK! Bug 1363756 is there to fix this.
               this._autoResizeWorkaroundTimer = window.setTimeout(() => {
                 this._viewContainer.style.removeProperty("height");
                 this._viewContainer.style.removeProperty("width");
@@ -639,16 +641,17 @@ this.PanelMultiView = class {
                 if (!reverse)
                   viewNode.style.removeProperty("margin-inline-start");
                 if (aAnchor)
                   aAnchor.removeAttribute("open");
 
                 this._viewContainer.removeAttribute("transition-reverse");
 
                 this._dispatchViewEvent(viewNode, "ViewShown");
+                this._updateKeyboardFocus(viewNode);
               }, { once: true });
             });
           }, { once: true });
         });
       } else if (!this.panelViews) {
         this._transitionHeight(() => {
           viewNode.setAttribute("current", true);
           this.node.setAttribute("viewtype", "subview");
@@ -982,16 +985,72 @@ this.PanelMultiView = class {
           this._mainView.removeAttribute("exceeding");
         }
         this._dispatchViewEvent(this.node, "PanelMultiViewHidden");
         break;
     }
   }
 
   /**
+   * Based on going up or down, select the previous or next focusable button
+   * in the current view.
+   *
+   * @param {Object}  navMap   the navigation keyboard map object for the view
+   * @param {Array}   buttons  an array of focusable buttons to select an item from.
+   * @param {Boolean} isDown   whether we're going down (true) or up (false) in this view.
+   *
+   * @return {DOMNode} the button we selected.
+   */
+  _updateSelectedKeyNav(navMap, buttons, isDown) {
+    let lastSelected = navMap.selected && navMap.selected.get();
+    let newButton = null;
+    let maxIdx = buttons.length - 1;
+    if (lastSelected) {
+      let buttonIndex = buttons.indexOf(lastSelected);
+      if (buttonIndex != -1) {
+        // Buttons may get selected whilst the panel is shown, so add an extra
+        // check here.
+        do {
+          buttonIndex = buttonIndex + (isDown ? 1 : -1);
+        } while (buttons[buttonIndex] && buttons[buttonIndex].disabled)
+        if (isDown && buttonIndex > maxIdx)
+          buttonIndex = 0;
+        else if (!isDown && buttonIndex < 0)
+          buttonIndex = maxIdx;
+        newButton = buttons[buttonIndex];
+      } else {
+        // The previously selected item is no longer selectable. Find the next item:
+        let allButtons = lastSelected.closest("panelview").getElementsByTagName("toolbarbutton");
+        let maxAllButtonIdx = allButtons.length - 1;
+        let allButtonIndex = allButtons.indexOf(lastSelected);
+        while (allButtonIndex >= 0 && allButtonIndex <= maxAllButtonIdx) {
+          allButtonIndex++;
+          // Check if the next button is in the list of focusable buttons.
+          buttonIndex = buttons.indexOf(allButtons[allButtonIndex]);
+          if (buttonIndex != -1) {
+            // If it is, just use that button if we were going down, or the previous one
+            // otherwise. If this was the first button, newButton will end up undefined,
+            // which is fine because we'll fall back to using the last button at the
+            // bottom of this method.
+            newButton = buttons[isDown ? buttonIndex : buttonIndex - 1];
+            break;
+          }
+        }
+      }
+    }
+
+    // If we couldn't find something, select the first or last item:
+    if (!newButton) {
+      newButton = buttons[isDown ? 0 : maxIdx];
+    }
+    navMap.selected = Cu.getWeakReference(newButton);
+    return newButton;
+  }
+
+  /**
    * Allow for navigating subview buttons using the arrow keys and the Enter key.
    * The Up and Down keys can be used to navigate the list up and down and the
    * Enter, Right or Left - depending on the text direction - key can be used to
    * simulate a click on the currently selected button.
    * The Right or Left key - depending on the text direction - can be used to
    * navigate to the previous view, functioning as a shortcut for the view's
    * back button.
    * Thus, in LTR mode:
@@ -1031,53 +1090,41 @@ this.PanelMultiView = class {
     };
 
     let keyCode = event.code;
     switch (keyCode) {
       case "ArrowDown":
       case "ArrowUp": {
         stop();
         let isDown = (keyCode == "ArrowDown");
-        let maxIdx = buttons.length - 1;
-        let buttonIndex = isDown ? 0 : maxIdx;
-        if (typeof navMap.selected == "number") {
-          // Buttons may get selected whilst the panel is shown, so add an extra
-          // check here.
-          do {
-            buttonIndex = navMap.selected = (navMap.selected + (isDown ? 1 : -1));
-          } while (buttons[buttonIndex] && buttons[buttonIndex].disabled)
-          if (isDown && buttonIndex > maxIdx)
-            buttonIndex = 0;
-          else if (!isDown && buttonIndex < 0)
-            buttonIndex = maxIdx;
-        }
-        let button = buttons[buttonIndex];
+        let button = this._updateSelectedKeyNav(navMap, buttons, isDown);
         button.focus();
-        navMap.selected = buttonIndex;
         break;
       }
       case "ArrowLeft":
       case "ArrowRight": {
         stop();
         let dir = this._dir;
         if ((dir == "ltr" && keyCode == "ArrowLeft") ||
             (dir == "rtl" && keyCode == "ArrowRight")) {
           if (this._canGoBack(view))
             this.goBack(view.backButton);
           break;
         }
         // If the current button is _not_ one that points to a subview, pressing
         // the arrow key shouldn't do anything.
-        if (!navMap.selected || !buttons[navMap.selected].classList.contains("subviewbutton-nav"))
+        if (!navMap.selected || !navMap.selected.get() ||
+            !navMap.selected.get().classList.contains("subviewbutton-nav")) {
           break;
+        }
         // Fall-through...
       }
       case "Space":
       case "Enter": {
-        let button = buttons[navMap.selected];
+        let button = navMap.selected && navMap.selected.get();
         if (!button)
           break;
         stop();
 
         // Unfortunately, 'tabindex' doesn't execute the default action, so
         // we explicitly do this here.
         // We are sending a command event and then a click event.
         // This is done in order to mimic a "real" mouse click event.
@@ -1089,31 +1136,36 @@ this.PanelMultiView = class {
       }
     }
   }
 
   /**
    * Clear all traces of keyboard navigation happening right now.
    *
    * @param {panelview} view View to reset the key navigation attributes of.
-   *                         Defaults to `this._currentSubView`.
+   *                         If no view is passed, all navigation attributes for
+   *                         this panelmultiview are cleared.
    */
-  _resetKeyNavigation(view = this._currentSubView) {
-    let navMap = this._keyNavigationMap.get(view);
-    this._keyNavigationMap.clear();
-    if (!navMap)
-      return;
+  _resetKeyNavigation(view) {
+    let viewToBlur = view || this._currentSubView;
+    let navMap = this._keyNavigationMap.get(viewToBlur);
+    if (navMap && navMap.selected && navMap.selected.get()) {
+      navMap.selected.get().blur();
+    }
 
-    let buttons = this._getNavigableElements(view);
-    if (!buttons.length)
-      return;
-
-    let button = buttons[navMap.selected];
-    if (button)
-      button.blur();
+    // We clear the entire key navigation map ONLY if *no* view was passed in.
+    // This happens e.g. when the popup is hidden completely, or the user moves
+    // their mouse.
+    // If a view is passed in, we just delete the map for that view. This happens
+    // when going back from a view (which resets the map for that view only)
+    if (view) {
+      this._keyNavigationMap.delete(view);
+    } else {
+      this._keyNavigationMap.clear();
+    }
   }
 
   /**
    * Retrieve the button elements from a view node that can be used for navigation
    * using the keyboard; enabled buttons and the back button, if visible.
    *
    * @param  {nsIDOMNode} view
    * @return {Array}
@@ -1125,16 +1177,28 @@ this.PanelMultiView = class {
     let dwu = this._dwu;
     return buttons.filter(button => {
       let bounds = dwu.getBoundsWithoutFlushing(button);
       return bounds.width > 0 && bounds.height > 0;
     });
   }
 
   /**
+   * Focus the last selected element in the view, if any.
+   *
+   * @param {panelview} view the view in which to update keyboard focus.
+   */
+  _updateKeyboardFocus(view) {
+    let navMap = this._keyNavigationMap.get(view);
+    if (navMap && navMap.selected && navMap.selected.get()) {
+      navMap.selected.get().focus();
+    }
+  }
+
+  /**
    * If the main view or a subview contains wrapping elements, the attribute
    * "descriptionheightworkaround" should be set on the view to force all the
    * wrapping "description", "label" or "toolbarbutton" elements to a fixed
    * height. If the attribute is set and the visibility, contents, or width
    * of any of these elements changes, this function should be called to
    * refresh the calculated heights.
    *
    * This may trigger a synchronous layout.
--- a/browser/components/customizableui/test/browser_panel_keyboard_navigation.js
+++ b/browser/components/customizableui/test/browser_panel_keyboard_navigation.js
@@ -128,12 +128,17 @@ add_task(async function testLeftRightKey
   EventUtils.synthesizeKey("KEY_ArrowRight", { code: "ArrowRight" });
   await promise;
 
   // Hitting ArrowLeft should navigate us back.
   promise = BrowserTestUtils.waitForEvent(PanelUI.mainView, "ViewShown");
   EventUtils.synthesizeKey("KEY_ArrowLeft", { code: "ArrowLeft" });
   await promise;
 
+  focusedElement = document.commandDispatcher.focusedElement;
+  Assert.equal(focusedElement.id, kHelpButtonId,
+               "Help button should be focused again now that we're back in the main view");
+
   promise = promisePanelHidden(window);
   PanelUI.hide();
   await promise;
 });
+