diff --git a/src/main/browser/grab-guest-overlay-follow.test.ts b/src/main/browser/grab-guest-overlay-follow.test.ts new file mode 100644 index 00000000000..d7e7fce6d8b --- /dev/null +++ b/src/main/browser/grab-guest-overlay-follow.test.ts @@ -0,0 +1,236 @@ +import { runInNewContext } from 'node:vm' +import { afterEach, describe, expect, it } from 'vitest' +import { BROWSER_GRAB_SELECTED_ATTRIBUTE } from '../../shared/browser-annotation-viewport-bridge' +import { buildGuestOverlayScript } from './grab-guest-script' + +type Rect = { x: number; y: number; width: number; height: number } + +const guestAborts: (() => Promise)[] = [] + +afterEach(async () => { + await Promise.all(guestAborts.splice(0).map((abort) => abort())) +}) + +async function armGuest(previousMarker?: string) { + const { Window } = await import('happy-dom') + const window = new Window() + guestAborts.push(() => window.happyDOM.abort()) + const document = window.document + + // The overlay lives in a closed shadow root; keep a handle so the test can read the outline. + const shadowRoots: ReturnType[] = [] + const attachShadow = window.HTMLElement.prototype.attachShadow + window.HTMLElement.prototype.attachShadow = function (init) { + const root = attachShadow.call(this, init) + shadowRoots.push(root) + return root + } + + const scroller = document.createElement('div') + const target = document.createElement('button') + if (previousMarker !== undefined) { + target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, previousMarker) + } + scroller.appendChild(target) + document.body.appendChild(scroller) + let targetRect: Rect = { x: 40, y: 300, width: 120, height: 30 } + const contentSize = { width: 120, height: 30 } + const rootSize = { width: 1000, height: 800 } + const spacing = { padding: 0, border: 0 } + target.getBoundingClientRect = () => + new window.DOMRect(targetRect.x, targetRect.y, targetRect.width, targetRect.height) + document.elementFromPoint = () => target + + const frames: (() => void)[] = [] + const resizeCallbacks: (() => void)[] = [] + const observers: { + callback: () => void + boxes: Map + }[] = [] + const measure = (element: unknown, box: 'content-box' | 'border-box') => { + if (element === target) { + const spacingSize = box === 'border-box' ? 2 * (spacing.padding + spacing.border) : 0 + return { width: contentSize.width + spacingSize, height: contentSize.height + spacingSize } + } + return { ...rootSize } + } + const context = { + window, + document, + requestAnimationFrame: (callback: () => void) => frames.push(callback), + cancelAnimationFrame: () => {}, + ResizeObserver: class { + private boxes = new Map< + unknown, + { box: 'content-box' | 'border-box'; width: number; height: number } + >() + constructor(callback: () => void) { + resizeCallbacks.push(callback) + observers.push({ callback, boxes: this.boxes }) + } + observe(element: unknown, options?: { box?: 'content-box' | 'border-box' }): void { + const box = options?.box ?? 'content-box' + this.boxes.set(element, { box, ...measure(element, box) }) + } + disconnect(): void { + this.boxes.clear() + } + } + } + expect(runInNewContext(buildGuestOverlayScript('arm'), context)).toBe(true) + const grab: { + host: { dispatchEvent: (event: unknown) => void } + freezeHighlight: (selectionId: string) => void + cleanup: () => void + } = Object.getOwnPropertyDescriptor(window, '__orcaGrab')?.value + const flushFrames = (): void => { + for (const frame of frames.splice(0)) { + frame() + } + } + const outline = (): Rect => { + // The outline is the first box inside the overlay container. + const box = shadowRoots[0].firstElementChild?.firstElementChild + if (!(box instanceof window.HTMLElement)) { + throw new Error('outline not rendered') + } + const style = box.style + return { + x: Number.parseFloat(style.left), + y: Number.parseFloat(style.top), + width: Number.parseFloat(style.width), + height: Number.parseFloat(style.height) + } + } + + // Hover the target, then click-freeze it like awaitClick does. + grab.host.dispatchEvent(new window.MouseEvent('mousemove', { clientX: 50, clientY: 310 })) + flushFrames() + grab.freezeHighlight('selection-1') + + return { + window, + scroller, + target, + grab, + outline, + flushFrames, + resizeCallbacks, + notifyResizes: () => { + let notifications = 0 + for (const observer of observers) { + let changed = false + for (const [element, previous] of observer.boxes) { + const current = measure(element, previous.box) + if (current.width !== previous.width || current.height !== previous.height) { + observer.boxes.set(element, { box: previous.box, ...current }) + changed = true + } + } + if (changed) { + observer.callback() + notifications++ + } + } + return notifications + }, + setTargetSpacing: (padding: number, border: number) => { + spacing.padding = padding + spacing.border = border + targetRect = { + ...targetRect, + width: contentSize.width + 2 * (padding + border), + height: contentSize.height + 2 * (padding + border) + } + }, + setRootContentSize: (width: number, height: number) => { + rootSize.width = width + rootSize.height = height + }, + moveTarget: (rect: Rect) => { + targetRect = rect + } + } +} + +describe('grab overlay after the click', () => { + it.each([ + ['border', 0, 8], + ['padding', 8, 0] + ])('follows %s-only outer growth with a fixed content box', async (_kind, padding, border) => { + const guest = await armGuest() + guest.setTargetSpacing(padding, border) + + expect(guest.notifyResizes()).toBe(1) + guest.flushFrames() + + expect(guest.outline()).toEqual({ x: 40, y: 300, width: 136, height: 46 }) + }) + + it('re-measures when the document root content box changes', async () => { + const guest = await armGuest() + guest.moveTarget({ x: 40, y: 250, width: 120, height: 30 }) + guest.setRootContentSize(980, 800) + + expect(guest.notifyResizes()).toBe(1) + guest.flushFrames() + + expect(guest.outline()).toEqual({ x: 40, y: 250, width: 120, height: 30 }) + }) + + it('keeps the outline on the element when a box inside the page scrolls', async () => { + const guest = await armGuest() + expect(guest.outline()).toEqual({ x: 40, y: 300, width: 120, height: 30 }) + + guest.moveTarget({ x: 40, y: 180, width: 120, height: 30 }) + guest.scroller.dispatchEvent(new guest.window.Event('scroll')) + guest.flushFrames() + + expect(guest.outline()).toEqual({ x: 40, y: 180, width: 120, height: 30 }) + }) + + it('re-measures on window resize and when the element changes size', async () => { + const guest = await armGuest() + + guest.moveTarget({ x: 20, y: 250, width: 90, height: 24 }) + guest.window.dispatchEvent(new guest.window.Event('resize')) + guest.flushFrames() + expect(guest.outline()).toEqual({ x: 20, y: 250, width: 90, height: 24 }) + + guest.moveTarget({ x: 20, y: 250, width: 90, height: 80 }) + for (const callback of guest.resizeCallbacks) { + callback() + } + guest.flushFrames() + expect(guest.outline()).toEqual({ x: 20, y: 250, width: 90, height: 80 }) + }) + + it('marks the selected element for the viewport bridge and clears it on teardown', async () => { + const guest = await armGuest() + expect(guest.target.getAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE)).toBe('selection-1') + + guest.grab.cleanup() + expect(guest.target.hasAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE)).toBe(false) + + guest.moveTarget({ x: 0, y: 0, width: 1, height: 1 }) + guest.scroller.dispatchEvent(new guest.window.Event('scroll')) + expect(guest.flushFrames).not.toThrow() + }) + + it('restores a page-owned marker value on teardown', async () => { + const guest = await armGuest('page-owned') + expect(guest.target.getAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE)).toBe('selection-1') + + guest.grab.cleanup() + expect(guest.target.getAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE)).toBe('page-owned') + }) + + it('does not overwrite a page marker changed after the selection', async () => { + const guest = await armGuest('page-owned') + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'page-updated') + + guest.grab.cleanup() + + expect(guest.target.getAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE)).toBe('page-updated') + }) +}) diff --git a/src/main/browser/grab-guest-overlay-script.ts b/src/main/browser/grab-guest-overlay-script.ts index a7c2b097264..233d0739a0c 100644 --- a/src/main/browser/grab-guest-overlay-script.ts +++ b/src/main/browser/grab-guest-overlay-script.ts @@ -1,3 +1,5 @@ +import { BROWSER_GRAB_SELECTED_ATTRIBUTE } from '../../shared/browser-annotation-viewport-bridge' + export const GRAB_GUEST_OVERLAY_SCRIPT = ` var host = document.createElement('div'); host.id = '__orca-grab-host'; host.style.cssText = 'position:fixed;top:0;left:0;width:100vw;height:100vh;z-index:2147483647;pointer-events:all;cursor:crosshair;'; @@ -75,6 +77,45 @@ export const GRAB_GUEST_OVERLAY_SCRIPT = ` var host = document.createElement('d // full-viewport click catcher that receives all pointer events. host.addEventListener('mousemove', onPointerMove); + var selectedAttribute = ${JSON.stringify(BROWSER_GRAB_SELECTED_ATTRIBUTE)}; + var selectedEl = null; + var selectedValue = null; + var previousSelectedValue = null; + var frozenFrame = 0; + var frozenResizeObserver = null; + + function refreshFrozenHighlight() { + frozenFrame = 0; + if (selectedEl && selectedEl.isConnected) { + updateHighlight(selectedEl); + } else { + highlightBox.style.display = 'none'; + hoverLabel.style.display = 'none'; + } + } + + function scheduleFrozenHighlight() { + if (!frozenFrame) frozenFrame = requestAnimationFrame(refreshFrozenHighlight); + } + + function stopTrackingSelection() { + window.removeEventListener('scroll', scheduleFrozenHighlight, true); + window.removeEventListener('resize', scheduleFrozenHighlight, true); + if (frozenResizeObserver) frozenResizeObserver.disconnect(); + frozenResizeObserver = null; + if (frozenFrame) cancelAnimationFrame(frozenFrame); + frozenFrame = 0; + if (selectedEl && selectedEl.getAttribute(selectedAttribute) === selectedValue) { + try { + if (previousSelectedValue === null) selectedEl.removeAttribute(selectedAttribute); + else selectedEl.setAttribute(selectedAttribute, previousSelectedValue); + } catch(e) {} + } + selectedEl = null; + selectedValue = null; + previousSelectedValue = null; + } + // Store state for awaitClick/finalize/teardown access window.__orcaGrab = { host: host, @@ -84,13 +125,34 @@ export const GRAB_GUEST_OVERLAY_SCRIPT = ` var host = document.createElement('d // the renderer shows the copy menu. Disabling pointer-events on the host // lets the cursor return to normal and prevents the crosshair from showing // over the dropdown menu's area in the webview. - freezeHighlight: function() { + freezeHighlight: function(selectionId) { host.removeEventListener('mousemove', onPointerMove); host.style.pointerEvents = 'none'; host.style.cursor = 'default'; + // Why: the outline must follow the element after the click — page and inner-box + // scroll (capture), resize/zoom and the element's own size changes. + selectedEl = currentEl; + if (!selectedEl) return; + // Why: the isolated-world viewport bridge finds the selection by this marker; it + // cannot see this world's objects, and the page must not see the bridge's token. + previousSelectedValue = selectedEl.getAttribute(selectedAttribute); + selectedValue = selectionId; + selectedEl.setAttribute(selectedAttribute, selectionId); + highlightBox.style.transition = 'none'; + window.addEventListener('scroll', scheduleFrozenHighlight, true); + window.addEventListener('resize', scheduleFrozenHighlight, true); + if (typeof ResizeObserver === 'function') { + frozenResizeObserver = new ResizeObserver(scheduleFrozenHighlight); + frozenResizeObserver.observe(selectedEl, { box: 'border-box' }); + // A scrollbar appearing narrows the page without a window resize. + if (selectedEl !== document.documentElement) { + frozenResizeObserver.observe(document.documentElement); + } + } }, cleanup: function() { host.removeEventListener('mousemove', onPointerMove); + stopTrackingSelection(); try { host.remove(); } catch(e) {} delete window.__orcaGrab; } diff --git a/src/main/browser/grab-guest-script.test.ts b/src/main/browser/grab-guest-script.test.ts index c64e2c52cf3..80a5cb8bbf1 100644 --- a/src/main/browser/grab-guest-script.test.ts +++ b/src/main/browser/grab-guest-script.test.ts @@ -7,8 +7,8 @@ import { clampGrabPayload } from './browser-grab-payload' describe('buildGuestOverlayScript', () => { it.each([ - ['arm', '07cffca05c4c9dab10bdcf301deab24e033edd07c6cd235bb364e1a139720a0a'], - ['awaitClick', 'b6b65b2b53c8719f1d10f93954cf867d99e43e14dbd1ca0a92e5067b168a126c'], + ['arm', '7786b94188002262ad921efb05cf4e4e22ad605b3fdb5bee6e040381f8dd95e7'], + ['awaitClick', '80ab6e9d7fe561791bab56e44e8dbee49698c03fa1ea87a1014e41143d463efd'], ['finalize', '91bd9836b0536c9579e0d4648d30679c0b4a5893d9a43110a70e67d6804fd291'], ['extractHover', 'cf0ee3ac61669daefa7db9389233c1abfe9f0fb9e7257300c761987aac914b02'], ['teardown', '732efde1022745f26dd4250d2891a663023eecafdf025fd66dde87781a985d81'] diff --git a/src/main/browser/grab-guest-selection-scripts.ts b/src/main/browser/grab-guest-selection-scripts.ts index 9eb3651ca67..ce1f6e83fde 100644 --- a/src/main/browser/grab-guest-selection-scripts.ts +++ b/src/main/browser/grab-guest-selection-scripts.ts @@ -42,7 +42,7 @@ export const AWAIT_CLICK_SCRIPT = `(async function() { // Why: freeze the highlight instead of removing it so the user sees // which element was selected while the copy menu is shown. Teardown // happens later when the renderer calls setGrabMode(false) or re-arms. - grab.freezeHighlight(); + grab.freezeHighlight(payload.page.capturedAt); resolve(payload); } @@ -64,7 +64,7 @@ export const AWAIT_CLICK_SCRIPT = `(async function() { } var payload = extractSelectedPayload(el); if (!payload) return; - grab.freezeHighlight(); + grab.freezeHighlight(payload.page.capturedAt); resolve({ __orcaContextMenu: true, payload: payload }); } diff --git a/src/main/ipc/browser-annotation-viewport-ipc.test.ts b/src/main/ipc/browser-annotation-viewport-ipc.test.ts new file mode 100644 index 00000000000..d31d9867f3d --- /dev/null +++ b/src/main/ipc/browser-annotation-viewport-ipc.test.ts @@ -0,0 +1,60 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + handlers: new Map unknown>(), + getAuthorizedGuest: vi.fn(() => ({})), + setAnnotationViewportBridge: vi.fn().mockResolvedValue(true) +})) + +vi.mock('electron', () => ({ + ipcMain: { + handle: (channel: string, handler: (event: unknown, args: unknown) => unknown) => + mocks.handlers.set(channel, handler), + removeHandler: (channel: string) => mocks.handlers.delete(channel) + } +})) + +vi.mock('../browser/browser-manager', () => ({ browserManager: mocks })) +vi.mock('./browser-renderer-trust', () => ({ isTrustedBrowserRenderer: () => true })) +vi.mock('../browser/browser-client-page-metadata-transport', () => ({ + publishBrowserClientPageMetadata: vi.fn() +})) + +import { registerBrowserGuestViewHandlers } from './browser-guest-view-ipc' + +beforeEach(() => { + mocks.handlers.clear() + mocks.getAuthorizedGuest.mockClear() + mocks.setAnnotationViewportBridge.mockClear() + registerBrowserGuestViewHandlers() +}) + +describe('browser annotation viewport IPC', () => { + it('passes the pending selection identity through to the isolated bridge', async () => { + const handler = mocks.handlers.get('browser:setAnnotationViewportBridge') + expect(handler).toBeDefined() + await handler?.( + { sender: { id: 91 } }, + { + browserPageId: 'page-1', + emitViewport: true, + enabled: true, + markers: [], + selectionId: '2026-10-12T19:30:00.000Z', + token: 'annotationviewporttoken' + } + ) + + expect(mocks.setAnnotationViewportBridge).toHaveBeenCalledWith( + 'page-1', + { + emitViewport: true, + enabled: true, + markers: [], + selectionId: '2026-10-12T19:30:00.000Z', + token: 'annotationviewporttoken' + }, + expect.any(Function) + ) + }) +}) diff --git a/src/main/ipc/browser-guest-view-ipc.ts b/src/main/ipc/browser-guest-view-ipc.ts index 8aa13a57ba9..0193eeff8a0 100644 --- a/src/main/ipc/browser-guest-view-ipc.ts +++ b/src/main/ipc/browser-guest-view-ipc.ts @@ -112,6 +112,8 @@ export function registerBrowserGuestViewHandlers(): void { typeof args.enabled !== 'boolean' || typeof args.emitViewport !== 'boolean' || !isValidBrowserAnnotationViewportBridgeMarkers(args.markers) || + (args.selectionId !== undefined && + (typeof args.selectionId !== 'string' || args.selectionId.length > 80)) || !isValidBrowserAnnotationViewportBridgeToken(args.token) ) { return false @@ -132,6 +134,7 @@ export function registerBrowserGuestViewHandlers(): void { enabled: args.enabled, emitViewport: args.emitViewport, markers: args.markers, + selectionId: args.selectionId, token: args.token }, resolveGuest diff --git a/src/renderer/src/components/browser-pane/annotate/browser-annotation-viewport-message.ts b/src/renderer/src/components/browser-pane/annotate/browser-annotation-viewport-message.ts new file mode 100644 index 00000000000..3f392f7883a --- /dev/null +++ b/src/renderer/src/components/browser-pane/annotate/browser-annotation-viewport-message.ts @@ -0,0 +1,23 @@ +import type { Dispatch, SetStateAction } from 'react' +import { BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX } from '../../../../../shared/browser-annotation-viewport-bridge' +import { parseBrowserAnnotationViewportReport } from '../../../../../shared/browser-annotation-viewport-report' +import { + applyBrowserAnnotationViewportReport, + type BrowserOverlayViewport +} from '../describe-page/browser-annotation-geometry' + +export function consumeBrowserAnnotationViewportMessage( + event: { message?: string }, + token: string, + setViewport: Dispatch> +): void { + const message = typeof event.message === 'string' ? event.message : '' + const prefix = `${BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX}${token}:` + if (!message.startsWith(prefix)) { + return + } + const report = parseBrowserAnnotationViewportReport(message.slice(prefix.length)) + if (report) { + setViewport((current) => applyBrowserAnnotationViewportReport(current, report)) + } +} diff --git a/src/renderer/src/components/browser-pane/annotate/browser-guest-annotate-overlays.tsx b/src/renderer/src/components/browser-pane/annotate/browser-guest-annotate-overlays.tsx index c8141bd0685..5dbf6b254fd 100644 --- a/src/renderer/src/components/browser-pane/annotate/browser-guest-annotate-overlays.tsx +++ b/src/renderer/src/components/browser-pane/annotate/browser-guest-annotate-overlays.tsx @@ -99,12 +99,15 @@ export function BrowserGuestAnnotateOverlays({ {pendingAnnotationPayload ? ( + getBrowserOverlayAnchor( + pendingAnnotationPayload, + containerRef.current, + webviewRef.current, + browserOverlayViewport, + cardHeight + ) + } portalContainer={containerRef.current} onAdd={handleAddBrowserAnnotation} onCancel={handleCancelPendingBrowserAnnotation} diff --git a/src/renderer/src/components/browser-pane/annotate/guest-annotation-viewport-bridge.test.ts b/src/renderer/src/components/browser-pane/annotate/guest-annotation-viewport-bridge.test.ts index 69625c91ab3..144fabd91fe 100644 --- a/src/renderer/src/components/browser-pane/annotate/guest-annotation-viewport-bridge.test.ts +++ b/src/renderer/src/components/browser-pane/annotate/guest-annotation-viewport-bridge.test.ts @@ -62,7 +62,12 @@ describe('current-document annotation marker projection', () => { ) syncGuestAnnotationViewportBridge({ ...args, pendingPayload: notes[1].payload }) expect(setAnnotationViewportBridge).toHaveBeenLastCalledWith( - expect.objectContaining({ enabled: true, emitViewport: true, markers: [] }) + expect.objectContaining({ + enabled: true, + emitViewport: true, + markers: [], + selectionId: notes[1].payload.page.capturedAt + }) ) syncGuestAnnotationViewportBridge({ ...args, diff --git a/src/renderer/src/components/browser-pane/annotate/guest-annotation-viewport-bridge.ts b/src/renderer/src/components/browser-pane/annotate/guest-annotation-viewport-bridge.ts index 6e28c0fa9d1..1d07b1499ac 100644 --- a/src/renderer/src/components/browser-pane/annotate/guest-annotation-viewport-bridge.ts +++ b/src/renderer/src/components/browser-pane/annotate/guest-annotation-viewport-bridge.ts @@ -44,6 +44,7 @@ export function syncGuestAnnotationViewportBridge({ emitViewport: pendingPayload !== null, enabled: surfaceActive && (pendingPayload !== null || markers.length > 0), markers, + selectionId: pendingPayload?.page.capturedAt, token }) .catch(() => { diff --git a/src/renderer/src/components/browser-pane/annotate/pending-browser-annotation-card.tsx b/src/renderer/src/components/browser-pane/annotate/pending-browser-annotation-card.tsx index 554ded9f019..e9a1edc7e58 100644 --- a/src/renderer/src/components/browser-pane/annotate/pending-browser-annotation-card.tsx +++ b/src/renderer/src/components/browser-pane/annotate/pending-browser-annotation-card.tsx @@ -1,5 +1,5 @@ import { ImeTextarea } from '@/lib/ime-text-field' -import { useState } from 'react' +import { useEffect, useState } from 'react' import { CornerDownLeft, MessageSquarePlus } from 'lucide-react' import { Button } from '@/components/ui/button' import { ToggleGroup, ToggleGroupItem } from '@/components/ui/toggle-group' @@ -19,17 +19,30 @@ import { export function PendingBrowserAnnotationCard({ payload, - anchor, + resolveAnchor, portalContainer, onAdd, onCancel }: { payload: BrowserGrabPayload - anchor: BrowserOverlayAnchor + /** Anchor for a card of the given measured height; null until the card has rendered once. */ + resolveAnchor: (cardHeight: number | null) => BrowserOverlayAnchor portalContainer: HTMLElement | null onAdd: (comment: string, intent: BrowserAnnotationIntent) => void onCancel: () => void }): React.JSX.Element { + const [card, setCard] = useState(null) + const [cardHeight, setCardHeight] = useState(null) + useEffect(() => { + if (!card || typeof ResizeObserver === 'undefined') { + return + } + // offsetHeight, not the bounding box: the open animation scales the card. + const observer = new ResizeObserver(() => setCardHeight(card.offsetHeight || null)) + observer.observe(card) + return () => observer.disconnect() + }, [card]) + const anchor = resolveAnchor(cardHeight) const [comment, setComment] = useState('') const [intent, setIntent] = useState('change') const trimmed = comment.trim() @@ -51,6 +64,7 @@ export function PendingBrowserAnnotationCard({ /> grabPayloadRef.current === grab.payload ? grab.payload : null ) + // The live box measured for the previous selection must not place this one's card. + setBrowserOverlayViewport((current) => + current.target ? { ...current, target: undefined } : current + ) return } if (!grab.contextMenu) { @@ -170,6 +174,7 @@ export function useBrowserPageGrabAnnotations({ grab.contextMenu, grabIntent, recordFeatureInteraction, + setBrowserOverlayViewport, showGrabToast ]) diff --git a/src/renderer/src/components/browser-pane/describe-page/browser-annotation-geometry.test.ts b/src/renderer/src/components/browser-pane/describe-page/browser-annotation-geometry.test.ts index a2706cd75c4..b454c92bfb2 100644 --- a/src/renderer/src/components/browser-pane/describe-page/browser-annotation-geometry.test.ts +++ b/src/renderer/src/components/browser-pane/describe-page/browser-annotation-geometry.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest' import type { BrowserGrabPayload } from '../../../../../shared/browser-grab-types' import { + applyBrowserAnnotationViewportReport, createBrowserAnnotationPayload, getBrowserOverlayAnchor, getLiveBrowserAnnotationRect @@ -111,3 +112,94 @@ describe('browser annotation geometry', () => { expect(createBrowserAnnotationPayload(source).target.selector).toBe('button') }) }) + +describe('browser annotation card follows the live element', () => { + const box = (left: number, top: number, width: number, height: number) => ({ + getBoundingClientRect: () => ({ left, top, width, height }) + }) + // Clicked at viewport (10, 20) with window scroll (5, 15). + const clicked = payload({ isFixed: false }) + + it('uses the guest-measured box over the click-time one after a box inside the page scrolls', () => { + // Window scroll did not move, so the click-time math alone would leave the card in place. + const viewport = applyBrowserAnnotationViewportReport( + { scrollX: 0, scrollY: 0, version: 0 }, + { scrollX: 5, scrollY: 15, target: { x: 10, y: 300, width: 40, height: 20 } } + ) + expect(getLiveBrowserAnnotationRect(clicked, viewport)).toEqual({ + x: 10, + y: 300, + width: 40, + height: 20 + }) + const pane = box(0, 0, 900, 900) + expect(getBrowserOverlayAnchor(clicked, pane, pane, viewport)).toEqual({ + x: 30, + y: 320, + side: 'bottom' + }) + }) + + it('follows the element after a pane resize reflows it', () => { + const viewport = applyBrowserAnnotationViewportReport( + { scrollX: 5, scrollY: 15, version: 1 }, + { scrollX: 5, scrollY: 15, target: { x: 200, y: 20, width: 300, height: 20 } } + ) + const pane = box(0, 0, 600, 900) + expect(getBrowserOverlayAnchor(clicked, pane, pane, viewport)).toEqual({ + x: 350, + y: 40, + side: 'bottom' + }) + }) + + it('converts guest pixels to pane pixels when the page is zoomed', () => { + // 120% page zoom: the guest lays out 1000 CSS pixels across a 1200-pixel-wide view. + const viewport = applyBrowserAnnotationViewportReport( + { scrollX: 0, scrollY: 0, version: 0 }, + { + scrollX: 0, + scrollY: 0, + viewportWidth: 1000, + target: { x: 100, y: 50, width: 50, height: 10 } + } + ) + const pane = box(0, 0, 1200, 900) + expect(getBrowserOverlayAnchor(clicked, pane, pane, viewport)).toEqual({ + x: 150, + y: 72, + side: 'bottom' + }) + }) + + it('picks the side with the measured card height once the card has rendered', () => { + const fixed = payload({ + isFixed: true, + rectViewport: { x: 100, y: 40, width: 100, height: 20 } + }) + const pane = box(0, 0, 900, 310) + const viewport = { scrollX: 0, scrollY: 0, version: 0 } + // 250 pixels below the target: too shallow for the assumed 240-pixel card plus margin. + expect(getBrowserOverlayAnchor(fixed, pane, pane, viewport).side).toBe('right') + expect(getBrowserOverlayAnchor(fixed, pane, pane, viewport, 200).side).toBe('bottom') + }) + + it('keeps the same viewport state when a report changes nothing', () => { + const target = { x: 1, y: 2, width: 3, height: 4 } + const report = { scrollX: 1, scrollY: 2, viewportWidth: 800, target } + const first = applyBrowserAnnotationViewportReport( + { scrollX: 0, scrollY: 0, version: 0 }, + report + ) + expect(first.version).toBe(1) + expect(applyBrowserAnnotationViewportReport(first, { ...report, target: { ...target } })).toBe( + first + ) + const released = applyBrowserAnnotationViewportReport(first, { + scrollX: 1, + scrollY: 2, + viewportWidth: 800 + }) + expect(released).toEqual({ scrollX: 1, scrollY: 2, viewportWidth: 800, version: 2 }) + }) +}) diff --git a/src/renderer/src/components/browser-pane/describe-page/browser-annotation-geometry.ts b/src/renderer/src/components/browser-pane/describe-page/browser-annotation-geometry.ts index bc6c37e31b4..78f1479976d 100644 --- a/src/renderer/src/components/browser-pane/describe-page/browser-annotation-geometry.ts +++ b/src/renderer/src/components/browser-pane/describe-page/browser-annotation-geometry.ts @@ -6,6 +6,7 @@ import type { BrowserGrabRect, BrowserPageAnnotation } from '../../../../../shared/browser-grab-types' +import type { BrowserAnnotationViewportReport } from '../../../../../shared/browser-annotation-viewport-report' import { translate } from '@/i18n/i18n' export type BrowserOverlaySide = 'top' | 'bottom' | 'left' | 'right' @@ -46,12 +47,17 @@ export type BrowserOverlayViewport = { scrollX: number scrollY: number version: number + /** Guest `innerWidth` at the last report. */ + viewportWidth?: number + /** Live box of the pending selection, re-measured by the guest; absent until it reports one. */ + target?: BrowserGrabRect } export const EMPTY_BROWSER_ANNOTATIONS: BrowserPageAnnotation[] = [] -// Footprint of PendingBrowserAnnotationCard (measured at 352x237), rounded up: it never grows -// past this, so a band narrower than it plus the margin below cannot hold the card. +// Footprint of PendingBrowserAnnotationCard (measured at 352x237), rounded up. The height is only +// the fallback until the card has measured itself; the width is fixed by the card's CSS, and its +// rendered width is squeezed by the side it sits on, so measuring it would feed back into the pick. const PENDING_ANNOTATION_CARD_HEIGHT = 240 const PENDING_ANNOTATION_CARD_WIDTH = 352 // PopoverContent's sideOffset (10) plus its collisionPadding (12). @@ -82,8 +88,11 @@ const BROWSER_OVERLAY_SIDES = ['bottom', 'top', 'right', 'left'] as const * target instead of beside it. A tall target, or a short pane, leaves neither vertical band deep * enough — the horizontal ones usually still are. */ -function pickBrowserOverlaySide(space: Record): BrowserOverlaySide { - const vertical = PENDING_ANNOTATION_CARD_HEIGHT + PENDING_ANNOTATION_CARD_MARGIN +function pickBrowserOverlaySide( + space: Record, + cardHeight: number +): BrowserOverlaySide { + const vertical = cardHeight + PENDING_ANNOTATION_CARD_MARGIN const horizontal = PENDING_ANNOTATION_CARD_WIDTH + PENDING_ANNOTATION_CARD_MARGIN const room: Record = { bottom: space.bottom / vertical, @@ -103,30 +112,40 @@ export function getBrowserOverlayAnchor( payload: BrowserGrabPayload, container: OverlayRectSource | null, webview: OverlayRectSource | null, - viewport: BrowserOverlayViewport + viewport: BrowserOverlayViewport, + cardHeight: number | null = null ): BrowserOverlayAnchor { const containerRect = container?.getBoundingClientRect() const webviewRect = webview?.getBoundingClientRect() const rect = getLiveBrowserAnnotationRect(payload, viewport) + // Guest pixels differ from pane pixels whenever page zoom differs from the app's zoom. + const guestWidth = viewport.viewportWidth ?? payload.page.viewportWidth + const scale = + webviewRect && webviewRect.width > 0 && guestWidth > 0 ? webviewRect.width / guestWidth : 1 const offsetX = (webviewRect?.left ?? 0) - (containerRect?.left ?? 0) const offsetY = (webviewRect?.top ?? 0) - (containerRect?.top ?? 0) const containerWidth = containerRect?.width ?? 0 const containerHeight = containerRect?.height ?? 0 - const elementLeft = offsetX + rect.x - const elementRight = elementLeft + rect.width - const elementTop = offsetY + rect.y - const elementBottom = elementTop + rect.height - const side = pickBrowserOverlaySide({ - top: elementTop, - bottom: containerHeight - elementBottom, - left: elementLeft, - right: containerWidth - elementRight - }) + const width = rect.width * scale + const height = rect.height * scale + const elementLeft = offsetX + rect.x * scale + const elementRight = elementLeft + width + const elementTop = offsetY + rect.y * scale + const elementBottom = elementTop + height + const side = pickBrowserOverlaySide( + { + top: elementTop, + bottom: containerHeight - elementBottom, + left: elementLeft, + right: containerWidth - elementRight + }, + cardHeight ?? PENDING_ANNOTATION_CARD_HEIGHT + ) const edge = { - top: { x: elementLeft + rect.width / 2, y: elementTop }, - bottom: { x: elementLeft + rect.width / 2, y: elementBottom }, - left: { x: elementLeft, y: elementTop + rect.height / 2 }, - right: { x: elementRight, y: elementTop + rect.height / 2 } + top: { x: elementLeft + width / 2, y: elementTop }, + bottom: { x: elementLeft + width / 2, y: elementBottom }, + left: { x: elementLeft, y: elementTop + height / 2 }, + right: { x: elementRight, y: elementTop + height / 2 } }[side] return { x: clampNumber(edge.x, 12, Math.max(12, containerWidth - 12)), @@ -143,6 +162,9 @@ export function getLiveBrowserAnnotationRect( payload: BrowserGrabPayload, viewport: BrowserOverlayViewport ): BrowserGrabRect { + if (viewport.target) { + return viewport.target + } if (payload.target.isFixed) { return payload.target.rectViewport } @@ -154,3 +176,26 @@ export function getLiveBrowserAnnotationRect( y: payload.target.rectPage.y - scrollY } } + +/** Folds a guest viewport report in, keeping the same state object when nothing moved. */ +export function applyBrowserAnnotationViewportReport( + current: BrowserOverlayViewport, + report: BrowserAnnotationViewportReport +): BrowserOverlayViewport { + const unchanged = + current.scrollX === report.scrollX && + current.scrollY === report.scrollY && + current.viewportWidth === report.viewportWidth && + sameRect(current.target, report.target) + if (unchanged) { + return current.version === 0 ? { ...current, version: 1 } : current + } + return { ...report, version: current.version + 1 } +} + +function sameRect(a: BrowserGrabRect | undefined, b: BrowserGrabRect | undefined): boolean { + return ( + a === b || + (!!a && !!b && a.x === b.x && a.y === b.y && a.width === b.width && a.height === b.height) + ) +} diff --git a/src/renderer/src/components/browser-pane/host-guest/browser-page-annotation-document.test.ts b/src/renderer/src/components/browser-pane/host-guest/browser-page-annotation-document.test.ts index 2ff2ebbab08..e0a65879f9c 100644 --- a/src/renderer/src/components/browser-pane/host-guest/browser-page-annotation-document.test.ts +++ b/src/renderer/src/components/browser-pane/host-guest/browser-page-annotation-document.test.ts @@ -1,5 +1,6 @@ // @vitest-environment happy-dom import { afterEach, describe, expect, it, vi } from 'vitest' +import { BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX } from '../../../../../shared/browser-annotation-viewport-bridge' import { createBrowserMockApi, createTestStore } from '@/store/slices/browser-slice-test-harness' import { makeAnnotation } from '@/store/slices/browser-annotation-test-fixture' import { syncGuestAnnotationViewportBridge } from '../annotate/guest-annotation-viewport-bridge' @@ -106,6 +107,34 @@ function navigationEvent( } describe('browser annotation document boundaries', () => { + it('still accepts token-authenticated live target reports', () => { + const h = createHarness() + const report = JSON.stringify({ + scrollX: 0, + scrollY: 0, + viewportWidth: 900, + target: { x: 40, y: 120, width: 120, height: 30 } + }) + h.navigation.handleAnnotationViewportMessage({ + message: `${BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX}wrong:${report}` + }) + expect(h.setBrowserOverlayViewport).not.toHaveBeenCalled() + + h.navigation.handleAnnotationViewportMessage({ + message: `${BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX}token:${report}` + }) + expect(h.setBrowserOverlayViewport).toHaveBeenCalledOnce() + expect( + h.setBrowserOverlayViewport.mock.calls[0][0]({ scrollX: 0, scrollY: 0, version: 0 }) + ).toEqual({ + scrollX: 0, + scrollY: 0, + viewportWidth: 900, + target: { x: 40, y: 120, width: 120, height: 30 }, + version: 1 + }) + }) + it('retains the exact saved notes on actual same-URL loading-start, including untracked loads', () => { const h = createHarness() const saved = h.store.getState().browserAnnotationsByPageId[h.pageId] diff --git a/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.ts b/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.ts index 1c08005a6fc..7af4659a1d0 100644 --- a/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.ts +++ b/src/renderer/src/components/browser-pane/host-guest/browser-page-webview-navigation-handlers.ts @@ -1,5 +1,4 @@ import type { Dispatch, MutableRefObject, RefObject, SetStateAction } from 'react' -import { BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX } from '../../../../../shared/browser-annotation-viewport-bridge' import { normalizeBrowserNavigationUrl, redactKagiSessionToken @@ -8,6 +7,7 @@ import type { BrowserLoadError } from '../../../../../shared/browser-workspace-t import { BROWSER_GUEST_RECOVERY_ERROR_CODE } from './browser-page-guest-recovery' import { rememberLiveBrowserUrl } from '../describe-page/live-browser-url-registry' import type { BrowserOverlayViewport } from '../describe-page/browser-annotation-geometry' +import { consumeBrowserAnnotationViewportMessage } from '../annotate/browser-annotation-viewport-message' import { getBrowserDisplayTitle, isChromiumErrorPage, @@ -184,29 +184,11 @@ export function createBrowserPageWebviewNavigationHandlers({ } const handleAnnotationViewportMessage = (event: { message?: string }): void => { - const message = typeof event.message === 'string' ? event.message : '' - const prefix = `${BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX}${annotationViewportBridgeTokenRef.current}:` - if (!message.startsWith(prefix)) { - return - } - try { - const next = JSON.parse(message.slice(prefix.length)) as { - scrollX?: unknown - scrollY?: unknown - } - const scrollX = - typeof next.scrollX === 'number' && Number.isFinite(next.scrollX) ? next.scrollX : 0 - const scrollY = - typeof next.scrollY === 'number' && Number.isFinite(next.scrollY) ? next.scrollY : 0 - setBrowserOverlayViewport((current) => { - if (current.scrollX === scrollX && current.scrollY === scrollY) { - return current.version === 0 ? { ...current, version: 1 } : current - } - return { scrollX, scrollY, version: current.version + 1 } - }) - } catch { - // Ignore unrelated or malformed guest console output. - } + consumeBrowserAnnotationViewportMessage( + event, + annotationViewportBridgeTokenRef.current, + setBrowserOverlayViewport + ) } return { diff --git a/src/renderer/src/components/browser-pane/workspace-doc/HtmlDocPreview.tsx b/src/renderer/src/components/browser-pane/workspace-doc/HtmlDocPreview.tsx index 3aef6bd8f7c..927dc0e8117 100644 --- a/src/renderer/src/components/browser-pane/workspace-doc/HtmlDocPreview.tsx +++ b/src/renderer/src/components/browser-pane/workspace-doc/HtmlDocPreview.tsx @@ -129,15 +129,23 @@ export function HtmlDocPreview({ [filePath, hostLabel, worktreeRoot] ) const isUnavailable = state === 'unavailable' || failureReason !== null - const { grab, markup, annotationSend, grabAnnotations, browserOverlayViewport, elementTools } = - useDocPreviewGuestTools({ - previewId, - worktreeId, - grantId, - webviewRef, - containerRef, - toolsReady: state === 'ready' && !isUnavailable - }) + const { + grab, + markup, + annotationSend, + grabAnnotations, + browserOverlayViewport, + handleAnnotationViewportMessage, + resetBrowserOverlayViewport, + elementTools + } = useDocPreviewGuestTools({ + previewId, + worktreeId, + grantId, + webviewRef, + containerRef, + toolsReady: state === 'ready' && !isUnavailable + }) // Not `document`: shadowing the global inside a component is how a stray DOM call silently // starts reading a plain object. const previewDocument = useMemo( @@ -150,6 +158,7 @@ export function HtmlDocPreview({ let detach: (() => void) | undefined let loadFailed = false const onLoadStarted = (): void => { + resetBrowserOverlayViewport() loadFailed = false setFailureReason(null) setAssetFailures([]) @@ -231,6 +240,7 @@ export function HtmlDocPreview({ onLoadStopped, onLoadFailed, onNavigated: syncHistory, + onViewportMessage: handleAnnotationViewportMessage, onTitleUpdated: (event) => { useAppStore.getState().updateBrowserPageState(previewId, { title: event.title }) // A rename only — the mount already recorded this document's visit. @@ -269,6 +279,8 @@ export function HtmlDocPreview({ previewId, remintCount, resetDirectoryAccess, + handleAnnotationViewportMessage, + resetBrowserOverlayViewport, resetHistory, syncHistory, worktreeId diff --git a/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-webview-attach.test.ts b/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-webview-attach.test.ts index e057e0a7d3c..8293fb8f5f8 100644 --- a/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-webview-attach.test.ts +++ b/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-webview-attach.test.ts @@ -22,7 +22,8 @@ it('restores pointer input when a drag ends after attaching a document preview', onLoadStopped: vi.fn(), onLoadFailed: vi.fn(), onNavigated: vi.fn(), - onTitleUpdated: vi.fn() + onTitleUpdated: vi.fn(), + onViewportMessage: vi.fn() }) try { diff --git a/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-webview-attach.ts b/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-webview-attach.ts index 19989f0a709..ffeb8b51926 100644 --- a/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-webview-attach.ts +++ b/src/renderer/src/components/browser-pane/workspace-doc/doc-preview-webview-attach.ts @@ -16,7 +16,8 @@ export function attachDocPreviewWebview({ onLoadStopped, onLoadFailed, onNavigated, - onTitleUpdated + onTitleUpdated, + onViewportMessage }: { previewId: string container: HTMLDivElement @@ -27,6 +28,7 @@ export function attachDocPreviewWebview({ onLoadFailed: (event: Electron.DidFailLoadEvent) => void onNavigated: () => void onTitleUpdated: (event: Electron.PageTitleUpdatedEvent) => void + onViewportMessage: (event: { message?: string }) => void }): { webview: Electron.WebviewTag; detach: () => void; reload: () => void } { const webview = document.createElement('webview') as Electron.WebviewTag // Why no allowpopups: the guest's preload intercepts a trusted click on a link before Chromium @@ -51,6 +53,7 @@ export function attachDocPreviewWebview({ // Why the document names its own tab: a preview is a browser tab, and this is how every other // one is named. What the document cannot do is name it the grant it is served over. webview.addEventListener('page-title-updated', onTitleUpdated) + webview.addEventListener('console-message', onViewportMessage) // Register before append so a guest attached mid-drag cannot swallow the pointer stream. registerPersistentWebview(previewId, webview) container.appendChild(webview) @@ -65,6 +68,7 @@ export function attachDocPreviewWebview({ webview.removeEventListener('did-navigate', onNavigated) webview.removeEventListener('did-navigate-in-page', onNavigated) webview.removeEventListener('page-title-updated', onTitleUpdated) + webview.removeEventListener('console-message', onViewportMessage) moveFocusToRendererBeforeWebviewDetach(webview) webview.remove() if (webviewRegistry.get(previewId) === webview) { diff --git a/src/renderer/src/components/browser-pane/workspace-doc/use-doc-preview-guest-tools.test.ts b/src/renderer/src/components/browser-pane/workspace-doc/use-doc-preview-guest-tools.test.ts index fc1f7952acf..d6a3a86454c 100644 --- a/src/renderer/src/components/browser-pane/workspace-doc/use-doc-preview-guest-tools.test.ts +++ b/src/renderer/src/components/browser-pane/workspace-doc/use-doc-preview-guest-tools.test.ts @@ -7,14 +7,28 @@ import { act, createElement, useRef } from 'react' import { createRoot, type Root } from 'react-dom/client' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { BrowserGrabPayload } from '../../../../../shared/browser-grab-types' +import { BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX } from '../../../../../shared/browser-annotation-viewport-bridge' +import { makeAnnotation } from '@/store/slices/browser-annotation-test-fixture' +import { getBrowserOverlayAnchor } from '../describe-page/browser-annotation-geometry' +import { attachDocPreviewWebview } from './doc-preview-webview-attach' -const calls = vi.hoisted(() => ({ - annotationSend: [] as { browserTabId: string }[], - grabAnnotations: [] as { browserTabId: string; toolTargetId: string }[], - grabMode: [] as string[], - viewportBridge: [] as { toolTargetId: string }[] +type GuestToolCalls = { + annotationSend: { browserTabId: string }[] + grabAnnotations: { browserTabId: string; toolTargetId: string }[] + grabMode: string[] + viewportBridge: { toolTargetId: string; token: string }[] +} + +const calls = vi.hoisted(() => ({ + annotationSend: [], + grabAnnotations: [], + grabMode: [], + viewportBridge: [] })) +let pendingPayload: BrowserGrabPayload | null = null + vi.mock('@/components/browser-pane/annotate/use-browser-page-annotation-send', () => ({ useBrowserPageAnnotationSend: (args: { browserTabId: string }) => { calls.annotationSend.push({ browserTabId: args.browserTabId }) @@ -28,7 +42,11 @@ vi.mock('@/components/browser-pane/annotate/use-browser-page-grab-annotations', browserTabId: args.browserTabId, toolTargetId: args.toolTargetId }) - return { pendingAnnotationPayload: null, grabIntent: null, startGrabIntent: () => undefined } + return { + pendingAnnotationPayload: pendingPayload, + grabIntent: null, + startGrabIntent: () => undefined + } } })) @@ -44,8 +62,8 @@ vi.mock('@/components/browser-pane/annotate/useGrabMode', () => ({ })) vi.mock('@/components/browser-pane/annotate/guest-annotation-viewport-bridge', () => ({ - syncGuestAnnotationViewportBridge: (args: { toolTargetId: string }) => { - calls.viewportBridge.push({ toolTargetId: args.toolTargetId }) + syncGuestAnnotationViewportBridge: (args: { toolTargetId: string; token: string }) => { + calls.viewportBridge.push({ toolTargetId: args.toolTargetId, token: args.token }) } })) @@ -60,7 +78,7 @@ const SECOND_GRANT = 'b'.repeat(32) function Harness({ grantId }: { grantId: string | null }): null { const webviewRef = useRef(null) const containerRef = useRef(null) - useDocPreviewGuestTools({ + latestTools = useDocPreviewGuestTools({ previewId: PREVIEW_ID, worktreeId: 'wt-1', grantId, @@ -73,6 +91,14 @@ function Harness({ grantId }: { grantId: string | null }): null { let container: HTMLDivElement let root: Root +let latestTools: ReturnType | null = null + +function currentTools(): ReturnType { + if (!latestTools) { + throw new Error('preview tools not rendered') + } + return latestTools +} function render(grantId: string | null): void { act(() => { @@ -81,10 +107,13 @@ function render(grantId: string | null): void { } beforeEach(() => { + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true) calls.annotationSend.length = 0 calls.grabAnnotations.length = 0 calls.grabMode.length = 0 calls.viewportBridge.length = 0 + pendingPayload = null + latestTools = null container = document.createElement('div') document.body.appendChild(container) root = createRoot(container) @@ -93,6 +122,7 @@ beforeEach(() => { afterEach(() => { act(() => root.unmount()) container.remove() + vi.unstubAllGlobals() }) describe('useDocPreviewGuestTools ids', () => { @@ -131,4 +161,63 @@ describe('useDocPreviewGuestTools ids', () => { expect(calls.grabAnnotations.at(-1)?.toolTargetId).toBe('') expect(calls.viewportBridge).toHaveLength(0) }) + + it('moves the pending card from a live report after inner scroll and removes the listener on detach', () => { + const selected = makeAnnotation(PREVIEW_ID).payload + selected.page.viewportWidth = 900 + selected.target.rectPage = { x: 40, y: 300, width: 120, height: 30 } + selected.target.rectViewport = { x: 40, y: 300, width: 120, height: 30 } + pendingPayload = selected + render(FIRST_GRANT) + const token = calls.viewportBridge.at(-1)?.token + expect(token).toBeTruthy() + + const previewContainer = document.createElement('div') + document.body.appendChild(previewContainer) + const attached = attachDocPreviewWebview({ + previewId: PREVIEW_ID, + container: previewContainer, + url: 'orca-preview://grant/index.html', + ariaLabel: 'HTML preview', + onLoadStarted: () => currentTools().resetBrowserOverlayViewport(), + onLoadStopped: vi.fn(), + onLoadFailed: vi.fn(), + onNavigated: vi.fn(), + onTitleUpdated: vi.fn(), + onViewportMessage: currentTools().handleAnnotationViewportMessage + }) + const pane = { + getBoundingClientRect: () => ({ left: 0, top: 0, width: 900, height: 900 }) + } + const cardY = (): number => + getBrowserOverlayAnchor(selected, pane, pane, currentTools().browserOverlayViewport).y + const send = (message: string): void => { + act(() => { + attached.webview.dispatchEvent(Object.assign(new Event('console-message'), { message })) + }) + } + + try { + expect(cardY()).toBe(330) + const report = JSON.stringify({ + scrollX: 0, + scrollY: 0, + viewportWidth: 900, + target: { x: 40, y: 120, width: 120, height: 30 } + }) + send(`${BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX}wrong-token:${report}`) + expect(cardY()).toBe(330) + send(`${BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX}${token}:${report}`) + expect(cardY()).toBe(150) + + act(() => attached.webview.dispatchEvent(new Event('did-start-loading'))) + expect(cardY()).toBe(330) + attached.detach() + send(`${BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX}${token}:${report}`) + expect(cardY()).toBe(330) + } finally { + attached.detach() + previewContainer.remove() + } + }) }) diff --git a/src/renderer/src/components/browser-pane/workspace-doc/use-doc-preview-guest-tools.ts b/src/renderer/src/components/browser-pane/workspace-doc/use-doc-preview-guest-tools.ts index 83706aa3fcd..23ff5fe3677 100644 --- a/src/renderer/src/components/browser-pane/workspace-doc/use-doc-preview-guest-tools.ts +++ b/src/renderer/src/components/browser-pane/workspace-doc/use-doc-preview-guest-tools.ts @@ -1,7 +1,8 @@ -import { useEffect, useMemo, useRef, useState, type MutableRefObject } from 'react' +import { useCallback, useEffect, useMemo, useRef, useState, type MutableRefObject } from 'react' import { createBrowserUuid } from '@/lib/browser-uuid' import { useShortcutLabel } from '@/hooks/useShortcutLabel' import { syncGuestAnnotationViewportBridge } from '@/components/browser-pane/annotate/guest-annotation-viewport-bridge' +import { consumeBrowserAnnotationViewportMessage } from '@/components/browser-pane/annotate/browser-annotation-viewport-message' import { useBrowserPageAnnotationSend } from '@/components/browser-pane/annotate/use-browser-page-annotation-send' import { useBrowserPageGrabAnnotations } from '@/components/browser-pane/annotate/use-browser-page-grab-annotations' import { useBrowserPageMarkupCapture } from '@/components/browser-pane/annotate/use-browser-page-markup-capture' @@ -36,6 +37,8 @@ export function useDocPreviewGuestTools({ annotationSend: ReturnType grabAnnotations: ReturnType browserOverlayViewport: BrowserOverlayViewport + handleAnnotationViewportMessage: (event: { message?: string }) => void + resetBrowserOverlayViewport: () => void elementTools: BrowserChromeElementTools } { // Why still empty before the first grant: the page is only a tool target once a document is on @@ -48,6 +51,19 @@ export function useDocPreviewGuestTools({ scrollY: 0, version: 0 }) + const handleAnnotationViewportMessage = useCallback( + (event: { message?: string }): void => + consumeBrowserAnnotationViewportMessage( + event, + annotationViewportBridgeTokenRef.current, + setBrowserOverlayViewport + ), + [] + ) + const resetBrowserOverlayViewport = useCallback( + (): void => setBrowserOverlayViewport({ scrollX: 0, scrollY: 0, version: 0 }), + [] + ) const grabElementShortcut = useShortcutLabel('browser.grabElement') const grab = useGrabMode(toolTargetId) @@ -100,5 +116,14 @@ export function useDocPreviewGuestTools({ ] ) - return { grab, markup, annotationSend, grabAnnotations, browserOverlayViewport, elementTools } + return { + grab, + markup, + annotationSend, + grabAnnotations, + browserOverlayViewport, + handleAnnotationViewportMessage, + resetBrowserOverlayViewport, + elementTools + } } diff --git a/src/shared/browser-annotation-viewport-bridge.ts b/src/shared/browser-annotation-viewport-bridge.ts index c9846eb3116..a1356f8647a 100644 --- a/src/shared/browser-annotation-viewport-bridge.ts +++ b/src/shared/browser-annotation-viewport-bridge.ts @@ -1,4 +1,5 @@ import type { BrowserGrabRect } from './browser-grab-types' +import { isValidBrowserGrabRect } from './browser-annotation-viewport-report' export type BrowserAnnotationViewportBridgeMarker = { id: string @@ -13,6 +14,7 @@ export type BrowserSetAnnotationViewportBridgeArgs = { enabled: boolean emitViewport: boolean markers: BrowserAnnotationViewportBridgeMarker[] + selectionId?: string token: string } @@ -20,35 +22,19 @@ export type BrowserAnnotationViewportBridgeOptions = { enabled: boolean emitViewport: boolean markers: BrowserAnnotationViewportBridgeMarker[] + selectionId?: string token: string } export const BROWSER_ANNOTATION_VIEWPORT_BRIDGE_WORLD_ID = 1207 export const BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX = '__orca_annotation_viewport__:' +// Set by the grab overlay on the element it froze on, so the isolated bridge can measure it. +export const BROWSER_GRAB_SELECTED_ATTRIBUTE = 'data-orca-grab-selected' export function isValidBrowserAnnotationViewportBridgeToken(value: unknown): value is string { return typeof value === 'string' && /^[a-zA-Z0-9_-]{16,80}$/.test(value) } -function isValidBrowserGrabRect(value: unknown): value is BrowserGrabRect { - if (!value || typeof value !== 'object') { - return false - } - const rect = value as Record - return ( - typeof rect.x === 'number' && - Number.isFinite(rect.x) && - typeof rect.y === 'number' && - Number.isFinite(rect.y) && - typeof rect.width === 'number' && - Number.isFinite(rect.width) && - rect.width >= 0 && - typeof rect.height === 'number' && - Number.isFinite(rect.height) && - rect.height >= 0 - ) -} - export function isValidBrowserAnnotationViewportBridgeMarkers( value: unknown ): value is BrowserAnnotationViewportBridgeMarker[] { @@ -81,6 +67,7 @@ export function buildBrowserAnnotationViewportBridgeScript({ emitViewport, enabled, markers, + selectionId, token }: BrowserAnnotationViewportBridgeOptions): string { return `(() => { @@ -89,10 +76,12 @@ export function buildBrowserAnnotationViewportBridgeScript({ const enabled = ${JSON.stringify(enabled)}; const emitViewportMessages = ${JSON.stringify(emitViewport)}; const markers = ${JSON.stringify(markers)}; + const selectionId = ${JSON.stringify(selectionId ?? null)}; const token = ${JSON.stringify(token)}; const prefix = ${JSON.stringify(BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX)}; const stateKey = '__orcaBrowserAnnotationViewportBridge'; const hostAttribute = 'data-orca-browser-annotation-overlay'; + const selectedAttribute = ${JSON.stringify(BROWSER_GRAB_SELECTED_ATTRIBUTE)}; const markerSize = 24; const removeOverlay = (state) => { @@ -114,6 +103,9 @@ export function buildBrowserAnnotationViewportBridgeScript({ if (state.onBeforeUnload) { window.removeEventListener('beforeunload', state.onBeforeUnload); } + if (state.targetObserver) { + state.targetObserver.disconnect(); + } removeOverlay(state); }; @@ -132,10 +124,50 @@ export function buildBrowserAnnotationViewportBridgeScript({ scrollY: toNumber(window.scrollY, toNumber(window.pageYOffset, 0)) }); - const emitViewport = () => { + // Why: the card must follow the element itself, not a click-time copy; the grab overlay + // marks it in the shared DOM because this world cannot see the page's objects. + const trackTarget = (state) => { + const current = state.target; + let next = null; + if (emitViewportMessages && selectionId) { + for (const candidate of document.querySelectorAll('[' + selectedAttribute + ']')) { + if (candidate.getAttribute(selectedAttribute) !== selectionId) continue; + if (next) { + next = null; + break; + } + next = candidate; + } + } + if (next !== current) { + state.target = next; + if (state.targetObserver) { + state.targetObserver.disconnect(); + if (next) { + state.targetObserver.observe(next, { box: 'border-box' }); + if (next !== document.documentElement) { + state.targetObserver.observe(document.documentElement); + } + } + } + } + return next; + }; + + const readTarget = (target) => { + if (!target) return null; + const rect = target.getBoundingClientRect(); + return { x: rect.x, y: rect.y, width: rect.width, height: rect.height }; + }; + + const emitViewport = (state) => { + const target = trackTarget(state); if (!emitViewportMessages) return; try { - console.debug(prefix + token + ':' + JSON.stringify(readViewport())); + const report = readViewport(); + report.viewportWidth = toNumber(window.innerWidth, 0); + report.target = readTarget(target); + console.debug(prefix + token + ':' + JSON.stringify(report)); } catch (e) {} }; @@ -238,7 +270,9 @@ export function buildBrowserAnnotationViewportBridgeScript({ markers: [], shadowRoot: null, requestUpdate: null, - onBeforeUnload: null + onBeforeUnload: null, + target: null, + targetObserver: null }; state.requestUpdate = () => { @@ -246,9 +280,12 @@ export function buildBrowserAnnotationViewportBridgeScript({ state.raf = requestAnimationFrame(() => { state.raf = 0; positionMarkers(state); - state.emitViewport(); + state.emitViewport(state); }); }; + if (typeof ResizeObserver === 'function') { + state.targetObserver = new ResizeObserver(state.requestUpdate); + } // Navigation can hold IPC cleanup until the next document finishes loading. state.onBeforeUnload = () => { diff --git a/src/shared/browser-annotation-viewport-live-target.test.ts b/src/shared/browser-annotation-viewport-live-target.test.ts new file mode 100644 index 00000000000..94e58f9b9ff --- /dev/null +++ b/src/shared/browser-annotation-viewport-live-target.test.ts @@ -0,0 +1,296 @@ +import { runInNewContext } from 'node:vm' +import { afterEach, describe, expect, it } from 'vitest' +import { + BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX, + BROWSER_GRAB_SELECTED_ATTRIBUTE, + buildBrowserAnnotationViewportBridgeScript +} from './browser-annotation-viewport-bridge' +import { + parseBrowserAnnotationViewportReport, + type BrowserAnnotationViewportReport +} from './browser-annotation-viewport-report' + +type Rect = { x: number; y: number; width: number; height: number } + +const TOKEN = 'live-target-test-token' +const guestAborts: (() => Promise)[] = [] + +afterEach(async () => { + await Promise.all(guestAborts.splice(0).map((abort) => abort())) +}) + +async function createGuest(targetIsRoot = false) { + const { Window } = await import('happy-dom') + const window = new Window({ width: 1000, height: 800 }) + guestAborts.push(() => window.happyDOM.abort()) + const document = window.document + const scroller = document.createElement('div') + const target = targetIsRoot ? document.documentElement : document.createElement('section') + if (!targetIsRoot) { + scroller.appendChild(target) + document.body.appendChild(scroller) + } + let targetRect: Rect = { x: 40, y: 300, width: 120, height: 30 } + const contentSize = { width: 120, height: 30 } + const rootSize = { width: 1000, height: 800 } + const spacing = { padding: 0, border: 0 } + target.getBoundingClientRect = () => + new window.DOMRect(targetRect.x, targetRect.y, targetRect.width, targetRect.height) + + const frames: (() => void)[] = [] + const resizeCallbacks: (() => void)[] = [] + const observers: { + callback: () => void + boxes: Map + }[] = [] + const measure = (element: unknown, box: 'content-box' | 'border-box') => { + if (element === target) { + const spacingSize = box === 'border-box' ? 2 * (spacing.padding + spacing.border) : 0 + return { width: contentSize.width + spacingSize, height: contentSize.height + spacingSize } + } + return { ...rootSize } + } + const reports: BrowserAnnotationViewportReport[] = [] + const prefix = `${BROWSER_ANNOTATION_VIEWPORT_MESSAGE_PREFIX}${TOKEN}:` + const context = { + window, + document, + requestAnimationFrame: (callback: () => void) => frames.push(callback), + cancelAnimationFrame: () => {}, + ResizeObserver: class { + private boxes = new Map< + unknown, + { box: 'content-box' | 'border-box'; width: number; height: number } + >() + constructor(callback: () => void) { + resizeCallbacks.push(callback) + observers.push({ callback, boxes: this.boxes }) + } + observe(element: unknown, options?: { box?: 'content-box' | 'border-box' }): void { + const box = options?.box ?? 'content-box' + this.boxes.set(element, { box, ...measure(element, box) }) + } + disconnect(): void { + this.boxes.clear() + } + }, + console: { + debug: (message: string) => { + const report = parseBrowserAnnotationViewportReport(message.slice(prefix.length)) + if (message.startsWith(prefix) && report) { + reports.push(report) + } + } + } + } + const install = (emitViewport: boolean, selectionId = 'selection-1'): void => { + runInNewContext( + buildBrowserAnnotationViewportBridgeScript({ + emitViewport, + enabled: true, + token: TOKEN, + markers: [], + selectionId + }), + context + ) + } + const flushFrames = (): void => { + for (const frame of frames.splice(0)) { + frame() + } + } + return { + window, + scroller, + target, + install, + flushFrames, + resizeCallbacks, + notifyResizes: () => { + let notifications = 0 + for (const observer of observers) { + let changed = false + for (const [element, previous] of observer.boxes) { + const current = measure(element, previous.box) + if (current.width !== previous.width || current.height !== previous.height) { + observer.boxes.set(element, { box: previous.box, ...current }) + changed = true + } + } + if (changed) { + observer.callback() + notifications++ + } + } + return notifications + }, + setTargetSpacing: (padding: number, border: number) => { + spacing.padding = padding + spacing.border = border + targetRect = { + ...targetRect, + width: contentSize.width + 2 * (padding + border), + height: contentSize.height + 2 * (padding + border) + } + }, + setRootContentSize: (width: number, height: number) => { + rootSize.width = width + rootSize.height = height + }, + lastReport: () => reports.at(-1), + moveTarget: (rect: Rect) => { + targetRect = rect + } + } +} + +describe('annotation viewport bridge live target', () => { + it.each([ + ['border', 0, 8], + ['padding', 8, 0] + ])('reports %s-only outer growth with a fixed content box', async (_kind, padding, border) => { + const guest = await createGuest() + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + guest.install(true) + guest.flushFrames() + guest.setTargetSpacing(padding, border) + + expect(guest.notifyResizes()).toBe(1) + guest.flushFrames() + + expect(guest.lastReport()?.target).toEqual({ x: 40, y: 300, width: 136, height: 46 }) + }) + + it('keeps a selected document root on outer-box observation', async () => { + const guest = await createGuest(true) + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + guest.install(true) + guest.flushFrames() + guest.setTargetSpacing(0, 8) + + expect(guest.notifyResizes()).toBe(1) + guest.flushFrames() + + expect(guest.lastReport()?.target).toEqual({ x: 40, y: 300, width: 136, height: 46 }) + }) + + it('re-reports when the document root content box changes', async () => { + const guest = await createGuest() + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + guest.install(true) + guest.flushFrames() + guest.moveTarget({ x: 40, y: 250, width: 120, height: 30 }) + guest.setRootContentSize(980, 800) + + expect(guest.notifyResizes()).toBe(1) + guest.flushFrames() + + expect(guest.lastReport()?.target).toEqual({ x: 40, y: 250, width: 120, height: 30 }) + }) + + it('reports the selected element measured live, including scroll inside a box', async () => { + const guest = await createGuest() + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + guest.install(true) + guest.flushFrames() + expect(guest.lastReport()).toEqual({ + scrollX: 0, + scrollY: 0, + viewportWidth: 1000, + target: { x: 40, y: 300, width: 120, height: 30 } + }) + + // A box scrolling inside the page leaves window scroll unchanged. + guest.moveTarget({ x: 40, y: 120, width: 120, height: 30 }) + guest.scroller.dispatchEvent(new guest.window.Event('scroll')) + guest.flushFrames() + expect(guest.lastReport()?.target).toEqual({ x: 40, y: 120, width: 120, height: 30 }) + }) + + it('ignores an earlier page-owned selection marker', async () => { + const guest = await createGuest() + const decoy = guest.window.document.createElement('div') + decoy.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'page-owned') + decoy.getBoundingClientRect = () => new guest.window.DOMRect(600, 400, 40, 20) + guest.window.document.body.insertBefore(decoy, guest.scroller) + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + + guest.install(true) + guest.flushFrames() + + expect(guest.lastReport()?.target).toEqual({ x: 40, y: 300, width: 120, height: 30 }) + }) + + it('switches to a new selection when the bridge is reused', async () => { + const guest = await createGuest() + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + guest.install(true) + guest.flushFrames() + + const next = guest.window.document.createElement('button') + next.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-2') + next.getBoundingClientRect = () => new guest.window.DOMRect(200, 100, 80, 20) + guest.window.document.body.appendChild(next) + guest.install(true, 'selection-2') + guest.flushFrames() + + expect(guest.lastReport()?.target).toEqual({ x: 200, y: 100, width: 80, height: 20 }) + }) + + it('does not report a target when two elements copy the same selection value', async () => { + const guest = await createGuest() + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + const decoy = guest.window.document.createElement('div') + decoy.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + guest.window.document.body.insertBefore(decoy, guest.scroller) + + guest.install(true) + guest.flushFrames() + + expect(guest.lastReport()?.target).toBeUndefined() + }) + + it('re-reports on resize and when the element changes size', async () => { + const guest = await createGuest() + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + guest.install(true) + guest.flushFrames() + + guest.moveTarget({ x: 10, y: 200, width: 80, height: 30 }) + guest.window.dispatchEvent(new guest.window.Event('resize')) + guest.flushFrames() + expect(guest.lastReport()?.target).toEqual({ x: 10, y: 200, width: 80, height: 30 }) + + guest.moveTarget({ x: 10, y: 200, width: 80, height: 90 }) + for (const callback of guest.resizeCallbacks) { + callback() + } + guest.flushFrames() + expect(guest.lastReport()?.target).toEqual({ x: 10, y: 200, width: 80, height: 90 }) + }) + + it('reports no target once the grab overlay has released the element', async () => { + const guest = await createGuest() + guest.target.setAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE, 'selection-1') + guest.install(true) + guest.flushFrames() + + guest.target.removeAttribute(BROWSER_GRAB_SELECTED_ATTRIBUTE) + guest.window.dispatchEvent(new guest.window.Event('scroll')) + guest.flushFrames() + expect(guest.lastReport()?.target).toBeUndefined() + }) +}) + +describe('parseBrowserAnnotationViewportReport', () => { + it('keeps only well-formed numbers and boxes', () => { + expect( + parseBrowserAnnotationViewportReport( + JSON.stringify({ scrollX: 'x', scrollY: 4, viewportWidth: -1, target: { x: 1 } }) + ) + ).toEqual({ scrollX: 0, scrollY: 4 }) + expect(parseBrowserAnnotationViewportReport('not json')).toBeNull() + expect(parseBrowserAnnotationViewportReport('7')).toBeNull() + }) +}) diff --git a/src/shared/browser-annotation-viewport-report.ts b/src/shared/browser-annotation-viewport-report.ts new file mode 100644 index 00000000000..25906ca4c34 --- /dev/null +++ b/src/shared/browser-annotation-viewport-report.ts @@ -0,0 +1,54 @@ +import type { BrowserGrabRect } from './browser-grab-types' + +/** What the bridge reports while a pending annotation card is open, in guest CSS pixels. */ +export type BrowserAnnotationViewportReport = { + scrollX: number + scrollY: number + /** Guest `innerWidth`; with the guest view's on-screen width it gives the page zoom scale. */ + viewportWidth?: number + /** Live box of the selected element; absent when the guest no longer has it. */ + target?: BrowserGrabRect +} + +export function isValidBrowserGrabRect(value: unknown): value is BrowserGrabRect { + if (!value || typeof value !== 'object') { + return false + } + const rect = value as Record + return ( + typeof rect.x === 'number' && + Number.isFinite(rect.x) && + typeof rect.y === 'number' && + Number.isFinite(rect.y) && + typeof rect.width === 'number' && + Number.isFinite(rect.width) && + rect.width >= 0 && + typeof rect.height === 'number' && + Number.isFinite(rect.height) && + rect.height >= 0 + ) +} + +const finiteOr = (value: unknown, fallback: number): number => + typeof value === 'number' && Number.isFinite(value) ? value : fallback + +export function parseBrowserAnnotationViewportReport( + json: string +): BrowserAnnotationViewportReport | null { + let next: Partial> | null + try { + next = JSON.parse(json) + } catch { + return null + } + if (!next || typeof next !== 'object') { + return null + } + const viewportWidth = finiteOr(next.viewportWidth, 0) + return { + scrollX: finiteOr(next.scrollX, 0), + scrollY: finiteOr(next.scrollY, 0), + ...(viewportWidth > 0 ? { viewportWidth } : {}), + ...(isValidBrowserGrabRect(next.target) ? { target: next.target } : {}) + } +}