From aa928105b4c7954073d63aed712c799616785aaa Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Thu, 26 Feb 2026 19:13:45 +0100 Subject: [PATCH] Fix tooltips on thumbnails and checkbox (bug 2019714) And fix an issue with some integration tests where the manage button was disabled (hence not in the tab cycle). --- l10n/en-US/viewer.ftl | 15 ++++---- test/integration/test_utils.mjs | 6 ++- test/integration/thumbnail_view_spec.mjs | 48 +++++++++++++----------- web/pdf_thumbnail_view.js | 25 ++++++++---- web/viewer.html | 4 +- 5 files changed, 58 insertions(+), 40 deletions(-) diff --git a/l10n/en-US/viewer.ftl b/l10n/en-US/viewer.ftl index 6ac456c58..97a2d0469 100644 --- a/l10n/en-US/viewer.ftl +++ b/l10n/en-US/viewer.ftl @@ -192,8 +192,9 @@ pdfjs-additional-layers = Additional Layers # Variables: # $page (Number) - the page number -pdfjs-thumb-page-title = - .title = Page { $page } +# $total (Number) - the number of pages +pdfjs-thumb-page-title1 = + .title = Page { $page } of { $total } # Variables: # $page (Number) - the page number @@ -202,8 +203,8 @@ pdfjs-thumb-page-canvas = # Variables: # $page (Number) - the page number -pdfjs-thumb-page-checkbox = - .aria-label = Select page { $page } +pdfjs-thumb-page-checkbox1 = + .title = Select page { $page } ## Find panel button title and messages @@ -690,11 +691,11 @@ pdfjs-editor-add-comment-button = ## - layers. ## The thumbnails view is used to edit the pdf: remove/insert pages, ... -pdfjs-toggle-views-manager-button = - .title = Toggle Sidebar +pdfjs-toggle-views-manager-button1 = + .title = Manage pages pdfjs-toggle-views-manager-notification-button = .title = Toggle Sidebar (document contains thumbnails/outline/attachments/layers) -pdfjs-toggle-views-manager-button-label = Toggle Sidebar +pdfjs-toggle-views-manager-button1-label = Manage pages pdfjs-views-manager-sidebar = .aria-label = Sidebar diff --git a/test/integration/test_utils.mjs b/test/integration/test_utils.mjs index b3f3416c7..6f8762826 100644 --- a/test/integration/test_utils.mjs +++ b/test/integration/test_utils.mjs @@ -262,7 +262,7 @@ function getAnnotationSelector(id) { } function getThumbnailSelector(pageNumber) { - return `.thumbnailImageContainer[data-l10n-args='{"page":${pageNumber}}']`; + return `.thumbnailImageContainer[data-l10n-args^='{"page":${pageNumber}']`; } async function getSpanRectFromText(page, pageNumber, text) { @@ -645,6 +645,7 @@ function waitForEditorMovedInDOM(page) { } async function scrollIntoView(page, selector) { + await page.waitForSelector(selector, { visible: true }); const handle = await page.evaluateHandle( sel => [ new Promise(resolve => { @@ -982,6 +983,9 @@ async function showViewsManager(page) { "#outerContainer:not(.viewsManagerMoving).viewsManagerOpen", { visible: true } ); + await page.waitForSelector("#viewsManagerStatusActionButton:not(:disabled)", { + visible: true, + }); } // Unicode bidi isolation characters, Fluent adds these markers to the text. diff --git a/test/integration/thumbnail_view_spec.mjs b/test/integration/thumbnail_view_spec.mjs index 28e11fd4d..506f636c1 100644 --- a/test/integration/thumbnail_view_spec.mjs +++ b/test/integration/thumbnail_view_spec.mjs @@ -2,6 +2,7 @@ import { awaitPromise, closePages, FSI, + getThumbnailSelector, kbFocusNext, loadAndWait, PDI, @@ -9,10 +10,7 @@ import { } from "./test_utils.mjs"; function waitForThumbnailVisible(page, pageNum) { - return page.waitForSelector( - `.thumbnailImageContainer[data-l10n-args='{"page":${pageNum}}']`, - { visible: true } - ); + return page.waitForSelector(getThumbnailSelector(pageNum), { visible: true }); } async function waitForMenu(page, buttonSelector, visible = true) { @@ -56,6 +54,14 @@ describe("PDF Thumbnail View", () => { await page.waitForSelector(`${thumbSelector}[src^="blob:http:"]`, { visible: true, }); + + const title = await page.$eval( + getThumbnailSelector(1), + el => el.title + ); + expect(title) + .withContext(`In ${browserName}`) + .toBe(`Page ${FSI}1${PDI} of ${FSI}14${PDI}`); }) ); }); @@ -110,7 +116,7 @@ describe("PDF Thumbnail View", () => { for (const pageNum of [14, 1, 13, 2]) { await goToPage(page, pageNum); - const thumbSelector = `.thumbnailImageContainer[data-l10n-args='{"page":${pageNum}}']`; + const thumbSelector = getThumbnailSelector(pageNum); await page.waitForSelector( `.thumbnail ${thumbSelector}[aria-current="page"]`, { visible: true } @@ -158,26 +164,25 @@ describe("PDF Thumbnail View", () => { await kbFocusNext(page); await page.waitForSelector( - `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`, + `#thumbnailsView ${getThumbnailSelector(1)}:focus`, { visible: true } ); await page.keyboard.press("ArrowDown"); await page.waitForSelector( - `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":2}']:focus`, + `#thumbnailsView ${getThumbnailSelector(2)}:focus`, { visible: true } ); await page.keyboard.press("ArrowUp"); - await page.waitForSelector( - `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`, - { visible: true } - ); + await page.waitForSelector(`${getThumbnailSelector(1)}:focus`, { + visible: true, + }); await page.keyboard.press("ArrowDown"); await page.keyboard.press("ArrowDown"); await page.waitForSelector( - `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":3}']:focus`, + `#thumbnailsView ${getThumbnailSelector(3)}:focus`, { visible: true } ); @@ -190,13 +195,13 @@ describe("PDF Thumbnail View", () => { await page.keyboard.press("End"); await page.waitForSelector( - `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":14}']:focus`, + `#thumbnailsView ${getThumbnailSelector(14)}:focus`, { visible: true } ); await page.keyboard.press("Home"); await page.waitForSelector( - `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`, + `#thumbnailsView ${getThumbnailSelector(1)}:focus`, { visible: true } ); }) @@ -322,17 +327,17 @@ describe("PDF Thumbnail View", () => { await closePages(pages); }); - it("should have accessible label on checkbox", async () => { + it("should have a title on the checkbox", async () => { await Promise.all( pages.map(async ([browserName, page]) => { await showViewsManager(page); await waitForThumbnailVisible(page, 1); - const ariaLabel = await page.$eval( + const title = await page.$eval( `.thumbnail[page-number="1"] input[type="checkbox"]`, - el => el.getAttribute("aria-label") + el => el.title ); - expect(ariaLabel) + expect(title) .withContext(`In ${browserName}`) .toBe(`Select page ${FSI}1${PDI}`); }) @@ -478,10 +483,9 @@ describe("PDF Thumbnail View", () => { await kbFocusNext(page); // Verify we're on the first thumbnail - await page.waitForSelector( - `#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`, - { visible: true } - ); + await page.waitForSelector(`${getThumbnailSelector(1)}:focus`, { + visible: true, + }); // Tab to checkbox await kbFocusNext(page); diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 9438f5ae5..d01d89b2d 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -131,6 +131,8 @@ class PDFThumbnailView extends RenderableView { imageContainer.tabIndex = -1; imageContainer.draggable = false; imageContainer.setAttribute("page-number", id); + imageContainer.setAttribute("data-l10n-id", "pdfjs-thumb-page-title1"); + imageContainer.setAttribute("data-l10n-args", this.#getPageL10nArgs(true)); const image = (this.image = document.createElement("img")); imageContainer.append(image); @@ -139,8 +141,8 @@ class PDFThumbnailView extends RenderableView { 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); + checkbox.setAttribute("data-l10n-id", "pdfjs-thumb-page-checkbox1"); + checkbox.setAttribute("data-l10n-args", this.#getPageL10nArgs()); thumbnailContainer.append(checkbox); this.pasteButton = null; } @@ -331,8 +333,8 @@ class PDFThumbnailView extends RenderableView { reducedCanvas.toBlob(resolve); const blob = await promise; image.src = URL.createObjectURL(blob); - imageContainer.setAttribute("data-l10n-id", "pdfjs-thumb-page-canvas"); - imageContainer.setAttribute("data-l10n-args", this.#pageL10nArgs); + image.setAttribute("data-l10n-id", "pdfjs-thumb-page-canvas"); + image.setAttribute("data-l10n-args", this.#getPageL10nArgs()); imageContainer.classList.remove("missingThumbnailImage"); if (!FeatureTest.isOffscreenCanvasSupported) { // Clean up the canvas element since it is no longer needed. @@ -525,8 +527,11 @@ class PDFThumbnailView extends RenderableView { return canvas; } - get #pageL10nArgs() { - return JSON.stringify({ page: this.pageLabel ?? this.id }); + #getPageL10nArgs(hasTotal = false) { + return JSON.stringify({ + page: this.pageLabel ?? this.id, + total: hasTotal ? this.linkService.pagesCount : undefined, + }); } /** @@ -534,8 +539,12 @@ class PDFThumbnailView extends RenderableView { */ setPageLabel(label) { this.pageLabel = typeof label === "string" ? label : null; - this.imageContainer.setAttribute("data-l10n-args", this.#pageL10nArgs); - this.checkbox?.setAttribute("data-l10n-args", this.#pageL10nArgs); + this.imageContainer.setAttribute( + "data-l10n-args", + this.#getPageL10nArgs(true) + ); + this.image.setAttribute("data-l10n-args", this.#getPageL10nArgs()); + this.checkbox?.setAttribute("data-l10n-args", this.#getPageL10nArgs()); } } diff --git a/web/viewer.html b/web/viewer.html index 2ea208d70..93dac8e10 100644 --- a/web/viewer.html +++ b/web/viewer.html @@ -111,12 +111,12 @@ See https://github.com/adobe-type-tools/cmap-resources class="toolbarButton" type="button" tabindex="0" - data-l10n-id="pdfjs-toggle-views-manager-button" + data-l10n-id="pdfjs-toggle-views-manager-button1" aria-expanded="false" aria-haspopup="true" aria-controls="viewsManager" > - +