Fix the keyboard accessibility of the manage button in the thumbnails view (bug 2015916)

Author: calixteman

test/integration/thumbnail_view_spec.mjs

@@ -12,6 +12,21 @@ function waitForThumbnailVisible(page, pageNum) {
   );
 }
 
+async function waitForMenu(page, buttonSelector, visible = true) {
+  return page.waitForFunction(
+    (selector, vis) => {
+      const button = document.querySelector(selector);
+      if (!button) {
+        return false;
+      }
+      return button.getAttribute("aria-expanded") === (vis ? "true" : "false");
+    },
+    {},
+    buttonSelector,
+    visible
+  );
+}
+
 describe("PDF Thumbnail View", () => {
   describe("Works without errors", () => {
     let pages;
@@ -201,4 +216,105 @@ describe("PDF Thumbnail View", () => {
       );
     });
   });
+
+  describe("The manage dropdown menu", () => {
+    let pages;
+
+    beforeEach(async () => {
+      pages = await loadAndWait(
+        "tracemonkey.pdf",
+        "#viewsManagerToggleButton",
+        null,
+        null,
+        { enableSplitMerge: true }
+      );
+    });
+
+    afterEach(async () => {
+      await closePages(pages);
+    });
+
+    async function enableMenuItems(page) {
+      await page.evaluate(() => {
+        document
+          .querySelectorAll("#viewsManagerStatusActionOptions button")
+          .forEach(button => {
+            button.disabled = false;
+          });
+      });
+    }
+
+    it("should open with Enter key and remain open", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#viewsManagerToggleButton");
+          await waitForThumbnailVisible(page, 1);
+
+          await enableMenuItems(page);
+
+          // Focus the manage button
+          await kbFocusNext(page);
+          await kbFocusNext(page);
+          await page.waitForSelector("#viewsManagerStatusActionButton:focus", {
+            visible: true,
+          });
+
+          // Press Enter to open the menu
+          await page.keyboard.press("Enter");
+
+          await waitForMenu(page, "#viewsManagerStatusActionButton");
+
+          // Verify first menu item can be focused
+          await page.waitForSelector("#viewsManagerStatusActionCopy:focus", {
+            visible: true,
+          });
+
+          // Close menu with Escape
+          await page.keyboard.press("Escape");
+          await waitForMenu(page, "#viewsManagerStatusActionButton", false);
+        })
+      );
+    });
+
+    it("should open with Space key and remain open", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#viewsManagerToggleButton");
+          await waitForThumbnailVisible(page, 1);
+
+          await enableMenuItems(page);
+
+          // Focus the manage button
+          await kbFocusNext(page);
+          await kbFocusNext(page);
+          await page.waitForSelector("#viewsManagerStatusActionButton:focus", {
+            visible: true,
+          });
+
+          // Press Space to open the menu
+          await page.keyboard.press(" ");
+
+          await waitForMenu(page, "#viewsManagerStatusActionButton");
+
+          // Verify first menu item can be focused
+          await page.waitForSelector("#viewsManagerStatusActionCopy:focus", {
+            visible: true,
+          });
+
+          // Navigate menu items with arrow keys
+          await page.keyboard.press("ArrowDown");
+          await page.waitForSelector("#viewsManagerStatusActionCut:focus", {
+            visible: true,
+          });
+
+          // Menu should still be open
+          await waitForMenu(page, "#viewsManagerStatusActionButton");
+
+          // Close menu with Escape
+          await page.keyboard.press("Escape");
+          await waitForMenu(page, "#viewsManagerStatusActionButton", false);
+        })
+      );
+    });
+  });
 });

web/menu.js

@@ -70,6 +70,36 @@ class Menu {
     this.#lastIndex = -1;
   }
 
+  /**
+   * Open the menu.
+   */
+  #openMenu() {
+    if (this.#openMenuAC) {
+      return;
+    }
+
+    const menu = this.#menu;
+    this.#triggeringButton.ariaExpanded = "true";
+    this.#openMenuAC = new AbortController();
+    const signal = AbortSignal.any([
+      this.#menuAC.signal,
+      this.#openMenuAC.signal,
+    ]);
+    window.addEventListener(
+      "pointerdown",
+      ({ target }) => {
+        if (
+          !this.#triggeringButton.contains(target) &&
+          !menu.contains(target)
+        ) {
+          this.#closeMenu();
+        }
+      },
+      { signal }
+    );
+    window.addEventListener("blur", this.#closeMenu.bind(this), { signal });
+  }
+
   /**
    * Set up the menu.
    */
