Skip to content

Commit bd260b2

Browse files
calixtemancalixteman
authored
Merge pull request #20746 from calixteman/bug2019714
Fix tooltips on thumbnails and checkbox (bug 2019714)
2 parents 45b0f8b + aa92810 commit bd260b2

5 files changed

Lines changed: 58 additions & 40 deletions

File tree

‎l10n/en-US/viewer.ftl‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -192,8 +192,9 @@ pdfjs-additional-layers = Additional Layers
192192

193193
# Variables:
194194
# $page (Number) - the page number
195-
pdfjs-thumb-page-title =
196-
.title = Page { $page }
195+
# $total (Number) - the number of pages
196+
pdfjs-thumb-page-title1 =
197+
.title = Page { $page } of { $total }
197198
198199
# Variables:
199200
# $page (Number) - the page number
@@ -202,8 +203,8 @@ pdfjs-thumb-page-canvas =
202203
203204
# Variables:
204205
# $page (Number) - the page number
205-
pdfjs-thumb-page-checkbox =
206-
.aria-label = Select page { $page }
206+
pdfjs-thumb-page-checkbox1 =
207+
.title = Select page { $page }
207208
208209
## Find panel button title and messages
209210

@@ -690,11 +691,11 @@ pdfjs-editor-add-comment-button =
690691
## - layers.
691692
## The thumbnails view is used to edit the pdf: remove/insert pages, ...
692693

693-
pdfjs-toggle-views-manager-button =
694-
.title = Toggle Sidebar
694+
pdfjs-toggle-views-manager-button1 =
695+
.title = Manage pages
695696
pdfjs-toggle-views-manager-notification-button =
696697
.title = Toggle Sidebar (document contains thumbnails/outline/attachments/layers)
697-
pdfjs-toggle-views-manager-button-label = Toggle Sidebar
698+
pdfjs-toggle-views-manager-button1-label = Manage pages
698699
699700
pdfjs-views-manager-sidebar =
700701
.aria-label = Sidebar

‎test/integration/test_utils.mjs‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ function getAnnotationSelector(id) {
262262
}
263263

264264
function getThumbnailSelector(pageNumber) {
265-
return `.thumbnailImageContainer[data-l10n-args='{"page":${pageNumber}}']`;
265+
return `.thumbnailImageContainer[data-l10n-args^='{"page":${pageNumber}']`;
266266
}
267267

