Bug 1454865: PanelMultiView: When entering a subview using the keyboard, focus the first button after the Back button in the subview. r=Gijs
authorJames Teh <jteh@mozilla.com>
Mon, 15 Apr 2019 01:38:15 +0000
changeset 469466 2e5c5542353aa0d9f3c84523a687f3cddbc31c97
parent 469465 8329ed3a30b94e4467806fd5b9f66782cb1d9132
child 469467 91fe817e863159b414b9e37c363899b6872af691
push id112792
push userncsoregi@mozilla.com
push dateMon, 15 Apr 2019 09:49:11 +0000
treeherdermozilla-inbound@a57f27d3ccd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1454865
milestone68.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 1454865: PanelMultiView: When entering a subview using the keyboard, focus the first button after the Back button in the subview. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D26067
browser/components/customizableui/PanelMultiView.jsm
browser/components/customizableui/test/browser_PanelMultiView_focus.js
browser/components/customizableui/test/browser_panel_keyboard_navigation.js
--- a/browser/components/customizableui/PanelMultiView.jsm
+++ b/browser/components/customizableui/PanelMultiView.jsm
@@ -639,16 +639,20 @@ var PanelMultiView = class extends Assoc
 
     // Do not re-enter the process if navigation is already in progress. Since
     // there is only one active view at any given time, we can do this check
     // safely, even considering that during the navigation process the actual
     // view to which prevPanelView refers will change.
     if (!prevPanelView.active) {
       return;
     }
+    // If prevPanelView._doingKeyboardActivation is true, it will be reset to
+    // false synchronously. Therefore, we must capture it before we use any
+    // "await" statements.
+    let doingKeyboardActivation = prevPanelView._doingKeyboardActivation;
     // Marking the view that is about to scrolled out of the visible area as
     // inactive will prevent re-entrancy and also disable keyboard navigation.
     // From this point onwards, "await" statements can be used safely.
     prevPanelView.active = false;
 
     // Provide visual feedback while navigation is in progress, starting before
     // the transition starts and ending when the previous view is invisible.
     if (anchor) {
@@ -687,16 +691,17 @@ var PanelMultiView = class extends Assoc
 
       await this._transitionViews(prevPanelView.node, viewNode, false, anchor);
     } finally {
       if (anchor) {
         anchor.removeAttribute("open");
       }
     }
 
+    nextPanelView.focusWhenActive = doingKeyboardActivation;
     this._activateView(nextPanelView);
   }
 
   /**
    * Navigates backwards by sliding out the most recent subview.
    */
   goBack() {
     this._goBack().catch(Cu.reportError);
@@ -809,17 +814,17 @@ var PanelMultiView = class extends Assoc
   /**
    * Activates the specified view and raises the ViewShown event, unless the
    * view was closed in the meantime.
    */
   _activateView(panelView) {
     if (panelView.isOpenIn(this)) {
       panelView.active = true;
       if (panelView.focusWhenActive) {
-        panelView.focusFirstNavigableElement();
+        panelView.focusFirstNavigableElement(false, true);
         panelView.focusWhenActive = false;
       }
       panelView.dispatchCustomEvent("ViewShown");
 
       // Re-enable panel autopositioning.
       this._panel.autoPosition = true;
     }
   }
@@ -1476,23 +1481,29 @@ var PanelView = class extends Associated
     }
   }
 
   /**
    * Focuses and moves keyboard selection to the first navigable element.
    * This is a no-op if there are no navigable elements.
    *
    * @param {Boolean} homeKey   `true` if this is for the home key.
+   * @param {Boolean} skipBack   `true` if the Back button should be skipped.
    */
