From 383e3128ebba0410985cdb9fd475928ae754c4c0 Mon Sep 17 00:00:00 2001 From: drakezhang Date: Wed, 9 Sep 2026 15:53:35 +0800 Subject: [PATCH] fix(geometry): normalize snapshot layout bounds and scroll together --- .../tools/__tests__/geometry-context.test.ts | 102 +++++++- .../src/tools/__tests__/observation.test.ts | 44 +++- .../snapshot-coordinates.browser.test.ts | 237 ++++++++++++++++++ .../src/tools/geometry/coordinate-types.ts | 67 ++++- .../src/tools/geometry/frame-context.ts | 27 +- apps/extension/src/tools/observation.ts | 2 +- .../src/tools/vom/__tests__/capture.test.ts | 85 ++++++- .../tools/vom/__tests__/frame-capture.test.ts | 33 ++- apps/extension/src/tools/vom/capture.ts | 81 ++++-- .../regression/snapshot-coordinates/README.md | 44 ++++ .../snapshot-coordinates/chrome.mjs | 99 ++++++++ .../snapshot-coordinates.case.json | 33 +++ .../snapshot-coordinates.fixture.mjs | 52 ++++ evals/browser/tests/eval.test.mjs | 13 +- 14 files changed, 856 insertions(+), 63 deletions(-) create mode 100644 apps/extension/src/tools/__tests__/snapshot-coordinates.browser.test.ts create mode 100644 evals/browser/cases/regression/snapshot-coordinates/README.md create mode 100644 evals/browser/cases/regression/snapshot-coordinates/chrome.mjs create mode 100644 evals/browser/cases/regression/snapshot-coordinates/snapshot-coordinates.case.json create mode 100644 evals/browser/cases/regression/snapshot-coordinates/snapshot-coordinates.fixture.mjs diff --git a/apps/extension/src/tools/__tests__/geometry-context.test.ts b/apps/extension/src/tools/__tests__/geometry-context.test.ts index 3b295f0..34bac76 100644 --- a/apps/extension/src/tools/__tests__/geometry-context.test.ts +++ b/apps/extension/src/tools/__tests__/geometry-context.test.ts @@ -5,9 +5,10 @@ import { rectPolygon } from "../geometry"; import { projectSnapshotRect, screenshotPageRect, + snapshotCoordinates, snapshotViewportRect, } from "../geometry/coordinate-types"; -import { GeometryContext } from "../geometry/frame-context"; +import { GeometryContext, snapshotLayoutScale } from "../geometry/frame-context"; import type { CdpRunner } from "../shared"; const target = { tabId: 4 }; @@ -22,7 +23,11 @@ const graph: CdpFrameGraph = { function driver() { const send = vi.fn(async (_tab: number, method: string): Promise => { if (method === "Page.getLayoutMetrics") - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 1000 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 1000 }, + }; if (method === "DOM.getBoxModel") return { model: { content: [52.5, 612.5, 427.5, 612.5, 427.5, 862.5, 52.5, 862.5] } }; if (method === "DOM.resolveNode") return { object: { objectId: "owner" } }; @@ -35,6 +40,63 @@ function driver() { } describe("measurement geometry context", () => { + it.each([0.8, 1, 1.25, 2])("normalizes snapshot layout units at scale %s", (scale) => { + const measured = snapshotLayoutScale({ + visualViewport: { clientWidth: 997.25, zoom: 1.1 }, + cssVisualViewport: { clientWidth: 997.25 / scale, zoom: 1.1 }, + // Rounded layout dimensions must not supply the scale. + layoutViewport: { clientWidth: 997 }, + cssLayoutViewport: { clientWidth: Math.floor(997.25 / scale) }, + }); + expect(measured).toBeCloseTo(scale, 12); + const coordinates = snapshotCoordinates( + { scrollOffsetX: 80 * scale, scrollOffsetY: 200 * scale }, + measured, + { x: 999, y: 999 }, + ); + expect( + snapshotViewportRect( + [160 * scale, 320 * scale, 120 * scale, 40 * scale], + { target }, + coordinates, + )?.rect, + ).toEqual({ x: 80, y: 120, width: 120, height: 40 }); + }); + + it("uses CSS root scroll only for missing snapshot offsets, without scaling it twice", () => { + const coordinates = snapshotCoordinates({ scrollOffsetX: 80 }, 2, { x: 999, y: 100 }); + expect(snapshotViewportRect([200, 800, 240, 80], { target }, coordinates)?.rect).toEqual({ + x: 60, + y: 300, + width: 120, + height: 40, + }); + expect(snapshotCoordinates({ scrollOffsetX: 0 }, 2)).toBeNull(); + expect( + snapshotCoordinates({ scrollOffsetX: NaN, scrollOffsetY: 0 }, 2, { x: 0, y: 0 }), + ).toBeNull(); + }); + + it("does not infer snapshot units from missing, invalid, or CSS-only metrics", () => { + expect(snapshotLayoutScale({ cssLayoutViewport: { clientWidth: 800 } })).toBeNull(); + for (const raw of [0, -1, NaN, Infinity]) { + expect( + snapshotLayoutScale({ + visualViewport: { clientWidth: raw }, + cssVisualViewport: { clientWidth: 800 }, + }), + ).toBeNull(); + } + expect( + snapshotLayoutScale({ + visualViewport: { clientWidth: 0, clientHeight: 600 }, + cssVisualViewport: { clientWidth: 0, clientHeight: 300 }, + }), + ).toBe(2); + expect(snapshotCoordinates({ scrollOffsetX: 0, scrollOffsetY: 0 }, null)).toBeNull(); + expect(snapshotViewportRect([10, 20, 120, 40], { target }, null)).toBeNull(); + }); + it("shares in-flight frame measurements, but never shares them with the next context", async () => { const cdp = driver(); const context = new GeometryContext(cdp, 4); @@ -61,14 +123,20 @@ describe("measurement geometry context", () => { height: 1000, }); if (projection.status !== "available") throw new Error("expected available projection"); - const input = snapshotViewportRect([17, 23, 120, 40], source, { x: 0, y: 0 }); + const input = snapshotViewportRect([17, 23, 120, 40], source, { + layoutUnitsPerCssPixel: 1, + scrollCss: { x: 0, y: 0 }, + }); expect(projectSnapshotRect(input!, projection.projection)).toEqual({ x: 73.75, y: 641.25, width: 150, height: 50, }); - const scrolled = snapshotViewportRect([17, 23, 120, 40], source, { x: 0, y: 10 }); + const scrolled = snapshotViewportRect([17, 23, 120, 40], source, { + layoutUnitsPerCssPixel: 1, + scrollCss: { x: 0, y: 10 }, + }); expect(projectSnapshotRect(scrolled!, projection.projection)?.y).toBe(628.75); await context.snapshotProjection(source, 10, [], { width: 1000, height: 1000 }); expect(cdp.calls.mock.calls.filter(([, method]) => method === "DOM.resolveNode")).toHaveLength( @@ -173,7 +241,7 @@ describe("measurement geometry context", () => { const input = snapshotViewportRect( [30, 50, 120, 40], { target, frameId: "main" }, - { x: 10, y: 20 }, + { layoutUnitsPerCssPixel: 1, scrollCss: { x: 10, y: 20 } }, )!; const projection = { source: { target, frameId: "main" }, @@ -194,8 +262,20 @@ describe("measurement geometry context", () => { source: { target: { tabId: 5 }, frameId: "main" }, }), ).toBeNull(); - expect(snapshotViewportRect([0, 0, NaN, 40], { target }, { x: 0, y: 0 })).toBeNull(); - expect(snapshotViewportRect([0, 0, 10, 40], { target }, { x: Infinity, y: 0 })).toBeNull(); + expect( + snapshotViewportRect( + [0, 0, NaN, 40], + { target }, + { layoutUnitsPerCssPixel: 1, scrollCss: { x: 0, y: 0 } }, + ), + ).toBeNull(); + expect( + snapshotViewportRect( + [0, 0, 10, 40], + { target }, + { layoutUnitsPerCssPixel: 1, scrollCss: { x: Infinity, y: 0 } }, + ), + ).toBeNull(); }); it("does not turn a border quad or a failed owner read into a content projection", async () => { @@ -228,7 +308,7 @@ describe("measurement geometry context", () => { const input = snapshotViewportRect( [0, 0, 300, 200], { target, frameId: "nested" }, - { x: 0, y: 0 }, + { layoutUnitsPerCssPixel: 1, scrollCss: { x: 0, y: 0 } }, ); expect(projectSnapshotRect(input!, projection.projection)).toEqual({ x: 60, @@ -278,7 +358,11 @@ describe("measurement geometry context", () => { peak = Math.max(peak, active); await new Promise((resolve) => release.push(resolve)); active--; - return { cssLayoutViewport: { clientWidth: 100, clientHeight: 100 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 100, clientHeight: 100 }, + }; }); const controller = new AbortController(); const context = new GeometryContext( diff --git a/apps/extension/src/tools/__tests__/observation.test.ts b/apps/extension/src/tools/__tests__/observation.test.ts index 8c24651..76d989c 100644 --- a/apps/extension/src/tools/__tests__/observation.test.ts +++ b/apps/extension/src/tools/__tests__/observation.test.ts @@ -65,7 +65,11 @@ function makeFakeCdp(handlers: Record unknown>) { sent.push({ method, params }); const handler = handlers[method]; if (!handler && method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1280, clientHeight: 720 }, + }; } if (!handler) throw new Error(`unexpected CDP call ${method}`); return handler(params); @@ -2494,7 +2498,11 @@ describe("handleSnapshot", () => { if (method === "Accessibility.enable") return {}; if (method === "Accessibility.getFullAXTree") return { nodes }; if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, + }; } if (method === "DOMSnapshot.enable") return {}; if (method === "DOMSnapshot.captureSnapshot") throw new Error("snapshot unsupported"); @@ -2581,7 +2589,11 @@ describe("handleSnapshot", () => { if (method === "Accessibility.enable") return {}; if (method === "Accessibility.getFullAXTree") return { nodes: [root, button] }; if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, + }; } if (method === "DOMSnapshot.enable") return {}; if (method === "DOMSnapshot.captureSnapshot") { @@ -2672,7 +2684,11 @@ describe("handleSnapshot", () => { if (method === "Accessibility.enable") return {}; if (method === "Accessibility.getFullAXTree") return { nodes: [root, button] }; if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, + }; } if (method === "DOMSnapshot.enable") return {}; if (method === "DOMSnapshot.captureSnapshot") { @@ -2766,7 +2782,11 @@ describe("handleSnapshot", () => { if (method === "Accessibility.enable") return {}; if (method === "Accessibility.getFullAXTree") return { nodes: [root, button] }; if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, + }; } if (method === "DOMSnapshot.enable") return {}; if (method === "DOMSnapshot.captureSnapshot") { @@ -2931,7 +2951,11 @@ describe("handleSnapshot", () => { }; } if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 }, + }; } if (method === "DOMSnapshot.enable") return {}; if (method === "DOMSnapshot.captureSnapshot") { @@ -3144,6 +3168,8 @@ describe("handleSnapshot", () => { } const VP_METRICS = { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, }; @@ -3157,6 +3183,8 @@ describe("handleSnapshot", () => { strings, documents: [ { + scrollOffsetX: 0, + scrollOffsetY: 0, frameId: "main", nodes: { parentIndex: [-1, 0, 1], @@ -3177,6 +3205,8 @@ describe("handleSnapshot", () => { }, }, { + scrollOffsetX: 0, + scrollOffsetY: 0, frameId: "child", nodes: { parentIndex: [-1, 0, 1], @@ -3488,7 +3518,7 @@ describe("handleSnapshot", () => { maxTokens, }); expect(result.text.match(/@warning/g)).toHaveLength(1); - expect(result.text).toContain("iframe geometry incomplete"); + expect(result.text).toContain("geometry incomplete"); expect(result.text.startsWith("@vom 1\n")).toBe(true); if (maxTokens === undefined) { expect(result.truncated).toBe(false); diff --git a/apps/extension/src/tools/__tests__/snapshot-coordinates.browser.test.ts b/apps/extension/src/tools/__tests__/snapshot-coordinates.browser.test.ts new file mode 100644 index 0000000..5ef89d9 --- /dev/null +++ b/apps/extension/src/tools/__tests__/snapshot-coordinates.browser.test.ts @@ -0,0 +1,237 @@ +// @vitest-environment node +import { describe, expect, it } from "vitest"; +import type { CdpFrame, CdpFrameGraph, CdpTarget } from "@/browser-driver/frame-graph"; +import { GeometryContext } from "../geometry/frame-context"; +import type { CdpRunner } from "../shared"; +import { captureViewModel } from "../vom/capture"; +import { captureFrameData } from "../vom/frame-capture"; + +type Send = >( + method: string, + params?: object, + sessionId?: string, +) => 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 }; + +// 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 owners = {}; + for (const frame of document.querySelectorAll('iframe')) { + const rect = frame.getBoundingClientRect(); + const style = getComputedStyle(frame); + const scale = rect.width / frame.offsetWidth; + owners[frame.name] = { + x: rect.x + (frame.clientLeft + parseFloat(style.paddingLeft)) * scale, + y: rect.y + (frame.clientTop + parseFloat(style.paddingTop)) * scale, + scale, + }; + } + return { probe: { x: box.x, y: box.y, w: box.width, h: box.height }, 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) => { + const evalRoot = new URL("../../../../../evals/browser/", import.meta.url); + const { createEvalServer } = await import(new URL("lib/server.mjs", evalRoot).href); + const { withChrome } = await import( + new URL("cases/regression/snapshot-coordinates/chrome.mjs", evalRoot).href + ); + const server = createEvalServer(); + const { baseUrl } = await server.start(); + try { + await withChrome( + { executable: process.env.BSK_GEOMETRY_CHROME, ...configuration }, + async (send: Send) => { + const { targetId } = await send<{ targetId: string }>("Target.createTarget", { + url: "about:blank", + }); + const { sessionId: rootSession } = await send<{ sessionId: string }>( + "Target.attachToTarget", + { targetId, flatten: true }, + ); + await send( + "Page.navigate", + { url: `${baseUrl}/snapshot-coordinates?run=coordinates` }, + rootSession, + ); + await expect + .poll( + () => + server + .snapshot("coordinates") + .events.some( + (event: { type: string; data: { root?: boolean } }) => + event.type === "geometry.ready" && event.data.root, + ), + { timeout: 10_000 }, + ) + .toBe(true); + + const targets = await send<{ targetInfos: { targetId: string; type: string }[] }>( + "Target.getTargets", + ); + const sessions = [rootSession]; + for (const target of targets.targetInfos.filter((target) => target.type === "iframe")) { + const { sessionId } = await send<{ sessionId: string }>("Target.attachToTarget", { + targetId: target.targetId, + flatten: true, + }); + sessions.push(sessionId); + } + expect(sessions.length).toBe(2); // The cross-site fixture must actually be an OOPIF. + const frames: CdpFrame[] = []; + const names = new Map(); + const sessionFor = (target: CdpTarget) => target.sessionId ?? rootSession; + for (const sessionId of sessions) { + const { frameTree } = await send<{ frameTree: Tree }>( + "Page.getFrameTree", + {}, + sessionId, + ); + const target: CdpTarget = { + tabId: 1, + ...(sessionId === rootSession ? {} : { sessionId }), + }; + const visit = (tree: Tree, parentFrameId = tree.frame.parentId) => { + frames.push({ frameId: tree.frame.id, parentFrameId, target }); + names.set(tree.frame.id, tree.frame.name ?? ""); + for (const child of tree.childFrames ?? []) visit(child, tree.frame.id); + }; + visit(frameTree); + } + expect(frames).toHaveLength(5); + for (const frame of frames) { + if (!frame.parentFrameId) continue; + const parent = frames.find((item) => item.frameId === frame.parentFrameId)!; + const owner = await send<{ backendNodeId: number }>( + "DOM.getFrameOwner", + { frameId: frame.frameId }, + sessionFor(parent.target), + ); + frame.ownerBackendNodeId = owner.backendNodeId; + } + const graph: CdpFrameGraph = { rootFrameId: frames[0].frameId, frames }; + const calls: string[] = []; + const cdp: CdpRunner = { + send: (tabId, method, params) => { + calls.push(`${tabId}:${method}`); + return send(method, params, rootSession); + }, + sendToTarget: (target, method, params) => { + calls.push(`${target.sessionId ?? target.tabId}:${method}`); + return send(method, params, sessionFor(target)); + }, + getFrameGraph: async () => graph, + }; + const oracles = new Map(); + for (const frame of frames) { + const session = sessionFor(frame.target); + const { executionContextId } = await send<{ executionContextId: number }>( + "Page.createIsolatedWorld", + { frameId: frame.frameId, worldName: "coordinate-oracle" }, + session, + ); + await expect + .poll( + async () => { + const ready = await send<{ result: { value: boolean } }>( + "Runtime.evaluate", + { + expression: 'document.documentElement.dataset.geometryReady === "true"', + contextId: executionContextId, + returnByValue: true, + }, + session, + ); + return ready.result.value; + }, + { timeout: 5_000 }, + ) + .toBe(true); + // Establish scroll after cross-process target creation/layout settles. + const scroll = frame.parentFrameId + ? names.get(frame.frameId) === "nested" + ? [10, 20] + : [40, 100] + : [80, 240]; + await send( + "Runtime.evaluate", + { + expression: `(async () => { scrollTo(${scroll.join(",")}); await new Promise(resolve => requestAnimationFrame(() => requestAnimationFrame(resolve))); })()`, + contextId: executionContextId, + awaitPromise: true, + }, + session, + ); + const reply = await send<{ result: { value: Oracle } }>( + "Runtime.evaluate", + { expression: oracleExpression, contextId: executionContextId, returnByValue: true }, + session, + ); + oracles.set(frame.frameId, reply.result.value); + } + const geometry = new GeometryContext(cdp, 1, graph); + const captured = await captureViewModel(cdp, 1, { geometry }); + await captureFrameData(cdp, 1, captured, undefined, geometry); + expect(captured.frameGeometryIssues).toEqual([]); + for (const frame of frames) { + const node = captured.frameNodes + ?.get(frame.frameId) + ?.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: captured.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, + ); + const metrics = await send<{ + cssVisualViewport: { zoom: number }; + visualViewport: { clientWidth: number }; + }>("Page.getLayoutMetrics", {}, rootSession); + expect(metrics.cssVisualViewport.zoom).toBeCloseTo(configuration.zoom, 4); + }, + ); + } finally { + await server.stop(); + } + }, 30_000); +}); diff --git a/apps/extension/src/tools/geometry/coordinate-types.ts b/apps/extension/src/tools/geometry/coordinate-types.ts index 289ee11..5b2c48e 100644 --- a/apps/extension/src/tools/geometry/coordinate-types.ts +++ b/apps/extension/src/tools/geometry/coordinate-types.ts @@ -25,6 +25,12 @@ export interface UnavailableFrameProjection { ownerBackendNodeId: number; } +export interface UnavailableSnapshotCoordinates { + status: "unavailable"; + source: CoordinateOwner; + reason: "snapshot-coordinates-unavailable"; +} + export type SnapshotProjectionResult = | { status: "available"; projection: SnapshotProjection } | UnavailableFrameProjection; @@ -32,25 +38,74 @@ export type SnapshotProjectionResult = /** Descendants inherit the failed boundary without attempting another owner read. */ export type FrameProjectionIssue = | UnavailableFrameProjection - | { status: "blocked"; source: CoordinateOwner; cause: UnavailableFrameProjection }; + | UnavailableSnapshotCoordinates + | { + status: "blocked"; + source: CoordinateOwner; + cause: UnavailableFrameProjection | UnavailableSnapshotCoordinates; + }; export type FrameProjectionState = SnapshotProjectionResult | FrameProjectionIssue; -/** DOMSnapshot bounds are document-relative CSS pixels, independent of raster DPR. */ +export interface SnapshotCoordinates { + layoutUnitsPerCssPixel: number; + scrollCss: { x: number; y: number }; +} + +/** Snapshot scroll offsets share the raw layout units of its bounds. Only a + * target's root document may fall back to that target's CSS layout viewport. */ +export function snapshotCoordinates( + document: { scrollOffsetX?: number; scrollOffsetY?: number }, + layoutUnitsPerCssPixel: number | null, + rootScrollCss?: { x?: number; y?: number }, +): SnapshotCoordinates | null { + if ( + layoutUnitsPerCssPixel === null || + !Number.isFinite(layoutUnitsPerCssPixel) || + layoutUnitsPerCssPixel <= 0 + ) + return null; + const x = + document.scrollOffsetX === undefined + ? rootScrollCss?.x + : document.scrollOffsetX / layoutUnitsPerCssPixel; + const y = + document.scrollOffsetY === undefined + ? rootScrollCss?.y + : document.scrollOffsetY / layoutUnitsPerCssPixel; + if (x === undefined || y === undefined || !Number.isFinite(x) || !Number.isFinite(y)) return null; + return { layoutUnitsPerCssPixel, scrollCss: { x, y } }; +} + +/** Convert raw document-relative Blink layout units exactly once, before any + * CSS viewport clipping or frame projection. This scale is not screenshot zoom. */ export function snapshotViewportRect( bounds: number[], owner: CoordinateOwner, - scroll: { x: number; y: number }, + coordinates: SnapshotCoordinates | null, ): FrameViewportRect | null { - if (bounds.length < 4 || !bounds.slice(0, 4).every(Number.isFinite)) return null; + if (!coordinates || bounds.length < 4 || !bounds.slice(0, 4).every(Number.isFinite)) return null; + const { layoutUnitsPerCssPixel: scale, scrollCss: scroll } = coordinates; const [x, y, width, height] = bounds; - if (width <= 0 || height <= 0 || !Number.isFinite(scroll.x) || !Number.isFinite(scroll.y)) { + if ( + !Number.isFinite(scale) || + scale <= 0 || + width <= 0 || + height <= 0 || + !Number.isFinite(scroll.x) || + !Number.isFinite(scroll.y) + ) { return null; } return { space: "frame-viewport-css", owner, - rect: { x: x - scroll.x, y: y - scroll.y, width, height }, + rect: { + x: x / scale - scroll.x, + y: y / scale - scroll.y, + width: width / scale, + height: height / scale, + }, }; } diff --git a/apps/extension/src/tools/geometry/frame-context.ts b/apps/extension/src/tools/geometry/frame-context.ts index 6669f83..c728096 100644 --- a/apps/extension/src/tools/geometry/frame-context.ts +++ b/apps/extension/src/tools/geometry/frame-context.ts @@ -18,8 +18,8 @@ import type { CoordinateOwner, CssViewport, SnapshotProjectionResult } from "./c import { readSnapshotOwnerSizes } from "./snapshot-owner-sizes"; export interface LayoutMetrics { - cssVisualViewport?: { zoom?: number }; - visualViewport?: { zoom?: number }; + cssVisualViewport?: { zoom?: number; clientWidth?: number; clientHeight?: number }; + visualViewport?: { zoom?: number; clientWidth?: number; clientHeight?: number }; cssLayoutViewport?: { clientWidth?: number; clientHeight?: number; @@ -29,6 +29,29 @@ export interface LayoutMetrics { layoutViewport?: { clientWidth?: number; clientHeight?: number; pageX?: number; pageY?: number }; } +/** Chromium emits these paired floating-point dimensions before/after dividing + * by LayoutZoomFactor. Integer layout viewport dimensions lose precision at + * fractional zoom. Neither visualViewport.scale nor .zoom is this conversion. + * See InspectorPageAgent::getLayoutMetrics in Chromium. */ +export function snapshotLayoutScale(metrics: LayoutMetrics): number | null { + for (const dimension of ["clientWidth", "clientHeight"] as const) { + const raw = metrics.visualViewport?.[dimension]; + const css = metrics.cssVisualViewport?.[dimension]; + if ( + raw !== undefined && + css !== undefined && + Number.isFinite(raw) && + Number.isFinite(css) && + raw > 0 && + css > 0 + ) { + const scale = raw / css; + if (Number.isFinite(scale) && scale > 0) return scale; + } + } + return null; +} + /** The legacy field is a compatibility fallback, never a source of raster DPR. */ export function cssViewport(metrics: LayoutMetrics): CssViewport { const value = metrics.cssLayoutViewport ?? metrics.layoutViewport; diff --git a/apps/extension/src/tools/observation.ts b/apps/extension/src/tools/observation.ts index 4dca1bd..6f3cae2 100644 --- a/apps/extension/src/tools/observation.ts +++ b/apps/extension/src/tools/observation.ts @@ -1136,7 +1136,7 @@ export async function captureVomObservation( // Reserve space using the renderer's character-based token estimate. Like VOM // headers, this integrity notice remains visible even under a tiny token budget. const geometryNotice = captured.frameGeometryIssues?.length - ? "@warning iframe geometry incomplete: some frame content has no top-level coordinates.\n" + ? "@warning geometry incomplete: some page or frame content has no top-level coordinates.\n" : ""; const rendered = renderVom(decoratedScene, { maxDepth: options.maxDepth, diff --git a/apps/extension/src/tools/vom/__tests__/capture.test.ts b/apps/extension/src/tools/vom/__tests__/capture.test.ts index 25d76c4..b70ca2d 100644 --- a/apps/extension/src/tools/vom/__tests__/capture.test.ts +++ b/apps/extension/src/tools/vom/__tests__/capture.test.ts @@ -211,7 +211,11 @@ function makeCdp(snapshot: unknown) { if (method === "DOMSnapshot.enable") return {}; if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, + }; } if (method === "Runtime.evaluate") { return { @@ -299,6 +303,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return hoverTriggerSnapshotReply(); if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, }; } @@ -362,6 +368,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return twoHoverTriggerSnapshotReply(); if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, }; } @@ -410,6 +418,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return nestedHoverTriggerSnapshotReply(); if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, }; } @@ -743,6 +753,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return fakeSnapshotReply(); if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 200 }, }; } @@ -755,7 +767,46 @@ describe("captureViewModel", () => { expect(div?.rect).toMatchObject({ y: 0, h: 600 }); }); - it("keeps CSS snapshot bounds independent of legacy metrics ratio", async () => { + it("normalizes snapshot-owned scroll before using a stale CSS metrics fallback", async () => { + const snapshot = { + strings: ["html", "button"], + documents: [ + { + frameId: "root", + scrollOffsetX: 40, + scrollOffsetY: 200, + nodes: { + parentIndex: [-1, 0], + nodeName: [0, 1], + backendNodeId: [10, 11], + attributes: [[], []], + }, + layout: { nodeIndex: [1], bounds: [[200, 800, 240, 80]] }, + }, + ], + }; + const cdp: CdpRunner = { + send: vi.fn(async (_tab: number, method: string) => { + if (method === "DOMSnapshot.captureSnapshot") return snapshot; + if (method === "Page.getLayoutMetrics") + return { + visualViewport: { clientWidth: 2000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 999, pageY: 999 }, + }; + return {}; + }) as CdpRunner["send"], + }; + const captured = await captureViewModel(cdp, 4); + expect(captured.nodes.find((node) => node.backendNodeId === 11)).toMatchObject({ + localRect: { x: 80, y: 300, w: 120, h: 40 }, + rect: { x: 80, y: 300, w: 120, h: 40 }, + rendered: true, + }); + expect(captured.frameGeometryIssues).toEqual([]); + }); + + it("normalizes raw snapshot bounds before viewport clipping", async () => { const S = ["html", "body", "div", "position", "fixed", "static", "pointer-events", "auto"]; const i = (s: string) => S.indexOf(s); const snapshot = { @@ -789,6 +840,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 2000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, layoutViewport: { clientWidth: 2000, clientHeight: 1600 }, }; @@ -806,8 +859,8 @@ describe("captureViewModel", () => { expect(nodes.find((n) => n.backendNodeId === 12)?.localRect).toEqual({ x: 0, y: 0, - w: 2000, - h: 1600, + w: 1000, + h: 800, }); }); @@ -864,6 +917,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, }; } @@ -954,6 +1009,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, }; } @@ -1050,6 +1107,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, }; } @@ -1116,6 +1175,8 @@ describe("captureViewModel", () => { attributes: [[], [], []], contentDocumentIndex: { index: [2], value: [2] }, }, + scrollOffsetX: 0, + scrollOffsetY: 0, layout: { nodeIndex: [1, 2], styles: [ @@ -1136,6 +1197,8 @@ describe("captureViewModel", () => { backendNodeId: [30, 31], attributes: [[], [i("type"), i("text")]], }, + scrollOffsetX: 0, + scrollOffsetY: 0, layout: { nodeIndex: [1], styles: [[i("static"), i("auto")]], @@ -1156,6 +1219,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 0 }, }; } @@ -1230,6 +1295,8 @@ describe("captureViewModel", () => { if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "Page.getLayoutMetrics") { return { + visualViewport: { clientWidth: 2000 }, + cssVisualViewport: { clientWidth: 1000 }, cssLayoutViewport: { clientWidth: 1000, clientHeight: 800, pageX: 0, pageY: 100 }, layoutViewport: { clientWidth: 2000, clientHeight: 1600 }, }; @@ -1238,7 +1305,7 @@ describe("captureViewModel", () => { }) as unknown as (tabId: number, method: string, params?: object) => Promise, }; const { nodes } = await captureViewModel(cdp, 4); - expect(nodes.find((n) => n.backendNodeId === 12)?.rect?.y).toBe(400 - 100); + expect(nodes.find((n) => n.backendNodeId === 12)?.rect?.y).toBe(400 / 2 - 100); }); it("collectOverlayExcludedBackendIds walks the pierced overlay host subtree", async () => { @@ -1269,6 +1336,8 @@ function siblingCaptureFixture( beforeReply: (method: string, params: Record) => Promise = async () => {}, ) { const document = (id: number, owners: number[], childIndexes: number[]) => ({ + scrollOffsetX: 0, + scrollOffsetY: 0, frameId: `frame-${id}`, nodes: { parentIndex: [-1, ...owners.map(() => 0)], @@ -1300,7 +1369,11 @@ function siblingCaptureFixture( await beforeReply(method, args); if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "Page.getLayoutMetrics") - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 }, + }; if (method === "DOM.getBoxModel") return { model: { content: [0, 0, 200, 0, 200, 100, 0, 100] } }; if (method === "DOM.resolveNode") return { object: { objectId: String(args.backendNodeId) } }; diff --git a/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts b/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts index 89512ec..1e7d5b5 100644 --- a/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts +++ b/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts @@ -25,6 +25,8 @@ function childSnapshot(frameId: string, backendNodeId: number) { strings, documents: [ { + scrollOffsetX: 0, + scrollOffsetY: 0, frameId, nodes: { parentIndex: [-1, 0], @@ -75,7 +77,11 @@ describe("captureFrameData", () => { }; const reply = async (_target: unknown, method: string) => { if (method === "Page.getLayoutMetrics") - return { cssLayoutViewport: { clientWidth: 300, clientHeight: 200 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 300, clientHeight: 200 }, + }; if (method === "DOMSnapshot.captureSnapshot") return snapshot; if (method === "DOM.getBoxModel") return { model: { content: [50, 100, 350, 100, 350, 300, 50, 300] } }; @@ -126,7 +132,11 @@ describe("captureFrameData", () => { }; const sendToTarget = vi.fn(async (target, method) => { if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 300, clientHeight: 200, pageX: 0, pageY: 0 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 300, clientHeight: 200, pageX: 0, pageY: 0 }, + }; } if (method === "DOMSnapshot.enable" || method === "Accessibility.enable") return {}; if (method === "DOMSnapshot.captureSnapshot") { @@ -147,7 +157,11 @@ describe("captureFrameData", () => { return { model: { content: [x, 100, x + 300, 100, x + 300, 300, x, 300] } }; } if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 }, + }; } throw new Error(`unexpected root ${method}`); }) as CdpRunner["send"], @@ -211,7 +225,11 @@ describe("captureFrameData", () => { if (method === "Accessibility.getFullAXTree") return { nodes: [] }; if (method === "DOM.getBoxModel") throw new Error("owner geometry unavailable"); if (method === "Page.getLayoutMetrics") { - return { cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 } }; + return { + visualViewport: { clientWidth: 1000 }, + cssVisualViewport: { clientWidth: 1000 }, + cssLayoutViewport: { clientWidth: 1000, clientHeight: 800 }, + }; } throw new Error(`unexpected root ${method}`); }) as CdpRunner["send"], @@ -254,7 +272,12 @@ describe("captureFrameData", () => { expect.objectContaining({ backendDOMNodeId: 101, frameId: "child" }), ]); expect(childDocument?.domNodes.find((node) => node.backendNodeId === 101)).toEqual( - expect.objectContaining({ rect: null, localRect: { x: 10, y: 20, w: 100, h: 40 } }), + expect.objectContaining({ rect: null, localRect: null, rendered: true }), ); + expect(captured.frameGeometryIssues).toContainEqual({ + status: "unavailable", + source: { target: { tabId: 4, sessionId: "child-session" }, frameId: "child" }, + reason: "snapshot-coordinates-unavailable", + }); }); }); diff --git a/apps/extension/src/tools/vom/capture.ts b/apps/extension/src/tools/vom/capture.ts index 91702d3..0c894d9 100644 --- a/apps/extension/src/tools/vom/capture.ts +++ b/apps/extension/src/tools/vom/capture.ts @@ -12,9 +12,15 @@ import { type FrameProjectionIssue, type FrameProjectionState, projectSnapshotRect, + type SnapshotCoordinates, + snapshotCoordinates, snapshotViewportRect, } from "../geometry/coordinate-types"; -import { cssViewport, GeometryContext, type LayoutMetrics } from "../geometry/frame-context"; +import { + GeometryContext, + type LayoutMetrics, + snapshotLayoutScale, +} from "../geometry/frame-context"; import type { CdpRunner } from "../shared"; import { clearHover, ProbeBudget, waitForHover } from "./hover-perception"; @@ -356,8 +362,8 @@ interface FrameContext { ownerFrameBackendNodeId: number | null; projection: FrameProjectionState; target: CdpTarget; - scrollX: number; - scrollY: number; + coordinates: SnapshotCoordinates | null; + layoutUnitsPerCssPixel: number | null; } function str(strings: string[], idx: number | undefined): string { @@ -892,7 +898,7 @@ function parseDocumentNodes( const input = snapshotViewportRect( b, { target: context.target, frameId: context.frameId }, - { x: context.scrollX, y: context.scrollY }, + context.coordinates, ); if (input) { const { x, y, width: w, height: h } = input.rect; @@ -912,7 +918,11 @@ function parseDocumentNodes( const visibility = str(strings, styleRow[visibilityCol]) || "visible"; const opacity = str(strings, styleRow[opacityCol]) || "1"; rendered = - localRect !== null && + !!b && + b.length >= 4 && + b.slice(0, 4).every(Number.isFinite) && + b[2] > 0 && + b[3] > 0 && visibility !== "hidden" && visibility !== "collapse" && (Number.parseFloat(opacity) || 0) > 0; @@ -1009,10 +1019,13 @@ async function parseChildFrameDocuments( const iframeBackendId = parentBackendIds[nodeArrayIdx]; if (visited.has(childDocIndex) || !childDoc || iframeBackendId === undefined) return []; const source = { target: parentContext.target, frameId: snapshotFrameId(childDoc, strings) }; + const coordinates = snapshotCoordinates(childDoc, parentContext.layoutUnitsPerCssPixel); const parent = parentContext.projection; const projection: FrameProjectionState = parent.status === "available" - ? { status: "unavailable", source, ownerBackendNodeId: iframeBackendId } + ? coordinates + ? { status: "unavailable", source, ownerBackendNodeId: iframeBackendId } + : { status: "unavailable", source, reason: "snapshot-coordinates-unavailable" } : { status: "blocked", source, cause: parent.status === "blocked" ? parent.cause : parent }; return [ { @@ -1020,6 +1033,7 @@ async function parseChildFrameDocuments( childDoc, iframeBackendId, source, + coordinates, projection: projection as FrameProjectionState, }, ]; @@ -1041,6 +1055,7 @@ async function parseChildFrameDocuments( Array.from({ length: Math.min(4, children.length) }, async () => { while (cursor < children.length && !signal?.aborted && !failure) { const child = children[cursor++]; + if (!child.coordinates) continue; try { // Owner quads are target-relative; do not apply the parent transform twice. child.projection = await geometry.snapshotProjection( @@ -1061,7 +1076,14 @@ async function parseChildFrameDocuments( throwIfAborted(signal); // Completion order must not change document traversal or result insertion order. - for (const { childDocIndex, childDoc, iframeBackendId, source, projection } of children) { + for (const { + childDocIndex, + childDoc, + iframeBackendId, + source, + projection, + coordinates, + } of children) { throwIfAborted(signal); if (projection.status !== "available") frameGeometryIssues.push(projection); const childContext: FrameContext = { @@ -1069,8 +1091,8 @@ async function parseChildFrameDocuments( target: source.target, ownerFrameBackendNodeId: iframeBackendId, projection, - scrollX: childDoc.scrollOffsetX ?? 0, - scrollY: childDoc.scrollOffsetY ?? 0, + coordinates, + layoutUnitsPerCssPixel: parentContext.layoutUnitsPerCssPixel, }; const nextVisited = new Set(visited); nextVisited.add(childDocIndex); @@ -1129,10 +1151,11 @@ export async function captureViewModel( console.debug("[bsk capture] layout metrics unavailable", error); } throwIfAborted(options.signal); - const measured = cssViewport(metrics); - const viewport: Viewport = { width: measured.width, height: measured.height }; - const scrollX = measured.scrollX; - const scrollY = measured.scrollY; + const viewport: Viewport = { + width: metrics.cssLayoutViewport?.clientWidth ?? 0, + height: metrics.cssLayoutViewport?.clientHeight ?? 0, + }; + const layoutUnitsPerCssPixel = snapshotLayoutScale(metrics); await cdp.send(tabId, "DOMSnapshot.enable", {}); throwIfAborted(options.signal); @@ -1155,19 +1178,28 @@ export async function captureViewModel( }; } + const source = { target, frameId: snapshotFrameId(doc0, strings) }; + const coordinates = snapshotCoordinates(doc0, layoutUnitsPerCssPixel, { + x: metrics.cssLayoutViewport?.pageX, + y: metrics.cssLayoutViewport?.pageY, + }); const topContext: FrameContext = { frameId: snapshotFrameId(doc0, strings), ownerFrameBackendNodeId: null, target, - projection: { - status: "available", - projection: { - source: { target, frameId: snapshotFrameId(doc0, strings) }, - geometry: { sourceClips: [], edges: [], topViewport: viewport }, - }, - }, - scrollX, - scrollY, + projection: + coordinates && + [viewport.width, viewport.height].every((size) => Number.isFinite(size) && size > 0) + ? { + status: "available", + projection: { + source, + geometry: { sourceClips: [], edges: [], topViewport: viewport }, + }, + } + : { status: "unavailable", source, reason: "snapshot-coordinates-unavailable" }, + coordinates, + layoutUnitsPerCssPixel, }; const mainParsed = parseDocumentNodes(doc0, strings, topContext); const nodes = mainParsed.nodes; @@ -1214,7 +1246,10 @@ export async function captureViewModel( viewport, iframeNodes, frameNodes, - frameGeometryIssues: frameParsed.frameGeometryIssues, + frameGeometryIssues: [ + ...(topContext.projection.status === "available" ? [] : [topContext.projection]), + ...frameParsed.frameGeometryIssues, + ], frameOwnerBackendNodeIds: frameParsed.frameOwnerBackendNodeIds, frameParentIds: frameParsed.frameParentIds, ...(topContext.frameId ? { rootFrameId: topContext.frameId } : {}), diff --git a/evals/browser/cases/regression/snapshot-coordinates/README.md b/evals/browser/cases/regression/snapshot-coordinates/README.md new file mode 100644 index 0000000..4e7f950 --- /dev/null +++ b/evals/browser/cases/regression/snapshot-coordinates/README.md @@ -0,0 +1,44 @@ +# DOMSnapshot layout-unit regression (#195) + +Source: https://github.com/Tencent/BrowserSkill/pull/195 + +DOMSnapshot bounds and document scroll offsets retain Blink layout units. Treating them as CSS +pixels changes both positions and sizes under device scaling/browser zoom, before iframe +projection or viewport clipping even starts. + +The fixture contains a scrolled root, a scrolled same-process iframe, an OOPIF served from the +other loopback hostname, and a nested iframe in each. Owners have borders, padding and CSS +transforms. The geometry test compares the production capture's local and top-level rectangles +against independent DOM border boxes, with at most 2 CSS pixels of layout rounding. It also +checks that each target reads layout metrics only once. No credentials or personal tabs are used. + +Run the geometric assertions 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 test launches and cleans up its own headless Chrome profiles. It explicitly verifies the +requested browser zoom and OOPIF topology, so unsupported setups fail rather than silently +testing a different configuration. It is skipped in ordinary unit runs unless the executable +is specified. No browser download or new package dependency is required. + +The corpus smoke case checks fixture readiness and observable semantics through the real CLI: + +```sh +BSK_AUTO_UPDATE=off pnpm eval:browser smoke --case snapshot-coordinates --bsk ./target/debug/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. diff --git a/evals/browser/cases/regression/snapshot-coordinates/chrome.mjs b/evals/browser/cases/regression/snapshot-coordinates/chrome.mjs new file mode 100644 index 0000000..93c7bb3 --- /dev/null +++ b/evals/browser/cases/regression/snapshot-coordinates/chrome.mjs @@ -0,0 +1,99 @@ +import { spawn } from "node:child_process"; +import { once } from "node:events"; +import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +// This regression owns its browser/profile. It never attaches to a user's Chrome. +export async function withChrome({ executable, deviceScale, zoom }, run) { + const profile = await mkdtemp(join(tmpdir(), "bsk-snapshot-coordinates-")); + let chrome; + let socket; + const pending = new Map(); + try { + await mkdir(join(profile, "Default")); + await writeFile( + join(profile, "Default", "Preferences"), + JSON.stringify({ + partition: { default_zoom_level: { x: Math.log(zoom) / Math.log(1.2) } }, + }), + ); + chrome = spawn( + executable, + [ + "--headless=new", + "--no-first-run", + "--no-default-browser-check", + "--disable-extensions", + "--site-per-process", + "--remote-debugging-port=0", + "--window-size=1600,1200", + `--force-device-scale-factor=${deviceScale}`, + `--user-data-dir=${profile}`, + "about:blank", + ], + { stdio: ["ignore", "ignore", "pipe"] }, + ); + const endpoint = await new Promise((resolve, reject) => { + let output = ""; + const timeout = setTimeout( + () => reject(new Error(`Chrome startup timed out: ${output}`)), + 15_000, + ); + const fail = (error) => { + clearTimeout(timeout); + reject(error); + }; + chrome.once("error", fail); + chrome.once("exit", (code) => fail(new Error(`Chrome exited (${code}): ${output}`))); + chrome.stderr.on("data", (chunk) => { + output += chunk; + const match = output.match(/DevTools listening on (ws:\/\/\S+)/); + if (match) { + clearTimeout(timeout); + resolve(match[1]); + } + }); + }); + socket = new WebSocket(endpoint); + await new Promise((resolve, reject) => { + socket.addEventListener("open", resolve, { once: true }); + socket.addEventListener("error", reject, { once: true }); + }); + socket.addEventListener("message", ({ data }) => { + const reply = JSON.parse(data); + const request = pending.get(reply.id); + if (!request) return; + clearTimeout(request.timeout); + pending.delete(reply.id); + if (reply.error) request.reject(new Error(JSON.stringify(reply.error))); + else request.resolve(reply.result); + }); + let sequence = 0; + const send = (method, params = {}, sessionId) => + new Promise((resolve, reject) => { + const id = ++sequence; + const timeout = setTimeout(() => { + pending.delete(id); + reject(new Error(`CDP timed out: ${method}`)); + }, 10_000); + pending.set(id, { resolve, reject, timeout }); + socket.send(JSON.stringify({ id, method, params, sessionId })); + }); + await run(send); + } finally { + for (const request of pending.values()) { + clearTimeout(request.timeout); + request.reject(new Error("regression browser closed")); + } + socket?.close(); + if (chrome && chrome.exitCode === null && chrome.signalCode === null) { + const exited = once(chrome, "exit"); + const timeout = setTimeout(() => chrome.kill("SIGKILL"), 5_000); + chrome.kill(); + await exited; + clearTimeout(timeout); + } + await rm(profile, { recursive: true, force: true }); + } +} diff --git a/evals/browser/cases/regression/snapshot-coordinates/snapshot-coordinates.case.json b/evals/browser/cases/regression/snapshot-coordinates/snapshot-coordinates.case.json new file mode 100644 index 0000000..cb45b05 --- /dev/null +++ b/evals/browser/cases/regression/snapshot-coordinates/snapshot-coordinates.case.json @@ -0,0 +1,33 @@ +{ + "$schema": "../../../schemas/case.schema.json", + "schemaVersion": 1, + "id": "snapshot-coordinates", + "title": "Scrolled and nested frame coordinate fixture", + "suite": "regression", + "tags": ["geometry", "iframe", "scroll"], + "fixture": { "startPath": "/snapshot-coordinates" }, + "prompts": { + "en": "Open {url}, observe the page and its frames, report the GEOMETRY marker, then close the session.", + "zh-CN": "打开 {url}，观察页面及其 iframe，报告 GEOMETRY 标记，然后关闭会话。" + }, + "coverage": ["session.start", "session.stop", "page.navigate", "inspect.observe"], + "assertions": { + "site": [ + { + "label": "root fixture scrolled and settled", + "type": "geometry.ready", + "where": { "data.root": true }, + "minCount": 1 + } + ], + "response": [{ "label": "observed the fixture marker", "includes": "GEOMETRY-195" }], + "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/snapshot-coordinates/snapshot-coordinates.fixture.mjs b/evals/browser/cases/regression/snapshot-coordinates/snapshot-coordinates.fixture.mjs new file mode 100644 index 0000000..d4f2312 --- /dev/null +++ b/evals/browser/cases/regression/snapshot-coordinates/snapshot-coordinates.fixture.mjs @@ -0,0 +1,52 @@ +import { page, withRun } from "../../../lib/fixtures.mjs"; + +export default { + id: "snapshot-coordinates", + routes: ["/snapshot-coordinates", "/snapshot-coordinates/frame", "/snapshot-coordinates/nested"], + render({ pathname, runId, query }) { + const root = pathname === "/snapshot-coordinates"; + const nested = pathname.endsWith("/nested"); + return page({ + title: "Snapshot coordinate regression", + body: ` + + + ${root ? `` : ""} + ${!root && !nested ? `` : ""} + `, + script: ` + history.scrollRestoration = "manual"; + const cross = document.querySelector("#cross"); + if (cross) { + const url = new URL(${JSON.stringify(withRun("/snapshot-coordinates/frame", runId))}, location.href); + url.hostname = location.hostname === "127.0.0.1" ? "localhost" : "127.0.0.1"; + // Isolate unit conversion from the pre-existing OOPIF scrollbar-width + // projection issue. ?classic-scrollbars retains that separate reproduction. + if (!new URLSearchParams(location.search).has("classic-scrollbars")) url.searchParams.set("scrollbars", "none"); + cross.src = url.href; + } + window.addEventListener("load", () => { + scrollTo(${root ? "80, 240" : nested ? "10, 20" : "40, 100"}); + requestAnimationFrame(() => requestAnimationFrame(() => { + window.geometryReady = true; + document.documentElement.dataset.geometryReady = "true"; + browserEval.send("geometry.ready", { root: ${root} }); + })); + }); + `, + }); + }, +}; diff --git a/evals/browser/tests/eval.test.mjs b/evals/browser/tests/eval.test.mjs index 2bc73de..c0ab22c 100644 --- a/evals/browser/tests/eval.test.mjs +++ b/evals/browser/tests/eval.test.mjs @@ -41,17 +41,22 @@ test("case manifests are discovered, ordered, and grouped into suites", () => { "diagnostics", "mobile-emulation", "generated-form", + "snapshot-coordinates", ], ); - assert.deepEqual(repositorySummary(cases, fixtureRegistry).suites, { core: 6, matrix: 1 }); + assert.deepEqual(repositorySummary(cases, fixtureRegistry).suites, { + core: 6, + matrix: 1, + regression: 1, + }); }); 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, 7); - assert.equal(summary.fixtureModules, 8); - assert.equal(summary.fixtureRoutes, 12); + assert.equal(summary.cases, 8); + assert.equal(summary.fixtureModules, 9); + assert.equal(summary.fixtureRoutes, 15); }); test("manifest validation rejects unknown operations and incomplete workflow steps", () => {