268268
async function getSpanRectFromText(page, pageNumber, text) {
@@ -645,6 +645,7 @@ function waitForEditorMovedInDOM(page) {
645645
}
646646

647647
async function scrollIntoView(page, selector) {
648+
await page.waitForSelector(selector, { visible: true });
648649
const handle = await page.evaluateHandle(
649650
sel => [
650651
new Promise(resolve => {
@@ -982,6 +983,9 @@ async function showViewsManager(page) {
982983
"#outerContainer:not(.viewsManagerMoving).viewsManagerOpen",
983984
{ visible: true }
984985
);
986+
await page.waitForSelector("#viewsManagerStatusActionButton:not(:disabled)", {
987+
visible: true,
988+
});
985989
}
986990

987991
// Unicode bidi isolation characters, Fluent adds these markers to the text.

‎test/integration/thumbnail_view_spec.mjs‎

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,15 @@ import {
22
awaitPromise,
33
closePages,
44
FSI,
5+
getThumbnailSelector,
56
kbFocusNext,
67
loadAndWait,
78
PDI,
89
showViewsManager,
910
} from "./test_utils.mjs";
1011

1112
function waitForThumbnailVisible(page, pageNum) {
12-
return page.waitForSelector(
13-
`.thumbnailImageContainer[data-l10n-args='{"page":${pageNum}}']`,
14-
{ visible: true }
15-
);
13+
return page.waitForSelector(getThumbnailSelector(pageNum), { visible: true });
1614
}
1715

1816
async function waitForMenu(page, buttonSelector, visible = true) {
@@ -56,6 +54,14 @@ describe("PDF Thumbnail View", () => {
5654
await page.waitForSelector(`${thumbSelector}[src^="blob:http:"]`, {
5755
visible: true,
5856
});
57+
58+
const title = await page.$eval(
59+
getThumbnailSelector(1),
60+
el => el.title
61+
);
62+
expect(title)
63+
.withContext(`In ${browserName}`)
64+
.toBe(`Page ${FSI}1${PDI} of ${FSI}14${PDI}`);
5965
})
6066
);
6167
});
@@ -110,7 +116,7 @@ describe("PDF Thumbnail View", () => {
110116

111117
for (const pageNum of [14, 1, 13, 2]) {
112118
await goToPage(page, pageNum);
113-
const thumbSelector = `.thumbnailImageContainer[data-l10n-args='{"page":${pageNum}}']`;
119+
const thumbSelector = getThumbnailSelector(pageNum);
114120
await page.waitForSelector(
115121
`.thumbnail ${thumbSelector}[aria-current="page"]`,
116122
{ visible: true }
@@ -158,26 +164,25 @@ describe("PDF Thumbnail View", () => {
158164

159165
await kbFocusNext(page);
160166
await page.waitForSelector(
161-
`#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`,
167+
`#thumbnailsView ${getThumbnailSelector(1)}:focus`,
162168
{ visible: true }
163169
);
164170

165171
await page.keyboard.press("ArrowDown");
166172
await page.waitForSelector(
167-
`#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":2}']:focus`,
173+
`#thumbnailsView ${getThumbnailSelector(2)}:focus`,
168174
{ visible: true }
169175
);
170176

171177
await page.keyboard.press("ArrowUp");
172-
await page.waitForSelector(
173-
`#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`,
174-
{ visible: true }
175-
);
178+
await page.waitForSelector(`${getThumbnailSelector(1)}:focus`, {
179+
visible: true,
180+
});
176181

177182
await page.keyboard.press("ArrowDown");
178183
await page.keyboard.press("ArrowDown");
179184
await page.waitForSelector(
180-
`#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":3}']:focus`,
185+
`#thumbnailsView ${getThumbnailSelector(3)}:focus`,
181186
{ visible: true }
182187
);
183188

@@ -190,13 +195,13 @@ describe("PDF Thumbnail View", () => {
190195

191196
await page.keyboard.press("End");
192197
await page.waitForSelector(
193-
`#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":14}']:focus`,
198+
`#thumbnailsView ${getThumbnailSelector(14)}:focus`,
194199
{ visible: true }
195200
);
196201

197202
await page.keyboard.press("Home");
198203
await page.waitForSelector(
199-
`#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`,
204+
`#thumbnailsView ${getThumbnailSelector(1)}:focus`,
200205
{ visible: true }
201206
);
202207
})
@@ -322,17 +327,17 @@ describe("PDF Thumbnail View", () => {
322327
await closePages(pages);
323328
});
324329

325-
it("should have accessible label on checkbox", async () => {
330+
it("should have a title on the checkbox", async () => {
326331
await Promise.all(
327332
pages.map(async ([browserName, page]) => {
328333
await showViewsManager(page);
329334
await waitForThumbnailVisible(page, 1);
330335

331-
const ariaLabel = await page.$eval(
336+
const title = await page.$eval(
332337
`.thumbnail[page-number="1"] input[type="checkbox"]`,
333-
el => el.getAttribute("aria-label")
338+
el => el.title
334339
);
335-
expect(ariaLabel)
340+
expect(title)
336341
.withContext(`In ${browserName}`)
337342
.toBe(`Select page ${FSI}1${PDI}`);
338343
})
@@ -478,10 +483,9 @@ describe("PDF Thumbnail View", () => {
478483
await kbFocusNext(page);
479484

480485
// Verify we're on the first thumbnail
481-
await page.waitForSelector(
482-
`#thumbnailsView .thumbnailImageContainer[data-l10n-args='{"page":1}']:focus`,
483-
{ visible: true }
484-
);
486+
await page.waitForSelector(`${getThumbnailSelector(1)}:focus`, {
487+
visible: true,
488+
});
485489

486490
// Tab to checkbox
487491
await kbFocusNext(page);

‎web/pdf_thumbnail_view.js‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ class PDFThumbnailView extends RenderableView {
131131
imageContainer.tabIndex = -1;
132132
imageContainer.draggable = false;
133133
imageContainer.setAttribute("page-number", id);
134+
imageContainer.setAttribute("data-l10n-id", "pdfjs-thumb-page-title1");
135+
imageContainer.setAttribute("data-l10n-args", this.#getPageL10nArgs(true));
134136

135137
const image = (this.image = document.createElement("img"));
136138
imageContainer.append(image);
@@ -139,8 +141,8 @@ class PDFThumbnailView extends RenderableView {
139141
const checkbox = (this.checkbox = document.createElement("input"));
140142
checkbox.type = "checkbox";
141143
checkbox.tabIndex = -1;
142-
checkbox.setAttribute("data-l10n-id", "pdfjs-thumb-page-checkbox");
143-
checkbox.setAttribute("data-l10n-args", this.#pageL10nArgs);
144+
checkbox.setAttribute("data-l10n-id", "pdfjs-thumb-page-checkbox1");
145+
checkbox.setAttribute("data-l10n-args", this.#getPageL10nArgs());
144146
thumbnailContainer.append(checkbox);
145147
this.pasteButton = null;
146148
}
@@ -331,8 +333,8 @@ class PDFThumbnailView extends RenderableView {
331333
reducedCanvas.toBlob(resolve);
332334
const blob = await promise;
333335
image.src = URL.createObjectURL(blob);
334-
imageContainer.setAttribute("data-l10n-id", "pdfjs-thumb-page-canvas");
335-
imageContainer.setAttribute("data-l10n-args", this.#pageL10nArgs);
336+
image.setAttribute("data-l10n-id", "pdfjs-thumb-page-canvas");
337+
image.setAttribute("data-l10n-args", this.#getPageL10nArgs());
336338
imageContainer.classList.remove("missingThumbnailImage");
337339
if (!FeatureTest.isOffscreenCanvasSupported) {
338340
// Clean up the canvas element since it is no longer needed.
@@ -525,17 +527,24 @@ class PDFThumbnailView extends RenderableView {
525527
return canvas;
526528
}
527529

528-
get #pageL10nArgs() {
529-
return JSON.stringify({ page: this.pageLabel ?? this.id });
530+
#getPageL10nArgs(hasTotal = false) {
531+
return JSON.stringify({
532+
page: this.pageLabel ?? this.id,
533+
total: hasTotal ? this.linkService.pagesCount : undefined,
534+
});
530535
}
531536

532537
/**
533538
* @param {string|null} label
534539
*/
535540
setPageLabel(label) {
536541
this.pageLabel = typeof label === "string" ? label : null;
537-
this.imageContainer.setAttribute("data-l10n-args", this.#pageL10nArgs);
538-
this.checkbox?.setAttribute("data-l10n-args", this.#pageL10nArgs);
542+
this.imageContainer.setAttribute(
543+
"data-l10n-args",
544+
this.#getPageL10nArgs(true)
545+
);
546+
this.image.setAttribute("data-l10n-args", this.#getPageL10nArgs());
547+
this.checkbox?.setAttribute("data-l10n-args", this.#getPageL10nArgs());
539548
}
540549
}
541550

‎web/viewer.html‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,12 @@
111111
class="toolbarButton"
112112
type="button"
113113
tabindex="0"
114-
data-l10n-id="pdfjs-toggle-views-manager-button"
114+
data-l10n-id="pdfjs-toggle-views-manager-button1"
115115
aria-expanded="false"
116116
aria-haspopup="true"
117117
aria-controls="viewsManager"
118118
>
119-
<span data-l10n-id="pdfjs-toggle-views-manager-button-label"></span>
119+
<span data-l10n-id="pdfjs-toggle-views-manager-button1-label"></span>
120120
</button>
121121
<div
122122
id="viewsManager"

0 commit comments

Comments
 (0)