From 7fce3eac939bb710567234b10e11bfce7a4286c6 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Wed, 12 Mar 2025 20:37:02 +0100 Subject: [PATCH] [Editor] Fix the role of the different editors in order to make them interactive elements (bug 1953290) Having some interactive elements forces the screen readers to switch to form mode and consequently they delegate the keyboard stuff to the browser. This patch sets an aria label on each editor in order to have a better description than just 'application'. --- l10n/en-US/viewer.ftl | 17 +++++++--- src/display/editor/alt_text.js | 2 +- src/display/editor/editor.js | 36 ++++++++++++---------- src/display/editor/highlight.js | 1 + src/display/editor/ink.js | 1 + src/display/editor/signature.js | 4 +-- src/display/editor/stamp.js | 9 ++---- test/integration/signature_editor_spec.mjs | 2 +- test/integration/stamp_editor_spec.mjs | 4 +-- 9 files changed, 41 insertions(+), 35 deletions(-) diff --git a/l10n/en-US/viewer.ftl b/l10n/en-US/viewer.ftl index e8c32ae45..172ceb2bf 100644 --- a/l10n/en-US/viewer.ftl +++ b/l10n/en-US/viewer.ftl @@ -324,6 +324,19 @@ pdfjs-editor-signature-button = .title = Add signature pdfjs-editor-signature-button-label = Add signature +## Default editor aria labels + +# “Highlight” is a noun, the string is used on the editor for highlights. +pdfjs-editor-highlight-editor = + .aria-label = Highlight editor +# “Drawing” is a noun, the string is used on the editor for drawings. +pdfjs-editor-ink-editor = + .aria-label = Drawing editor +pdfjs-editor-signature-editor = + .aria-label = Signature editor +pdfjs-editor-stamp-editor = + .aria-label = Image editor + ## Remove button for the various kind of editor. pdfjs-editor-remove-ink-button = @@ -360,10 +373,6 @@ pdfjs-editor-signature-add-signature-button-label = Add new signature pdfjs-free-text2 = .aria-label = Text Editor .default-content = Start typing… -pdfjs-ink = - .aria-label = Draw Editor -pdfjs-ink-canvas = - .aria-label = User-created image ## Alt-text dialog diff --git a/src/display/editor/alt_text.js b/src/display/editor/alt_text.js index 6ccdea85f..201d12014 100644 --- a/src/display/editor/alt_text.js +++ b/src/display/editor/alt_text.js @@ -330,7 +330,7 @@ class AltText { button.append(tooltip); } - const element = this.#editor.getImageForAltText(); + const element = this.#editor.getElementForAltText(); element?.setAttribute("aria-describedby", tooltip.id); } } diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index 8cf46aa0a..91ad4500d 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -1142,13 +1142,17 @@ class AnnotationEditor { * @returns {HTMLDivElement | null} */ render() { - this.div = document.createElement("div"); - this.div.setAttribute("data-editor-rotation", (360 - this.rotation) % 360); - this.div.className = this.name; - this.div.setAttribute("id", this.id); - this.div.tabIndex = this.#disabled ? -1 : 0; + const div = (this.div = document.createElement("div")); + div.setAttribute("data-editor-rotation", (360 - this.rotation) % 360); + div.className = this.name; + div.setAttribute("id", this.id); + div.tabIndex = this.#disabled ? -1 : 0; + div.setAttribute("role", "application"); + if (this.defaultL10nId) { + div.setAttribute("data-l10n-id", this.defaultL10nId); + } if (!this._isVisible) { - this.div.classList.add("hidden"); + div.classList.add("hidden"); } this.setInForeground(); @@ -1156,23 +1160,22 @@ class AnnotationEditor { const [parentWidth, parentHeight] = this.parentDimensions; if (this.parentRotation % 180 !== 0) { - this.div.style.maxWidth = `${((100 * parentHeight) / parentWidth).toFixed( + div.style.maxWidth = `${((100 * parentHeight) / parentWidth).toFixed( + 2 + )}%`; + div.style.maxHeight = `${((100 * parentWidth) / parentHeight).toFixed( 2 )}%`; - this.div.style.maxHeight = `${( - (100 * parentWidth) / - parentHeight - ).toFixed(2)}%`; } const [tx, ty] = this.getInitialTranslation(); this.translate(tx, ty); - bindEvents(this, this.div, ["pointerdown"]); + bindEvents(this, div, ["keydown", "pointerdown"]); if (this.isResizable && this._uiManager._supportsPinchToZoom) { this.#touchManager ||= new TouchManager({ - container: this.div, + container: div, isPinchingDisabled: () => !this.isSelected, onPinchStart: this.#touchPinchStartCallback.bind(this), onPinching: this.#touchPinchCallback.bind(this), @@ -1183,7 +1186,7 @@ class AnnotationEditor { this._uiManager._editorUndoBar?.hide(); - return this.div; + return div; } #touchPinchStartCallback() { @@ -1692,7 +1695,6 @@ class AnnotationEditor { if (this.isResizable) { this.#createResizers(); this.#resizersDiv.classList.remove("hidden"); - bindEvents(this, this.div, ["keydown"]); } } @@ -1892,8 +1894,8 @@ class AnnotationEditor { /** * @returns {HTMLElement | null} the element requiring an alt text. */ - getImageForAltText() { - return null; + getElementForAltText() { + return this.div; } /** diff --git a/src/display/editor/highlight.js b/src/display/editor/highlight.js index c361313a7..0706ba867 100644 --- a/src/display/editor/highlight.js +++ b/src/display/editor/highlight.js @@ -111,6 +111,7 @@ class HighlightEditor extends AnnotationEditor { this.#methodOfCreation = params.methodOfCreation || ""; this.#text = params.text || ""; this._isDraggable = false; + this.defaultL10nId = "pdfjs-editor-highlight-editor"; if (params.highlightId > -1) { this.#isFreeHighlight = true; diff --git a/src/display/editor/ink.js b/src/display/editor/ink.js index a3062bd03..b15ac6253 100644 --- a/src/display/editor/ink.js +++ b/src/display/editor/ink.js @@ -68,6 +68,7 @@ class InkEditor extends DrawingEditor { constructor(params) { super({ ...params, name: "inkEditor" }); this._willKeepAspectRatio = true; + this.defaultL10nId = "pdfjs-editor-ink-editor"; } /** @inheritdoc */ diff --git a/src/display/editor/signature.js b/src/display/editor/signature.js index 7fef24c31..40569256b 100644 --- a/src/display/editor/signature.js +++ b/src/display/editor/signature.js @@ -79,6 +79,7 @@ class SignatureEditor extends DrawingEditor { this._willKeepAspectRatio = true; this.#signatureData = params.signatureData || null; this.#description = null; + this.defaultL10nId = "pdfjs-editor-signature-editor"; } /** @inheritdoc */ @@ -156,7 +157,6 @@ class SignatureEditor extends DrawingEditor { } super.render(); - this.div.setAttribute("role", "figure"); if (this._drawId === null) { if (this.#signatureData) { @@ -259,7 +259,7 @@ class SignatureEditor extends DrawingEditor { const { outline } = (this.#signatureData = data); this.#isExtracted = outline instanceof ContourDrawOutline; this.#description = description; - this.div.setAttribute("aria-label", description); + this.div.setAttribute("aria-description", description); let drawingOptions; if (this.#isExtracted) { drawingOptions = SignatureEditor.getDefaultDrawingOptions(); diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js index 28b427075..b9416211c 100644 --- a/src/display/editor/stamp.js +++ b/src/display/editor/stamp.js @@ -56,6 +56,7 @@ class StampEditor extends AnnotationEditor { super({ ...params, name: "stampEditor" }); this.#bitmapUrl = params.bitmapUrl; this.#bitmapFile = params.bitmapFile; + this.defaultL10nId = "pdfjs-editor-stamp-editor"; } /** @inheritdoc */ @@ -353,7 +354,6 @@ class StampEditor extends AnnotationEditor { super.render(); this.div.hidden = true; - this.div.setAttribute("role", "figure"); this.addAltTextButton(); @@ -474,7 +474,7 @@ class StampEditor extends AnnotationEditor { action: "inserted_image", }); if (this.#bitmapFileName) { - canvas.setAttribute("aria-label", this.#bitmapFileName); + this.div.setAttribute("aria-description", this.#bitmapFileName); } } @@ -686,11 +686,6 @@ class StampEditor extends AnnotationEditor { ); } - /** @inheritdoc */ - getImageForAltText() { - return this.#canvas; - } - #serializeBitmap(toUrl) { if (toUrl) { if (this.#isSvg) { diff --git a/test/integration/signature_editor_spec.mjs b/test/integration/signature_editor_spec.mjs index a1a3678b4..757bea460 100644 --- a/test/integration/signature_editor_spec.mjs +++ b/test/integration/signature_editor_spec.mjs @@ -184,7 +184,7 @@ describe("Signature Editor", () => { // Check the aria label. await page.waitForSelector( - `${editorSelector}[aria-label="Hello World"]` + `${editorSelector}[aria-description="Hello World"]` ); // Edit the description. diff --git a/test/integration/stamp_editor_spec.mjs b/test/integration/stamp_editor_spec.mjs index b76a84e79..9ab373453 100644 --- a/test/integration/stamp_editor_spec.mjs +++ b/test/integration/stamp_editor_spec.mjs @@ -361,9 +361,7 @@ describe("Stamp Editor", () => { await page.click(saveButtonSelector); // Check that the canvas has an aria-describedby attribute. - await page.waitForSelector( - `${editorSelector} canvas[aria-describedby]` - ); + await page.waitForSelector(`${editorSelector}[aria-describedby]`); // Wait for the alt-text button to have the correct icon. await page.waitForSelector(`${buttonSelector}.done`);