Skip to content

Commit

Permalink
Fix intermittent failures when moving a freetext annotation in integr…
Browse files Browse the repository at this point in the history 
…ation 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.
  • Loading branch information
@calixteman
calixteman committed Dec 8, 2024
1 parent 6177bb5 commit 3c31ba4
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 82 deletions.
5 changes: 5 additions & 0 deletions src/display/editor/editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
123 changes: 41 additions & 82 deletions test/integration/freetext_editor_spec.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
switchToEditor,
waitForAnnotationEditorLayer,
waitForAnnotationModeChanged,
waitForEditorMovedInDOM,
waitForSelectedEditor,
waitForSerialized,
waitForStorageEntries,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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))
Expand All @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -2294,7 +2269,9 @@ describe("FreeText Editor", () => {
const selectorEditor = getEditorSelector(1);
let xy = await getXY(page, selectorEditor);
while ((await getRect(page, selectorEditor)).y > y0 - height) {
const handle = await waitForEditorMovedInDOM(page);
await kbBigMoveUp(page);
await awaitPromise(handle);
await waitForPositionChange(page, selectorEditor, xy);
xy = await getXY(page, selectorEditor);
}
Expand Down Expand Up @@ -2732,12 +2709,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);
Expand All @@ -2762,12 +2734,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);

Expand Down Expand Up @@ -2797,12 +2764,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);
Expand All @@ -2827,12 +2789,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);

Expand Down
9 changes: 9 additions & 0 deletions test/integration/test_utils.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 => [
Expand Down Expand Up @@ -862,6 +870,7 @@ export {
waitAndClick,
waitForAnnotationEditorLayer,
waitForAnnotationModeChanged,
waitForEditorMovedInDOM,
waitForEntryInStorage,
waitForEvent,
waitForNoElement,
Expand Down

0 comments on commit 3c31ba4

Please sign in to comment.