Merge pull request #20677 from calixteman/bug2016007

calixteman · web-flow · commit 30ed527a8092 · 2026-02-18T19:56:09.000+01:00
Add the possibility to navigate with the keyboard to go from a checkbox to an other in the thumbnail view (bug 2016007)
diff --git a/test/integration/thumbnail_view_spec.mjs b/test/integration/thumbnail_view_spec.mjs
@@ -140,12 +140,6 @@ describe("PDF Thumbnail View", () => {
       await closePages(pages);
     });
 
-    async function isElementFocused(page, selector) {
-      await page.waitForSelector(selector, { visible: true });
-
-      return page.$eval(selector, el => el === document.activeElement);
-    }
-
     it("should navigate with the keyboard", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
@@ -156,57 +150,40 @@ describe("PDF Thumbnail View", () => {
           await waitForThumbnailVisible(page, 3);
 
           await kbFocusNext(page);
-          expect(await isElementFocused(page, "#viewsManagerSelectorButton"))
-            .withContext(`In ${browserName}`)
-            .toBe(true);
+          await page.waitForSelector("#viewsManagerSelectorButton:focus", {
+            visible: true,
+          });
 
           await kbFocusNext(page);
-          expect(
-            await isElementFocused(page, "#viewsManagerStatusActionButton")
-          )
-            .withContext(`In ${browserName}`)
-            .toBe(true);
+          await page.waitForSelector("#viewsManagerStatusActionButton:focus", {
+            visible: true,
+          });
 
           await kbFocusNext(page);
-          expect(
-            await isElementFocused(
-              page,
-              `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']`
-            )
-          )
-            .withContext(`In ${browserName}`)
-            .toBe(true);
+          await page.waitForSelector(
+            `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`,
+            { visible: true }
+          );
 
           await page.keyboard.press("ArrowDown");
-          expect(
-            await isElementFocused(
-              page,
-              `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":2}']`
-            )
-          )
-            .withContext(`In ${browserName}`)
-            .toBe(true);
+          await page.waitForSelector(
+            `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":2}']:focus`,
+            { visible: true }
+          );
 
           await page.keyboard.press("ArrowUp");
-          expect(
-            await isElementFocused(
-              page,
-              `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']`
-            )
-          )
-            .withContext(`In ${browserName}`)
-            .toBe(true);
+          await page.waitForSelector(
+            `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`,
+            { visible: true }
+          );
 
           await page.keyboard.press("ArrowDown");
           await page.keyboard.press("ArrowDown");
-          expect(
-            await isElementFocused(
-              page,
-              `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":3}']`
-            )
-          )
-            .withContext(`In ${browserName}`)
-            .toBe(true);
+          await page.waitForSelector(
+            `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":3}']:focus`,
+            { visible: true }
+          );
+
           await page.keyboard.press("Enter");
           const currentPage = await page.$eval(
             "#pageNumber",
@@ -215,24 +192,16 @@ describe("PDF Thumbnail View", () => {
           expect(currentPage).withContext(`In ${browserName}`).toBe(3);
 
           await page.keyboard.press("End");
-          expect(
-            await isElementFocused(
-              page,
-              `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":14}']`
-            )
-          )
-            .withContext(`In ${browserName}`)
-            .toBe(true);
+          await page.waitForSelector(
+            `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":14}']:focus`,
+            { visible: true }
+          );
 
           await page.keyboard.press("Home");
-          expect(
-            await isElementFocused(
-              page,
-              `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']`
-            )
-          )
-            .withContext(`In ${browserName}`)
-            .toBe(true);
+          await page.waitForSelector(
+            `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`,
+            { visible: true }
+          );
         })
       );
     });
@@ -443,4 +412,87 @@ describe("PDF Thumbnail View", () => {
       );
     });
   });
+
+  describe("Checkbox keyboard navigation", () => {
+    let pages;
+
+    beforeEach(async () => {
+      pages = await loadAndWait(
+        "tracemonkey.pdf",
+        "#viewsManagerToggleButton",
+        null,
+        null,
+        { enableSplitMerge: true }
+      );
+    });
+
+    afterEach(async () => {
+      await closePages(pages);
+    });
+
+    it("should focus checkboxes with Tab key", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#viewsManagerToggleButton");
+
+          await waitForThumbnailVisible(page, 1);
+
+          // Focus the first thumbnail button
+          await kbFocusNext(page);
+          await kbFocusNext(page);
+          await kbFocusNext(page);
+
+          // Verify we're on the first thumbnail
+          await page.waitForSelector(
+            `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`,
+            { visible: true }
+          );
+
+          // Tab to checkbox
+          await kbFocusNext(page);
+          await page.waitForSelector(
+            `.thumbnail[page-number="1"] input[type="checkbox"]:focus`,
+            { visible: true }
+          );
+        })
+      );
+    });
+
+    it("should navigate checkboxes with arrow keys", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#viewsManagerToggleButton");
+
+          await waitForThumbnailVisible(page, 1);
+          await waitForThumbnailVisible(page, 2);
+
+          // Navigate to first checkbox
+          await kbFocusNext(page);
+          await kbFocusNext(page);
+          await kbFocusNext(page);
+          await kbFocusNext(page);
+
+          // Verify first checkbox is focused
+          await page.waitForSelector(
+            `.thumbnail[page-number="1"] input[type="checkbox"]:focus`,
+            { visible: true }
+          );
+
+          // Navigate to next checkbox with ArrowDown
+          await page.keyboard.press("ArrowDown");
+          await page.waitForSelector(
+            `.thumbnail[page-number="2"] input[type="checkbox"]:focus`,
+            { visible: true }
+          );
+
+          // Navigate back with ArrowUp
+          await page.keyboard.press("ArrowUp");
+          await page.waitForSelector(
+            `.thumbnail[page-number="1"] input[type="checkbox"]:focus`,
+            { visible: true }
+          );
+        })
+      );
+    });
+  });
 });
diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js
@@ -121,16 +121,6 @@ class PDFThumbnailView extends RenderableView {
     thumbnailContainer.setAttribute("page-number", id);
     thumbnailContainer.setAttribute("page-id", id);
 
-    if (enableSplitMerge) {
-      const checkbox = (this.checkbox = document.createElement("input"));
-      checkbox.type = "checkbox";
-      checkbox.tabIndex = -1;
-      checkbox.setAttribute("data-l10n-id", "pdfjs-thumb-page-checkbox");
-      checkbox.setAttribute("data-l10n-args", this.#pageL10nArgs);
-      thumbnailContainer.append(checkbox);
-      this.pasteButton = null;
-    }
-
     const imageContainer = (this.imageContainer =
       document.createElement("div"));
     thumbnailContainer.append(imageContainer);
@@ -145,6 +135,17 @@ class PDFThumbnailView extends RenderableView {
 
     const image = (this.image = document.createElement("img"));
     imageContainer.append(image);
+
+    if (enableSplitMerge) {
+      const checkbox = (this.checkbox = document.createElement("input"));
+      checkbox.type = "checkbox";
+      checkbox.tabIndex = -1;
+      checkbox.setAttribute("data-l10n-id", "pdfjs-thumb-page-checkbox");
+      checkbox.setAttribute("data-l10n-args", this.#pageL10nArgs);
+      thumbnailContainer.append(checkbox);
+      this.pasteButton = null;
+    }
+
     this.#updateDims();
 
     container.append(thumbnailContainer);
@@ -271,9 +272,15 @@ class PDFThumbnailView extends RenderableView {
     if (isCurrent) {
       imageContainer.ariaCurrent = "page";
       imageContainer.tabIndex = 0;
+      if (this.checkbox) {
+        this.checkbox.tabIndex = 0;
+      }
     } else {
       imageContainer.ariaCurrent = false;
       imageContainer.tabIndex = -1;
+      if (this.checkbox) {
+        this.checkbox.tabIndex = -1;
+      }
     }
   }
 
diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js
@@ -848,34 +848,41 @@ class PDFThumbnailViewer {
       }
     });
     this.container.addEventListener("keydown", e => {
+      const { target } = e;
+      const isCheckbox =
+        target instanceof HTMLInputElement && target.type === "checkbox";
+
       switch (e.key) {
         case "ArrowLeft":
-          this.#goToNextItem(e.target, false, true);
+          this.#goToNextItem(target, false, true, isCheckbox);
           stopEvent(e);
           break;
         case "ArrowRight":
-          this.#goToNextItem(e.target, true, true);
+          this.#goToNextItem(target, true, true, isCheckbox);
           stopEvent(e);
           break;
         case "ArrowDown":
-          this.#goToNextItem(e.target, true, false);
+          this.#goToNextItem(target, true, false, isCheckbox);
           stopEvent(e);
           break;
         case "ArrowUp":
-          this.#goToNextItem(e.target, false, false);
+          this.#goToNextItem(target, false, false, isCheckbox);
           stopEvent(e);
           break;
         case "Home":
-          this._thumbnails[0].imageContainer.focus();
+          this.#focusThumbnailElement(this._thumbnails[0], isCheckbox);
           stopEvent(e);
           break;
         case "End":
-          this._thumbnails.at(-1).imageContainer.focus();
+          this.#focusThumbnailElement(this._thumbnails.at(-1), isCheckbox);
           stopEvent(e);
           break;
         case "Enter":
         case " ":
-          this.#goToPage(e);
+          if (!isCheckbox) {
+            this.#goToPage(e);
+          }
+          // For checkboxes, let the default behavior handle toggling
           break;
       }
     });
@@ -1048,13 +1055,29 @@ class PDFThumbnailViewer {
     }
   }
 
+  /**
+   * Focus either the checkbox or image of a thumbnail.
+   * @param {PDFThumbnailView} thumbnail
+   * @param {boolean} focusCheckbox - If true, focus checkbox; otherwise focus
+   *   image
+   */
+  #focusThumbnailElement(thumbnail, focusCheckbox) {
+    if (focusCheckbox && thumbnail.checkbox) {
+      thumbnail.checkbox.focus();
+    } else {
+      thumbnail.imageContainer.focus();
+    }
+  }
+
   /**
    * Go to the next/previous menu item.
    * @param {HTMLElement} element
    * @param {boolean} forward
    * @param {boolean} horizontal
+   * @param {boolean} navigateCheckboxes - If true, focus checkboxes;
+   *   otherwise focus images
    */
-  #goToNextItem(element, forward, horizontal) {
+  #goToNextItem(element, forward, horizontal, navigateCheckboxes = false) {
     let currentPageNumber = parseInt(
       element.parentElement.getAttribute("page-number"),
       10
@@ -1097,7 +1120,7 @@ class PDFThumbnailViewer {
       }
     }
     if (nextThumbnail) {
-      nextThumbnail.imageContainer.focus();
+      this.#focusThumbnailElement(nextThumbnail, navigateCheckboxes);
     }
   }
 
diff --git a/web/views_manager.css b/web/views_manager.css
@@ -590,6 +590,7 @@
         display: inline-flex;
         justify-content: center;
         align-items: center;
+        flex-direction: row-reverse;
         gap: 16px;
         width: auto;
         height: auto;
