From 71f7e6d4b6e5c8297164d236902cb943f020368e Mon Sep 17 00:00:00 2001 From: Ljy-0827 Date: Wed, 26 Aug 2026 14:51:28 +0800 Subject: [PATCH] fix(vom): canvas bug fix --- .../src/tools/__tests__/mouse-input.test.ts | 119 ++++++++++++++++++ apps/extension/src/tools/interaction.ts | 39 +----- apps/extension/src/tools/mouse-input.ts | 96 ++++++++++++++ .../src/tools/surface-point-action.ts | 47 ++----- .../test-fixtures/rendered-surfaces/README.md | 8 ++ .../rendered-surfaces/frame.html | 28 ++++- .../rendered-surfaces/frames.html | 4 +- 7 files changed, 265 insertions(+), 76 deletions(-) create mode 100644 apps/extension/src/tools/__tests__/mouse-input.test.ts create mode 100644 apps/extension/src/tools/mouse-input.ts diff --git a/apps/extension/src/tools/__tests__/mouse-input.test.ts b/apps/extension/src/tools/__tests__/mouse-input.test.ts new file mode 100644 index 0000000..51e4c26 --- /dev/null +++ b/apps/extension/src/tools/__tests__/mouse-input.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it, vi } from "vitest"; +import { dispatchMouseClick } from "../mouse-input"; +import type { CdpRunner } from "../shared"; + +function fakeCdp(onSend?: (method: string, params: Record) => void): { + cdp: CdpRunner; + events: Record[]; +} { + const events: Record[] = []; + return { + events, + cdp: { + send: vi.fn(async (_tabId, method, params) => { + const event = params as Record; + events.push(event); + onSend?.(method, event); + return {} as never; + }), + }, + }; +} + +describe("dispatchMouseClick", () => { + it("emits one move, press, and release sequence", async () => { + const { cdp, events } = fakeCdp(); + + expect( + await dispatchMouseClick( + cdp, + 4, + { x: 12, y: 34 }, + { + button: "left", + clickCount: 1, + modifiers: 0, + moveSettleMs: 0, + }, + ), + ).toBe("completed"); + + expect(events).toEqual([ + { + type: "mouseMoved", + x: 12, + y: 34, + modifiers: 0, + }, + { + type: "mousePressed", + x: 12, + y: 34, + button: "left", + clickCount: 1, + modifiers: 0, + }, + { + type: "mouseReleased", + x: 12, + y: 34, + button: "left", + clickCount: 1, + modifiers: 0, + }, + ]); + }); + + it("releases a pressed button when cancellation arrives", async () => { + const controller = new AbortController(); + const { cdp, events } = fakeCdp((_method, event) => { + if (event.type === "mousePressed") controller.abort(); + }); + + expect( + await dispatchMouseClick( + cdp, + 4, + { x: 1, y: 2 }, + { + button: "left", + clickCount: 1, + modifiers: 0, + signal: controller.signal, + moveSettleMs: 0, + }, + ), + ).toBe("cancelled"); + expect(events.at(-1)).toMatchObject({ type: "mouseReleased" }); + }); + + it("settles pointer-move hit testing before pressing", async () => { + vi.useFakeTimers(); + try { + const { cdp, events } = fakeCdp(); + const click = dispatchMouseClick( + cdp, + 4, + { x: 1, y: 2 }, + { + button: "left", + clickCount: 1, + modifiers: 0, + moveSettleMs: 32, + }, + ); + + await vi.advanceTimersByTimeAsync(31); + expect(events.map((event) => event.type)).toEqual(["mouseMoved"]); + await vi.advanceTimersByTimeAsync(1); + await click; + expect(events.map((event) => event.type)).toEqual([ + "mouseMoved", + "mousePressed", + "mouseReleased", + ]); + } finally { + vi.useRealTimers(); + } + }); +}); diff --git a/apps/extension/src/tools/interaction.ts b/apps/extension/src/tools/interaction.ts index 0f29ec9..fb1dc9b 100644 --- a/apps/extension/src/tools/interaction.ts +++ b/apps/extension/src/tools/interaction.ts @@ -32,6 +32,7 @@ import { attachDialogs, markDialogCursor } from "./dialogs"; import { backendNodeToObject } from "./element-geometry"; import { rpcError } from "./errors"; import { resolveNodeGeometry, scrollElementAndFramesIntoView } from "./frame-geometry"; +import { dispatchMouseClick } from "./mouse-input"; import { type CdpRunner, type ChromeTabsApi, @@ -285,47 +286,15 @@ export async function handleClick( } try { - // Move first so hover state activates, then press → release. - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseMoved", - x: centre.x, - y: centre.y, - modifiers, - }); - if (throwIfAborted(deps.signal)) { - return { code: "cancelled", message: "click aborted" }; - } - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mousePressed", - x: centre.x, - y: centre.y, + const dispatch = await dispatchMouseClick(deps.cdp, target.tabId, centre, { button, clickCount, modifiers, + signal: deps.signal, }); - if (throwIfAborted(deps.signal)) { - try { - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseReleased", - x: centre.x, - y: centre.y, - button, - clickCount, - modifiers, - }); - } catch (err) { - console.debug("[bsk interaction] best-effort mouseReleased after abort failed", err); - } + if (dispatch === "cancelled") { return { code: "cancelled", message: "click aborted" }; } - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseReleased", - x: centre.x, - y: centre.y, - button, - clickCount, - modifiers, - }); } catch (err) { return { code: "cdp_failed", diff --git a/apps/extension/src/tools/mouse-input.ts b/apps/extension/src/tools/mouse-input.ts new file mode 100644 index 0000000..c0100b3 --- /dev/null +++ b/apps/extension/src/tools/mouse-input.ts @@ -0,0 +1,96 @@ +import type { MouseButton } from "@/transport/types"; +import type { Point } from "./geometry"; +import type { CdpRunner } from "./shared"; + +interface MouseClickDispatchOptions { + button: MouseButton; + clickCount: number; + modifiers: number; + signal?: AbortSignal; + moveSettleMs?: number; +} + +type MouseClickDispatchResult = "completed" | "cancelled"; + +function waitForSettle(ms: number, signal: AbortSignal | undefined): Promise { + if (ms <= 0) return Promise.resolve(signal?.aborted !== true); + if (signal?.aborted) return Promise.resolve(false); + return new Promise((resolve) => { + let settled = false; + const finish = (completed: boolean) => { + if (settled) return; + settled = true; + clearTimeout(timer); + signal?.removeEventListener("abort", onAbort); + resolve(completed); + }; + const timer = setTimeout(() => finish(true), ms); + const onAbort = () => finish(false); + signal?.addEventListener("abort", onAbort, { once: true }); + }); +} + +/** + * Dispatch one coherent CDP mouse click in top-viewport CSS coordinates. + * + * Keeping the sequence here prevents DOM and Surface interaction paths from + * duplicating cancellation and best-effort release behavior. Callers that + * target asynchronously hit-tested content may request a settle interval. + */ +export async function dispatchMouseClick( + cdp: CdpRunner, + tabId: number, + point: Point, + options: MouseClickDispatchOptions, +): Promise { + const { button, clickCount, modifiers, signal, moveSettleMs = 0 } = options; + let pressed = false; + const release = () => + cdp.send(tabId, "Input.dispatchMouseEvent", { + type: "mouseReleased", + x: point.x, + y: point.y, + button, + clickCount, + modifiers, + }); + + try { + await cdp.send(tabId, "Input.dispatchMouseEvent", { + type: "mouseMoved", + x: point.x, + y: point.y, + modifiers, + }); + if (!(await waitForSettle(moveSettleMs, signal))) return "cancelled"; + + await cdp.send(tabId, "Input.dispatchMouseEvent", { + type: "mousePressed", + x: point.x, + y: point.y, + button, + clickCount, + modifiers, + }); + pressed = true; + + if (signal?.aborted) { + await release(); + pressed = false; + return "cancelled"; + } + + await release(); + pressed = false; + return "completed"; + } catch (error) { + if (pressed) { + try { + await release(); + } catch (releaseError) { + console.debug("[bsk mouse-input] best-effort mouse release failed", releaseError); + } + } + throw error; + } +} diff --git a/apps/extension/src/tools/surface-point-action.ts b/apps/extension/src/tools/surface-point-action.ts index e61b661..79fc8c0 100644 --- a/apps/extension/src/tools/surface-point-action.ts +++ b/apps/extension/src/tools/surface-point-action.ts @@ -4,6 +4,7 @@ import type { ClickParams, ClickResult, RpcError } from "@/transport/types"; import { attachDialogs, markDialogCursor } from "./dialogs"; import { rpcError } from "./errors"; import { resolveNodeGeometry } from "./frame-geometry"; +import { dispatchMouseClick } from "./mouse-input"; import { type CdpRunner, type ChromeTabsApi, @@ -34,6 +35,10 @@ export interface SurfacePointActionDeps { bypassOverlay?: (tabId: number, enabled: boolean) => Promise; } +// Canvas-style controls commonly update their pointer hit-test on the next +// animation frame. Leave a scheduling boundary after moving and before pressing. +const SURFACE_POINTER_MOVE_SETTLE_MS = 32; + function stale(message: string): RpcError { return rpcError("permission_denied", "surface_capture_stale", message); } @@ -249,54 +254,22 @@ export async function handleSurfacePointClick( const dialogCursor = markDialogCursor(deps.cdp, target.tabId); let bypassEnabled = false; - let pressed = false; - const release = () => - deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseReleased", - x: point.x, - y: point.y, - button: "left", - clickCount: 1, - modifiers: 0, - }); try { if (deps.bypassOverlay) { await deps.bypassOverlay(target.tabId, true); bypassEnabled = true; } - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mouseMoved", - x: point.x, - y: point.y, - modifiers: 0, - }); - const beforePressAbort = aborted(deps.signal); - if (beforePressAbort) return beforePressAbort; - await deps.cdp.send(target.tabId, "Input.dispatchMouseEvent", { - type: "mousePressed", - x: point.x, - y: point.y, + const dispatch = await dispatchMouseClick(deps.cdp, target.tabId, point, { button: "left", clickCount: 1, modifiers: 0, + signal: deps.signal, + moveSettleMs: SURFACE_POINTER_MOVE_SETTLE_MS, }); - pressed = true; - const afterPressAbort = aborted(deps.signal); - if (afterPressAbort) { - await release(); - pressed = false; - return afterPressAbort; + if (dispatch === "cancelled") { + return { code: "cancelled", message: "surface point click aborted" }; } - await release(); - pressed = false; } catch (error) { - if (pressed) { - try { - await release(); - } catch (releaseError) { - console.debug("[bsk surface-point] best-effort mouse release failed", releaseError); - } - } return { code: "cdp_failed", message: error instanceof Error ? error.message : String(error) }; } finally { if (bypassEnabled && deps.bypassOverlay) { diff --git a/apps/extension/test-fixtures/rendered-surfaces/README.md b/apps/extension/test-fixtures/rendered-surfaces/README.md index d0f10a0..a377507 100644 --- a/apps/extension/test-fixtures/rendered-surfaces/README.md +++ b/apps/extension/test-fixtures/rendered-surfaces/README.md @@ -77,6 +77,14 @@ Expected Surface labels: The cross-origin frame uses `http://localhost:4173` while the parent uses `http://127.0.0.1:4173`. With Chromium site isolation this exercises the OOPIF path without an external service. +Each frame Canvas also exposes a `pointer status` live region. A successful +screenshot-bound point click must report +`down button=0 buttons=1 hoverReady=true; click received` and turn the Canvas +yellow. The hover flag is set on the animation frame after `pointermove`, so the +fixture verifies both child-document delivery and the scheduling boundary +required before `pointerdown`, rather than merely validating projected +coordinates or a successful CDP response. + ### `viewport.html` Run with a `900x700` Agent Window. diff --git a/apps/extension/test-fixtures/rendered-surfaces/frame.html b/apps/extension/test-fixtures/rendered-surfaces/frame.html index cc42cd4..da17b81 100644 --- a/apps/extension/test-fixtures/rendered-surfaces/frame.html +++ b/apps/extension/test-fixtures/rendered-surfaces/frame.html @@ -8,6 +8,7 @@ body { padding: 12px; background: #fdfefe; } canvas { display: block; width: 220px; height: 90px; background: #d5f5e3; border: 2px solid #1e8449; } iframe { width: 280px; height: 150px; margin-top: 10px; border: 2px solid #85929e; } + #pointer-status { margin: 8px 0 0; min-height: 20px; } @@ -16,11 +17,34 @@ const params = new URLSearchParams(location.search); const canvas = document.querySelector("#frame-canvas"); if (params.get("canvas") === "0") canvas.remove(); - else canvas.setAttribute("aria-label", params.get("label") || "fixture-frame-default"); + else { + canvas.setAttribute("aria-label", params.get("label") || "fixture-frame-default"); + if (params.get("interactive") === "1") { + const status = document.createElement("p"); + status.id = "pointer-status"; + status.setAttribute("aria-live", "polite"); + status.textContent = "pointer status: idle"; + canvas.insertAdjacentElement("afterend", status); + let hoverReady = false; + canvas.addEventListener("pointermove", () => { + hoverReady = false; + requestAnimationFrame(() => { + hoverReady = true; + }); + }); + canvas.addEventListener("pointerdown", (event) => { + status.textContent = `pointer status: down button=${event.button} buttons=${event.buttons} hoverReady=${hoverReady}`; + if (event.buttons === 1 && hoverReady) canvas.style.background = "#f9e79f"; + }); + canvas.addEventListener("click", () => { + status.textContent += "; click received"; + }); + } + } if (params.get("nested") === "1") { const iframe = document.createElement("iframe"); iframe.title = "nested fixture frame"; - iframe.src = "frame.html?label=fixture-nested-frame"; + iframe.src = "frame.html?label=fixture-nested-frame&interactive=1"; document.body.append(iframe); } diff --git a/apps/extension/test-fixtures/rendered-surfaces/frames.html b/apps/extension/test-fixtures/rendered-surfaces/frames.html index 6355030..0f780d9 100644 --- a/apps/extension/test-fixtures/rendered-surfaces/frames.html +++ b/apps/extension/test-fixtures/rendered-surfaces/frames.html @@ -14,8 +14,8 @@

Frame matrix

- - + +