From a462f17d0145daecb7c33de0ee565474ed127e5a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 04:03:25 +0000 Subject: [PATCH 1/8] Initial plan From 404e1e4da1e51760b42069fcff15dff71dce6ef7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 04:15:17 +0000 Subject: [PATCH 2/8] Fix waitForText timeout regression in Playwright helper --- lib/helper/Playwright.js | 50 ++++++++++------------------------ test/helper/Playwright_test.js | 24 ++++++++++++++++ 2 files changed, 38 insertions(+), 36 deletions(-) diff --git a/lib/helper/Playwright.js b/lib/helper/Playwright.js index ef91c8d67..a9d09da8e 100644 --- a/lib/helper/Playwright.js +++ b/lib/helper/Playwright.js @@ -2795,42 +2795,20 @@ class Playwright extends Helper { } } - const timeoutGap = waitTimeout + 1000 - - // We add basic timeout to make sure we don't wait forever - // We apply 2 strategies here: wait for text as innert text on page (wide strategy) - older - // or we use native Playwright matcher to wait for text in element (narrow strategy) - newer - // If a user waits for text on a page they are mostly expect it to be there, so wide strategy can be helpful even PW strategy is available - - // Use a flag to stop retries when race resolves - let shouldStop = false - let timeoutId - - const racePromise = Promise.race([ - new Promise((_, reject) => { - timeoutId = setTimeout(() => reject(errorMessage), waitTimeout) - }), - this.page.waitForFunction(text => document.body && document.body.innerText.indexOf(text) > -1, text, { timeout: timeoutGap }), - promiseRetry( - async (retry, number) => { - // Stop retrying if race has resolved - if (shouldStop) { - throw new Error('Operation cancelled') - } - const textPresent = await contextObject - .locator(`:has-text(${JSON.stringify(text)})`) - .first() - .isVisible() - if (!textPresent) retry(errorMessage) - }, - { retries: 10, minTimeout: 100, maxTimeout: 500, factor: 1.5 }, - ), - ]) - - // Clean up when race resolves/rejects - return racePromise.finally(() => { - if (timeoutId) clearTimeout(timeoutId) - shouldStop = true + // Use a simple Promise.race with two complementary strategies: + // 1. page.waitForFunction - checks document.body.innerText (broad search) + // 2. contextObject.locator with :has-text - uses Playwright's native text matching + // Both approaches will wait for the full timeout duration + return Promise.race([ + // Strategy 1: Check document body innerText + this.page.waitForFunction(text => document.body && document.body.innerText.indexOf(text) > -1, text, { timeout: waitTimeout }), + // Strategy 2: Use Playwright's native text locator + contextObject + .locator(`:has-text(${JSON.stringify(text)})`) + .first() + .waitFor({ timeout: waitTimeout, state: 'visible' }), + ]).catch(err => { + throw new Error(errorMessage) }) } diff --git a/test/helper/Playwright_test.js b/test/helper/Playwright_test.js index f02e4a6ec..02edcf5e2 100644 --- a/test/helper/Playwright_test.js +++ b/test/helper/Playwright_test.js @@ -778,6 +778,30 @@ describe('Playwright', function () { .then(() => I.seeInField('#text2', 'London'))) }) + describe('#waitForText timeout fix', () => { + it('should wait for the full timeout duration when text is not found', async function () { + this.timeout(10000) // Allow up to 10 seconds for this test + + const startTime = Date.now() + const timeoutSeconds = 3 // 3 second timeout + + try { + await I.amOnPage('/') + await I.waitForText('ThisTextDoesNotExistAnywhere12345', timeoutSeconds) + // Should not reach here + throw new Error('waitForText should have thrown an error') + } catch (error) { + const elapsedTime = Date.now() - startTime + const expectedTimeout = timeoutSeconds * 1000 + + // Verify it waited close to the full timeout (allow 500ms tolerance) + assert.ok(elapsedTime >= expectedTimeout - 500, `Expected to wait at least ${expectedTimeout - 500}ms, but waited ${elapsedTime}ms`) + assert.ok(elapsedTime <= expectedTimeout + 1000, `Expected to wait at most ${expectedTimeout + 1000}ms, but waited ${elapsedTime}ms`) + assert.ok(error.message.includes('was not found on page after'), `Expected error message about text not found, got: ${error.message}`) + } + }) + }) + describe('#grabHTMLFrom', () => { it('should grab inner html from an element using xpath query', () => I.amOnPage('/') From 4b40471655c5225f52d639ac80f56349cd262bef Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 04:19:23 +0000 Subject: [PATCH 3/8] Add comprehensive tests for waitForText timeout behavior --- test/helper/Playwright_test.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/helper/Playwright_test.js b/test/helper/Playwright_test.js index 02edcf5e2..658d48dd0 100644 --- a/test/helper/Playwright_test.js +++ b/test/helper/Playwright_test.js @@ -800,6 +800,40 @@ describe('Playwright', function () { assert.ok(error.message.includes('was not found on page after'), `Expected error message about text not found, got: ${error.message}`) } }) + + it('should return quickly when text is found', async function () { + this.timeout(5000) + + const startTime = Date.now() + + await I.amOnPage('/') + await I.waitForText('TestEd', 10) // This text should exist on the test page + + const elapsedTime = Date.now() - startTime + // Should find text quickly, within 2 seconds + assert.ok(elapsedTime < 2000, `Expected to find text quickly but took ${elapsedTime}ms`) + }) + + it('should work correctly with context parameter and proper timeout', async function () { + this.timeout(8000) + + const startTime = Date.now() + const timeoutSeconds = 2 + + try { + await I.amOnPage('/') + await I.waitForText('NonExistentTextInBody', timeoutSeconds, 'body') + throw new Error('Should have thrown timeout error') + } catch (error) { + const elapsedTime = Date.now() - startTime + const expectedTimeout = timeoutSeconds * 1000 + + // Verify proper timeout behavior with context + assert.ok(elapsedTime >= expectedTimeout - 500, `Expected to wait at least ${expectedTimeout - 500}ms, but waited ${elapsedTime}ms`) + assert.ok(elapsedTime <= expectedTimeout + 1000, `Expected to wait at most ${expectedTimeout + 1000}ms, but waited ${elapsedTime}ms`) + assert.ok(error.message.includes('was not found on page after'), `Expected timeout error message, got: ${error.message}`) + } + }) }) describe('#grabHTMLFrom', () => { From 9339e5ce03025abcda4ce2ecc63a879fdf06fd8d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 08:20:05 +0000 Subject: [PATCH 4/8] Changes before error encountered Co-authored-by: kobenguyent <7845001+kobenguyent@users.noreply.github.com> --- lib/helper/Playwright.js | 41 +++++++++++++++++++++++++++------------- 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/lib/helper/Playwright.js b/lib/helper/Playwright.js index a9d09da8e..51a0b3125 100644 --- a/lib/helper/Playwright.js +++ b/lib/helper/Playwright.js @@ -2776,19 +2776,26 @@ class Playwright extends Helper { .locator(`${locator.isCustom() ? `${locator.type}=${locator.value}` : locator.simplify()} >> text=${text}`) .first() .waitFor({ timeout: waitTimeout, state: 'visible' }) + .catch(e => { + throw new Error(errorMessage) + }) } if (locator.isXPath()) { - return contextObject.waitForFunction( - ([locator, text, $XPath]) => { - eval($XPath) - const el = $XPath(null, locator) - if (!el.length) return false - return el[0].innerText.indexOf(text) > -1 - }, - [locator.value, text, $XPath.toString()], - { timeout: waitTimeout }, - ) + return contextObject + .waitForFunction( + ([locator, text, $XPath]) => { + eval($XPath) + const el = $XPath(null, locator) + if (!el.length) return false + return el[0].innerText.indexOf(text) > -1 + }, + [locator.value, text, $XPath.toString()], + { timeout: waitTimeout }, + ) + .catch(e => { + throw new Error(errorMessage) + }) } } catch (e) { throw new Error(`${errorMessage}\n${e.message}`) @@ -2796,12 +2803,20 @@ class Playwright extends Helper { } // Use a simple Promise.race with two complementary strategies: - // 1. page.waitForFunction - checks document.body.innerText (broad search) + // 1. page.waitForFunction - checks entire document text content including title, body, etc. // 2. contextObject.locator with :has-text - uses Playwright's native text matching // Both approaches will wait for the full timeout duration return Promise.race([ - // Strategy 1: Check document body innerText - this.page.waitForFunction(text => document.body && document.body.innerText.indexOf(text) > -1, text, { timeout: waitTimeout }), + // Strategy 1: Check entire document text content (including title) + this.page.waitForFunction( + text => { + // Check document text content - includes title and all visible text + const documentText = document.documentElement.innerText || document.documentElement.textContent || '' + return documentText.indexOf(text) > -1 + }, + text, + { timeout: waitTimeout }, + ), // Strategy 2: Use Playwright's native text locator contextObject .locator(`:has-text(${JSON.stringify(text)})`) From 10f97e6a64868683e84eb568872c3d425317efe5 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 11:56:52 +0000 Subject: [PATCH 5/8] Fix waitForText timeout regression by checking title text --- lib/helper/Playwright.js | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/lib/helper/Playwright.js b/lib/helper/Playwright.js index 51a0b3125..5246548b9 100644 --- a/lib/helper/Playwright.js +++ b/lib/helper/Playwright.js @@ -2802,22 +2802,27 @@ class Playwright extends Helper { } } - // Use a simple Promise.race with two complementary strategies: - // 1. page.waitForFunction - checks entire document text content including title, body, etc. - // 2. contextObject.locator with :has-text - uses Playwright's native text matching - // Both approaches will wait for the full timeout duration + // Use two complementary strategies that both respect the full timeout: + // Strategy 1: waitForFunction checks body text AND title text + // Strategy 2: native Playwright locator for visible elements return Promise.race([ - // Strategy 1: Check entire document text content (including title) - this.page.waitForFunction( + // Strategy 1: Check both body text and title text (fixes original bug) + contextObject.waitForFunction( text => { - // Check document text content - includes title and all visible text - const documentText = document.documentElement.innerText || document.documentElement.textContent || '' - return documentText.indexOf(text) > -1 + // Check body text (original behavior) + if (document.body && document.body.innerText.indexOf(text) > -1) { + return true + } + // Also check document title (new addition to fix title text detection) + if (document.title && document.title.indexOf(text) > -1) { + return true + } + return false }, text, { timeout: waitTimeout }, ), - // Strategy 2: Use Playwright's native text locator + // Strategy 2: Use Playwright's native text locator for visible elements contextObject .locator(`:has-text(${JSON.stringify(text)})`) .first() From 0d51f890a47a1a0f5f65b41e6f2d89d8e954e93d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 12:38:29 +0000 Subject: [PATCH 6/8] Changes before error encountered Co-authored-by: kobenguyent <7845001+kobenguyent@users.noreply.github.com> --- lib/helper/Playwright.js | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/lib/helper/Playwright.js b/lib/helper/Playwright.js index 5246548b9..43b1f6842 100644 --- a/lib/helper/Playwright.js +++ b/lib/helper/Playwright.js @@ -2802,34 +2802,31 @@ class Playwright extends Helper { } } - // Use two complementary strategies that both respect the full timeout: - // Strategy 1: waitForFunction checks body text AND title text - // Strategy 2: native Playwright locator for visible elements - return Promise.race([ - // Strategy 1: Check both body text and title text (fixes original bug) - contextObject.waitForFunction( + // Simple approach: use waitForFunction that checks all text sources + // This handles both body text and title text in one strategy + return contextObject + .waitForFunction( text => { // Check body text (original behavior) if (document.body && document.body.innerText.indexOf(text) > -1) { return true } - // Also check document title (new addition to fix title text detection) + // Check document title (fixes the TestEd in title issue) if (document.title && document.title.indexOf(text) > -1) { return true } + // Also check document.documentElement.innerText which includes title + if (document.documentElement && document.documentElement.innerText.indexOf(text) > -1) { + return true + } return false }, text, { timeout: waitTimeout }, - ), - // Strategy 2: Use Playwright's native text locator for visible elements - contextObject - .locator(`:has-text(${JSON.stringify(text)})`) - .first() - .waitFor({ timeout: waitTimeout, state: 'visible' }), - ]).catch(err => { - throw new Error(errorMessage) - }) + ) + .catch(err => { + throw new Error(errorMessage) + }) } /** From f40a8e13547f2827b201c9a0b3fdfd04eb308c55 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 14:30:33 +0000 Subject: [PATCH 7/8] Changes before error encountered Co-authored-by: kobenguyent <7845001+kobenguyent@users.noreply.github.com> --- lib/helper/Playwright.js | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/helper/Playwright.js b/lib/helper/Playwright.js index 43b1f6842..8763e8f1b 100644 --- a/lib/helper/Playwright.js +++ b/lib/helper/Playwright.js @@ -2802,31 +2802,36 @@ class Playwright extends Helper { } } - // Simple approach: use waitForFunction that checks all text sources - // This handles both body text and title text in one strategy - return contextObject - .waitForFunction( - text => { + // Based on original implementation but fixed to check title text and remove problematic promiseRetry + // Original used timeoutGap for waitForFunction to give it slightly more time than the locator + const timeoutGap = waitTimeout + 1000 + + return Promise.race([ + // Strategy 1: waitForFunction that checks both body AND title text + // Original only checked document.body.innerText, missing title text like "TestEd" + contextObject.waitForFunction( + function (text) { // Check body text (original behavior) - if (document.body && document.body.innerText.indexOf(text) > -1) { + if (document.body && document.body.innerText && document.body.innerText.indexOf(text) > -1) { return true } // Check document title (fixes the TestEd in title issue) if (document.title && document.title.indexOf(text) > -1) { return true } - // Also check document.documentElement.innerText which includes title - if (document.documentElement && document.documentElement.innerText.indexOf(text) > -1) { - return true - } return false }, text, - { timeout: waitTimeout }, - ) - .catch(err => { - throw new Error(errorMessage) - }) + { timeout: timeoutGap }, + ), + // Strategy 2: Native Playwright text locator (replaces problematic promiseRetry) + contextObject + .locator(`:has-text(${JSON.stringify(text)})`) + .first() + .waitFor({ timeout: waitTimeout }), + ]).catch(err => { + throw new Error(errorMessage) + }) } /** From 7f8ae35ec999e52601253cc6a1307fb920b92657 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 22 Aug 2025 14:42:03 +0000 Subject: [PATCH 8/8] Fix contextObject.waitForFunction error in iframe contexts Co-authored-by: kobenguyent <7845001+kobenguyent@users.noreply.github.com> --- lib/helper/Playwright.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/helper/Playwright.js b/lib/helper/Playwright.js index 8763e8f1b..31e27a9f8 100644 --- a/lib/helper/Playwright.js +++ b/lib/helper/Playwright.js @@ -2808,8 +2808,9 @@ class Playwright extends Helper { return Promise.race([ // Strategy 1: waitForFunction that checks both body AND title text + // Use this.page instead of contextObject because FrameLocator doesn't have waitForFunction // Original only checked document.body.innerText, missing title text like "TestEd" - contextObject.waitForFunction( + this.page.waitForFunction( function (text) { // Check body text (original behavior) if (document.body && document.body.innerText && document.body.innerText.indexOf(text) > -1) {