diff --git a/extensions/browser/src/browser/pw-session.get-page-for-targetid.test.ts b/extensions/browser/src/browser/pw-session.get-page-for-targetid.test.ts index 4ed5912754fb..48063f6d75a5 100644 --- a/extensions/browser/src/browser/pw-session.get-page-for-targetid.test.ts +++ b/extensions/browser/src/browser/pw-session.get-page-for-targetid.test.ts @@ -9,6 +9,9 @@ import { focusPageByTargetIdViaPlaywright, getPageForTargetId, listPagesViaPlaywright, + rememberRoleRefsForTarget, + restoreRoleRefsForTarget, + ensurePageState, setCdpConnectRetryDelayMsForTests, } from "./pw-session.js"; @@ -20,12 +23,15 @@ type MockPageSpec = { url?: string; title?: string; targetLookupError?: string; + navigateDuringTargetLookup?: boolean; + subframeNavigationDuringTargetLookup?: boolean; }; type BrowserMockBundle = { browser: import("playwright-core").Browser; browserClose: ReturnType; pages: import("playwright-core").Page[]; + pageHandlers: Array void>>>; pageActions: Array<{ bringToFront: ReturnType; close: ReturnType; @@ -39,12 +45,18 @@ function makeBrowser(pages: MockPageSpec[]): BrowserMockBundle { bringToFront: vi.fn(async () => {}), close: vi.fn(async () => {}), })); + const pageHandlers = pages.map(() => new Map void>>()); const pageObjects = pages.map((spec, index) => { const actions = pageActions[index]!; + const handlers = pageHandlers[index]!; + const mainFrame = { url: () => spec.url ?? `https://page-${index + 1}.example` }; const page = { - on: vi.fn(), + on: vi.fn((event: string, handler: (...args: unknown[]) => void) => { + handlers.set(event, [...(handlers.get(event) ?? []), handler]); + }), context: () => context, + mainFrame: () => mainFrame, title: vi.fn(async () => spec.title ?? spec.targetId ?? `page-${index + 1}`), url: vi.fn(() => spec.url ?? `https://page-${index + 1}.example`), bringToFront: actions.bringToFront, @@ -67,6 +79,16 @@ function makeBrowser(pages: MockPageSpec[]): BrowserMockBundle { if (spec?.targetLookupError) { throw new Error(spec.targetLookupError); } + if (spec?.navigateDuringTargetLookup) { + const pageIndex = pageObjects.indexOf(page); + pageHandlers[pageIndex]?.get("framenavigated")?.[0]?.(page.mainFrame()); + } + if (spec?.subframeNavigationDuringTargetLookup) { + const pageIndex = pageObjects.indexOf(page); + pageHandlers[pageIndex]?.get("framenavigated")?.[0]?.({ + url: () => "https://frame.example/new", + }); + } return { targetInfo: { targetId: spec?.targetId } }; }), detach: vi.fn(async () => {}), @@ -81,7 +103,7 @@ function makeBrowser(pages: MockPageSpec[]): BrowserMockBundle { close: browserClose, } as unknown as import("playwright-core").Browser; - return { browser, browserClose, pages: pageObjects, pageActions }; + return { browser, browserClose, pages: pageObjects, pageHandlers, pageActions }; } function installBrowser(pages: MockPageSpec[]): BrowserMockBundle { @@ -160,6 +182,58 @@ describe("pw-session getPageForTargetId", () => { expect(resolved).toBe(pages[1]); }); + it("binds a resolved replacement page to target-cache invalidation", async () => { + const cdpUrl = "http://127.0.0.1:18792"; + const targetId = "TARGET_REPLACED"; + rememberRoleRefsForTarget({ + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "Old document" } }, + }); + const { pages, pageHandlers } = installBrowser([{ targetId }]); + const page = await getPageForTargetId({ cdpUrl, targetId }); + const navigationHandler = pageHandlers[0]?.get("framenavigated")?.[0]; + navigationHandler?.(page.mainFrame()); + restoreRoleRefsForTarget({ cdpUrl, targetId, page }); + + expect(navigationHandler).toBeTypeOf("function"); + expect(ensurePageState(page).roleRefs).toBeUndefined(); + expect(page).toBe(pages[0]); + }); + + it("invalidates target refs when navigation races target lookup", async () => { + const cdpUrl = "http://127.0.0.1:18792"; + const targetId = "TARGET_RACE"; + rememberRoleRefsForTarget({ + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "Old document" } }, + }); + installBrowser([{ targetId, navigateDuringTargetLookup: true }]); + + const page = await getPageForTargetId({ cdpUrl, targetId }); + restoreRoleRefsForTarget({ cdpUrl, targetId, page }); + + expect(ensurePageState(page).roleRefs).toBeUndefined(); + }); + + it("invalidates page-wide aria refs when subframe navigation races target lookup", async () => { + const cdpUrl = "http://127.0.0.1:18792"; + const targetId = "TARGET_ARIA_RACE"; + rememberRoleRefsForTarget({ + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "Old embedded document" } }, + mode: "aria", + }); + installBrowser([{ targetId, subframeNavigationDuringTargetLookup: true }]); + + const page = await getPageForTargetId({ cdpUrl, targetId }); + restoreRoleRefsForTarget({ cdpUrl, targetId, page }); + + expect(ensurePageState(page).roleRefs).toBeUndefined(); + }); + it("focuses and closes only the exact target when URLs are identical", async () => { const { pageActions } = installBrowser([ { targetId: "TARGET_A", url: "https://same.example" }, diff --git a/extensions/browser/src/browser/pw-session.test.ts b/extensions/browser/src/browser/pw-session.test.ts index 4625dd61058a..e24c1e6436ef 100644 --- a/extensions/browser/src/browser/pw-session.test.ts +++ b/extensions/browser/src/browser/pw-session.test.ts @@ -1,7 +1,7 @@ // Browser tests cover pw session plugin behavior. import fs from "node:fs/promises"; import path from "node:path"; -import type { Page } from "playwright-core"; +import type { Frame, Page } from "playwright-core"; import { afterEach, describe, expect, it, vi } from "vitest"; import { DEFAULT_DOWNLOAD_DIR } from "./paths.js"; import { createDownloadCaptureForPage } from "./pw-download-capture.js"; @@ -12,6 +12,7 @@ import { refLocator, rememberRoleRefsForTarget, restoreRoleRefsForTarget, + storeRoleRefsForTarget, } from "./pw-session.js"; import { BROWSER_REF_MARKER_ATTRIBUTE } from "./pw-session.page-cdp.js"; @@ -29,8 +30,12 @@ afterEach(() => { function fakePage(): { page: Page; handlers: Map void>>; + mainFrame: { url: () => string }; + selectedFrame: Frame; mocks: { on: ReturnType; + frameGetByRole: ReturnType; + frameQuery: ReturnType; getByRole: ReturnType; frameLocator: ReturnType; locator: ReturnType; @@ -56,17 +61,37 @@ function fakePage(): { getByRole: vi.fn(() => ({ nth: vi.fn(() => ({ ok: true })) })), locator: vi.fn(() => ({ nth: vi.fn(() => ({ ok: true })) })), })); - const locator = vi.fn(() => ({ nth: vi.fn(() => ({ ok: true })) })); + const frameGetByRole = vi.fn(() => ({ nth: vi.fn(() => ({ ok: true })) })); + const frameQuery = vi.fn(() => ({ nth: vi.fn(() => ({ ok: true })) })); + const selectedFrame = { + url: () => "https://frame.example.com", + getByRole: frameGetByRole, + locator: frameQuery, + } as unknown as Frame; + const locator = vi.fn(() => ({ + nth: vi.fn(() => ({ ok: true })), + elementHandle: vi.fn(async () => ({ + contentFrame: vi.fn(async () => selectedFrame), + })), + })); + const mainFrame = { url: () => "https://test.example.com" }; const page = { on, off, getByRole, frameLocator, locator, + mainFrame: () => mainFrame, } as unknown as Page; - return { page, handlers, mocks: { on, getByRole, frameLocator, locator } }; + return { + page, + handlers, + mainFrame, + selectedFrame, + mocks: { on, frameGetByRole, frameQuery, getByRole, frameLocator, locator }, + }; } function firstSavePath(saveAs: MutableDownload["saveAs"]): string { @@ -82,15 +107,20 @@ function firstSavePath(saveAs: MutableDownload["saveAs"]): string { } describe("pw-session refLocator", () => { - it("uses frameLocator for role refs when snapshot was scoped to a frame", () => { - const { page, mocks } = fakePage(); + it("uses the captured Frame for refs from a frame-scoped snapshot", () => { + const { page, selectedFrame, mocks } = fakePage(); const state = ensurePageState(page); state.roleRefs = { e1: { role: "button", name: "OK" } }; state.roleRefsFrameSelector = "iframe#main"; + state.roleRefsFrame = selectedFrame; refLocator(page, "e1"); - expect(mocks.frameLocator).toHaveBeenCalledWith("iframe#main"); + expect(mocks.frameGetByRole).toHaveBeenCalledWith("button", { + name: "OK", + exact: true, + }); + expect(mocks.frameLocator).not.toHaveBeenCalled(); }); it("uses page getByRole for role refs by default", () => { @@ -142,7 +172,7 @@ describe("pw-session refLocator", () => { }); describe("pw-session role refs cache", () => { - it("restores refs for a different Page instance (same CDP targetId)", () => { + it("restores unscoped refs for a different Page instance", () => { const cdpUrl = "http://127.0.0.1:9222"; const targetId = "t1"; @@ -150,15 +180,136 @@ describe("pw-session role refs cache", () => { cdpUrl, targetId, refs: { e1: { role: "button", name: "OK" } }, - frameSelector: "iframe#main", + mode: "role", }); const { page, mocks } = fakePage(); restoreRoleRefsForTarget({ cdpUrl, targetId, page }); refLocator(page, "e1"); - expect(mocks.frameLocator).toHaveBeenCalledWith("iframe#main"); + expect(mocks.getByRole).toHaveBeenCalled(); }); + + it("does not restore frame-scoped refs onto a replacement Page", () => { + const cdpUrl = "http://127.0.0.1:9222"; + const targetId = "frame-target"; + rememberRoleRefsForTarget({ + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "Old frame" } }, + frameSelector: "iframe#main", + mode: "role", + }); + + const { page } = fakePage(); + restoreRoleRefsForTarget({ cdpUrl, targetId, page }); + + expect(ensurePageState(page).roleRefs).toBeUndefined(); + }); + + it("invalidates cached refs for replacement Pages after main-frame navigation", () => { + const cdpUrl = "http://127.0.0.1:9222"; + const targetId = "t1"; + + const { page: pageA, handlers, mainFrame } = fakePage(); + storeRoleRefsForTarget({ + page: pageA, + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "Page A" } }, + mode: "role", + }); + + handlers.get("framenavigated")?.[0]?.(mainFrame); + expect(ensurePageState(pageA).roleRefs).toBeUndefined(); + + const { page: pageB } = fakePage(); + restoreRoleRefsForTarget({ cdpUrl, targetId, page: pageB }); + expect(ensurePageState(pageB).roleRefs).toBeUndefined(); + }); + + it("restores fresh post-navigation refs for a replacement Page", () => { + const cdpUrl = "http://127.0.0.1:9222"; + const targetId = "t1"; + + const { page: pageA, handlers, mainFrame } = fakePage(); + storeRoleRefsForTarget({ + page: pageA, + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "Page A" } }, + mode: "role", + }); + handlers.get("framenavigated")?.[0]?.(mainFrame); + + storeRoleRefsForTarget({ + page: pageA, + cdpUrl, + targetId, + refs: { e1: { role: "heading", name: "Page B" } }, + mode: "aria", + }); + + const { page: pageB } = fakePage(); + restoreRoleRefsForTarget({ cdpUrl, targetId, page: pageB }); + expect(ensurePageState(pageB).roleRefs).toEqual({ + e1: { role: "heading", name: "Page B" }, + }); + expect(ensurePageState(pageB).roleRefsMode).toBe("aria"); + }); + + it("does not let an obsolete Page invalidate a newer cache generation", () => { + const cdpUrl = "http://127.0.0.1:9222"; + const targetId = "shared-target"; + const { page: oldPage, handlers, mainFrame } = fakePage(); + storeRoleRefsForTarget({ + page: oldPage, + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "Old document" } }, + mode: "role", + }); + + const { page: currentPage } = fakePage(); + storeRoleRefsForTarget({ + page: currentPage, + cdpUrl, + targetId, + refs: { e1: { role: "heading", name: "Current document" } }, + mode: "aria", + }); + handlers.get("framenavigated")?.[0]?.(mainFrame); + + const { page: replacementPage } = fakePage(); + restoreRoleRefsForTarget({ cdpUrl, targetId, page: replacementPage }); + expect(ensurePageState(replacementPage).roleRefs).toEqual({ + e1: { role: "heading", name: "Current document" }, + }); + expect(ensurePageState(replacementPage).roleRefsMode).toBe("aria"); + }); + + it.each(["framenavigated", "framedetached"] as const)( + "invalidates page-wide aria refs when a subframe emits %s", + (event) => { + const cdpUrl = "http://127.0.0.1:9222"; + const targetId = `aria-target-${event}`; + const { page, handlers } = fakePage(); + storeRoleRefsForTarget({ + page, + cdpUrl, + targetId, + refs: { e1: { role: "button", name: "Embedded" } }, + mode: "aria", + }); + + handlers.get(event)?.[0]?.({ url: () => "https://frame.example/new" }); + + expect(ensurePageState(page).roleRefs).toBeUndefined(); + const { page: replacementPage } = fakePage(); + restoreRoleRefsForTarget({ cdpUrl, targetId, page: replacementPage }); + expect(ensurePageState(replacementPage).roleRefs).toBeUndefined(); + }, + ); }); describe("pw-session ensurePageState", () => { @@ -691,4 +842,104 @@ describe("pw-session ensurePageState", () => { expect(state2.errors).toStrictEqual([]); expect(state2.requests).toStrictEqual([]); }); + + it.each([ + { mode: "role" as const, frameSelector: undefined }, + { mode: "aria" as const, frameSelector: "iframe#content" }, + ])("clears $mode role refs on main-frame navigation", ({ mode, frameSelector }) => { + const { page, handlers, mainFrame, selectedFrame } = fakePage(); + const state = ensurePageState(page); + + storeRoleRefsForTarget({ + page, + cdpUrl: "http://127.0.0.1:9222", + targetId: "t1", + refs: { e1: { role: "button", name: "Save" } }, + frameSelector, + frame: frameSelector ? selectedFrame : undefined, + mode, + }); + expect(state.roleRefs).toBeDefined(); + expect(state.roleRefsMode).toBe(mode); + expect(state.roleRefsFrameSelector).toBe(frameSelector); + + handlers.get("framenavigated")?.[0]?.(mainFrame); + + expect(state.roleRefs).toBeUndefined(); + expect(state.roleRefsMode).toBeUndefined(); + expect(state.roleRefsFrameSelector).toBeUndefined(); + expect(() => refLocator(page, "e1")).toThrow(/Unknown ref/); + }); + + it("preserves role refs on subframe navigation", () => { + const { page, handlers } = fakePage(); + const state = ensurePageState(page); + + storeRoleRefsForTarget({ + page, + cdpUrl: "http://127.0.0.1:9222", + targetId: "t1", + refs: { e1: { role: "button", name: "MainBtn" } }, + mode: "role", + }); + expect(Object.keys(state.roleRefs!)).toHaveLength(1); + + const subframe = { url: () => "https://ads.example.com/widget" }; + handlers.get("framenavigated")?.[0]?.(subframe); + + expect(state.roleRefs).toBeDefined(); + expect(Object.keys(state.roleRefs!)).toHaveLength(1); + }); + + it.each(["framenavigated", "framedetached"] as const)( + "clears frame-scoped role refs on %s", + (event) => { + const { page, handlers, selectedFrame } = fakePage(); + const state = ensurePageState(page); + + storeRoleRefsForTarget({ + page, + cdpUrl: "http://127.0.0.1:9222", + targetId: "t1", + refs: { e1: { role: "button", name: "Inside frame" } }, + frameSelector: "iframe#content", + frame: selectedFrame, + mode: "role", + }); + + handlers.get(event)?.[0]?.({ url: () => "https://ads.example.com" }); + expect(state.roleRefs).toBeDefined(); + + handlers.get(event)?.[0]?.(selectedFrame); + expect(state.roleRefs).toBeUndefined(); + expect(state.roleRefsFrameSelector).toBeUndefined(); + }, + ); + + it("allows new snapshot to store fresh refs after navigation clear", () => { + const { page, handlers, mainFrame } = fakePage(); + const state = ensurePageState(page); + + storeRoleRefsForTarget({ + page, + cdpUrl: "http://127.0.0.1:9222", + targetId: "t1", + refs: { e1: { role: "button", name: "PageA-Btn" } }, + mode: "role", + }); + + handlers.get("framenavigated")?.[0]?.(mainFrame); + expect(state.roleRefs).toBeUndefined(); + + storeRoleRefsForTarget({ + page, + cdpUrl: "http://127.0.0.1:9222", + targetId: "t1", + refs: { e1: { role: "heading", name: "PageB Title" } }, + mode: "aria", + }); + expect(state.roleRefs).toBeDefined(); + expect(state.roleRefs!.e1.role).toBe("heading"); + expect(state.roleRefsMode).toBe("aria"); + }); }); diff --git a/extensions/browser/src/browser/pw-session.ts b/extensions/browser/src/browser/pw-session.ts index 8926b4fa48f6..80409c69576f 100644 --- a/extensions/browser/src/browser/pw-session.ts +++ b/extensions/browser/src/browser/pw-session.ts @@ -15,6 +15,7 @@ import type { BrowserContext, ConsoleMessage, Dialog, + Frame, Page, Request, Response, @@ -183,13 +184,22 @@ type PageState = { roleRefs?: Record; roleRefsMode?: "role" | "aria"; roleRefsFrameSelector?: string; + roleRefsFrame?: Frame; + /** Target-cache entry owned by the current role refs. */ + roleRefsTargetKey?: string; + /** Cache generation restored or stored by this Page. */ + roleRefsTargetGeneration?: number; + /** Main-frame navigation observed before this Page could be bound to its target. */ + roleRefsInvalidBeforeGeneration?: number; + /** Any frame changed before target binding; invalidates only page-wide aria refs. */ + roleRefsAriaInvalidBeforeGeneration?: number; }; type RoleRefs = NonNullable; type RoleRefsCacheEntry = { refs: RoleRefs; - frameSelector?: string; mode?: NonNullable; + generation: number; }; type ContextState = { @@ -205,6 +215,7 @@ const observedPages = new WeakSet(); // for the same CDP target across requests. const roleRefsByTarget = new Map(); const MAX_ROLE_REFS_CACHE = 50; +let roleRefsCacheGeneration = 0; const MAX_CONSOLE_MESSAGES = 500; const MAX_PAGE_ERRORS = 200; @@ -522,6 +533,33 @@ function roleRefsKey(cdpUrl: string, targetId: string) { return targetKey(cdpUrl, targetId); } +function bindRoleRefsTarget(page: Page, cdpUrl: string, targetId?: string | null): void { + const normalizedTargetId = normalizeOptionalString(targetId ?? undefined); + if (!normalizedTargetId) { + return; + } + const state = ensurePageState(page); + const key = roleRefsKey(cdpUrl, normalizedTargetId); + const invalidBeforeGeneration = state.roleRefsInvalidBeforeGeneration; + const ariaInvalidBeforeGeneration = state.roleRefsAriaInvalidBeforeGeneration; + const cached = roleRefsByTarget.get(key); + if ( + cached && + ((invalidBeforeGeneration !== undefined && cached.generation <= invalidBeforeGeneration) || + (ariaInvalidBeforeGeneration !== undefined && + cached.mode === "aria" && + cached.generation <= ariaInvalidBeforeGeneration)) + ) { + roleRefsByTarget.delete(key); + } + state.roleRefsInvalidBeforeGeneration = undefined; + state.roleRefsAriaInvalidBeforeGeneration = undefined; + state.roleRefsTargetKey = key; + if (!state.roleRefs) { + state.roleRefsTargetGeneration = roleRefsByTarget.get(key)?.generation; + } +} + function isBlockedTarget(cdpUrl: string, targetId?: string): boolean { const normalizedTargetId = normalizeOptionalString(targetId) ?? ""; if (!normalizedTargetId) { @@ -796,15 +834,23 @@ export function rememberRoleRefsForTarget(opts: { refs: RoleRefs; frameSelector?: string; mode?: NonNullable; -}): void { +}): number | undefined { const targetId = normalizeOptionalString(opts.targetId) ?? ""; if (!targetId) { - return; + return undefined; } - roleRefsByTarget.set(roleRefsKey(opts.cdpUrl, targetId), { + const key = roleRefsKey(opts.cdpUrl, targetId); + // A selector cannot preserve frame identity across replacement Page objects. + // Frame-scoped refs remain local and require a fresh snapshot after reconnect. + if (opts.frameSelector) { + roleRefsByTarget.delete(key); + return undefined; + } + const generation = ++roleRefsCacheGeneration; + roleRefsByTarget.set(key, { refs: opts.refs, - ...(opts.frameSelector ? { frameSelector: opts.frameSelector } : {}), ...(opts.mode ? { mode: opts.mode } : {}), + generation, }); while (roleRefsByTarget.size > MAX_ROLE_REFS_CACHE) { const first = roleRefsByTarget.keys().next(); @@ -813,6 +859,7 @@ export function rememberRoleRefsForTarget(opts: { } roleRefsByTarget.delete(first.value); } + return generation; } /** Store role refs on the page and target cache. */ @@ -822,17 +869,25 @@ export function storeRoleRefsForTarget(opts: { targetId?: string; refs: RoleRefs; frameSelector?: string; + frame?: Frame; mode: NonNullable; }): void { + if (opts.frameSelector && !opts.frame) { + throw new Error("Frame-scoped role refs require their resolved frame."); + } const state = ensurePageState(opts.page); state.roleRefs = opts.refs; state.roleRefsFrameSelector = opts.frameSelector; + state.roleRefsFrame = opts.frame; state.roleRefsMode = opts.mode; const targetId = normalizeOptionalString(opts.targetId); if (!targetId) { + state.roleRefsTargetKey = undefined; + state.roleRefsTargetGeneration = undefined; return; } - rememberRoleRefsForTarget({ + bindRoleRefsTarget(opts.page, opts.cdpUrl, targetId); + state.roleRefsTargetGeneration = rememberRoleRefsForTarget({ cdpUrl: opts.cdpUrl, targetId, refs: opts.refs, @@ -841,6 +896,33 @@ export function storeRoleRefsForTarget(opts: { }); } +function clearRoleRefs(state: PageState): void { + if (state.roleRefsTargetKey) { + const cached = roleRefsByTarget.get(state.roleRefsTargetKey); + // A delayed event from an obsolete Page must not erase refs that a newer + // wrapper stored for the same target after this Page's generation. + if (cached?.generation === state.roleRefsTargetGeneration) { + roleRefsByTarget.delete(state.roleRefsTargetKey); + } + } + state.roleRefs = undefined; + state.roleRefsMode = undefined; + state.roleRefsFrameSelector = undefined; + state.roleRefsFrame = undefined; + state.roleRefsTargetKey = undefined; + state.roleRefsTargetGeneration = undefined; +} + +function currentTargetRoleRefsMode( + state: PageState, +): NonNullable | undefined { + if (!state.roleRefsTargetKey) { + return undefined; + } + const cached = roleRefsByTarget.get(state.roleRefsTargetKey); + return cached && cached.generation === state.roleRefsTargetGeneration ? cached.mode : undefined; +} + /** Restore cached role refs onto a newly resolved page. */ export function restoreRoleRefsForTarget(opts: { cdpUrl: string; @@ -851,7 +933,9 @@ export function restoreRoleRefsForTarget(opts: { if (!targetId) { return; } - const cached = roleRefsByTarget.get(roleRefsKey(opts.cdpUrl, targetId)); + const cacheKey = roleRefsKey(opts.cdpUrl, targetId); + bindRoleRefsTarget(opts.page, opts.cdpUrl, targetId); + const cached = roleRefsByTarget.get(cacheKey); if (!cached) { return; } @@ -859,8 +943,9 @@ export function restoreRoleRefsForTarget(opts: { if (state.roleRefs) { return; } + state.roleRefsTargetKey = cacheKey; + state.roleRefsTargetGeneration = cached.generation; state.roleRefs = cached.refs; - state.roleRefsFrameSelector = cached.frameSelector; state.roleRefsMode = cached.mode; } @@ -982,6 +1067,45 @@ export function ensurePageState(page: Page): PageState { finish(); } }); + page.on("framenavigated", (frame) => { + // Clear role refs on main-frame navigation so stale refs from the + // previous page are never used to locate elements on the new page. + // Unscoped refs survive subframe navigation. Frame-scoped refs are + // invalid only when their exact Frame replaces its document. + const isMainFrame = frame === page.mainFrame(); + const targetWasBound = state.roleRefsTargetKey !== undefined; + if (!targetWasBound) { + // Target discovery is asynchronous. Remember an early navigation so + // binding removes only cache generations that already existed. Refs a + // newer Page stores after this event must survive the delayed lookup. + if (isMainFrame) { + state.roleRefsInvalidBeforeGeneration = roleRefsCacheGeneration; + } else { + state.roleRefsAriaInvalidBeforeGeneration = roleRefsCacheGeneration; + } + } + const pageWideAriaRefs = + state.roleRefsMode === "aria" || currentTargetRoleRefsMode(state) === "aria"; + if (isMainFrame || pageWideAriaRefs || frame === state.roleRefsFrame) { + // Replacement Page objects restore from this target cache, so local + // clearing alone could resurrect refs from the previous document. + clearRoleRefs(state); + } + }); + page.on("framedetached", (frame) => { + if (!state.roleRefsTargetKey) { + if (frame === page.mainFrame()) { + state.roleRefsInvalidBeforeGeneration = roleRefsCacheGeneration; + } else { + state.roleRefsAriaInvalidBeforeGeneration = roleRefsCacheGeneration; + } + } + const pageWideAriaRefs = + state.roleRefsMode === "aria" || currentTargetRoleRefsMode(state) === "aria"; + if (pageWideAriaRefs || frame === state.roleRefsFrame) { + clearRoleRefs(state); + } + }); page.on("close", () => { clearArmedDialogResponse(state); for (const controller of state.dialogAbortControllers) { @@ -1307,6 +1431,7 @@ async function partitionAccessiblePages(opts: { cdpUrl: string; pages: Page[] }) blockedCount += 1; continue; } + ensurePageState(page); const targetId = await pageTargetId(page).catch(() => null); // Fail closed when we cannot resolve a target id while this session has // quarantined targets; otherwise a blocked tab can become selectable. @@ -1322,6 +1447,7 @@ async function partitionAccessiblePages(opts: { cdpUrl: string; pages: Page[] }) blockedCount += 1; continue; } + bindRoleRefsTarget(page, opts.cdpUrl, targetId); accessible.push({ page, targetId }); } return { accessible, blockedCount }; @@ -1362,13 +1488,15 @@ async function getPageForTargetIdOnce(opts: { } throw new Error("No pages available in the connected browser."); } - const first = accessible[0].page; + const first = accessible[0]; if (!opts.targetId) { - return first; + bindRoleRefsTarget(first.page, opts.cdpUrl, first.targetId); + return first.page; } - const found = accessible.find((entry) => entry.targetId === opts.targetId)?.page; + const found = accessible.find((entry) => entry.targetId === opts.targetId); if (found) { - return found; + bindRoleRefsTarget(found.page, opts.cdpUrl, found.targetId); + return found.page; } throw new BrowserTabNotFoundError(); } @@ -1863,9 +1991,7 @@ export function refLocator(page: Page, ref: string) { if (/^e\d+$/.test(normalized)) { const state = pageStates.get(page); if (state?.roleRefsMode === "aria") { - const scope = state.roleRefsFrameSelector - ? page.frameLocator(state.roleRefsFrameSelector) - : page; + const scope = state.roleRefsFrame ?? page; return scope.locator(`aria-ref=${normalized}`); } const info = state?.roleRefs?.[normalized]; @@ -1874,9 +2000,7 @@ export function refLocator(page: Page, ref: string) { `Unknown ref "${normalized}". Run a new snapshot and use a ref from that snapshot.`, ); } - const scope = state?.roleRefsFrameSelector - ? page.frameLocator(state.roleRefsFrameSelector) - : page; + const scope = state?.roleRefsFrame ?? page; const locAny = scope as unknown as { getByRole: ( role: never, @@ -1897,9 +2021,7 @@ export function refLocator(page: Page, ref: string) { `Unknown ref "${normalized}". Run a new snapshot and use a ref from that snapshot.`, ); } - const scope = state.roleRefsFrameSelector - ? page.frameLocator(state.roleRefsFrameSelector) - : page; + const scope = state.roleRefsFrame ?? page; if (info.domMarker) { return scope.locator(`[${BROWSER_REF_MARKER_ATTRIBUTE}="${normalized}"]`); } diff --git a/extensions/browser/src/browser/pw-tools-core.snapshot.test.ts b/extensions/browser/src/browser/pw-tools-core.snapshot.test.ts index a68ce7adba2d..7086f1dddb14 100644 --- a/extensions/browser/src/browser/pw-tools-core.snapshot.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.snapshot.test.ts @@ -58,6 +58,16 @@ function requireScopedCdpClientOptions(): ScopedCdpClientOptions { return options as ScopedCdpClientOptions; } +function makeAriaSnapshotPage(ariaSnapshot: ReturnType) { + const mainFrame = { id: "main-frame" }; + return { + ariaSnapshot, + mainFrame: () => mainFrame, + on: vi.fn(), + off: vi.fn(), + }; +} + describe("pw-tools-core aria snapshot storage", () => { beforeEach(() => { vi.clearAllMocks(); @@ -149,7 +159,7 @@ describe("pw-tools-core aria snapshot storage", () => { it("forwards an explicit timeoutMs into the role-aria Playwright ariaSnapshot call", async () => { const ariaSnapshotMock = vi.fn().mockResolvedValue(""); - const page = { ariaSnapshot: ariaSnapshotMock }; + const page = makeAriaSnapshotPage(ariaSnapshotMock); getPageForTargetId.mockResolvedValue(page); const mod = await import("./pw-tools-core.snapshot.js"); @@ -165,7 +175,7 @@ describe("pw-tools-core aria snapshot storage", () => { it("uses the default snapshot timeout for non-finite role-aria timeouts", async () => { const ariaSnapshotMock = vi.fn().mockResolvedValue(""); - const page = { ariaSnapshot: ariaSnapshotMock }; + const page = makeAriaSnapshotPage(ariaSnapshotMock); getPageForTargetId.mockResolvedValue(page); const mod = await import("./pw-tools-core.snapshot.js"); @@ -179,9 +189,112 @@ describe("pw-tools-core aria snapshot storage", () => { expect(ariaSnapshotMock).toHaveBeenCalledWith({ mode: "ai", timeout: 5000 }); }); + it("rejects page-wide refs when a subframe navigates during capture", async () => { + const mainFrame = { id: "main-frame" }; + const subframe = { id: "subframe" }; + const handlers = new Map void>(); + const page = { + ariaSnapshot: vi.fn(async () => { + handlers.get("framenavigated")?.(subframe); + return '- button "Save"'; + }), + mainFrame: () => mainFrame, + on: vi.fn((event: string, handler: (frame: unknown) => void) => { + handlers.set(event, handler); + }), + off: vi.fn(), + }; + getPageForTargetId.mockResolvedValue(page); + + const mod = await import("./pw-tools-core.snapshot.js"); + await expect( + mod.snapshotRoleViaPlaywright({ + cdpUrl: "http://127.0.0.1:9222", + targetId: "tab-1", + refsMode: "aria", + }), + ).rejects.toThrow("Frame changed while its browser snapshot was being captured"); + + expect(storeRoleRefsForTarget).not.toHaveBeenCalled(); + }); + + it("stores frame-scoped refs with the exact captured frame", async () => { + const ariaSnapshot = vi.fn(async () => '- button "Save"'); + const frame = { id: "frame-1", locator: vi.fn(() => ({ ariaSnapshot })) }; + const page = { + locator: vi.fn(() => ({ + elementHandle: vi.fn(async () => ({ + contentFrame: vi.fn(async () => frame), + dispose: vi.fn(async () => {}), + })), + })), + on: vi.fn(), + off: vi.fn(), + }; + getPageForTargetId.mockResolvedValue(page); + + const mod = await import("./pw-tools-core.snapshot.js"); + await mod.snapshotRoleViaPlaywright({ + cdpUrl: "http://127.0.0.1:9222", + targetId: "tab-1", + frameSelector: "iframe#content", + }); + + expect(storeRoleRefsForTarget).toHaveBeenCalledWith( + expect.objectContaining({ + page, + frameSelector: "iframe#content", + frame, + }), + ); + expect(page.off).toHaveBeenCalledWith("framenavigated", expect.any(Function)); + }); + + it.each(["framenavigated", "framedetached"] as const)( + "rejects frame-scoped refs when that frame emits %s during capture", + async (event) => { + const handlers = new Map void>(); + const frame = { + id: "frame-1", + locator: vi.fn(() => ({ + ariaSnapshot: vi.fn(async () => { + handlers.get(event)?.(frame); + return '- button "Save"'; + }), + })), + }; + const page = { + locator: vi.fn(() => ({ + elementHandle: vi.fn(async () => ({ + contentFrame: vi.fn(async () => frame), + dispose: vi.fn(async () => {}), + })), + })), + on: vi.fn((eventName: string, handler: (frame: unknown) => void) => { + handlers.set(eventName, handler); + }), + off: vi.fn(), + }; + getPageForTargetId.mockResolvedValue(page); + + const mod = await import("./pw-tools-core.snapshot.js"); + await expect( + mod.snapshotRoleViaPlaywright({ + cdpUrl: "http://127.0.0.1:9222", + targetId: "tab-1", + frameSelector: "iframe#content", + }), + ).rejects.toThrow("Frame changed while its browser snapshot was being captured"); + + expect(storeRoleRefsForTarget).not.toHaveBeenCalled(); + expect(page.off).toHaveBeenCalledWith("framenavigated", expect.any(Function)); + expect(page.off).toHaveBeenCalledWith("framedetached", expect.any(Function)); + }, + ); + it("uses the default snapshot timeout for non-finite ai snapshot timeouts", async () => { const ariaSnapshotMock = vi.fn().mockResolvedValue(""); - const page = { ariaSnapshot: ariaSnapshotMock }; + const page = makeAriaSnapshotPage(ariaSnapshotMock); getPageForTargetId.mockResolvedValue(page); const mod = await import("./pw-tools-core.snapshot.js"); @@ -199,7 +312,7 @@ describe("pw-tools-core aria snapshot storage", () => { const second = `- button "Hidden ${"X".repeat(100)} 🙂" [ref=e2]`; const marker = "[...TRUNCATED - page too large]"; const ariaSnapshotMock = vi.fn().mockResolvedValue(`${first}\n${second}`); - const page = { ariaSnapshot: ariaSnapshotMock }; + const page = makeAriaSnapshotPage(ariaSnapshotMock); getPageForTargetId.mockResolvedValue(page); const mod = await import("./pw-tools-core.snapshot.js"); @@ -227,7 +340,7 @@ describe("pw-tools-core aria snapshot storage", () => { const ariaSnapshotMock = vi .fn() .mockResolvedValue(`${first}\n- button "Hidden ${"X".repeat(100)}" [ref=e2]`); - const page = { ariaSnapshot: ariaSnapshotMock }; + const page = makeAriaSnapshotPage(ariaSnapshotMock); getPageForTargetId.mockResolvedValue(page); const mod = await import("./pw-tools-core.snapshot.js"); diff --git a/extensions/browser/src/browser/pw-tools-core.snapshot.ts b/extensions/browser/src/browser/pw-tools-core.snapshot.ts index 0bb3add7834c..9eb88e664be9 100644 --- a/extensions/browser/src/browser/pw-tools-core.snapshot.ts +++ b/extensions/browser/src/browser/pw-tools-core.snapshot.ts @@ -8,7 +8,7 @@ import { normalizeOptionalString, } from "openclaw/plugin-sdk/string-coerce-runtime"; import { truncateUtf16Safe } from "openclaw/plugin-sdk/text-utility-runtime"; -import type { Page } from "playwright-core"; +import type { Frame, Page } from "playwright-core"; import type { SsrFPolicy } from "../infra/net/ssrf.js"; import { ACT_MAX_VIEWPORT_DIMENSION } from "./act-policy.js"; import { type AriaSnapshotNode, formatAriaSnapshot, type RawAXNode } from "./cdp.js"; @@ -267,27 +267,61 @@ export async function snapshotAiViaPlaywright(opts: { ssrfPolicy: opts.ssrfPolicy, }); - let snapshot = await page.ariaSnapshot({ - mode: "ai", - timeout: resolveSnapshotTimeoutMs(opts.timeoutMs), - }); - if (opts.urls) { - snapshot = appendSnapshotUrls(snapshot, await collectSnapshotUrls(page)); - } - const built = buildRoleSnapshotFromAiSnapshot(snapshot); - const finalized = finalizeRoleSnapshot({ - snapshot, - refs: built.refs, - maxChars: opts.maxChars, - }); - storeRoleRefsForTarget({ + return await withSnapshotFrameGuard({ page, - cdpUrl: opts.cdpUrl, - targetId: opts.targetId, - refs: finalized.refs, - mode: "aria", + run: async (isFrameCurrent) => { + let snapshot = await page.ariaSnapshot({ + mode: "ai", + timeout: resolveSnapshotTimeoutMs(opts.timeoutMs), + }); + if (opts.urls) { + snapshot = appendSnapshotUrls(snapshot, await collectSnapshotUrls(page)); + } + const built = buildRoleSnapshotFromAiSnapshot(snapshot); + const finalized = finalizeRoleSnapshot({ + snapshot, + refs: built.refs, + maxChars: opts.maxChars, + }); + assertSnapshotFrameCurrent(isFrameCurrent); + storeRoleRefsForTarget({ + page, + cdpUrl: opts.cdpUrl, + targetId: opts.targetId, + refs: finalized.refs, + mode: "aria", + }); + return finalized; + }, }); - return finalized; +} + +function assertSnapshotFrameCurrent(isFrameCurrent: () => boolean): void { + if (!isFrameCurrent()) { + throw new Error("Frame changed while its browser snapshot was being captured; retry."); + } +} + +async function withSnapshotFrameGuard(opts: { + page: Page; + /** Omit for page-wide AI snapshots, whose refs can include every frame. */ + frame?: Frame; + run: (isFrameCurrent: () => boolean) => Promise; +}): Promise { + let frameCurrent = true; + const onFrameChanged = (frame: Frame) => { + if (!opts.frame || frame === opts.frame) { + frameCurrent = false; + } + }; + opts.page.on("framenavigated", onFrameChanged); + opts.page.on("framedetached", onFrameChanged); + try { + return await opts.run(() => frameCurrent); + } finally { + opts.page.off("framenavigated", onFrameChanged); + opts.page.off("framedetached", onFrameChanged); + } } async function finalizeRoleSnapshotViaPlaywright(params: { @@ -295,6 +329,8 @@ async function finalizeRoleSnapshotViaPlaywright(params: { cdpUrl: string; targetId?: string; frameSelector?: string; + frame?: Frame; + isFrameCurrent?: () => boolean; mode: "aria" | "role"; built: { snapshot: string; refs: RoleRefMap }; urls?: boolean; @@ -308,6 +344,9 @@ async function finalizeRoleSnapshotViaPlaywright(params: { const snapshot = params.urls ? appendSnapshotUrls(params.built.snapshot, await collectSnapshotUrls(params.page)) : params.built.snapshot; + if (params.isFrameCurrent) { + assertSnapshotFrameCurrent(params.isFrameCurrent); + } const finalized = finalizeRoleSnapshot({ snapshot, refs: params.built.refs, @@ -319,6 +358,7 @@ async function finalizeRoleSnapshotViaPlaywright(params: { targetId: params.targetId, refs: finalized.refs, ...(params.frameSelector ? { frameSelector: params.frameSelector } : {}), + ...(params.frame ? { frame: params.frame } : {}), mode: params.mode, }); return finalized; @@ -354,43 +394,70 @@ export async function snapshotRoleViaPlaywright(opts: { if (normalizeOptionalString(opts.selector) || normalizeOptionalString(opts.frameSelector)) { throw new Error("refs=aria does not support selector/frame snapshots yet."); } - const snapshot = await page.ariaSnapshot({ - mode: "ai", - timeout: ariaSnapshotTimeout, - }); - const built = buildRoleSnapshotFromAiSnapshot(snapshot, opts.options); - return await finalizeRoleSnapshotViaPlaywright({ + return await withSnapshotFrameGuard({ page, - cdpUrl: opts.cdpUrl, - targetId: opts.targetId, - built, - mode: "aria", - urls: opts.urls, - maxChars: opts.maxChars, + run: async (isFrameCurrent) => { + const snapshot = await page.ariaSnapshot({ + mode: "ai", + timeout: ariaSnapshotTimeout, + }); + const built = buildRoleSnapshotFromAiSnapshot(snapshot, opts.options); + return await finalizeRoleSnapshotViaPlaywright({ + page, + cdpUrl: opts.cdpUrl, + targetId: opts.targetId, + isFrameCurrent, + built, + mode: "aria", + urls: opts.urls, + maxChars: opts.maxChars, + }); + }, }); } const frameSelector = normalizeOptionalString(opts.frameSelector) ?? ""; const selector = normalizeOptionalString(opts.selector) ?? ""; - const locator = frameSelector - ? selector - ? page.frameLocator(frameSelector).locator(selector) - : page.frameLocator(frameSelector).locator(":root") - : selector - ? page.locator(selector) - : page.locator(":root"); - - const ariaSnapshot = await locator.ariaSnapshot({ timeout: ariaSnapshotTimeout }); - const built = buildRoleSnapshotFromAriaSnapshot(ariaSnapshot ?? "", opts.options); - return await finalizeRoleSnapshotViaPlaywright({ + const frameElement = frameSelector + ? await page.locator(frameSelector).elementHandle({ timeout: ariaSnapshotTimeout }) + : undefined; + let frame: Frame | undefined; + if (frameElement) { + try { + frame = (await frameElement.contentFrame()) ?? undefined; + } finally { + await frameElement.dispose(); + } + } + if (frameSelector && !frame) { + throw new Error("Frame was unavailable while its browser snapshot was being captured."); + } + return await withSnapshotFrameGuard({ page, - cdpUrl: opts.cdpUrl, - targetId: opts.targetId, - frameSelector: frameSelector || undefined, - built, - mode: "role", - urls: opts.urls, - maxChars: opts.maxChars, + frame: frame ?? page.mainFrame(), + run: async (isFrameCurrent) => { + const locator = frame + ? selector + ? frame.locator(selector) + : frame.locator(":root") + : selector + ? page.locator(selector) + : page.locator(":root"); + const ariaSnapshot = await locator.ariaSnapshot({ timeout: ariaSnapshotTimeout }); + const built = buildRoleSnapshotFromAriaSnapshot(ariaSnapshot ?? "", opts.options); + return await finalizeRoleSnapshotViaPlaywright({ + page, + cdpUrl: opts.cdpUrl, + targetId: opts.targetId, + frameSelector: frameSelector || undefined, + frame: frame ?? undefined, + isFrameCurrent, + built, + mode: "role", + urls: opts.urls, + maxChars: opts.maxChars, + }); + }, }); }