From 967e34046a5851b058948c4c8d8d2fa880209430 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Sat, 26 Apr 2025 19:15:20 +0200 Subject: [PATCH] Fix the "must work properly when selecting undo by keyboard" integration test This integration test fails intermittently because the undo button can only be activated if focus can be put on it, and that in turn can only happen if it's visible. The test tried to make sure that the undo bar is visible, but checking for the absence of the `hidden` attribute is unfortunately not enough to assert visibility according to Puppeteer documentation [1]. Moreover, the undo button wasn't checked at all. To fix the issue we let Puppeteer do the visibility detection for the undo bar by providing the `visible: true` option to `waitForSelector` [2]. This is consistent with the other tests that already do this, and also with the existing code that detects if the undo bar is hidden (which uses the `hidden: true` option of `waitForSelector`). Moreover, we wait for the undo button to be present before putting focus on it. For consistency, and to avoid intermittent failures elsewhere, we mirror this solution to the other undo bar/button tests of the various editors. [1] https://pptr.dev/api/puppeteer.elementhandle.isvisible [2] https://pptr.dev/api/puppeteer.waitforselectoroptions --- test/integration/freetext_editor_spec.mjs | 7 ++-- test/integration/highlight_editor_spec.mjs | 42 ++++++++++++++-------- test/integration/ink_editor_spec.mjs | 13 +++++-- test/integration/stamp_editor_spec.mjs | 8 +++-- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/test/integration/freetext_editor_spec.mjs b/test/integration/freetext_editor_spec.mjs index cb1729a63..bc5eb57c9 100644 --- a/test/integration/freetext_editor_spec.mjs +++ b/test/integration/freetext_editor_spec.mjs @@ -3233,8 +3233,11 @@ describe("FreeText Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); + await page.waitForSelector("#editorUndoBar", { visible: true }); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); await page.click("#editorUndoBarUndoButton"); await waitForSerialized(page, 1); await page.waitForSelector(editorSelector); @@ -3292,7 +3295,7 @@ describe("FreeText Editor", () => { await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); rect = await getRect(page, ".annotationEditorLayer"); const secondEditorSelector = getEditorSelector(1); const newData = "This is a new text box!"; diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs index f71a3d2e0..bf9204c7d 100644 --- a/test/integration/highlight_editor_spec.mjs +++ b/test/integration/highlight_editor_spec.mjs @@ -2204,8 +2204,11 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); await page.click("#editorUndoBarUndoButton"); await waitForSerialized(page, 1); await page.waitForSelector(editorSelector); @@ -2231,8 +2234,11 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); await page.click("#editorUndoBarUndoButton"); await page.waitForSelector("#editorUndoBar", { hidden: true }); }) @@ -2254,9 +2260,11 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); - await page.waitForSelector("#editorUndoBarCloseButton"); + await page.waitForSelector("#editorUndoBarCloseButton", { + visible: true, + }); await page.click("#editorUndoBarCloseButton"); await page.waitForSelector("#editorUndoBar", { hidden: true }); }) @@ -2278,7 +2286,7 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); const newRect = await getSpanRectFromText(page, 1, "Introduction"); const newX = newRect.x + newRect.width / 2; @@ -2306,7 +2314,7 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); await page.evaluate(() => window.print()); await page.waitForSelector("#editorUndoBar", { hidden: true }); @@ -2329,7 +2337,7 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); await page.click("#printButton"); await page.waitForSelector("#editorUndoBar", { hidden: true }); @@ -2352,7 +2360,7 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); await kbSave(page); await page.waitForSelector("#editorUndoBar", { hidden: true }); @@ -2375,7 +2383,7 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); await page.click("#secondaryToolbarToggleButton"); await page.click("#lastPage"); @@ -2399,7 +2407,7 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); await switchToHighlight(page, /* disable */ true); await page.waitForSelector("#editorUndoBar", { hidden: true }); @@ -2422,7 +2430,7 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); const pdfPath = path.join(__dirname, "../pdfs/basicapi.pdf"); const pdfData = fs.readFileSync(pdfPath).toString("base64"); const dataTransfer = await page.evaluateHandle(data => { @@ -2559,8 +2567,11 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); await page.focus("#editorUndoBarUndoButton"); // we have to simulate focus like this to avoid the wait await page.keyboard.press("Enter"); await waitForSerialized(page, 1); @@ -2572,8 +2583,11 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); await page.focus("#editorUndoBarUndoButton"); // we have to simulate focus like this to avoid the wait await page.keyboard.press(" "); await waitForSerialized(page, 1); @@ -2600,7 +2614,7 @@ describe("Highlight Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); await page.focus("#editorUndoBar"); await page.keyboard.press("Enter"); diff --git a/test/integration/ink_editor_spec.mjs b/test/integration/ink_editor_spec.mjs index 13000bfc4..5a1639208 100644 --- a/test/integration/ink_editor_spec.mjs +++ b/test/integration/ink_editor_spec.mjs @@ -896,8 +896,11 @@ describe("Ink Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); + await page.waitForSelector("#editorUndoBar", { visible: true }); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); await page.click("#editorUndoBarUndoButton"); await waitForSerialized(page, 1); await page.waitForSelector(editorSelector); @@ -966,7 +969,7 @@ describe("Ink Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); const newRect = await getRect(page, ".annotationEditorLayer"); const newXStart = newRect.x + 300; @@ -1093,9 +1096,15 @@ describe("Ink Editor", () => { await waitForSelectedEditor(page, editorSelector); await dragAndDrop(page, editorSelector, [[0, -30]], /* steps = */ 10); await waitForSerialized(page, 2); + await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 1); + await page.waitForSelector("#editorUndoBar", { visible: true }); + + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); await page.click("#editorUndoBarUndoButton"); await page.waitForSelector("#editorUndoBar", { hidden: true }); diff --git a/test/integration/stamp_editor_spec.mjs b/test/integration/stamp_editor_spec.mjs index 5dacf9ba4..709aef997 100644 --- a/test/integration/stamp_editor_spec.mjs +++ b/test/integration/stamp_editor_spec.mjs @@ -1605,9 +1605,11 @@ describe("Stamp Editor", () => { await page.waitForSelector(`${editorSelector} button.delete`); await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); + await page.waitForSelector("#editorUndoBar", { visible: true }); - await page.waitForSelector("#editorUndoBar:not([hidden])"); - + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); await page.click("#editorUndoBarUndoButton"); await waitForSerialized(page, 1); await page.waitForSelector(editorSelector); @@ -1660,7 +1662,7 @@ describe("Stamp Editor", () => { await page.click(`${editorSelector} button.delete`); await waitForSerialized(page, 0); - await page.waitForSelector("#editorUndoBar:not([hidden])"); + await page.waitForSelector("#editorUndoBar", { visible: true }); await page.click("#editorStampAddImage"); const newInput = await page.$("#stampEditorFileInput"); await newInput.uploadFile(