Fix waitForText timeout regression in Playwright helper (#5093)

* Initial plan

* Fix waitForText timeout regression in Playwright helper

* Add comprehensive tests for waitForText timeout behavior

* Changes before error encountered

Co-authored-by: kobenguyent <[email protected]>

* Fix waitForText timeout regression by checking title text

* Changes before error encountered

Co-authored-by: kobenguyent <[email protected]>

* Changes before error encountered

Co-authored-by: kobenguyent <[email protected]>

* Fix contextObject.waitForFunction error in iframe contexts

Co-authored-by: kobenguyent <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: kobenguyent <[email protected]>

Author: Copilot

lib/helper/Playwright.js

@@ -2779,61 +2779,62 @@ 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}`)
       }
     }
 
+    // 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
 
-    // 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')
+    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"
+      this.page.waitForFunction(
+        function (text) {
+          // Check body text (original behavior)
+          if (document.body && document.body.innerText && document.body.innerText.indexOf(text) > -1) {
+            return true
           }
-          const textPresent = await contextObject
-            .locator(`:has-text(${JSON.stringify(text)})`)
-            .first()
-            .isVisible()
-          if (!textPresent) retry(errorMessage)
+          // Check document title (fixes the TestEd in title issue)
+          if (document.title && document.title.indexOf(text) > -1) {
+            return true
+          }
+          return false
         },
-        { retries: 10, minTimeout: 100, maxTimeout: 500, factor: 1.5 },
+        text,
+        { timeout: timeoutGap },
       ),
-    ])
-
-    // Clean up when race resolves/rejects
-    return racePromise.finally(() => {
-      if (timeoutId) clearTimeout(timeoutId)
-      shouldStop = true
+      // 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)
     })
   }
 

test/helper/Playwright_test.js

@@ -778,6 +778,64 @@ 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}`)
+      }
+    })
+
+    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', () => {
     it('should grab inner html from an element using xpath query', () =>
       I.amOnPage('/')