-  focusFirstNavigableElement(homeKey = false) {
+  focusFirstNavigableElement(homeKey = false, skipBack = false) {
     // The home key is conceptually similar to the up/down arrow keys.
     let walker = homeKey ?
       this._arrowNavigableWalker : this._tabNavigableWalker;
     walker.currentNode = walker.root;
     this.selectedElement = walker.firstChild();
+    if (skipBack && walker.currentNode
+        && walker.currentNode.classList.contains("subviewbutton-back")
+        && walker.nextNode()) {
+      this.selectedElement = walker.currentNode;
+    }
     this.focusSelectedElement();
   }
 
   /**
    * Focuses and moves keyboard selection to the last navigable element.
    * This is a no-op if there are no navigable elements.
    *
    * @param {Boolean} endKey   `true` if this is for the end key.
@@ -1637,24 +1648,26 @@ var PanelView = class extends Associated
         if (tabOnly()) {
           break;
         }
         let button = this.selectedElement;
         if (!button)
           break;
         stop();
 
+        this._doingKeyboardActivation = true;
         // 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.
         // The command event executes the action, then the click event closes the menu.
         button.doCommand();
         let clickEvent = new event.target.ownerGlobal.MouseEvent("click", {"bubbles": true});
         button.dispatchEvent(clickEvent);
+        this._doingKeyboardActivation = false;
         break;
       }
     }
   }
 
   /**
    * Focus the last selected element in the view, if any.
    */
--- a/browser/components/customizableui/test/browser_PanelMultiView_focus.js
+++ b/browser/components/customizableui/test/browser_PanelMultiView_focus.js
@@ -6,41 +6,55 @@
 /**
  * Test the focus behavior when opening PanelViews.
  */
 
 const {PanelMultiView} = ChromeUtils.import("resource:///modules/PanelMultiView.jsm");
 
 let gAnchor;
 let gPanel;
+let gPanelMultiView;
 let gMainView;
 let gMainButton;
+let gMainSubButton;
+let gSubView;
+let gSubButton;
 
 add_task(async function setup() {
   let navBar = document.getElementById("nav-bar");
   gAnchor = document.createXULElement("toolbarbutton");
   // Must be focusable in order for key presses to work.
   gAnchor.style["-moz-user-focus"] = "normal";
   navBar.appendChild(gAnchor);
-  gPanel = document.createXULElement("panel");
-  navBar.appendChild(gPanel);
-  let panelMultiView = document.createXULElement("panelmultiview");
-  panelMultiView.setAttribute("mainViewId", "testMainView");
-  gPanel.appendChild(panelMultiView);
-  gMainView = document.createXULElement("panelview");
-  gMainView.id = "testMainView";
-  panelMultiView.appendChild(gMainView);
-  gMainButton = document.createXULElement("button");
-  gMainView.appendChild(gMainButton);
-
   let onPress = event => PanelMultiView.openPopup(gPanel, gAnchor, {
     triggerEvent: event,
   });
   gAnchor.addEventListener("keypress", onPress);
   gAnchor.addEventListener("click", onPress);
+  gPanel = document.createXULElement("panel");
+  navBar.appendChild(gPanel);
+  gPanelMultiView = document.createXULElement("panelmultiview");
+  gPanelMultiView.setAttribute("mainViewId", "testMainView");
+  gPanel.appendChild(gPanelMultiView);
+
+  gMainView = document.createXULElement("panelview");
+  gMainView.id = "testMainView";
+  gPanelMultiView.appendChild(gMainView);
+  gMainButton = document.createXULElement("button");
+  gMainView.appendChild(gMainButton);
+  gMainSubButton = document.createXULElement("button");
+  gMainView.appendChild(gMainSubButton);
+  gMainSubButton.addEventListener("command", () =>
+      gPanelMultiView.showSubView("testSubView", gMainSubButton));
+
+  gSubView = document.createXULElement("panelview");
+  gSubView.id = "testSubView";
+  gPanelMultiView.appendChild(gSubView);
+  gSubButton = document.createXULElement("button");
+  gSubView.appendChild(gSubButton);
 
   registerCleanupFunction(() => {
     gAnchor.remove();
     gPanel.remove();
   });
 });
 
 // Activate the main view by pressing a key. Focus should be moved inside.
@@ -59,8 +73,61 @@ add_task(async function testMainViewByKe
 add_task(async function testMainViewByClick() {
   await gCUITestUtils.openPanelMultiView(gPanel, gMainView,
     () => gAnchor.click());
   Assert.notEqual(document.activeElement, gMainButton,
     "Focus not on button in main view");
   await gCUITestUtils.hidePanelMultiView(gPanel,
     () => PanelMultiView.hidePopup(gPanel));
 });
+
+// Activate the subview by pressing a key. Focus should be moved to the first
+// button after the Back button.
+add_task(async function testSubViewByKeypress() {
+  await gCUITestUtils.openPanelMultiView(gPanel, gMainView,
+    () => gAnchor.click());
+  while (document.activeElement != gMainSubButton) {
+    EventUtils.synthesizeKey("KEY_Tab", {shiftKey: true});
+  }
+  let shown = BrowserTestUtils.waitForEvent(gSubView, "ViewShown");
+  EventUtils.synthesizeKey(" ");
+  await shown;
+  Assert.equal(document.activeElement, gSubButton,
+    "Focus on first button after Back button in subview");
+  await gCUITestUtils.hidePanelMultiView(gPanel,
+    () => PanelMultiView.hidePopup(gPanel));
+});
+
+// Activate the subview by clicking the mouse. Focus should not be moved
+// inside.
+add_task(async function testSubViewByClick() {
+  await gCUITestUtils.openPanelMultiView(gPanel, gMainView,
+    () => gAnchor.click());
+  let shown = BrowserTestUtils.waitForEvent(gSubView, "ViewShown");
+  gMainSubButton.click();
+  await shown;
+  let backButton = gSubView.querySelector(".subviewbutton-back");
+  Assert.notEqual(document.activeElement, backButton,
+    "Focus not on Back button in subview");
+  Assert.notEqual(document.activeElement, gSubButton,
+    "Focus not on button after Back button in subview");
+  await gCUITestUtils.hidePanelMultiView(gPanel,
+    () => PanelMultiView.hidePopup(gPanel));
+});
+
+// Test that focus is restored when going back to a previous view.
+add_task(async function testBackRestoresFocus() {
+  await gCUITestUtils.openPanelMultiView(gPanel, gMainView,
+    () => gAnchor.click());
+  while (document.activeElement != gMainSubButton) {
+    EventUtils.synthesizeKey("KEY_Tab", {shiftKey: true});
+  }
+  let shown = BrowserTestUtils.waitForEvent(gSubView, "ViewShown");
+  EventUtils.synthesizeKey(" ");
+  await shown;
+  shown = BrowserTestUtils.waitForEvent(gMainView, "ViewShown");
+  EventUtils.synthesizeKey("KEY_ArrowLeft");
+  await shown;
+  Assert.equal(document.activeElement, gMainSubButton,
+    "Focus on sub button in main view");
+  await gCUITestUtils.hidePanelMultiView(gPanel,
+    () => PanelMultiView.hidePopup(gPanel));
+});
--- a/browser/components/customizableui/test/browser_panel_keyboard_navigation.js
+++ b/browser/components/customizableui/test/browser_panel_keyboard_navigation.js
@@ -86,23 +86,30 @@ add_task(async function testEnterKeyBeha
   EventUtils.synthesizeKey("KEY_Enter");
   await promise;
 
   let helpButtons = getEnabledNavigableElementsForView(PanelUI.helpView);
   Assert.ok(helpButtons[0].classList.contains("subviewbutton-back"),
     "First button in help view should be a back button");
 
   // For posterity, check navigating the subview using up/ down arrow keys as well.
+  // When opening a subview, the first control *after* the Back button gets
+  // focus.
+  EventUtils.synthesizeKey("KEY_ArrowUp");
+  focusedElement = document.commandDispatcher.focusedElement;
+  Assert.equal(focusedElement, helpButtons[0],
+    "The Back button should be focused after navigating upward");
   for (let i = helpButtons.length - 1; i >= 0; --i) {
     let button = helpButtons[i];
     if (button.disabled)
       continue;
     EventUtils.synthesizeKey("KEY_ArrowUp");
     focusedElement = document.commandDispatcher.focusedElement;
-    Assert.equal(focusedElement, button, "The first button should be focused after navigating upward");
+    Assert.equal(focusedElement, button,
+      "The previous button should be focused after navigating upward");
   }
 
   // Make sure the back button is in focus again.
   while (focusedElement != helpButtons[0]) {
     EventUtils.synthesizeKey("KEY_ArrowDown");
     focusedElement = document.commandDispatcher.focusedElement;
   }