From eefbe03258f10b63f4a2f69fe4552f06d2563a5d Mon Sep 17 00:00:00 2001 From: drakezhang Date: Wed, 9 Sep 2026 15:58:28 +0800 Subject: [PATCH] fix(observation): preserve snapshot unit normalization after document extraction --- apps/extension/src/tools/observation.ts | 2 +- .../src/tools/vom/__tests__/capture.test.ts | 87 +++++++++++++++++-- .../src/tools/vom/__tests__/facts.test.ts | 2 +- .../tools/vom/__tests__/frame-capture.test.ts | 28 ++++-- apps/extension/src/tools/vom/capture.ts | 73 +++++++++++----- apps/extension/src/tools/vom/facts.ts | 4 +- apps/extension/src/tools/vom/normalize.ts | 15 ++-- apps/extension/src/tools/vom/snapshot.ts | 2 +- 8 files changed, 172 insertions(+), 41 deletions(-) diff --git a/apps/extension/src/tools/observation.ts b/apps/extension/src/tools/observation.ts index 61ae7dd..ea1f0a1 100644 --- a/apps/extension/src/tools/observation.ts +++ b/apps/extension/src/tools/observation.ts @@ -1138,7 +1138,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 a1b5dfb..1748dad 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 }, }; } @@ -528,6 +538,8 @@ describe("captureViewModel", () => { strings: S, documents: [ { + scrollOffsetX: 0, + scrollOffsetY: 0, frameId: "main", nodes: { parentIndex: [-1, 0, 1, 0, 3, 4], @@ -748,6 +760,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 }, }; } @@ -760,7 +774,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 = { @@ -794,6 +847,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 }, }; @@ -811,8 +866,8 @@ describe("captureViewModel", () => { expect(nodes.find((n) => n.backendNodeId === 12)?.localRect).toEqual({ x: 0, y: 0, - w: 2000, - h: 1600, + w: 1000, + h: 800, }); }); @@ -869,6 +924,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 }, }; } @@ -959,6 +1016,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 }, }; } @@ -1055,6 +1114,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 }, }; } @@ -1114,6 +1175,8 @@ describe("captureViewModel", () => { }, }, { + scrollOffsetX: 0, + scrollOffsetY: 0, nodes: { parentIndex: [-1, 0, 1], nodeName: [i("html"), i("body"), i("iframe")], @@ -1135,6 +1198,8 @@ describe("captureViewModel", () => { }, }, { + scrollOffsetX: 0, + scrollOffsetY: 0, nodes: { parentIndex: [-1, 0], nodeName: [i("body"), i("input")], @@ -1161,6 +1226,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 }, }; } @@ -1235,6 +1302,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 }, }; @@ -1243,7 +1312,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 () => { @@ -1274,6 +1343,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)], @@ -1305,7 +1376,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__/facts.test.ts b/apps/extension/src/tools/vom/__tests__/facts.test.ts index be44c37..2991049 100644 --- a/apps/extension/src/tools/vom/__tests__/facts.test.ts +++ b/apps/extension/src/tools/vom/__tests__/facts.test.ts @@ -77,7 +77,7 @@ describe("document facts", () => { ); expect(nodes[0].attrs).toEqual({ "aria-hidden": "true", inert: "" }); expect(nodes[0].layout).toEqual({ - boundsSpace: "snapshot-document-css", + boundsSpace: "snapshot-document-layout", bounds: [10, 20, 120, 40], styles: { position: "static", 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 024fca0..52951df 100644 --- a/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts +++ b/apps/extension/src/tools/vom/__tests__/frame-capture.test.ts @@ -26,6 +26,8 @@ function childSnapshot(frameId: string, backendNodeId: number) { strings, documents: [ { + scrollOffsetX: 0, + scrollOffsetY: 0, frameId, nodes: { parentIndex: [-1, 0], @@ -76,7 +78,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] } }; @@ -127,7 +133,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") { @@ -154,7 +164,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"], @@ -222,7 +236,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"], @@ -265,7 +283,7 @@ 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 }), ); }); }); diff --git a/apps/extension/src/tools/vom/capture.ts b/apps/extension/src/tools/vom/capture.ts index c96b02f..4b61c5e 100644 --- a/apps/extension/src/tools/vom/capture.ts +++ b/apps/extension/src/tools/vom/capture.ts @@ -5,8 +5,16 @@ import type { Viewport } from "@browser-skill/vom"; import type { CdpTarget } from "@/browser-driver/frame-graph"; import { evaluateHoverTrigger } from "@/lib/hover-trigger-policy"; import { OVERLAY_HOST_SELECTOR } from "../../lib/overlay-bridge"; -import { type FrameProjectionIssue, type FrameProjectionState } from "../geometry/coordinate-types"; -import { cssViewport, GeometryContext, type LayoutMetrics } from "../geometry/frame-context"; +import { + type FrameProjectionIssue, + type FrameProjectionState, + snapshotCoordinates, +} from "../geometry/coordinate-types"; +import { + GeometryContext, + type LayoutMetrics, + snapshotLayoutScale, +} from "../geometry/frame-context"; import type { CdpRunner } from "../shared"; import { isAbortError, throwIfAborted } from "./capture-abort"; import type { CapturedNode } from "./capture-types"; @@ -529,10 +537,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 [ { @@ -540,6 +551,7 @@ async function parseChildFrameDocuments( childDoc, iframeBackendId, source, + coordinates, projection: projection as FrameProjectionState, }, ]; @@ -561,6 +573,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( @@ -581,7 +594,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 = { @@ -589,8 +609,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); @@ -653,10 +673,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); @@ -679,19 +700,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 = await normalizeDocument(doc0, strings, topContext, options.signal); const nodes = mainParsed.nodes; @@ -743,7 +773,10 @@ export async function captureViewModel( viewport, iframeNodes, frameNodes, - frameGeometryIssues: frameParsed.frameGeometryIssues, + frameGeometryIssues: [ + ...(topContext.projection.status === "available" ? [] : [topContext.projection]), + ...frameParsed.frameGeometryIssues, + ], frameOwnerBackendNodeIds: frameParsed.frameOwnerBackendNodeIds, frameExcludedBackendNodeIds: frameParsed.frameExcludedBackendNodeIds, frameParentIds: frameParsed.frameParentIds, diff --git a/apps/extension/src/tools/vom/facts.ts b/apps/extension/src/tools/vom/facts.ts index c9087e1..e257d01 100644 --- a/apps/extension/src/tools/vom/facts.ts +++ b/apps/extension/src/tools/vom/facts.ts @@ -2,9 +2,9 @@ import { isOverlayHostNode } from "@/lib/overlay-bridge"; import { createCaptureCheckpoint } from "./capture-abort"; import type { CapturedNode } from "./capture-types"; -/** Bounds retain their snapshot document CSS units until normalization. */ +/** Bounds retain their raw snapshot document layout units until normalization. */ export interface SnapshotLayout { - readonly boundsSpace: "snapshot-document-css"; + readonly boundsSpace: "snapshot-document-layout"; bounds?: number[]; styles: Readonly>; } diff --git a/apps/extension/src/tools/vom/normalize.ts b/apps/extension/src/tools/vom/normalize.ts index 5db9dfd..66bbc4d 100644 --- a/apps/extension/src/tools/vom/normalize.ts +++ b/apps/extension/src/tools/vom/normalize.ts @@ -2,6 +2,7 @@ import type { CdpTarget } from "@/browser-driver/frame-graph"; import { type FrameProjectionState, projectSnapshotRect, + type SnapshotCoordinates, snapshotViewportRect, } from "../geometry/coordinate-types"; import { createCaptureCheckpoint } from "./capture-abort"; @@ -13,8 +14,8 @@ export interface FrameContext { ownerFrameBackendNodeId: number | null; projection: FrameProjectionState; target: CdpTarget; - scrollX: number; - scrollY: number; + coordinates: SnapshotCoordinates | null; + layoutUnitsPerCssPixel: number | null; } export interface NormalizedDocument { @@ -38,10 +39,11 @@ export async function normalizeDocument( if (pending) await pending; } const node = decoded.nodes[i]; + const bounds = node.layout?.bounds ?? []; const input = snapshotViewportRect( - node.layout?.bounds ?? [], + bounds, { target: context.target, frameId: context.frameId }, - { x: context.scrollX, y: context.scrollY }, + context.coordinates, ); const local = input?.rect; const rect = @@ -57,7 +59,10 @@ export async function normalizeDocument( localRect: local ? { x: local.x, y: local.y, w: local.width, h: local.height } : null, rect: rect ? { x: rect.x, y: rect.y, w: rect.width, h: rect.height } : null, rendered: - !!local && + bounds.length >= 4 && + bounds.slice(0, 4).every(Number.isFinite) && + bounds[2] > 0 && + bounds[3] > 0 && visibility !== "hidden" && visibility !== "collapse" && (Number.parseFloat(opacity) || 0) > 0, diff --git a/apps/extension/src/tools/vom/snapshot.ts b/apps/extension/src/tools/vom/snapshot.ts index 8993106..bac1b41 100644 --- a/apps/extension/src/tools/vom/snapshot.ts +++ b/apps/extension/src/tools/vom/snapshot.ts @@ -173,7 +173,7 @@ export async function decodeDocument( li === undefined ? undefined : { - boundsSpace: "snapshot-document-css" as const, + boundsSpace: "snapshot-document-layout" as const, bounds: dl?.bounds?.[li], styles, };