diff --git a/apps/extension/src/tools/__tests__/file-transfer.test.ts b/apps/extension/src/tools/__tests__/file-transfer.test.ts index dfea01a8..740630ee 100644 --- a/apps/extension/src/tools/__tests__/file-transfer.test.ts +++ b/apps/extension/src/tools/__tests__/file-transfer.test.ts @@ -436,6 +436,8 @@ describe("file transfer tools", () => { if (method === "DOM.getContentQuads") { return { quads: [[10, 20, 210, 20, 210, 120, 10, 120]] }; } + if (method === "Runtime.evaluate") + return { result: { value: { width: 400, height: 300 } } }; if (method === "Page.getLayoutMetrics") { return { cssLayoutViewport: { clientWidth: 400, clientHeight: 300 } }; } diff --git a/apps/extension/src/tools/__tests__/frame-geometry.test.ts b/apps/extension/src/tools/__tests__/frame-geometry.test.ts index b535ea05..54696f1c 100644 --- a/apps/extension/src/tools/__tests__/frame-geometry.test.ts +++ b/apps/extension/src/tools/__tests__/frame-geometry.test.ts @@ -13,7 +13,138 @@ import { import { GeometryContext } from "../geometry/frame-context"; import type { CdpRunner } from "../shared"; +function scrollbarDriver( + visible = { width: 185, height: 89 }, + frameSize: unknown = { width: 200, height: 100 }, +) { + const send = vi.fn(async (target: { sessionId?: string }, method: string) => { + if (method === "Runtime.evaluate") return { result: { value: frameSize } }; + if (method === "Page.getLayoutMetrics") + return { + cssLayoutViewport: { + clientWidth: target.sessionId ? visible.width : 800, + clientHeight: target.sessionId ? visible.height : 600, + }, + }; + if (method === "DOM.getBoxModel") + return { + model: { + content: target.sessionId + ? [0, 0, 200, 0, 200, 100, 0, 100] + : [100, 100, 500, 100, 500, 300, 100, 300], + }, + }; + if (method === "DOM.getContentQuads") + return { quads: [[180, 80, 230, 80, 230, 120, 180, 120]] }; + throw new Error(`unexpected ${method}`); + }); + const cdp: CdpRunner = { + send: (tabId, method) => send({ tabId } as never, method) as never, + sendToTarget: send as CdpRunner["sendToTarget"], + getFrameGraph: async () => ({ + rootFrameId: "main", + frames: [ + { frameId: "main", target: { tabId: 4 } }, + { + frameId: "child", + parentFrameId: "main", + ownerBackendNodeId: 10, + target: { tabId: 4, sessionId: "child" }, + }, + { + frameId: "same-child", + parentFrameId: "child", + ownerBackendNodeId: 20, + target: { tabId: 4, sessionId: "child" }, + }, + ], + }), + }; + return { cdp, send }; +} + describe("frame geometry projection", () => { + it.each([ + { width: 200, height: 100 }, + { width: 185, height: 100 }, + { width: 200, height: 89 }, + { width: 185, height: 89 }, + ])("preserves scale and clips scrollbar strips for visible viewport $width × $height", async (visible) => { + const { cdp, send } = scrollbarDriver(visible); + const geometry = await resolveNodeGeometry(cdp, 4, { + target: { tabId: 4, sessionId: "child" }, + frameId: "child", + backendNodeId: 101, + }); + expect(geometry).toMatchObject({ + topBounds: { + x: 460, + y: 260, + width: (visible.width - 180) * 2, + height: (visible.height - 80) * 2, + }, + actionPoint: { x: 460 + visible.width - 180, y: 260 + visible.height - 80 }, + targetActionPoint: { x: (180 + visible.width) / 2, y: (80 + visible.height) / 2 }, + }); + expect(send.mock.calls.filter(([, method]) => method === "Runtime.evaluate")).toHaveLength(1); + expect(send.mock.calls.filter(([, method]) => method === "Page.getLayoutMetrics")).toHaveLength( + 2, + ); + }); + + it("shares the full viewport read across same-target frames, and refreshes it next operation", async () => { + const { cdp, send } = scrollbarDriver(); + const context = new GeometryContext(cdp, 4); + await Promise.all([context.targetProjection("child"), context.targetProjection("same-child")]); + expect(send.mock.calls.filter(([, method]) => method === "Runtime.evaluate")).toHaveLength(1); + await new GeometryContext(cdp, 4).targetProjection("child"); + expect(send.mock.calls.filter(([, method]) => method === "Runtime.evaluate")).toHaveLength(2); + }); + + it.each([ + null, + {}, + { width: 0, height: 100 }, + { width: 200, height: NaN }, + ])("rejects unavailable full viewport dimensions: %j", async (size) => { + const { cdp } = scrollbarDriver(undefined, size); + expect(await new GeometryContext(cdp, 4).targetProjection("child")).toBeNull(); + }); + + it("clips an inner OOPIF against an intermediate OOPIF's occupied scrollbar strips", async () => { + const { cdp } = scrollbarDriver(); + const outer = cdp.sendToTarget!; + cdp.sendToTarget = async (target, method, params) => { + if (method === "DOM.getBoxModel") + return { model: { content: [150, 60, 250, 60, 250, 120, 150, 120] } } as never; + if (target.sessionId !== "inner") return outer(target, method, params); + if (method === "Runtime.evaluate") + return { result: { value: { width: 100, height: 60 } } } as never; + if (method === "Page.getLayoutMetrics") + return { cssLayoutViewport: { clientWidth: 90, clientHeight: 50 } } as never; + if (method === "DOM.getContentQuads") + return { quads: [[0, 0, 100, 0, 100, 60, 0, 60]] } as never; + throw new Error(`unexpected ${method}`); + }; + const graph = await cdp.getFrameGraph!(4); + graph.frames[2] = { + frameId: "inner", + parentFrameId: "child", + ownerBackendNodeId: 20, + target: { tabId: 4, sessionId: "inner" }, + }; + cdp.getFrameGraph = async () => graph; + expect( + await resolveNodeGeometry(cdp, 4, { + target: { tabId: 4, sessionId: "inner" }, + frameId: "inner", + backendNodeId: 101, + }), + ).toMatchObject({ + topBounds: { x: 400, y: 220, width: 70, height: 58 }, + }); + }); + it("keeps region bounds separate from polygon area", () => { const region = [ rectPolygon({ x: 0, y: 0, w: 10, h: 10 }), @@ -77,6 +208,8 @@ describe("frame geometry projection", () => { const cdp: CdpRunner = { send: send as CdpRunner["send"], sendToTarget: vi.fn(async (_target, method) => { + if (method === "Runtime.evaluate") + return { result: { value: { width: 200, height: 100 } } }; if (method === "Page.getLayoutMetrics") { return { cssLayoutViewport: { clientWidth: 200, clientHeight: 100 } }; } @@ -135,6 +268,8 @@ describe("frame geometry projection", () => { throw new Error(`unexpected root command ${method}`); }) as CdpRunner["send"], sendToTarget: vi.fn(async (_target, method) => { + if (method === "Runtime.evaluate") + return { result: { value: { width: 200, height: 100 } } }; if (method === "Page.getLayoutMetrics") { return { cssLayoutViewport: { clientWidth: 200, clientHeight: 100 } }; } @@ -196,6 +331,8 @@ describe("frame geometry projection", () => { }) as CdpRunner["send"], sendToTarget: vi.fn(async (_target, method) => { if (method === "DOM.scrollIntoViewIfNeeded") return {}; + if (method === "Runtime.evaluate") + return { result: { value: { width: 200, height: 100 } } }; if (method === "Page.getLayoutMetrics") { return { cssLayoutViewport: { clientWidth: 200, clientHeight: 100 } }; } diff --git a/apps/extension/src/tools/__tests__/human-loop.test.ts b/apps/extension/src/tools/__tests__/human-loop.test.ts index a500735f..d695233d 100644 --- a/apps/extension/src/tools/__tests__/human-loop.test.ts +++ b/apps/extension/src/tools/__tests__/human-loop.test.ts @@ -433,6 +433,7 @@ describe("handleRequestHelp", () => { if (method === "DOM.getContentQuads") { return { quads: [[10, 20, 110, 20, 110, 60, 10, 60]] }; } + if (method === "Runtime.evaluate") return { result: { value: { width: 200, height: 100 } } }; if (method === "Page.getLayoutMetrics") { return { cssLayoutViewport: { clientWidth: 200, clientHeight: 100 } }; } diff --git a/apps/extension/src/tools/__tests__/interaction.test.ts b/apps/extension/src/tools/__tests__/interaction.test.ts index 7cb1d4dd..a7a4382f 100644 --- a/apps/extension/src/tools/__tests__/interaction.test.ts +++ b/apps/extension/src/tools/__tests__/interaction.test.ts @@ -201,6 +201,7 @@ describe("handleClick", () => { if (method === "DOM.getContentQuads") { return { quads: [[10, 20, 110, 20, 110, 60, 10, 60]] }; } + if (method === "Runtime.evaluate") return { result: { value: { width: 200, height: 100 } } }; if (method === "Page.getLayoutMetrics") { return { cssLayoutViewport: { clientWidth: 200, clientHeight: 100 } }; } @@ -220,6 +221,7 @@ describe("handleClick", () => { { sessionId: "child-session", method: "DOM.scrollIntoViewIfNeeded" }, { sessionId: "child-session", method: "DOM.getContentQuads" }, { sessionId: "child-session", method: "Page.getLayoutMetrics" }, + { sessionId: "child-session", method: "Runtime.evaluate" }, ]); expect(fake.sent.filter((call) => call.method === "Input.dispatchMouseEvent")).toHaveLength(3); }); diff --git a/apps/extension/src/tools/__tests__/observation.test.ts b/apps/extension/src/tools/__tests__/observation.test.ts index 95f86908..1fc9feb8 100644 --- a/apps/extension/src/tools/__tests__/observation.test.ts +++ b/apps/extension/src/tools/__tests__/observation.test.ts @@ -3380,6 +3380,7 @@ describe("handleSnapshot", () => { throw new Error(`unexpected root CDP method ${method}`); }); const sendToTarget = vi.fn(async (_target, method: string) => { + if (method === "Runtime.evaluate") return { result: { value: { width: 400, height: 300 } } }; if (method === "Page.getLayoutMetrics" && ownerGeometry !== "unavailable") return { visualViewport: { clientWidth: 1000 }, diff --git a/apps/extension/src/tools/__tests__/snapshot-coordinates.browser.test.ts b/apps/extension/src/tools/__tests__/snapshot-coordinates.browser.test.ts index 686f23a8..2ee3ef17 100644 --- a/apps/extension/src/tools/__tests__/snapshot-coordinates.browser.test.ts +++ b/apps/extension/src/tools/__tests__/snapshot-coordinates.browser.test.ts @@ -1,6 +1,7 @@ // @vitest-environment node import { describe, expect, it } from "vitest"; import type { CdpFrame, CdpFrameGraph, CdpTarget } from "@/browser-driver/frame-graph"; +import { resolveNodeGeometry } from "../frame-geometry"; import type { CdpRunner } from "../shared"; import { captureObservationFacts } from "../vom/capture-coordinator"; @@ -11,12 +12,28 @@ type Send = >( ) => Promise; type Tree = { frame: { id: string; parentId?: string; name?: string }; childFrames?: Tree[] }; type Rect = { x: number; y: number; w: number; h: number }; -type Oracle = { probe: Rect; owners: Record }; +type Oracle = { + probes: Record; + viewport: { width: number; height: number }; + owners: Record; +}; + +function clip(rect: Rect, viewport: Oracle["viewport"]): Rect | null { + const x = Math.max(0, rect.x), + y = Math.max(0, rect.y); + const right = Math.min(viewport.width, rect.x + rect.w); + const bottom = Math.min(viewport.height, rect.y + rect.h); + return right > x && bottom > y ? { x, y, w: right - x, h: bottom - y } : null; +} // The independent oracle uses DOM border boxes and this fixture's axis-aligned // iframe transforms. No production snapshot conversion/projection builds expectations. const oracleExpression = `(() => { - const box = document.querySelector('#probe').getBoundingClientRect(); + const probes = {}; + for (const node of document.querySelectorAll('[data-geometry-probe]')) { + const box = node.getBoundingClientRect(); + probes[node.id] = { x: box.x, y: box.y, w: box.width, h: box.height }; + } const owners = {}; for (const frame of document.querySelectorAll('iframe')) { const rect = frame.getBoundingClientRect(); @@ -28,17 +45,31 @@ const oracleExpression = `(() => { scale, }; } - return { probe: { x: box.x, y: box.y, w: box.width, h: box.height }, owners }; + return { probes, viewport: { width: document.documentElement.clientWidth, height: document.documentElement.clientHeight }, owners }; })()`; describe.skipIf(!process.env.BSK_GEOMETRY_CHROME)("real DOMSnapshot coordinate contract", () => { - it.each([ - { deviceScale: 1, zoom: 1 }, - { deviceScale: 0.8, zoom: 1 }, - { deviceScale: 1, zoom: 1.25 }, - { deviceScale: 2, zoom: 1 }, - { deviceScale: 2, zoom: 0.8 }, - ])("matches DOM geometry at device scale $deviceScale and zoom $zoom", async (configuration) => { + it.each( + [ + { deviceScale: 1, zoom: 1 }, + { deviceScale: 0.8, zoom: 1 }, + { deviceScale: 1, zoom: 1.25 }, + { deviceScale: 2, zoom: 1 }, + { deviceScale: 2, zoom: 0.8 }, + ] + .flatMap((configuration) => [ + { ...configuration, fixture: "snapshot-coordinates", scrollbars: "none" }, + { ...configuration, fixture: "oopif-scrollbars", scrollbars: "both" }, + ]) + .concat( + ["vertical", "horizontal", "none"].map((scrollbars) => ({ + deviceScale: 1, + zoom: 1, + fixture: "oopif-scrollbars", + scrollbars, + })), + ), + )("$fixture: device scale $deviceScale, zoom $zoom, scrollbars $scrollbars", async (configuration) => { const evalRoot = new URL("../../../../../evals/browser/", import.meta.url); const { createEvalServer } = await import(new URL("lib/server.mjs", evalRoot).href); const { withChrome } = await import( @@ -59,7 +90,9 @@ describe.skipIf(!process.env.BSK_GEOMETRY_CHROME)("real DOMSnapshot coordinate c ); await send( "Page.navigate", - { url: `${baseUrl}/snapshot-coordinates?run=coordinates` }, + { + url: `${baseUrl}/${configuration.fixture}?run=coordinates&scrollbars=${configuration.scrollbars}`, + }, rootSession, ); await expect @@ -86,7 +119,7 @@ describe.skipIf(!process.env.BSK_GEOMETRY_CHROME)("real DOMSnapshot coordinate c }); sessions.push(sessionId); } - expect(sessions.length).toBe(2); // The cross-site fixture must actually be an OOPIF. + expect(sessions.length).toBe(configuration.fixture === "oopif-scrollbars" ? 3 : 2); const frames: CdpFrame[] = []; const names = new Map(); const sessionFor = (target: CdpTarget) => target.sessionId ?? rootSession; @@ -107,7 +140,7 @@ describe.skipIf(!process.env.BSK_GEOMETRY_CHROME)("real DOMSnapshot coordinate c }; visit(frameTree); } - expect(frames).toHaveLength(5); + expect(frames).toHaveLength(configuration.fixture === "oopif-scrollbars" ? 3 : 5); for (const frame of frames) { if (!frame.parentFrameId) continue; const parent = frames.find((item) => item.frameId === frame.parentFrameId)!; @@ -127,6 +160,11 @@ describe.skipIf(!process.env.BSK_GEOMETRY_CHROME)("real DOMSnapshot coordinate c }, sendToTarget: (target, method, params) => { calls.push(`${target.sessionId ?? target.tabId}:${method}`); + if ( + method === "Runtime.evaluate" && + (params as { expression?: string })?.expression?.includes("window.innerWidth") + ) + calls.push(`${target.sessionId ?? target.tabId}:viewport-size`); return send(method, params, sessionFor(target)); }, getFrameGraph: async () => graph, @@ -179,47 +217,130 @@ describe.skipIf(!process.env.BSK_GEOMETRY_CHROME)("real DOMSnapshot coordinate c ); oracles.set(frame.frameId, reply.result.value); } + const childViewport = await send<{ + result: { + value: { width: number; height: number; clientWidth: number; clientHeight: number }; + }; + }>( + "Runtime.evaluate", + { + expression: + "({ width: innerWidth, height: innerHeight, clientWidth: document.documentElement.clientWidth, clientHeight: document.documentElement.clientHeight })", + returnByValue: true, + }, + sessions[1], + ); + for (const dimension of ["Width", "Height"] as const) { + const occupied = + childViewport.result.value[dimension === "Width" ? "width" : "height"] - + childViewport.result.value[`client${dimension}`]; + if ( + configuration.scrollbars === "both" || + configuration.scrollbars === (dimension === "Width" ? "vertical" : "horizontal") + ) + expect(occupied).toBeGreaterThan(0); + else expect(occupied).toBe(0); + } const facts = await captureObservationFacts(cdp, 1); expect(facts.issues).toEqual([]); - for (const frame of frames) { - const node = facts.documents - .find((doc) => doc.frame.frameId === frame.frameId) - ?.domNodes.find((node) => node.attrs.id === "probe"); - expect(node, `missing probe in ${names.get(frame.frameId) || "root"}`).toBeDefined(); - const local = oracles.get(frame.frameId)!.probe; - const top = { ...local }; - let current = frame; - while (current.parentFrameId) { - const owner = oracles.get(current.parentFrameId)!.owners[names.get(current.frameId)!]; - top.x = owner.x + top.x * owner.scale; - top.y = owner.y + top.y * owner.scale; - top.w *= owner.scale; - top.h *= owner.scale; - current = frames.find((item) => item.frameId === current.parentFrameId)!; - } - expect( - node!.rect, - JSON.stringify({ - frame: names.get(frame.frameId), - local, - top, - viewport: facts.viewport, - }), - ).not.toBeNull(); - for (const key of ["x", "y", "w", "h"] as const) { - expect( - Math.abs(node!.localRect![key] - local[key]), - `${names.get(frame.frameId)} local ${key}`, - ).toBeLessThan(2); - expect( - Math.abs(node!.rect![key] - top[key]), - `${names.get(frame.frameId)} top ${key}`, - ).toBeLessThan(2); - } - } expect(calls.filter((call) => call.endsWith(":Page.getLayoutMetrics"))).toHaveLength( sessions.length, ); + // Identity verification also uses Runtime.evaluate; count the full viewport reads separately. + const viewportReadsBeforeLive = calls.filter((call) => call.endsWith(":viewport-size")); + expect(new Set(viewportReadsBeforeLive).size).toBe(viewportReadsBeforeLive.length); + expect(viewportReadsBeforeLive.length).toBeLessThanOrEqual(sessions.length - 1); + for (const frame of frames) { + for (const [id, local] of Object.entries(oracles.get(frame.frameId)!.probes)) { + const node = facts.documents + .find((doc) => doc.frame.frameId === frame.frameId) + ?.domNodes.find((node) => node.attrs.id === id); + expect(node, `missing ${id} in ${names.get(frame.frameId) || "root"}`).toBeDefined(); + let top = clip(local, oracles.get(frame.frameId)!.viewport); + let current = frame; + while (top && current.parentFrameId) { + const parentOracle = oracles.get(current.parentFrameId)!; + const owner = parentOracle.owners[names.get(current.frameId)!]; + top = clip( + { + x: owner.x + top.x * owner.scale, + y: owner.y + top.y * owner.scale, + w: top.w * owner.scale, + h: top.h * owner.scale, + }, + parentOracle.viewport, + ); + current = frames.find((item) => item.frameId === current.parentFrameId)!; + } + for (const key of ["x", "y", "w", "h"] as const) + expect( + Math.abs(node!.localRect![key] - local[key]), + `${names.get(frame.frameId)} ${id} local ${key}`, + ).toBeLessThan(2); + if (!top) { + expect(node!.rect, `${id} must be clipped out`).toBeNull(); + if (configuration.fixture === "oopif-scrollbars") + expect( + await resolveNodeGeometry(cdp, 1, { + target: frame.target, + frameId: frame.frameId, + backendNodeId: node!.backendNodeId, + }), + ).toMatchObject({ code: "permission_denied" }); + continue; + } + expect(node!.rect).not.toBeNull(); + for (const key of ["x", "y", "w", "h"] as const) + expect( + Math.abs(node!.rect![key] - top[key]), + `${names.get(frame.frameId)} ${id} top ${key}`, + ).toBeLessThan(2); + if (configuration.fixture !== "oopif-scrollbars") continue; + const live = await resolveNodeGeometry(cdp, 1, { + target: frame.target, + frameId: frame.frameId, + backendNodeId: node!.backendNodeId, + }); + if ("code" in live) throw new Error(live.message); + for (const [key, liveKey] of [ + ["x", "x"], + ["y", "y"], + ["w", "width"], + ["h", "height"], + ] as const) + expect( + Math.abs(live.topBounds[liveKey] - top[key]), + `${id} live ${key}`, + ).toBeLessThan(2); + const before = server.snapshot("coordinates").events.length; + for (const type of ["mousePressed", "mouseReleased"]) + await send( + "Input.dispatchMouseEvent", + { type, ...live.actionPoint, button: "left", clickCount: 1 }, + rootSession, + ); + await expect + .poll( + () => + server + .snapshot("coordinates") + .events.slice(before) + .some( + (event: { type: string; path: string; data: { probe?: string } }) => + event.type === "geometry.clicked" && + event.data.probe === id && + event.path === + (frame.parentFrameId + ? names.get(frame.frameId) === "nested" + ? "/oopif-scrollbars/nested" + : "/oopif-scrollbars/frame" + : "/oopif-scrollbars"), + ), + { timeout: 3000 }, + ) + .toBe(true); + } + } const metrics = await send<{ cssVisualViewport: { zoom: number }; visualViewport: { clientWidth: number }; diff --git a/apps/extension/src/tools/frame-geometry.ts b/apps/extension/src/tools/frame-geometry.ts index 0fb997ca..c9f0b7eb 100644 --- a/apps/extension/src/tools/frame-geometry.ts +++ b/apps/extension/src/tools/frame-geometry.ts @@ -161,7 +161,8 @@ export async function resolveNodeGeometry( return geometryError(`could not resolve frame geometry for ${address.frameId}`); topVisibleRegions = projectRegionToViewport(localRegion, projection); if (address.target.sessionId) { - const localViewport = projection.edges[0]?.sourceViewport ?? projection.topViewport; + const localViewport = await context.viewport(address.target); + if (!localViewport) return geometryError("could not resolve target viewport geometry"); const localVisibleRegions = localRegion .map((polygon) => clipPolygon( diff --git a/apps/extension/src/tools/geometry/frame-context.ts b/apps/extension/src/tools/geometry/frame-context.ts index c7280967..7956f2bf 100644 --- a/apps/extension/src/tools/geometry/frame-context.ts +++ b/apps/extension/src/tools/geometry/frame-context.ts @@ -9,8 +9,10 @@ import { type Polygon, type ProjectiveEdge, parseCdpQuad, + projectPolygon, type Quad, type Size, + viewportPolygon, } from "../geometry"; import { type CdpRunner, sendToCdpTarget } from "../shared"; import type { CoordinateOwner, CssViewport, SnapshotProjectionResult } from "./coordinate-types"; @@ -67,6 +69,7 @@ export function cssViewport(metrics: LayoutMetrics): CssViewport { /** One read-only measurement phase. Discard after scrolling or any later operation. */ export class GeometryContext { private readonly metrics = new Map>(); + private readonly frameViewports = new Map>(); private readonly owners = new Map>(); private readonly projections = new Map>(); private readonly snapshotOwners = new Map>(); @@ -157,6 +160,35 @@ export class GeometryContext { : null; } + /** The full OOPIF viewport maps to its owner's content quad. Layout metrics + * exclude occupied scrollbars and remain the clipping boundary, not the scale. */ + private frameViewport(target: CdpTarget): Promise { + const key = cdpTargetKey(target); + let promise = this.frameViewports.get(key); + if (!promise) { + promise = this.request<{ result?: { value?: Size }; exceptionDetails?: unknown }>( + target, + "Runtime.evaluate", + { + expression: "({ width: window.innerWidth, height: window.innerHeight })", + returnByValue: true, + }, + ).then(({ result, exceptionDetails }) => { + const size = result?.value; + return !exceptionDetails && + size && + Number.isFinite(size.width) && + Number.isFinite(size.height) && + size.width > 0 && + size.height > 0 + ? size + : null; + }); + this.frameViewports.set(key, promise); + } + return promise; + } + ownerContent(target: CdpTarget, backendNodeId: number): Promise { const key = `${cdpTargetKey(target)}:${backendNodeId}`; let promise = this.owners.get(key); @@ -342,12 +374,19 @@ export class GeometryContext { const parent = frames.get(root.parentFrameId); if (!parent || root.ownerBackendNodeId === undefined) return null; const destinationQuad = await this.ownerContent(parent.target, root.ownerBackendNodeId); - const source = await this.viewport(root.target); + if (!destinationQuad) return null; + const source = await this.frameViewport(root.target); + const visible = await this.viewport(root.target); const parentRoot = this.targetRoot(frames, parent); - if (!destinationQuad || !source || !parentRoot) return null; + if (!source || !visible || !parentRoot) return null; const destinationClips = await this.clips(frames, parent, parentRoot); if (!destinationClips) return null; - edges.push({ sourceViewport: source, destinationQuad, destinationClips }); + const edge = { sourceViewport: source, destinationQuad, destinationClips }; + // Keep scrollbar strips clipped at every target boundary, including + // intermediate OOPIFs. Project the clip with the same full-viewport scale. + if (source.width !== visible.width || source.height !== visible.height) + destinationClips.push(projectPolygon(viewportPolygon(visible), edge)); + edges.push(edge); root = parentRoot; } const topViewport = await this.viewport(root.target); diff --git a/apps/extension/src/tools/vom/__tests__/capture-coordinator.test.ts b/apps/extension/src/tools/vom/__tests__/capture-coordinator.test.ts index 8b7b8a09..07747b18 100644 --- a/apps/extension/src/tools/vom/__tests__/capture-coordinator.test.ts +++ b/apps/extension/src/tools/vom/__tests__/capture-coordinator.test.ts @@ -655,6 +655,7 @@ function childSnapshot(frameId: string, backendNodeId: number) { describe("OOPIF capture", () => { it("captures and positions multiple OOPIF documents missing from the root snapshot", async () => { const sendToTarget = vi.fn(async (target, method) => { + if (method === "Runtime.evaluate") return { result: { value: { width: 300, height: 200 } } }; if (method === "Page.getLayoutMetrics") { return { visualViewport: { clientWidth: 1000 }, diff --git a/evals/browser/cases/regression/oopif-scrollbars/README.md b/evals/browser/cases/regression/oopif-scrollbars/README.md new file mode 100644 index 00000000..e1223db2 --- /dev/null +++ b/evals/browser/cases/regression/oopif-scrollbars/README.md @@ -0,0 +1,41 @@ +# OOPIF occupied-scrollbar regression + +An iframe's content quad includes its child viewport's scrollbar space, while +`Page.getLayoutMetrics().cssLayoutViewport` excludes that space. Mapping the latter +onto the whole quad stretches positions and sizes. The full target-local viewport +must determine scale; the visible viewport must still clip content at every OOPIF +boundary and constrain target-local action points. + +This fixture alternates loopback hostnames across two nested OOPIFs. It includes +scrolling, borders, padding, scaled iframe owners, a partially clipped button and +a fully clipped button. Custom scrollbars occupy different widths and heights. +The `scrollbars` query parameter accepts `both` (default), `vertical`, `horizontal` +or `none`. + +Run the numerical regression with Node 22+ and a local Chrome executable: + +```sh +BSK_GEOMETRY_CHROME=/path/to/chrome pnpm --filter @browser-skill/extension exec vitest run \ + src/tools/__tests__/snapshot-coordinates.browser.test.ts +``` + +The existing browser runner is shared with the snapshot-unit regression. The test: + +- Verifies actual OOPIF targets and occupied scrollbar dimensions. +- Compares snapshot and live geometry with independent DOM rectangles and clipping. +- Dispatches real root-target clicks and verifies the intended frame/button received them. +- Rejects fully clipped controls and checks snapshot measurement reuse per target. +- Covers five device-scale/browser-zoom combinations plus single-axis and no-scrollbar cases. + +The runner creates and removes isolated browser profiles. It is opt-in and skipped +in normal unit runs when `BSK_GEOMETRY_CHROME` is unset. + +Run the CLI fixture smoke with a connected test extension: + +```sh +BSK_AUTO_UPDATE=off pnpm eval:browser smoke --case oopif-scrollbars --bsk ./target/debug/bsk +``` + +The smoke assertions verify both nested frames have occupied scrollbars, that the +marker is observed and that the session is closed. Numeric geometry and actual +click assertions are exercised by the browser test above, not by smoke alone. diff --git a/evals/browser/cases/regression/oopif-scrollbars/oopif-scrollbars.case.json b/evals/browser/cases/regression/oopif-scrollbars/oopif-scrollbars.case.json new file mode 100644 index 00000000..1f336a57 --- /dev/null +++ b/evals/browser/cases/regression/oopif-scrollbars/oopif-scrollbars.case.json @@ -0,0 +1,39 @@ +{ + "$schema": "../../../schemas/case.schema.json", + "schemaVersion": 1, + "id": "oopif-scrollbars", + "title": "Nested OOPIFs with occupied scrollbars and clipped controls", + "suite": "regression", + "tags": ["geometry", "iframe", "scroll"], + "fixture": { "startPath": "/oopif-scrollbars" }, + "prompts": { + "en": "Open {url}, observe the page and its frames, report the OOPIF-SCROLLBARS marker, then close the session.", + "zh-CN": "打开 {url},观察页面及其 iframe,报告 OOPIF-SCROLLBARS 标记,然后关闭会话。" + }, + "coverage": ["session.start", "session.stop", "page.navigate", "inspect.observe"], + "assertions": { + "site": [ + { + "label": "nested frame fixture settled", + "type": "geometry.ready", + "where": { "data.root": true }, + "minCount": 1 + }, + { + "label": "both child frames have occupied scrollbars", + "type": "geometry.scrollbars", + "where": { "data.vertical": true, "data.horizontal": true }, + "minCount": 2 + } + ], + "response": [{ "label": "observed the fixture marker", "includes": "OOPIF-SCROLLBARS" }], + "adapter": [{ "label": "browser session was closed", "key": "sessionStopped" }] + }, + "smoke": { + "steps": [ + { "action": "navigate" }, + { "action": "wait-site-event", "type": "geometry.ready", "where": { "root": true } }, + { "action": "observe" } + ] + } +} diff --git a/evals/browser/cases/regression/oopif-scrollbars/oopif-scrollbars.fixture.mjs b/evals/browser/cases/regression/oopif-scrollbars/oopif-scrollbars.fixture.mjs new file mode 100644 index 00000000..b2b46537 --- /dev/null +++ b/evals/browser/cases/regression/oopif-scrollbars/oopif-scrollbars.fixture.mjs @@ -0,0 +1,56 @@ +import { page, withRun } from "../../../lib/fixtures.mjs"; + +export default { + id: "oopif-scrollbars", + routes: ["/oopif-scrollbars", "/oopif-scrollbars/frame", "/oopif-scrollbars/nested"], + render({ pathname, runId, query }) { + const root = pathname === "/oopif-scrollbars"; + const nested = pathname.endsWith("/nested"); + const mode = query.get("scrollbars") ?? "both"; + return page({ + title: "OOPIF scrollbar geometry regression", + body: ` + + + ${!root ? '' : ""} + ${!nested ? `` : ""} + `, + script: ` + history.scrollRestoration = "manual"; + const child = document.querySelector("iframe"); + if (child) { + const url = new URL(${JSON.stringify(withRun(root ? "/oopif-scrollbars/frame" : "/oopif-scrollbars/nested", runId, { scrollbars: mode }))}, location.href); + url.hostname = location.hostname === "127.0.0.1" ? "localhost" : "127.0.0.1"; + child.src = url.href; + } + document.addEventListener("click", event => { + const probe = event.target.closest("[data-geometry-probe]"); + if (probe) browserEval.send("geometry.clicked", { probe: probe.id }); + }); + window.addEventListener("load", () => { + scrollTo(${root ? "80, 240" : nested ? "10, 20" : "40, 100"}); + requestAnimationFrame(() => requestAnimationFrame(() => { + document.documentElement.dataset.geometryReady = "true"; + if (!${root}) browserEval.send("geometry.scrollbars", { + vertical: innerWidth > document.documentElement.clientWidth, + horizontal: innerHeight > document.documentElement.clientHeight, + }); + browserEval.send("geometry.ready", { root: ${root} }); + })); + }); + `, + }); + }, +}; diff --git a/evals/browser/cases/regression/snapshot-coordinates/README.md b/evals/browser/cases/regression/snapshot-coordinates/README.md index 4e7f950f..403cd717 100644 --- a/evals/browser/cases/regression/snapshot-coordinates/README.md +++ b/evals/browser/cases/regression/snapshot-coordinates/README.md @@ -33,12 +33,9 @@ BSK_AUTO_UPDATE=off pnpm eval:browser smoke --case snapshot-coordinates --bsk ./ Smoke alone does **not** certify coordinate accuracy; CLI observations do not expose raw boxes. Use the geometric test above for the regression's numeric assertions. -## Separate existing boundary - -The OOPIF root uses non-occupying scrollbars in the unit-conversion regression. Root and -same-process frames retain normal scrollbars. Append `&classic-scrollbars` to the fixture URL -to reproduce the separate existing OOPIF projection issue: `targetProjection()` maps the CSS -layout viewport (which excludes occupying scrollbars) onto the entire owner content quad. -This inflates the projected coordinates when scrollbars occupy space. Main already has that -mapping; fixing it also changes shared live geometry and is outside this snapshot-unit fix. -The test does not relax its numeric tolerance to absorb that error. +## Related scrollbar regression + +The unit-conversion fixture keeps non-occupying OOPIF scrollbars by default. The same +browser test also runs the dedicated [OOPIF scrollbar fixture](../oopif-scrollbars/README.md), +which covers occupied scrollbars, two OOPIF boundaries, clipping and real clicks. +Append `&classic-scrollbars` to this fixture's URL to enable its normal OOPIF scrollbars. diff --git a/evals/browser/tests/eval.test.mjs b/evals/browser/tests/eval.test.mjs index c0ab22c3..bb8c4d72 100644 --- a/evals/browser/tests/eval.test.mjs +++ b/evals/browser/tests/eval.test.mjs @@ -41,22 +41,23 @@ test("case manifests are discovered, ordered, and grouped into suites", () => { "diagnostics", "mobile-emulation", "generated-form", + "oopif-scrollbars", "snapshot-coordinates", ], ); assert.deepEqual(repositorySummary(cases, fixtureRegistry).suites, { core: 6, matrix: 1, - regression: 1, + regression: 2, }); }); test("repository validation links every case to a fixture and valid workflow evidence", () => { assert.deepEqual(validateRepositoryCases(cases, fixtureRegistry), []); const summary = repositorySummary(cases, fixtureRegistry); - assert.equal(summary.cases, 8); - assert.equal(summary.fixtureModules, 9); - assert.equal(summary.fixtureRoutes, 15); + assert.equal(summary.cases, 9); + assert.equal(summary.fixtureModules, 10); + assert.equal(summary.fixtureRoutes, 18); }); test("manifest validation rejects unknown operations and incomplete workflow steps", () => {