From 16a94412e4d093b8880fc5ffbe7f486e9352eef3 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Sat, 11 Jan 2020 13:55:13 +0100 Subject: [PATCH] Remove the unused `id` properties from page and thumbnail canvas/image DOM elements (issue 11499) As described in the issue, having a DOM element with `id=page2` (or any other number) will automatically cause that element to become linkable through the URL hash. That's currently leading to some confusing and outright wrong behaviour, since it obviously only works for pages that have been loaded and rendered. For PDF documents the only officially supported way to reference a particular page through the URL hash is using the `#page=2` format, which also works for all pages regardless if they're loaded or not. As far as I can tell there's nothing in the PDF.js default viewer that actually depends on the page/thumbnail `id` at this point in time, hence why I believe that this removal ought to be safe. Just as a pre-caution this patch adds an `aria-label` to the page canvas, similar to the thumbnail canvas/image, to at least keep this information in the DOM. --- l10n/en-US/viewer.properties | 2 ++ l10n/sv-SE/viewer.properties | 2 ++ web/pdf_page_view.js | 6 +++++- web/pdf_thumbnail_view.js | 3 --- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/l10n/en-US/viewer.properties b/l10n/en-US/viewer.properties index 2dd7751aa..93dc4e313 100644 --- a/l10n/en-US/viewer.properties +++ b/l10n/en-US/viewer.properties @@ -148,6 +148,8 @@ thumbs_label=Thumbnails findbar.title=Find in Document findbar_label=Find +# LOCALIZATION NOTE (page_canvas): "{{page}}" will be replaced by the page number. +page_canvas=Page {{page}} # Thumbnails panel item (tooltip and alt text for images) # LOCALIZATION NOTE (thumb_page_title): "{{page}}" will be replaced by the page # number. diff --git a/l10n/sv-SE/viewer.properties b/l10n/sv-SE/viewer.properties index b1b60bd40..3d2f0cf43 100644 --- a/l10n/sv-SE/viewer.properties +++ b/l10n/sv-SE/viewer.properties @@ -148,6 +148,8 @@ thumbs_label=Miniatyrer findbar.title=Sök i dokument findbar_label=Sök +# LOCALIZATION NOTE (page_canvas): "{{page}}" will be replaced by the page number. +page_canvas=Sida {{page}} # Thumbnails panel item (tooltip and alt text for images) # LOCALIZATION NOTE (thumb_page_title): "{{page}}" will be replaced by the page # number. diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 319d59977..856f5bb4c 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -559,7 +559,11 @@ class PDFPageView { const viewport = this.viewport; const canvas = document.createElement("canvas"); - canvas.id = this.renderingId; + this.l10n + .get("page_canvas", { page: this.id }, "Page {{page}}") + .then(msg => { + canvas.setAttribute("aria-label", msg); + }); // Keep the canvas hidden until the first draw callback, or until drawing // is complete when `!this.renderingQueue`, to prevent black flickering. diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 5d8c9ae1f..3defbafb3 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -258,11 +258,9 @@ class PDFThumbnailView { if (this.renderingState !== RenderingStates.FINISHED) { return; } - const id = this.renderingId; const className = "thumbnailImage"; if (this.disableCanvasToImageConversion) { - this.canvas.id = id; this.canvas.className = className; this.l10n .get( @@ -279,7 +277,6 @@ class PDFThumbnailView { return; } const image = document.createElement("img"); - image.id = id; image.className = className; this.l10n .get(