@@ -80,23 +110,7 @@ class Menu {
         return;
       }
 
-      const menu = this.#menu;
-      this.#triggeringButton.ariaExpanded = "true";
-      this.#openMenuAC = new AbortController();
-      const signal = AbortSignal.any([
-        this.#menuAC.signal,
-        this.#openMenuAC.signal,
-      ]);
-      window.addEventListener(
-        "pointerdown",
-        ({ target }) => {
-          if (target !== this.#triggeringButton && !menu.contains(target)) {
-            this.#closeMenu();
-          }
-        },
-        { signal }
-      );
-      window.addEventListener("blur", this.#closeMenu.bind(this), { signal });
+      this.#openMenu();
     });
 
     const { signal } = this.#menuAC;
@@ -110,12 +124,10 @@ class Menu {
             stopEvent(e);
             break;
           case "ArrowDown":
-          case "Tab":
             this.#goToNextItem(e.target, true);
             stopEvent(e);
             break;
           case "ArrowUp":
-          case "ShiftTab":
             this.#goToNextItem(e.target, false);
             stopEvent(e);
             break;
@@ -124,15 +136,15 @@ class Menu {
               .find(
                 item => !item.disabled && !item.classList.contains("hidden")
               )
-              .focus();
+              ?.focus();
             stopEvent(e);
             break;
           case "End":
             this.#menuItems
               .findLast(
                 item => !item.disabled && !item.classList.contains("hidden")
               )
-              .focus();
+              ?.focus();
             stopEvent(e);
             break;
           default:
@@ -159,27 +171,27 @@ class Menu {
           case "Enter":
           case "ArrowDown":
           case "Home":
+            stopEvent(e);
             if (!this.#openMenuAC) {
-              this.#triggeringButton.click();
+              this.#openMenu();
             }
             this.#menuItems
               .find(
                 item => !item.disabled && !item.classList.contains("hidden")
               )
-              .focus();
-            stopEvent(e);
+              ?.focus();
             break;
           case "ArrowUp":
           case "End":
+            stopEvent(e);
             if (!this.#openMenuAC) {
-              this.#triggeringButton.click();
+              this.#openMenu();
             }
             this.#menuItems
               .findLast(
                 item => !item.disabled && !item.classList.contains("hidden")
               )
-              .focus();
-            stopEvent(e);
+              ?.focus();
             break;
           case "Escape":
             this.#closeMenu();

web/viewer.html

@@ -207,22 +207,22 @@
                           </button>
                           <menu id="viewsManagerStatusActionOptions" class="popupMenu">
                             <li>
-                              <button id="viewsManagerStatusActionCopy" class="noIcon" role="menuitem" type="button" tabindex="0" disabled>
+                              <button id="viewsManagerStatusActionCopy" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
                                 <span data-l10n-id="pdfjs-views-manager-pages-status-copy-button-label"></span>
                               </button>
                             </li>
                             <li>
-                              <button id="viewsManagerStatusActionCut" class="noIcon" role="menuitem" type="button" tabindex="0" disabled>
+                              <button id="viewsManagerStatusActionCut" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
                                 <span data-l10n-id="pdfjs-views-manager-pages-status-cut-button-label"></span>
                               </button>
                             </li>
                             <li>
-                              <button id="viewsManagerStatusActionDelete" class="noIcon" role="menuitem" type="button" tabindex="0" disabled>
+                              <button id="viewsManagerStatusActionDelete" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
                                 <span data-l10n-id="pdfjs-views-manager-pages-status-delete-button-label"></span>
                               </button>
                             </li>
                             <li>
-                              <button id="viewsManagerStatusActionSaveAs" class="noIcon" role="menuitem" type="button" tabindex="0" disabled>
+                              <button id="viewsManagerStatusActionSaveAs" class="noIcon" role="menuitem" type="button" tabindex="-1" disabled>
                                 <span data-l10n-id="pdfjs-views-manager-pages-status-save-as-button-label"></span>
                               </button>
                             </li>