diff --git a/CHANGELOG.md b/CHANGELOG.md index 42e7dd3ba0ea..01ad8b300d3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -340,6 +340,7 @@ Docs: https://docs.openclaw.ai - **Remote browser reliability:** bound persistent Playwright tab enumeration by the existing remote CDP timeout budget and retire timed-out connection attempts so late completions cannot restore a stuck connection. (#80147, #58968) Thanks @HemantSudarshan and @KeaneYan. - **Browser tab adoption:** preserve the prior implicit tab and stable aliases when new MCP, Playwright, or CDP targets fail final safety validation, abort after creation, or cannot be rediscovered; validate labels before creating tabs and limit managed cleanup to adopted targets. (#105301) Thanks @hugenshen. - **Browser snapshots:** preserve slash-only control names and keep ref-looking page text from retaining truncated refs or misplacing child frames. Fixes #130571. +- **Browser refs:** keep unnamed and overlong-named controls distinct from ordinary named controls to prevent wrong-target actions, and initialize raw ARIA DOM markers in their owning CDP session. - **Browser attachment downloads:** return managed URL, filename, and path metadata when direct Playwright navigation starts an attachment download, while validating final URLs before saving bytes and preserving single-owner explicit downloads. (#48045, #89416) Thanks @zhangguiping-xydt. - **Browser action downloads:** return managed URL, filename, and path metadata when agent actions trigger downloads, while preserving explicit ownership, validating final URLs before saving bytes, and quarantining policy-denied tabs without closing them. (#93250, #93307) Thanks @sunlit-deng. - **Managed browser cookie persistence:** initialize new isolated macOS headless profiles with a non-interactive encryption key while preserving existing profile keys, and close Chromium through CDP before bounded signal fallback so persistent logins survive graceful browser and Gateway restarts. (#96704, #98284) Thanks @TurboTheTurtle. diff --git a/config/assertion-safety-baseline.txt b/config/assertion-safety-baseline.txt index 1f4c10a45271..932602069cc7 100644 --- a/config/assertion-safety-baseline.txt +++ b/config/assertion-safety-baseline.txt @@ -85,7 +85,7 @@ extensions/browser/src/browser/paths.ts 1 extensions/browser/src/browser/playwright-core.runtime.ts 2 extensions/browser/src/browser/pw-download-capture.ts 1 extensions/browser/src/browser/pw-role-snapshot.ts 1 -extensions/browser/src/browser/pw-session-actions.ts 7 +extensions/browser/src/browser/pw-session-actions.ts 2 extensions/browser/src/browser/pw-session-cdp-transport.ts 1 extensions/browser/src/browser/pw-session-navigation.ts 1 extensions/browser/src/browser/pw-session.page-cdp.ts 4 diff --git a/docs/tools/browser-control.md b/docs/tools/browser-control.md index 17accdede6f7..ea93222e1e36 100644 --- a/docs/tools/browser-control.md +++ b/docs/tools/browser-control.md @@ -310,6 +310,7 @@ OpenClaw supports three "snapshot" styles: - Actions: `openclaw browser click e12`, `openclaw browser highlight e12`. - Internally, the ref is resolved via `getByRole(...)` (plus `nth()` for duplicates). - Names containing quotes, backslashes, or YAML punctuation remain actionable; use the ref rather than reconstructing a locator from the displayed name. + - A missing displayed name can mean an empty accessible name or one above Playwright's 900 UTF-16-unit limit; keep using the returned ref. - Add `--labels` to include a screenshot with overlayed `e12` labels. On Playwright-backed profiles this also returns per-ref bounding-box metadata (`annotations[]`). diff --git a/extensions/browser/src/browser/pw-role-snapshot.chromium.test.ts b/extensions/browser/src/browser/pw-role-snapshot.chromium.test.ts index e64cc3f52223..9b2678f3cb79 100644 --- a/extensions/browser/src/browser/pw-role-snapshot.chromium.test.ts +++ b/extensions/browser/src/browser/pw-role-snapshot.chromium.test.ts @@ -2,9 +2,19 @@ import path from "node:path"; import { afterEach, describe, expect, it } from "vitest"; import { useAutoCleanupTempDirTracker } from "../../test-support.js"; import { getPlaywrightCore } from "./playwright-core.runtime.js"; -import { closePlaywrightBrowserConnection } from "./pw-session.js"; +import { + closePlaywrightBrowserConnection, + refLocator, + restoreRoleRefsForTarget, +} from "./pw-session.js"; +import { BROWSER_REF_MARKER_ATTRIBUTE } from "./pw-session.page-cdp.js"; import { clickViaPlaywright, typeViaPlaywright } from "./pw-tools-core.interactions.actions.js"; -import { snapshotAiViaPlaywright, snapshotRoleViaPlaywright } from "./pw-tools-core.snapshot.js"; +import { + snapshotAiViaPlaywright, + snapshotAriaViaPlaywright, + snapshotRoleViaPlaywright, + storeAriaSnapshotRefsViaPlaywright, +} from "./pw-tools-core.snapshot.js"; import { getFreePort } from "./test-port.js"; const tempDirs = useAutoCleanupTempDirTracker(afterEach); @@ -12,7 +22,7 @@ const tempDirs = useAutoCleanupTempDirTracker(afterEach); describe.runIf(process.env.OPENCLAW_BROWSER_SNAPSHOT_E2E === "1")( "Chromium snapshot-to-action name fidelity", () => { - it("resolves encoded duplicate names, textboxes, and frame-qualified AI refs", async () => { + it("resolves encoded and omitted names, raw AX names, and native frame refs", async () => { const rootDir = tempDirs.make("openclaw-snapshot-labels-"); const port = await getFreePort(); const cdpUrl = `http://127.0.0.1:${port}`; @@ -26,11 +36,25 @@ describe.runIf(process.env.OPENCLAW_BROWSER_SNAPSHOT_E2E === "1")( ); try { const page = context.pages()[0] ?? (await context.newPage()); - await page.setContent('
'); + await page.setContent( + '
', + ); const buttonName = 'Save: "owner\'s" C:\\draft 🦞'; const inputName = 'Project "path"'; + const nameControls = [ + { id: "short", name: "Named control" }, + { id: "empty", name: "" }, + { id: "empty-again", name: "" }, + { id: "name-900", name: "x".repeat(900) }, + { id: "name-901", name: "x".repeat(901) }, + { id: "surrogates-900", name: "😀".repeat(450) }, + { id: "surrogates-901", name: "😀".repeat(450) + "x" }, + { id: "normalized-empty", name: "\u200b\u00ad" }, + { id: "normalized-899", name: " x \n".repeat(450) }, + { id: "normalized-901", name: " x \n".repeat(451) }, + ]; await page.evaluate( - ({ buttonName: label, inputName: inputLabel }) => { + ({ buttonName: label, inputName: inputLabel, nameControls: controls }) => { for (const id of ["first", "second"]) { const button = document.createElement("button"); button.textContent = label; @@ -48,10 +72,19 @@ describe.runIf(process.env.OPENCLAW_BROWSER_SNAPSHOT_E2E === "1")( document.querySelector("output")!.textContent = "/"; }); document.querySelector("main")!.append(slash); + for (const { id, name } of controls) { + const button = document.createElement("button"); + button.setAttribute("aria-label", name); + button.textContent = name ? "control" : ""; + button.addEventListener("click", () => { + document.querySelector("output")!.textContent = id; + }); + document.querySelector("main")!.append(button); + } document.querySelector("iframe")!.srcdoc = ""; }, - { buttonName, inputName }, + { buttonName, inputName, nameControls }, ); await page.frameLocator("iframe").getByRole("button").waitFor(); const session = await context.newCDPSession(page); @@ -68,14 +101,13 @@ describe.runIf(process.env.OPENCLAW_BROWSER_SNAPSHOT_E2E === "1")( options: { interactive: mode !== "role" }, }); const buttons = Object.entries(snapshot.refs).filter( - ([, value]) => value.role === "button" && value.name === buttonName, + ([, value]) => value.role === "button" && value.name !== "Frame: action", ); - expect(buttons, mode).toHaveLength(2); + const expectedButtons = ["first", "second", "/", ...nameControls.map(({ id }) => id)]; + expect(buttons, mode).toHaveLength(expectedButtons.length); for (const [index, [ref]] of buttons.entries()) { await clickViaPlaywright({ ...target, ref, timeoutMs: 1_000 }); - expect(await page.locator("output").textContent(), mode).toBe( - index ? "second" : "first", - ); + expect(await page.locator("output").textContent(), mode).toBe(expectedButtons[index]); } const input = Object.entries(snapshot.refs).find( ([, value]) => value.role === "textbox" && value.name === inputName, @@ -83,10 +115,6 @@ describe.runIf(process.env.OPENCLAW_BROWSER_SNAPSHOT_E2E === "1")( expect(input, mode).toBeDefined(); await typeViaPlaywright({ ...target, ref: input![0], text: mode, timeoutMs: 1_000 }); expect(await page.getByRole("textbox").inputValue()).toBe(mode); - const slash = Object.entries(snapshot.refs).find(([, value]) => value.name === "/"); - expect(slash, mode).toBeDefined(); - await clickViaPlaywright({ ...target, ref: slash![0], timeoutMs: 1_000 }); - expect(await page.locator("output").textContent(), mode).toBe("/"); } const snapshot = await snapshotAiViaPlaywright(target); const nested = Object.entries(snapshot.refs).find( @@ -97,6 +125,48 @@ describe.runIf(process.env.OPENCLAW_BROWSER_SNAPSHOT_E2E === "1")( expect(await page.frameLocator("iframe").getByRole("button").textContent()).toBe( "Frame clicked", ); + + await page.setContent( + "
", + ); + const rawControls = [ + { id: "raw-short", name: "Raw named control" }, + { id: "raw-empty", name: "" }, + { id: "raw-long", name: "x".repeat(901) }, + ]; + await page.evaluate((controls) => { + for (const { id, name } of controls) { + const button = document.createElement("button"); + button.setAttribute("aria-label", name); + button.textContent = name ? "control" : ""; + button.addEventListener("click", () => { + document.querySelector("output")!.textContent = id; + }); + document.querySelector("main")!.append(button); + } + }, rawControls); + const aria = await snapshotAriaViaPlaywright(target); + const rawButtons = aria.nodes.filter((node) => node.role.toLowerCase() === "button"); + expect(rawButtons).toHaveLength(rawControls.length); + expect( + await page + .getByRole("button", { name: "", exact: true }) + .getAttribute(BROWSER_REF_MARKER_ATTRIBUTE), + ).toBe(rawButtons[1]!.ref); + await clickViaPlaywright({ ...target, ref: rawButtons[1]!.ref, timeoutMs: 1_000 }); + expect(await page.locator("output").textContent()).toBe("raw-empty"); + // Real AX names with unavailable DOM ids exercise the existing role fallback. + // Restore onto the launcher's distinct Page wrapper to cross the target cache. + await storeAriaSnapshotRefsViaPlaywright({ + ...target, + nodes: aria.nodes.map(({ backendDOMNodeId: _backendId, ...node }) => node), + }); + expect(await page.locator(`[${BROWSER_REF_MARKER_ATTRIBUTE}]`).count()).toBe(0); + restoreRoleRefsForTarget({ ...target, page }); + for (const [index, node] of rawButtons.entries()) { + await refLocator(page, node.ref).click({ timeout: 1_000 }); + expect(await page.locator("output").textContent()).toBe(rawControls[index]!.id); + } } finally { await closePlaywrightBrowserConnection({ cdpUrl }); await context.close(); diff --git a/extensions/browser/src/browser/pw-session-actions.ts b/extensions/browser/src/browser/pw-session-actions.ts index c1ebf2a281db..9cc22bc0fa53 100644 --- a/extensions/browser/src/browser/pw-session-actions.ts +++ b/extensions/browser/src/browser/pw-session-actions.ts @@ -82,9 +82,10 @@ export function refLocator(page: Page, ref: string) { ? ref.slice(4) : ref; - if (/^e\d+$/.test(normalized)) { + const isRoleRef = /^e\d+$/.test(normalized); + if (isRoleRef || AX_REF_PATTERN.test(normalized)) { const state = pageStates.get(page); - if (state?.roleRefsMode === "aria") { + if (isRoleRef && state?.roleRefsMode === "aria") { const scope = state.roleRefsFrame ?? page; return scope.locator(`aria-ref=${normalized}`); } @@ -95,39 +96,15 @@ export function refLocator(page: Page, ref: string) { ); } const scope = state?.roleRefsFrame ?? page; - const locAny = scope as unknown as { - getByRole: ( - role: never, - opts?: { name?: string; exact?: boolean }, - ) => ReturnType; - }; - const locator = info.name - ? locAny.getByRole(info.role as never, { name: info.name, exact: true }) - : locAny.getByRole(info.role as never); - return info.nth !== undefined ? locator.nth(info.nth) : locator; - } - - if (AX_REF_PATTERN.test(normalized)) { - const state = pageStates.get(page); - const info = state?.roleRefs?.[normalized]; - if (!info) { - throw new Error( - `Unknown ref "${normalized}". Run a new snapshot and use a ref from that snapshot.`, - ); - } - const scope = state.roleRefsFrame ?? page; - if (info.domMarker) { + if (!isRoleRef && info.domMarker) { return scope.locator(`[${BROWSER_REF_MARKER_ATTRIBUTE}="${normalized}"]`); } - const locAny = scope as unknown as { - getByRole: ( - role: never, - opts?: { name?: string; exact?: boolean }, - ) => ReturnType; - }; - const locator = info.name - ? locAny.getByRole(info.role as never, { name: info.name, exact: true }) - : locAny.getByRole(info.role as never); + // Playwright omits empty names and names over 900 UTF-16 units from ARIA text. + // Match that exact bucket before nth; raw AX names (including "") stay explicit. + const locator = scope.getByRole(info.role as never, { + name: info.name ?? /^$|^.{901,}$/s, + exact: true, + }); return info.nth !== undefined ? locator.nth(info.nth) : locator; } diff --git a/extensions/browser/src/browser/pw-session.page-cdp.test.ts b/extensions/browser/src/browser/pw-session.page-cdp.test.ts index 01c99530a527..0f29ca03b5d3 100644 --- a/extensions/browser/src/browser/pw-session.page-cdp.test.ts +++ b/extensions/browser/src/browser/pw-session.page-cdp.test.ts @@ -58,9 +58,16 @@ describe("pw-session page-scoped CDP client", () => { expect(sessionDetach).toHaveBeenCalledTimes(1); }); - it("marks backend DOM refs on the page", async () => { + it("requests the document before marking backend DOM refs on the page", async () => { + let documentRequested = false; const sessionSend = vi.fn(async (method: string, params?: Record) => { + if (method === "DOM.getDocument") { + documentRequested = true; + } if (method === "DOM.pushNodesByBackendIdsToFrontend") { + if (!documentRequested) { + throw new Error("Document needs to be requested first"); + } expect(params).toEqual({ backendNodeIds: [42, 84] }); return { nodeIds: [101, 202] }; } @@ -89,7 +96,8 @@ describe("pw-session page-scoped CDP client", () => { expect(page.locator).toHaveBeenCalledWith(`[${BROWSER_REF_MARKER_ATTRIBUTE}]`); expect(evaluateAll).toHaveBeenCalledTimes(1); - expect(sessionSend).toHaveBeenNthCalledWith(1, "DOM.enable", undefined); + expect(marked).toEqual(new Set(["ax1", "ax2"])); + expect(sessionSend).toHaveBeenNthCalledWith(1, "DOM.getDocument", { depth: 0 }); expect(sessionSend).toHaveBeenNthCalledWith(2, "DOM.pushNodesByBackendIdsToFrontend", { backendNodeIds: [42, 84], }); @@ -103,7 +111,6 @@ describe("pw-session page-scoped CDP client", () => { name: BROWSER_REF_MARKER_ATTRIBUTE, value: "ax2", }); - expect(marked).toEqual(new Set(["ax1", "ax2"])); expect(sessionDetach).toHaveBeenCalledTimes(1); }); diff --git a/extensions/browser/src/browser/pw-session.page-cdp.ts b/extensions/browser/src/browser/pw-session.page-cdp.ts index e972600d85bd..04944853c8f8 100644 --- a/extensions/browser/src/browser/pw-session.page-cdp.ts +++ b/extensions/browser/src/browser/pw-session.page-cdp.ts @@ -99,7 +99,9 @@ export async function markBackendDomRefsOnPage(opts: { ) => Promise )(method, params); - await send("DOM.enable").catch(() => {}); + // Backend-id pushes require a bound document in this fresh session. + // getDocument also enables DOM; depth zero avoids fetching the subtree. + await send("DOM.getDocument", { depth: 0 }).catch(() => {}); const backendNodeIds = uniqueValues(refs.map((entry) => Math.floor(entry.backendDOMNodeId))); const pushed = (await send("DOM.pushNodesByBackendIdsToFrontend", { diff --git a/extensions/browser/src/browser/pw-session.test.ts b/extensions/browser/src/browser/pw-session.test.ts index 8557d3c11e70..4ce20dd6fb26 100644 --- a/extensions/browser/src/browser/pw-session.test.ts +++ b/extensions/browser/src/browser/pw-session.test.ts @@ -123,14 +123,19 @@ describe("pw-session refLocator", () => { expect(mocks.frameLocator).not.toHaveBeenCalled(); }); - it("uses page getByRole for role refs by default", () => { + it.each([ + { ref: "e1", name: "OK" }, + { ref: "e1", name: "" }, + { ref: "ax12", name: "OK" }, + { ref: "ax12", name: "" }, + ])("matches the exact name for unmarked $ref with name '$name'", ({ ref, name }) => { const { page, mocks } = fakePage(); const state = ensurePageState(page); - state.roleRefs = { e1: { role: "button", name: "OK" } }; + state.roleRefs = { [ref]: { role: "button", name } }; - refLocator(page, "e1"); + refLocator(page, ref); - expect(mocks.getByRole).toHaveBeenCalled(); + expect(mocks.getByRole).toHaveBeenCalledWith("button", { name, exact: true }); }); it("uses aria-ref locators when refs mode is aria", () => { @@ -153,16 +158,6 @@ describe("pw-session refLocator", () => { expect(mocks.locator).toHaveBeenCalledWith(`[${BROWSER_REF_MARKER_ATTRIBUTE}="ax12"]`); }); - it("falls back to role heuristics for ax refs without backend markers", () => { - const { page, mocks } = fakePage(); - const state = ensurePageState(page); - state.roleRefs = { ax12: { role: "button", name: "OK" } }; - - refLocator(page, "ax12"); - - expect(mocks.getByRole).toHaveBeenCalledWith("button", { name: "OK", exact: true }); - }); - it("rejects unknown ax refs instead of timing out on aria-ref locators", () => { const { page, mocks } = fakePage(); 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 7086f1dddb14..6f8c1c6b92d8 100644 --- a/extensions/browser/src/browser/pw-tools-core.snapshot.test.ts +++ b/extensions/browser/src/browser/pw-tools-core.snapshot.test.ts @@ -424,6 +424,7 @@ describe("pw-tools-core aria snapshot storage", () => { nodes: [ { ref: "ax1", role: "Button", name: "OK", backendDOMNodeId: 42, depth: 0 }, { ref: "ax2", role: "Button", name: "OK", backendDOMNodeId: 84, depth: 0 }, + { ref: "ax3", role: "Button", name: "", backendDOMNodeId: 126, depth: 0 }, ], }); @@ -434,6 +435,7 @@ describe("pw-tools-core aria snapshot storage", () => { refs: { ax1: { role: "button", name: "OK" }, ax2: { role: "button", name: "OK", nth: 1 }, + ax3: { role: "button", name: "" }, }, mode: "role", }); diff --git a/extensions/browser/src/browser/pw-tools-core.snapshot.ts b/extensions/browser/src/browser/pw-tools-core.snapshot.ts index c53aebe0e3b9..e31d6841fcf0 100644 --- a/extensions/browser/src/browser/pw-tools-core.snapshot.ts +++ b/extensions/browser/src/browser/pw-tools-core.snapshot.ts @@ -109,8 +109,8 @@ function buildStoredAriaRefs( for (const node of nodes) { const role = normalizeLowercaseStringOrEmpty(node.role) || "unknown"; - const name = node.name.trim() || undefined; - const key = `${role}:${name ?? ""}`; + const name = node.name.trim(); + const key = `${role}:${name}`; const nth = counts.get(key) ?? 0; counts.set(key, nth + 1); const refsForKey = refsByKey.get(key); @@ -121,7 +121,7 @@ function buildStoredAriaRefs( } refs[node.ref] = { role, - ...(name ? { name } : {}), + name, ...(nth > 0 ? { nth } : {}), ...(markedRefs.has(node.ref) ? { domMarker: true } : {}), };