From 74a377f72feb68e976ace12617384def4fe47a6f Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Sun, 8 Dec 2024 22:16:41 +0100 Subject: [PATCH] Fix intermittent failures when moving a freetext annotation in integration tests When an editor is moved with the keyboard or in dragging it, it is moved in the DOM in order to make screen readers happy. But this move is slightly postponed thanks to a setTimeout(..., 0). The failures were very likely due to the fact that intermittently the DOM move was done in the middle of the next key sequence which was making the move on screen failing. --- src/display/editor/editor.js | 5 + test/integration/freetext_editor_spec.mjs | 126 +++++++--------------- test/integration/test_utils.mjs | 9 ++ 3 files changed, 54 insertions(+), 86 deletions(-) diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index 50aff8624d838..2d61a2850fc32 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -1278,6 +1278,11 @@ class AnnotationEditor { this.#moveInDOMTimeout = setTimeout(() => { this.#moveInDOMTimeout = null; this.parent?.moveEditorInDOM(this); + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { + this._uiManager._eventBus.dispatch("editormovedindom", { + source: this, + }); + } }, 0); } diff --git a/test/integration/freetext_editor_spec.mjs b/test/integration/freetext_editor_spec.mjs index 085c75e6a6c8e..9f0b201ebce69 100644 --- a/test/integration/freetext_editor_spec.mjs +++ b/test/integration/freetext_editor_spec.mjs @@ -47,6 +47,7 @@ import { switchToEditor, waitForAnnotationEditorLayer, waitForAnnotationModeChanged, + waitForEditorMovedInDOM, waitForSelectedEditor, waitForSerialized, waitForStorageEntries, @@ -88,6 +89,17 @@ const waitForPositionChange = (page, selector, xy) => xy ); +const moveEditor = async (page, selector, n, pressKey) => { + let xy = await getXY(page, selector); + for (let i = 0; i < n; i++) { + const handle = await waitForEditorMovedInDOM(page); + await pressKey(); + await awaitPromise(handle); + await waitForPositionChange(page, selector, xy); + xy = await getXY(page, selector); + } +}; + const cancelFocusIn = async (page, selector) => { page.evaluate(sel => { const el = document.querySelector(sel); @@ -1968,12 +1980,9 @@ describe("FreeText Editor", () => { const [pageX, pageY] = await getFirstSerialized(page, x => x.rect); - let xy = await getXY(page, selectorEditor); - for (let i = 0; i < 20; i++) { - await page.keyboard.press("ArrowRight"); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 20, () => + page.keyboard.press("ArrowRight") + ); let [newX, newY] = await getFirstSerialized(page, x => x.rect); expect(Math.round(newX)) @@ -1983,11 +1992,9 @@ describe("FreeText Editor", () => { .withContext(`In ${browserName}`) .toEqual(Math.round(pageY)); - for (let i = 0; i < 20; i++) { - await page.keyboard.press("ArrowDown"); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 20, () => + page.keyboard.press("ArrowDown") + ); [newX, newY] = await getFirstSerialized(page, x => x.rect); expect(Math.round(newX)) @@ -1997,11 +2004,7 @@ describe("FreeText Editor", () => { .withContext(`In ${browserName}`) .toEqual(Math.round(pageY - 20)); - for (let i = 0; i < 2; i++) { - await kbBigMoveLeft(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 2, () => kbBigMoveLeft(page)); [newX, newY] = await getFirstSerialized(page, x => x.rect); expect(Math.round(newX)) @@ -2011,11 +2014,7 @@ describe("FreeText Editor", () => { .withContext(`In ${browserName}`) .toEqual(Math.round(pageY - 20)); - for (let i = 0; i < 2; i++) { - await kbBigMoveUp(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 2, () => kbBigMoveUp(page)); [newX, newY] = await getFirstSerialized(page, x => x.rect); expect(Math.round(newX)) @@ -2041,12 +2040,9 @@ describe("FreeText Editor", () => { const pageWidth = page2X - page1X; const selectorEditor = getEditorSelector(0); - let xy = await getXY(page, selectorEditor); - for (let i = 0; i < 5; i++) { - await page.keyboard.press("ArrowRight"); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 5, () => + page.keyboard.press("ArrowRight") + ); const [new1X, , new2X] = await getFirstSerialized(page, x => x.rect); const newWidth = new2X - new1X; @@ -2093,42 +2089,21 @@ describe("FreeText Editor", () => { visible: true, }); - let xy = await getXY(page, selectorEditor); - for (let i = 0; i < 20; i++) { - await page.keyboard.press("ArrowRight"); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 20, () => + page.keyboard.press("ArrowRight") + ); - for (let i = 0; i < 2; i++) { - await kbBigMoveDown(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 2, () => kbBigMoveDown(page)); - for (let i = 0; i < 20; i++) { - await page.keyboard.press("ArrowLeft"); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 20, () => + page.keyboard.press("ArrowLeft") + ); - for (let i = 0; i < 2; i++) { - await kbBigMoveUp(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 2, () => kbBigMoveUp(page)); - for (let i = 0; i < 2; i++) { - await kbBigMoveRight(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 2, () => kbBigMoveRight(page)); - for (let i = 0; i < 2; i++) { - await kbBigMoveLeft(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 2, () => kbBigMoveLeft(page)); await page.type(`${selectorEditor} .internal`, data); await page.keyboard.press("Escape"); @@ -2292,11 +2267,8 @@ describe("FreeText Editor", () => { const { y: y0, height } = await getRect(page, getEditorSelector(0)); const selectorEditor = getEditorSelector(1); - let xy = await getXY(page, selectorEditor); while ((await getRect(page, selectorEditor)).y > y0 - height) { - await kbBigMoveUp(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); + await moveEditor(page, selectorEditor, 1, () => kbBigMoveUp(page)); } // The editor must be moved in the DOM and potentially the focus @@ -2732,12 +2704,7 @@ describe("FreeText Editor", () => { visible: true, }); - let xy = await getXY(page, selectorEditor); - for (let i = 0; i < 5; i++) { - await kbBigMoveUp(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 5, () => kbBigMoveUp(page)); const data = "Hello PDF.js World !!"; await page.type(`${selectorEditor} .internal`, data); @@ -2762,12 +2729,7 @@ describe("FreeText Editor", () => { visible: true, }); - xy = await getXY(page, selectorEditor); - for (let i = 0; i < 5; i++) { - await kbBigMoveDown(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 5, () => kbBigMoveDown(page)); await page.type(`${selectorEditor} .internal`, data); @@ -2797,12 +2759,7 @@ describe("FreeText Editor", () => { visible: true, }); - let xy = await getXY(page, selectorEditor); - for (let i = 0; i < 10; i++) { - await kbBigMoveLeft(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 10, () => kbBigMoveLeft(page)); const data = "Hello PDF.js World !!"; await page.type(`${selectorEditor} .internal`, data); @@ -2827,12 +2784,9 @@ describe("FreeText Editor", () => { visible: true, }); - xy = await getXY(page, selectorEditor); - for (let i = 0; i < 10; i++) { - await kbBigMoveRight(page); - await waitForPositionChange(page, selectorEditor, xy); - xy = await getXY(page, selectorEditor); - } + await moveEditor(page, selectorEditor, 10, () => + kbBigMoveRight(page) + ); await page.type(`${selectorEditor} .internal`, data); diff --git a/test/integration/test_utils.mjs b/test/integration/test_utils.mjs index c52f5257bb299..c0d5bf838b3d3 100644 --- a/test/integration/test_utils.mjs +++ b/test/integration/test_utils.mjs @@ -558,6 +558,14 @@ function waitForPageRendered(page) { }); } +function waitForEditorMovedInDOM(page) { + return createPromise(page, resolve => { + window.PDFViewerApplication.eventBus.on("editormovedindom", resolve, { + once: true, + }); + }); +} + async function scrollIntoView(page, selector) { const handle = await page.evaluateHandle( sel => [ @@ -862,6 +870,7 @@ export { waitAndClick, waitForAnnotationEditorLayer, waitForAnnotationModeChanged, + waitForEditorMovedInDOM, waitForEntryInStorage, waitForEvent, waitForNoElement,