From 7d8b53bf7a456bed9a041bc10794a6383edc96c6 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Tue, 8 Aug 2023 19:47:24 +0200 Subject: [PATCH] [Editor] Move an the editor div in the DOM once a translation with the keyboard is done When moving an element in the DOM, the focus is potentially lost, so we need to make sure that the focused element before the translation will get back its focus after it. But we must take care to not execute any focus/blur callbacks because the user didn't do anything which should trigger such events: it's a detail of implementation. For example, when several editors are selected and moved, then at the end the same must be selected, so no element receive a focus event which will set it as selected. --- src/display/editor/annotation_editor_layer.js | 39 ++++++--- src/display/editor/editor.js | 11 ++- src/display/editor/freetext.js | 3 + src/display/editor/ink.js | 3 + test/integration/freetext_editor_spec.js | 82 +++++++++++++++++++ test/integration/ink_editor_spec.js | 9 ++ 6 files changed, 134 insertions(+), 13 deletions(-) diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index fdb738b1a..3a2815a31 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -320,19 +320,15 @@ class AnnotationEditorLayer { this.detach(editor); this.#uiManager.removeEditor(editor); - editor.div.style.display = "none"; - setTimeout(() => { - // When the div is removed from DOM the focus can move on the - // document.body, so we just slightly postpone the removal in - // order to let an element potentially grab the focus before - // the body. - editor.div.style.display = ""; - editor.div.remove(); - editor.isAttachedToDOM = false; - if (document.activeElement === document.body) { + if (editor.div.contains(document.activeElement)) { + setTimeout(() => { + // When the div is removed from DOM the focus can move on the + // document.body, so we need to move it back to the main container. this.#uiManager.focusMainContainer(); - } - }, 0); + }, 0); + } + editor.div.remove(); + editor.isAttachedToDOM = false; if (!this.#isCleaningUp) { this.addInkEditorIfNeeded(/* isCommitting = */ false); @@ -385,6 +381,25 @@ class AnnotationEditorLayer { } moveEditorInDOM(editor) { + const { activeElement } = document; + if (editor.div.contains(activeElement)) { + // When the div is moved in the DOM the focus can move somewhere else, + // so we want to be sure that the focus will stay on the editor but we + // don't want to call any focus callbacks, hence we disable them and only + // re-enable them when the editor has the focus. + editor._focusEventsAllowed = false; + setTimeout(() => { + editor.div.addEventListener( + "focusin", + () => { + editor._focusEventsAllowed = true; + }, + { once: true } + ); + activeElement.focus(); + }, 0); + } + this.#accessibilityManager?.moveElementInDOM( this.div, editor.div, diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index c0935ab61..6229ddae7 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -50,6 +50,8 @@ class AnnotationEditor { _uiManager = null; + _focusEventsAllowed = true; + #isDraggable = false; #zIndex = AnnotationEditor._zIndex++; @@ -190,6 +192,9 @@ class AnnotationEditor { * onfocus callback. */ focusin(event) { + if (!this._focusEventsAllowed) { + return; + } if (!this.#hasBeenSelected) { this.parent.setSelected(this); } else { @@ -202,6 +207,10 @@ class AnnotationEditor { * @param {FocusEvent} event */ focusout(event) { + if (!this._focusEventsAllowed) { + return; + } + if (!this.isAttachedToDOM) { return; } @@ -284,6 +293,7 @@ class AnnotationEditor { */ translateInPage(x, y) { this.#translate(this.pageDimensions, x, y); + this.parent.moveEditorInDOM(this); this.div.scrollIntoView({ block: "nearest" }); } @@ -790,7 +800,6 @@ class AnnotationEditor { this.fixAndSetPosition(); this.parent.moveEditorInDOM(this); - this.div.focus(); }; window.addEventListener("pointerup", pointerUpCallback); // If the user is using alt+tab during the dragging session, the pointerup diff --git a/src/display/editor/freetext.js b/src/display/editor/freetext.js index 0e3fc3a0b..d3dac2092 100644 --- a/src/display/editor/freetext.js +++ b/src/display/editor/freetext.js @@ -342,6 +342,9 @@ class FreeTextEditor extends AnnotationEditor { /** @inheritdoc */ focusin(event) { + if (!this._focusEventsAllowed) { + return; + } super.focusin(event); if (event.target !== this.editorDiv) { this.editorDiv.focus(); diff --git a/src/display/editor/ink.js b/src/display/editor/ink.js index a67676968..28cc4776a 100644 --- a/src/display/editor/ink.js +++ b/src/display/editor/ink.js @@ -634,6 +634,9 @@ class InkEditor extends AnnotationEditor { /** @inheritdoc */ focusin(event) { + if (!this._focusEventsAllowed) { + return; + } super.focusin(event); this.enableEditMode(); } diff --git a/test/integration/freetext_editor_spec.js b/test/integration/freetext_editor_spec.js index e69c0c364..a2366d10f 100644 --- a/test/integration/freetext_editor_spec.js +++ b/test/integration/freetext_editor_spec.js @@ -390,6 +390,7 @@ describe("FreeText Editor", () => { await clearAll(page); await page.mouse.click(rect.x + 200, rect.y + 100); + await page.waitForTimeout(10); for (let i = 0; i < 5; i++) { await page.type(`${getEditorSelector(9)} .internal`, "A"); @@ -2111,4 +2112,85 @@ describe("FreeText Editor", () => { ); }); }); + + describe("Freetext must stay focused after having been moved", () => { + let pages; + + beforeAll(async () => { + pages = await loadAndWait("empty.pdf", ".annotationEditorLayer"); + }); + + afterAll(async () => { + await closePages(pages); + }); + + it("must keep the focus", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.click("#editorFreeText"); + + const rect = await page.$eval(".annotationEditorLayer", el => { + const { x, y } = el.getBoundingClientRect(); + return { x, y }; + }); + + await page.mouse.click(rect.x + 100, rect.y + 100); + await page.waitForTimeout(10); + await page.type(`${getEditorSelector(0)} .internal`, "A"); + + // Commit. + await page.keyboard.press("Escape"); + await page.waitForTimeout(10); + + await page.mouse.click(rect.x + 110, rect.y + 150); + await page.waitForTimeout(10); + await page.type(`${getEditorSelector(0)} .internal`, "B"); + + // Commit. + await page.keyboard.press("Escape"); + await page.waitForTimeout(10); + + await page.mouse.click(rect.x + 115, rect.y + 155); + await page.waitForTimeout(10); + + const pos = n => + page.evaluate(sel => { + const editor = document.querySelector(sel); + return Array.prototype.indexOf.call( + editor.parentNode.childNodes, + editor + ); + }, getEditorSelector(n)); + + expect(await pos(0)) + .withContext(`In ${browserName}`) + .toEqual(0); + expect(await pos(1)) + .withContext(`In ${browserName}`) + .toEqual(1); + + for (let i = 0; i < 6; i++) { + await page.keyboard.down("Control"); + await page.keyboard.press("ArrowUp"); + await page.keyboard.up("Control"); + await page.waitForTimeout(1); + } + + await page.waitForTimeout(100); + const focused = await page.evaluate(sel => { + const editor = document.querySelector(sel); + return editor === document.activeElement; + }, getEditorSelector(1)); + expect(focused).withContext(`In ${browserName}`).toEqual(true); + + expect(await pos(0)) + .withContext(`In ${browserName}`) + .toEqual(1); + expect(await pos(1)) + .withContext(`In ${browserName}`) + .toEqual(0); + }) + ); + }); + }); }); diff --git a/test/integration/ink_editor_spec.js b/test/integration/ink_editor_spec.js index 7a40d250b..9a45d929c 100644 --- a/test/integration/ink_editor_spec.js +++ b/test/integration/ink_editor_spec.js @@ -50,23 +50,28 @@ describe("Ink Editor", () => { await page.mouse.down(); await page.mouse.move(x + 50, y + 50); await page.mouse.up(); + await page.waitForTimeout(10); await page.keyboard.press("Escape"); + await page.waitForTimeout(10); } await page.keyboard.down("Control"); await page.keyboard.press("a"); await page.keyboard.up("Control"); + await page.waitForTimeout(10); expect(await getSelectedEditors(page)) .withContext(`In ${browserName}`) .toEqual([0, 1, 2]); await page.keyboard.press("Backspace"); + await page.waitForTimeout(10); await page.keyboard.down("Control"); await page.keyboard.press("z"); await page.keyboard.up("Control"); + await page.waitForTimeout(10); expect(await getSelectedEditors(page)) .withContext(`In ${browserName}`) @@ -81,8 +86,10 @@ describe("Ink Editor", () => { await page.keyboard.down("Control"); await page.keyboard.press("a"); await page.keyboard.up("Control"); + await page.waitForTimeout(10); await page.keyboard.press("Backspace"); + await page.waitForTimeout(10); const rect = await page.$eval(".annotationEditorLayer", el => { // With Chrome something is wrong when serializing a DomRect, @@ -97,8 +104,10 @@ describe("Ink Editor", () => { await page.mouse.down(); await page.mouse.move(xStart + 50, yStart + 50); await page.mouse.up(); + await page.waitForTimeout(10); await page.keyboard.press("Escape"); + await page.waitForTimeout(10); const rectBefore = await page.$eval(".inkEditor canvas", el => { const { x, y } = el.getBoundingClientRect();