Skip to content

Commit 967e340

Browse files
timvandermeijtimvandermeij
committed
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
1 parent 9e1f93e commit 967e340

4 files changed

Lines changed: 49 additions & 21 deletions

File tree

‎test/integration/freetext_editor_spec.mjs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3233,8 +3233,11 @@ describe("FreeText Editor", () => {
32333233
await page.waitForSelector(`${editorSelector} button.delete`);
32343234
await page.click(`${editorSelector} button.delete`);
32353235
await waitForSerialized(page, 0);
3236+
await page.waitForSelector("#editorUndoBar", { visible: true });
32363237

3237-
await page.waitForSelector("#editorUndoBar:not([hidden])");
3238+
await page.waitForSelector("#editorUndoBarUndoButton", {
3239+
visible: true,
3240+
});
32383241
await page.click("#editorUndoBarUndoButton");
32393242
await waitForSerialized(page, 1);
32403243
await page.waitForSelector(editorSelector);
@@ -3292,7 +3295,7 @@ describe("FreeText Editor", () => {
32923295
await page.click(`${editorSelector} button.delete`);
32933296
await waitForSerialized(page, 0);
32943297

3295-
await page.waitForSelector("#editorUndoBar:not([hidden])");
3298+
await page.waitForSelector("#editorUndoBar", { visible: true });
32963299
rect = await getRect(page, ".annotationEditorLayer");
32973300
const secondEditorSelector = getEditorSelector(1);
32983301
const newData = "This is a new text box!";

‎test/integration/highlight_editor_spec.mjs‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2204,8 +2204,11 @@ describe("Highlight Editor", () => {
22042204
await page.waitForSelector(`${editorSelector} button.delete`);
22052205
await page.click(`${editorSelector} button.delete`);
22062206
await waitForSerialized(page, 0);
2207-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2207+
await page.waitForSelector("#editorUndoBar", { visible: true });
22082208

2209+
await page.waitForSelector("#editorUndoBarUndoButton", {
2210+
visible: true,
2211+
});
22092212
await page.click("#editorUndoBarUndoButton");
22102213
await waitForSerialized(page, 1);
22112214
await page.waitForSelector(editorSelector);
@@ -2231,8 +2234,11 @@ describe("Highlight Editor", () => {
22312234
await page.waitForSelector(`${editorSelector} button.delete`);
22322235
await page.click(`${editorSelector} button.delete`);
22332236
await waitForSerialized(page, 0);
2234-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2237+
await page.waitForSelector("#editorUndoBar", { visible: true });
22352238

2239+
await page.waitForSelector("#editorUndoBarUndoButton", {
2240+
visible: true,
2241+
});
22362242
await page.click("#editorUndoBarUndoButton");
22372243
await page.waitForSelector("#editorUndoBar", { hidden: true });
22382244
})
@@ -2254,9 +2260,11 @@ describe("Highlight Editor", () => {
22542260
await page.waitForSelector(`${editorSelector} button.delete`);
22552261
await page.click(`${editorSelector} button.delete`);
22562262
await waitForSerialized(page, 0);
2257-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2263+
await page.waitForSelector("#editorUndoBar", { visible: true });
22582264

2259-
await page.waitForSelector("#editorUndoBarCloseButton");
2265+
await page.waitForSelector("#editorUndoBarCloseButton", {
2266+
visible: true,
2267+
});
22602268
await page.click("#editorUndoBarCloseButton");
22612269
await page.waitForSelector("#editorUndoBar", { hidden: true });
22622270
})
@@ -2278,7 +2286,7 @@ describe("Highlight Editor", () => {
22782286
await page.waitForSelector(`${editorSelector} button.delete`);
22792287
await page.click(`${editorSelector} button.delete`);
22802288
await waitForSerialized(page, 0);
2281-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2289+
await page.waitForSelector("#editorUndoBar", { visible: true });
22822290

22832291
const newRect = await getSpanRectFromText(page, 1, "Introduction");
22842292
const newX = newRect.x + newRect.width / 2;
@@ -2306,7 +2314,7 @@ describe("Highlight Editor", () => {
23062314
await page.waitForSelector(`${editorSelector} button.delete`);
23072315
await page.click(`${editorSelector} button.delete`);
23082316
await waitForSerialized(page, 0);
2309-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2317+
await page.waitForSelector("#editorUndoBar", { visible: true });
23102318

23112319
await page.evaluate(() => window.print());
23122320
await page.waitForSelector("#editorUndoBar", { hidden: true });
@@ -2329,7 +2337,7 @@ describe("Highlight Editor", () => {
23292337
await page.waitForSelector(`${editorSelector} button.delete`);
23302338
await page.click(`${editorSelector} button.delete`);
23312339
await waitForSerialized(page, 0);
2332-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2340+
await page.waitForSelector("#editorUndoBar", { visible: true });
23332341

23342342
await page.click("#printButton");
23352343
await page.waitForSelector("#editorUndoBar", { hidden: true });
@@ -2352,7 +2360,7 @@ describe("Highlight Editor", () => {
23522360
await page.waitForSelector(`${editorSelector} button.delete`);
23532361
await page.click(`${editorSelector} button.delete`);
23542362
await waitForSerialized(page, 0);
2355-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2363+
await page.waitForSelector("#editorUndoBar", { visible: true });
23562364

23572365
await kbSave(page);
23582366
await page.waitForSelector("#editorUndoBar", { hidden: true });
@@ -2375,7 +2383,7 @@ describe("Highlight Editor", () => {
23752383
await page.waitForSelector(`${editorSelector} button.delete`);
23762384
await page.click(`${editorSelector} button.delete`);
23772385
await waitForSerialized(page, 0);
2378-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2386+
await page.waitForSelector("#editorUndoBar", { visible: true });
23792387

23802388
await page.click("#secondaryToolbarToggleButton");
23812389
await page.click("#lastPage");
@@ -2399,7 +2407,7 @@ describe("Highlight Editor", () => {
23992407
await page.waitForSelector(`${editorSelector} button.delete`);
24002408
await page.click(`${editorSelector} button.delete`);
24012409
await waitForSerialized(page, 0);
2402-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2410+
await page.waitForSelector("#editorUndoBar", { visible: true });
24032411

24042412
await switchToHighlight(page, /* disable */ true);
24052413
await page.waitForSelector("#editorUndoBar", { hidden: true });
@@ -2422,7 +2430,7 @@ describe("Highlight Editor", () => {
24222430
await page.waitForSelector(`${editorSelector} button.delete`);
24232431
await page.click(`${editorSelector} button.delete`);
24242432
await waitForSerialized(page, 0);
2425-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2433+
await page.waitForSelector("#editorUndoBar", { visible: true });
24262434
const pdfPath = path.join(__dirname, "../pdfs/basicapi.pdf");
24272435
const pdfData = fs.readFileSync(pdfPath).toString("base64");
24282436
const dataTransfer = await page.evaluateHandle(data => {
@@ -2559,8 +2567,11 @@ describe("Highlight Editor", () => {
25592567
await page.waitForSelector(`${editorSelector} button.delete`);
25602568
await page.click(`${editorSelector} button.delete`);
25612569
await waitForSerialized(page, 0);
2562-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2570+
await page.waitForSelector("#editorUndoBar", { visible: true });
25632571

2572+
await page.waitForSelector("#editorUndoBarUndoButton", {
2573+
visible: true,
2574+
});
25642575
await page.focus("#editorUndoBarUndoButton"); // we have to simulate focus like this to avoid the wait
25652576
await page.keyboard.press("Enter");
25662577
await waitForSerialized(page, 1);
@@ -2572,8 +2583,11 @@ describe("Highlight Editor", () => {
25722583
await page.waitForSelector(`${editorSelector} button.delete`);
25732584
await page.click(`${editorSelector} button.delete`);
25742585
await waitForSerialized(page, 0);
2575-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2586+
await page.waitForSelector("#editorUndoBar", { visible: true });
25762587

2588+
await page.waitForSelector("#editorUndoBarUndoButton", {
2589+
visible: true,
2590+
});
25772591
await page.focus("#editorUndoBarUndoButton"); // we have to simulate focus like this to avoid the wait
25782592
await page.keyboard.press(" ");
25792593
await waitForSerialized(page, 1);
@@ -2600,7 +2614,7 @@ describe("Highlight Editor", () => {
26002614
await page.waitForSelector(`${editorSelector} button.delete`);
26012615
await page.click(`${editorSelector} button.delete`);
26022616
await waitForSerialized(page, 0);
2603-
await page.waitForSelector("#editorUndoBar:not([hidden])");
2617+
await page.waitForSelector("#editorUndoBar", { visible: true });
26042618

26052619
await page.focus("#editorUndoBar");
26062620
await page.keyboard.press("Enter");

‎test/integration/ink_editor_spec.mjs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -896,8 +896,11 @@ describe("Ink Editor", () => {
896896
await page.waitForSelector(`${editorSelector} button.delete`);
897897
await page.click(`${editorSelector} button.delete`);
898898
await waitForSerialized(page, 0);
899+
await page.waitForSelector("#editorUndoBar", { visible: true });
899900

900-
await page.waitForSelector("#editorUndoBar:not([hidden])");
901+
await page.waitForSelector("#editorUndoBarUndoButton", {
902+
visible: true,
903+
});
901904
await page.click("#editorUndoBarUndoButton");
902905
await waitForSerialized(page, 1);
903906
await page.waitForSelector(editorSelector);
@@ -966,7 +969,7 @@ describe("Ink Editor", () => {
966969
await page.waitForSelector(`${editorSelector} button.delete`);
967970
await page.click(`${editorSelector} button.delete`);
968971
await waitForSerialized(page, 0);
969-
await page.waitForSelector("#editorUndoBar:not([hidden])");
972+
await page.waitForSelector("#editorUndoBar", { visible: true });
970973

971974
const newRect = await getRect(page, ".annotationEditorLayer");
972975
const newXStart = newRect.x + 300;
@@ -1093,9 +1096,15 @@ describe("Ink Editor", () => {
10931096
await waitForSelectedEditor(page, editorSelector);
10941097
await dragAndDrop(page, editorSelector, [[0, -30]], /* steps = */ 10);
10951098
await waitForSerialized(page, 2);
1099+
10961100
await page.waitForSelector(`${editorSelector} button.delete`);
10971101
await page.click(`${editorSelector} button.delete`);
10981102
await waitForSerialized(page, 1);
1103+
await page.waitForSelector("#editorUndoBar", { visible: true });
1104+
1105+
await page.waitForSelector("#editorUndoBarUndoButton", {
1106+
visible: true,
1107+
});
10991108
await page.click("#editorUndoBarUndoButton");
11001109
await page.waitForSelector("#editorUndoBar", { hidden: true });
11011110

‎test/integration/stamp_editor_spec.mjs‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1605,9 +1605,11 @@ describe("Stamp Editor", () => {
16051605
await page.waitForSelector(`${editorSelector} button.delete`);
16061606
await page.click(`${editorSelector} button.delete`);
16071607
await waitForSerialized(page, 0);
1608+
await page.waitForSelector("#editorUndoBar", { visible: true });
16081609

1609-
await page.waitForSelector("#editorUndoBar:not([hidden])");
1610-
1610+
await page.waitForSelector("#editorUndoBarUndoButton", {
1611+
visible: true,
1612+
});
16111613
await page.click("#editorUndoBarUndoButton");
16121614
await waitForSerialized(page, 1);
16131615
await page.waitForSelector(editorSelector);
@@ -1660,7 +1662,7 @@ describe("Stamp Editor", () => {
16601662
await page.click(`${editorSelector} button.delete`);
16611663
await waitForSerialized(page, 0);
16621664

1663-
await page.waitForSelector("#editorUndoBar:not([hidden])");
1665+
await page.waitForSelector("#editorUndoBar", { visible: true });
16641666
await page.click("#editorStampAddImage");
16651667
const newInput = await page.$("#stampEditorFileInput");
16661668
await newInput.uploadFile(

0 commit comments

Comments
 (0)