From 5c5c107ec5813d04ba0f414f115ebcf3e85a71aa Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Thu, 2 May 2024 19:19:26 +0200 Subject: [PATCH 1/2] Remove most `waitForTimeout` usage from the freetext editor integration tests This commit replaces a `waitForTimeout` occurrence with an equivalent `waitForSelector` expression, and removes two other `waitForTimeout` occurrences that are obsolete because we already wait for an observable event to trigger or class change to happen. Note that the other `waitForTimeout` occurrences in this file are either part of #17931 or remain until we find a good way to ensure that nothing happened (because currently there is nothing we can await there). --- test/integration/freetext_editor_spec.mjs | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/test/integration/freetext_editor_spec.mjs b/test/integration/freetext_editor_spec.mjs index 5bce50fb9b11d..2b21ba4bda9c0 100644 --- a/test/integration/freetext_editor_spec.mjs +++ b/test/integration/freetext_editor_spec.mjs @@ -56,9 +56,6 @@ const copyPaste = async page => { await kbCopy(page); await promise; - // eslint-disable-next-line no-restricted-syntax - await waitForTimeout(10); - promise = waitForEvent(page, "paste"); await kbPaste(page); await promise; @@ -1364,9 +1361,6 @@ describe("FreeText Editor", () => { // Enter in editing mode. await switchToFreeText(page); - // eslint-disable-next-line no-restricted-syntax - await waitForTimeout(200); - // Disable editing mode. await page.click("#editorFreeText"); await page.waitForSelector( @@ -2411,14 +2405,7 @@ describe("FreeText Editor", () => { // The editor must be moved in the DOM and potentially the focus // will be lost, hence there's a callback will get back the focus. - // eslint-disable-next-line no-restricted-syntax - await waitForTimeout(200); - - const focused = await page.evaluate(sel => { - const editor = document.querySelector(sel); - return editor === document.activeElement; - }, getEditorSelector(1)); - expect(focused).withContext(`In ${browserName}`).toEqual(true); + await page.waitForSelector(`${getEditorSelector(1)}:focus`); expect(await pos(0)) .withContext(`In ${browserName}`) From bb743389aaf068d59ec659b6c0d3f2582cbd98c9 Mon Sep 17 00:00:00 2001 From: Tim van der Meij Date: Thu, 2 May 2024 19:23:55 +0200 Subject: [PATCH 2/2] Use `waitForSelector` instead of `waitForFunction` for focus checks This commit replaces `waitForFunction` calls that use `document.activeElement` to wait for an element to get focus by simpler `waitForSelector` expressions that use the `:focus` selector. Note that we already use this in other tests, so this improves consistency too. --- test/integration/annotation_spec.mjs | 8 +------- test/integration/highlight_editor_spec.mjs | 12 +++--------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/test/integration/annotation_spec.mjs b/test/integration/annotation_spec.mjs index 7a6b123e332ea..579ee2d0ce1f9 100644 --- a/test/integration/annotation_spec.mjs +++ b/test/integration/annotation_spec.mjs @@ -74,13 +74,7 @@ describe("Annotation highlight", () => { pages.map(async ([browserName, page]) => { for (const i of [23, 22, 14]) { await page.click(`[data-annotation-id='${i}R']`); - await page.waitForFunction( - id => - document.activeElement === - document.querySelector(`#pdfjs_internal_id_${id}R`), - {}, - i - ); + await page.waitForSelector(`#pdfjs_internal_id_${i}R:focus`); } }) ); diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs index 793250926b25f..f506279ea19d7 100644 --- a/test/integration/highlight_editor_spec.mjs +++ b/test/integration/highlight_editor_spec.mjs @@ -1591,18 +1591,12 @@ describe("Highlight Editor", () => { await page.focus(getEditorSelector(1)); await kbFocusPrevious(page); - await page.waitForFunction( - sel => document.querySelector(sel) === document.activeElement, - {}, - `.page[data-page-number="1"] > .textLayer` + await page.waitForSelector( + `.page[data-page-number="1"] > .textLayer:focus` ); await kbFocusNext(page); - await page.waitForFunction( - sel => document.querySelector(sel) === document.activeElement, - {}, - getEditorSelector(1) - ); + await page.waitForSelector(`${getEditorSelector(1)}:focus`); }) ); });