diff --git a/packages/core/lib/v3/handlers/handlerUtils/actHandlerUtils.ts b/packages/core/lib/v3/handlers/handlerUtils/actHandlerUtils.ts index 0b2ac3ce5..78f09d9e3 100644 --- a/packages/core/lib/v3/handlers/handlerUtils/actHandlerUtils.ts +++ b/packages/core/lib/v3/handlers/handlerUtils/actHandlerUtils.ts @@ -77,6 +77,12 @@ export async function performUnderstudyMethod( args: Array.from(args), }); + // Arm the watcher before the action so we do not miss very fast URL changes, + // but start the timeout window only after the click returns. + const navWatcher = shouldWaitForPostClickSettle(method) + ? watchMainFrameUrlChangeStart(page, page.url()) + : null; + try { const handler = METHOD_HANDLER_MAP[method] ?? null; @@ -106,6 +112,11 @@ export async function performUnderstudyMethod( ); } } + if (navWatcher && (await navWatcher.wait(400))) { + await waitForDomNetworkQuiet(page.mainFrame(), domSettleTimeoutMs).catch( + () => {}, + ); + } } catch (e) { const msg = e instanceof Error ? e.message : String(e); const stack = e instanceof Error ? e.stack : undefined; @@ -126,10 +137,111 @@ export async function performUnderstudyMethod( } throw new UnderstudyCommandException(msg, e); } finally { + navWatcher?.dispose(); SessionFileLogger.logUnderstudyActionCompleted(); } } +function shouldWaitForPostClickSettle(method: string): boolean { + return method === "click" || method === "doubleClick"; +} + +function watchMainFrameUrlChangeStart( + page: Page, + initialUrl: string, +): { + wait: (timeoutMs: number) => Promise; + dispose: () => void; +} { + const session = page.mainFrame().session; + const initialMainFrameId = page.mainFrameId(); + let matched = false; + let disposed = false; + let waiter: + | { + resolve: (value: boolean) => void; + timer: ReturnType; + } + | undefined; + + const finish = (value: boolean) => { + if (disposed) return; + // Latch a successful match so wait() can return immediately when a very + // fast navigation starts before the post-click timeout window begins. + if (value) matched = true; + if (waiter) { + clearTimeout(waiter.timer); + const current = waiter; + waiter = undefined; + current.resolve(value); + } + }; + + const dispose = () => { + // dispose() is called from cleanup paths that can overlap, so it must be + // safe to run more than once. + if (disposed) return; + disposed = true; + session.off("Page.frameNavigated", onFrameNavigated); + session.off("Page.navigatedWithinDocument", onNavigatedWithinDocument); + if (waiter) { + clearTimeout(waiter.timer); + waiter.resolve(false); + waiter = undefined; + } + }; + + // Main-frame identity can shift during navigation. Treat both the original + // root frame id and the current page.mainFrameId() as authoritative. + const isMainFrame = (frameId: string): boolean => + frameId === initialMainFrameId || frameId === page.mainFrameId(); + + // We only treat main-frame events with an actual URL change as navigation + // start. Real pages can emit same-URL history events that should not trigger + // a post-click settle wait. + const onFrameNavigated = (event: Protocol.Page.FrameNavigatedEvent) => { + if ( + isMainFrame(event.frame.id) && + event.frame.url && + event.frame.url !== initialUrl + ) { + finish(true); + } + }; + + const onNavigatedWithinDocument = ( + event: Protocol.Page.NavigatedWithinDocumentEvent, + ) => { + if (isMainFrame(event.frameId) && event.url !== initialUrl) { + finish(true); + } + }; + + session.on("Page.frameNavigated", onFrameNavigated); + session.on("Page.navigatedWithinDocument", onNavigatedWithinDocument); + + return { + wait: async (timeoutMs: number) => { + // If navigation started while the click was still in flight, surface that + // immediately instead of burning the full timeout budget after the click. + if (matched) return true; + if (disposed) return false; + return await new Promise((resolve) => { + waiter = { + resolve, + timer: setTimeout(() => { + // Clear waiter before resolving so a late navigation event cannot + // race in and resolve the same wait() call twice. + waiter = undefined; + resolve(false); + }, timeoutMs), + }; + }); + }, + dispose, + }; +} + /* ===================== Handlers & Map ===================== */ const METHOD_HANDLER_MAP: Record< diff --git a/packages/core/tests/integration/perform-understudy-navigation-race.spec.ts b/packages/core/tests/integration/perform-understudy-navigation-race.spec.ts new file mode 100644 index 000000000..a2081f8ef --- /dev/null +++ b/packages/core/tests/integration/perform-understudy-navigation-race.spec.ts @@ -0,0 +1,110 @@ +import { expect, test } from "@playwright/test"; +import { V3 } from "../../lib/v3/v3.js"; +import { performUnderstudyMethod } from "../../lib/v3/handlers/handlerUtils/actHandlerUtils.js"; +import { closeV3 } from "./testUtils.js"; +import { v3DynamicTestConfig } from "./v3.dynamic.config.js"; + +async function loadDelayedDocumentNavigationFixture( + v3: V3, + delayMs: number, +): Promise<{ + page: ReturnType[number]; + sourceUrl: string; + targetUrl: string; +}> { + const page = v3.context.pages()[0]; + const sourceUrl = "https://example.com/"; + const targetUrl = `https://example.com/?nav-race=${delayMs}`; + + await page.goto(sourceUrl, { waitUntil: "load" }); + + await page.mainFrame().evaluate( + ({ delay, nextUrl }) => { + document.open(); + document.write(` + + + + + `); + document.close(); + + (window as typeof window & { __navScheduled?: boolean }).__navScheduled = + false; + document.getElementById("go")?.addEventListener("click", () => { + ( + window as typeof window & { __navScheduled?: boolean } + ).__navScheduled = true; + setTimeout(() => { + window.location.assign(nextUrl); + }, delay); + }); + }, + { delay: delayMs, nextUrl: targetUrl }, + ); + + return { + page, + sourceUrl, + targetUrl, + }; +} + +test.describe("performUnderstudyMethod navigation race", () => { + let v3: V3; + + test.beforeEach(async () => { + v3 = new V3(v3DynamicTestConfig); + await v3.init(); + }); + + test.afterEach(async () => { + await closeV3(v3); + }); + + test("waits for navigation that starts within 400ms of click", async () => { + const { page, targetUrl } = await loadDelayedDocumentNavigationFixture( + v3, + 250, + ); + + await performUnderstudyMethod( + page, + page.mainFrame(), + "click", + "xpath=/html/body/button", + [], + 3_000, + ); + + expect(page.url()).toBe(targetUrl); + }); + + test("does not wait for navigation that starts after 400ms of click", async () => { + const { page, sourceUrl, targetUrl } = + await loadDelayedDocumentNavigationFixture(v3, 900); + + await performUnderstudyMethod( + page, + page.mainFrame(), + "click", + "xpath=/html/body/button", + [], + 3_000, + ); + + expect(page.url()).toBe(sourceUrl); + expect( + await page.evaluate(() => { + return (window as typeof window & { __navScheduled?: boolean }) + .__navScheduled; + }), + ).toBe(true); + + await expect + .poll(() => page.url(), { + timeout: 3_000, + }) + .toBe(targetUrl); + }); +});