diff --git a/src/cursor.ts b/src/cursor.ts index 7214e92..7fd191c 100644 --- a/src/cursor.ts +++ b/src/cursor.ts @@ -17,13 +17,13 @@ export interface CursorHighlightOptions { } /** - * Enables a persistent cursor highlight that follows the mouse pointer. - * The highlight remains active until `resetCursor(page)` is called. - * Calling again replaces the existing highlight. + * Shared implementation for `cursorHighlight` (always replaces) and + * `ensureCursorHighlight` (no-op when a ring is already installed). */ -export async function cursorHighlight( +async function applyCursorHighlight( page: Page, - opts?: CursorHighlightOptions, + opts: CursorHighlightOptions | undefined, + skipIfPresent: boolean, ): Promise { const color = opts?.color ?? '#3b82f6'; const radius = opts?.radius ?? 20; @@ -33,82 +33,117 @@ export async function cursorHighlight( try { await page.evaluate( - ({ color, radius, pulse, clickRipple, opacity, attr, id }) => { - // Remove existing highlight - document.getElementById(id)?.remove(); - document.querySelectorAll(`[${attr}]`).forEach(el => el.remove()); - - // Inject keyframe styles - const style = document.createElement('style'); - style.setAttribute(attr, 'style'); - style.textContent = ` - @keyframes argo-cursor-pulse { - 0%, 100% { box-shadow: 0 0 0 2px ${color}${Math.round(opacity * 255).toString(16).padStart(2, '0')}, 0 0 ${radius * 0.6}px ${color}33; } - 50% { box-shadow: 0 0 0 3px ${color}${Math.round(opacity * 255 * 0.8).toString(16).padStart(2, '0')}, 0 0 ${radius}px ${color}55; } - } - @keyframes argo-cursor-ripple { - 0% { transform: translate(-50%, -50%) scale(1); opacity: ${opacity}; } - 100% { transform: translate(-50%, -50%) scale(3); opacity: 0; } + ({ color, radius, pulse, clickRipple, opacity, attr, id, skipIfPresent }) => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const w = window as any; + + // Idempotent path: the caller only wants a ring to exist, and one is + // already installed (or queued behind DOMContentLoaded). Bail before + // touching the DOM so same-document (SPA) navigations don't tear down + // and rebuild a perfectly good overlay. + if (skipIfPresent && (document.getElementById(id) || w.__argoCursorPending)) return; + + // Generation counter: a later call supersedes any install still + // waiting on DOMContentLoaded, so we never stack two rings. + const gen = (w.__argoCursorGen = (w.__argoCursorGen || 0) + 1); + + const install = (): void => { + if (w.__argoCursorGen !== gen) return; // superseded + w.__argoCursorPending = false; + + // Remove existing highlight. Invoke its stored cleanup first — + // otherwise the previous document-level mousemove/click listeners + // survive as leaks bound to a detached node. + const previous = document.getElementById(id); + if (previous) { + try { (previous as any).__cleanup?.(); } catch { /* best-effort */ } + previous.remove(); } - `; - document.head.appendChild(style); - - // Create highlight element - const dot = document.createElement('div'); - dot.id = id; - dot.setAttribute(attr, 'highlight'); - dot.style.cssText = ` - position: fixed; z-index: 99998; pointer-events: none; - width: ${radius * 2}px; height: ${radius * 2}px; - border-radius: 50%; - border: 2px solid ${color}; - opacity: ${opacity}; - transform: translate(-50%, -50%); - left: -100px; top: -100px; - transition: left 0.05s ease-out, top 0.05s ease-out; - ${pulse ? `animation: argo-cursor-pulse 1.5s ease-in-out infinite;` : `box-shadow: 0 0 0 2px ${color}${Math.round(opacity * 255).toString(16).padStart(2, '0')}, 0 0 ${radius * 0.6}px ${color}33;`} - `; - document.body.appendChild(dot); - - // Track mouse movement - const onMove = (e: MouseEvent) => { - dot.style.left = e.clientX + 'px'; - dot.style.top = e.clientY + 'px'; - }; - document.addEventListener('mousemove', onMove, true); + document.querySelectorAll(`[${attr}]`).forEach(el => el.remove()); - // Store cleanup reference - (dot as any).__cleanup = () => { - document.removeEventListener('mousemove', onMove, true); - }; + // Inject keyframe styles + const style = document.createElement('style'); + style.setAttribute(attr, 'style'); + style.textContent = ` + @keyframes argo-cursor-pulse { + 0%, 100% { box-shadow: 0 0 0 2px ${color}${Math.round(opacity * 255).toString(16).padStart(2, '0')}, 0 0 ${radius * 0.6}px ${color}33; } + 50% { box-shadow: 0 0 0 3px ${color}${Math.round(opacity * 255 * 0.8).toString(16).padStart(2, '0')}, 0 0 ${radius}px ${color}55; } + } + @keyframes argo-cursor-ripple { + 0% { transform: translate(-50%, -50%) scale(1); opacity: ${opacity}; } + 100% { transform: translate(-50%, -50%) scale(3); opacity: 0; } + } + `; + (document.head ?? document.documentElement).appendChild(style); + + // Create highlight element + const dot = document.createElement('div'); + dot.id = id; + dot.setAttribute(attr, 'highlight'); + dot.style.cssText = ` + position: fixed; z-index: 99998; pointer-events: none; + width: ${radius * 2}px; height: ${radius * 2}px; + border-radius: 50%; + border: 2px solid ${color}; + opacity: ${opacity}; + transform: translate(-50%, -50%); + left: -100px; top: -100px; + transition: left 0.05s ease-out, top 0.05s ease-out; + ${pulse ? `animation: argo-cursor-pulse 1.5s ease-in-out infinite;` : `box-shadow: 0 0 0 2px ${color}${Math.round(opacity * 255).toString(16).padStart(2, '0')}, 0 0 ${radius * 0.6}px ${color}33;`} + `; + document.body.appendChild(dot); - // Click ripple effect - if (clickRipple) { - const onClick = (e: MouseEvent) => { - const ripple = document.createElement('div'); - ripple.setAttribute(attr, 'ripple'); - ripple.style.cssText = ` - position: fixed; z-index: 99997; pointer-events: none; - width: ${radius * 2}px; height: ${radius * 2}px; - border-radius: 50%; - border: 2px solid ${color}; - left: ${e.clientX}px; top: ${e.clientY}px; - transform: translate(-50%, -50%); - animation: argo-cursor-ripple 0.4s ease-out forwards; - `; - document.body.appendChild(ripple); - setTimeout(() => ripple.remove(), 400); + // Track mouse movement + const onMove = (e: MouseEvent) => { + dot.style.left = e.clientX + 'px'; + dot.style.top = e.clientY + 'px'; }; - document.addEventListener('click', onClick, true); + document.addEventListener('mousemove', onMove, true); - const origCleanup = (dot as any).__cleanup; + // Store cleanup reference (dot as any).__cleanup = () => { - origCleanup(); - document.removeEventListener('click', onClick, true); + document.removeEventListener('mousemove', onMove, true); }; + + // Click ripple effect + if (clickRipple) { + const onClick = (e: MouseEvent) => { + const ripple = document.createElement('div'); + ripple.setAttribute(attr, 'ripple'); + ripple.style.cssText = ` + position: fixed; z-index: 99997; pointer-events: none; + width: ${radius * 2}px; height: ${radius * 2}px; + border-radius: 50%; + border: 2px solid ${color}; + left: ${e.clientX}px; top: ${e.clientY}px; + transform: translate(-50%, -50%); + animation: argo-cursor-ripple 0.4s ease-out forwards; + `; + document.body.appendChild(ripple); + setTimeout(() => ripple.remove(), 400); + }; + document.addEventListener('click', onClick, true); + + const origCleanup = (dot as any).__cleanup; + (dot as any).__cleanup = () => { + origCleanup(); + document.removeEventListener('click', onClick, true); + }; + } + }; + + // `framenavigated` fires at navigation commit — the parser may not have + // produced yet, and `document.body.appendChild` would throw a + // TypeError that isn't a disposal error (so it would surface as a + // warning and leave the page permanently cursor-less). Defer instead. + if (document.body) { + install(); + } else { + w.__argoCursorPending = true; + document.addEventListener('DOMContentLoaded', install, { once: true }); } }, - { color, radius, pulse, clickRipple, opacity, attr: CURSOR_ATTR, id: CURSOR_ID }, + { color, radius, pulse, clickRipple, opacity, attr: CURSOR_ATTR, id: CURSOR_ID, skipIfPresent }, ); } catch (err) { const msg = (err as Error)?.message ?? ''; @@ -118,12 +153,46 @@ export async function cursorHighlight( } } +/** + * Enables a persistent cursor highlight that follows the mouse pointer. + * The highlight remains active until `resetCursor(page)` is called. + * Calling again replaces the existing highlight. + */ +export async function cursorHighlight( + page: Page, + opts?: CursorHighlightOptions, +): Promise { + return applyCursorHighlight(page, opts, false); +} + +/** + * Installs the cursor highlight only when one isn't already present. + * + * Used by the automatic (`video.cursorHighlight`) path after a navigation: + * Playwright emits `framenavigated` for same-document history navigations too, + * so an unconditional reinstall would rebuild the ring on every SPA route + * change — resetting it off-screen until the next mouse event and leaking the + * previous document listeners. + */ +export async function ensureCursorHighlight( + page: Page, + opts?: CursorHighlightOptions, +): Promise { + return applyCursorHighlight(page, opts, true); +} + /** * Removes the cursor highlight and all related elements. */ export async function resetCursor(page: Page): Promise { try { await page.evaluate(({ attr, id }) => { + // Invalidate any install still queued behind DOMContentLoaded. + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const w = window as any; + w.__argoCursorGen = (w.__argoCursorGen || 0) + 1; + w.__argoCursorPending = false; + const dot = document.getElementById(id); if (dot && (dot as any).__cleanup) { (dot as any).__cleanup(); diff --git a/src/narration.ts b/src/narration.ts index 28408a0..c4eeec4 100644 --- a/src/narration.ts +++ b/src/narration.ts @@ -6,7 +6,7 @@ import type { Writable } from 'node:stream'; import { schedulePlacements, type Placement } from './tts/align.js'; import type { CameraMove } from './camera-move.js'; import { startCdpScreencast, type CdpScreencastHandle } from './cdp-screencast.js'; -import { cursorHighlight, type CursorHighlightOptions } from './cursor.js'; +import { cursorHighlight, ensureCursorHighlight, type CursorHighlightOptions } from './cursor.js'; /** * Subset of Playwright's Page we depend on — typed structurally so we don't @@ -319,22 +319,36 @@ export class NarrationTimeline { } // close legacy `else` branch — showActions + timeline anchor below run for both paths. + // Set for BOTH capture paths (CDP-direct and legacy). This also enables + // `_triggerPaint()` from `mark()` on the chromium + jpeg-stitch path, where + // it was previously a silent no-op because `_recordingPage` stayed null. this._recordingPage = page; - // Reinstall the pseudo-cursor after a top-level navigation because the - // browser replaces the document (and its overlay/listeners). Waiting for - // the injection before nudging paint keeps the first post-navigation frame - // from briefly appearing without the cursor overlay. + // CDP screencast only emits frames on paint. After page.goto() lands, + // Chrome can stay paused for the inter-paint window — gap-fill repeats + // the last (pre-nav) JPEG and the new page doesn't appear in the video + // until the next natural paint (could be seconds on heavy SPAs). Force + // a paint right after navigation so CDP unsticks promptly. + // + // When the automatic pseudo-cursor is on we also reinstall it, because a + // cross-document navigation replaces the document (and its overlay and + // listeners). `ensureCursorHighlight` no-ops when a ring is already there, + // which matters because Playwright emits `framenavigated` for + // same-document history navigations too — an unconditional reinstall would + // rebuild the ring on every SPA route change, resetting it off-screen until + // the next mouse event. The paint nudge deliberately does NOT wait on the + // injection (that would defer the very unsticking this listener exists for, + // since the evaluate blocks on the new document's execution context); we + // nudge again once the cursor lands so it shows up promptly either way. const navListener = (frame: { parentFrame: () => unknown }): void => { if (frame.parentFrame() !== null) return; // ignore subframe navs if (automaticCursor) { - void cursorHighlight( - page as unknown as Parameters[0], + void ensureCursorHighlight( + page as unknown as Parameters[0], automaticCursor, - ).then(() => this._triggerPaint()); - } else { - this._triggerPaint(); + ).finally(() => this._triggerPaint()); } + this._triggerPaint(); }; this._navListener = navListener; page.on('framenavigated', navListener); diff --git a/tests/cursor.test.ts b/tests/cursor.test.ts index 2e0a9c5..8404858 100644 --- a/tests/cursor.test.ts +++ b/tests/cursor.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { cursorHighlight, resetCursor } from '../src/cursor.js'; +import { cursorHighlight, ensureCursorHighlight, resetCursor } from '../src/cursor.js'; import type { Page } from '@playwright/test'; function createMockPage() { @@ -85,3 +85,176 @@ describe('resetCursor', () => { await expect(resetCursor(page)).resolves.toBeUndefined(); }); }); + +// --------------------------------------------------------------------------- +// Browser-side behaviour. The function handed to `page.evaluate` is captured +// from the mock and executed against a minimal DOM stub, so the injection +// logic itself (not just its arguments) is under test. +// --------------------------------------------------------------------------- + +class FakeEl { + id = ''; + textContent = ''; + style: any = { cssText: '' }; + children: FakeEl[] = []; + parent: FakeEl | null = null; + private attrs = new Map(); + constructor(public tagName: string) {} + setAttribute(k: string, v: string) { this.attrs.set(k, v); } + hasAttribute(k: string) { return this.attrs.has(k); } + appendChild(c: FakeEl) { c.parent = this; this.children.push(c); return c; } + remove() { + if (!this.parent) return; + this.parent.children = this.parent.children.filter((x) => x !== this); + this.parent = null; + } +} + +function createFakeDom({ withBody }: { withBody: boolean }) { + const documentElement = new FakeEl('html'); + const head = new FakeEl('head'); + documentElement.appendChild(head); + let body: FakeEl | null = null; + if (withBody) body = documentElement.appendChild(new FakeEl('body')); + + const listeners: Array<{ type: string; fn: any }> = []; + const walk = (el: FakeEl): FakeEl[] => [el, ...el.children.flatMap(walk)]; + const all = () => walk(documentElement); + + const document: any = { + documentElement, + head, + get body() { return body; }, + createElement: (tag: string) => new FakeEl(tag), + getElementById: (id: string) => all().find((e) => e.id === id) ?? null, + querySelectorAll: (sel: string) => { + const attr = sel.replace(/^\[|\]$/g, ''); + return all().filter((e) => e.hasAttribute(attr)); + }, + addEventListener: (type: string, fn: any) => { listeners.push({ type, fn }); }, + removeEventListener: (type: string, fn: any) => { + const i = listeners.findIndex((l) => l.type === type && l.fn === fn); + if (i >= 0) listeners.splice(i, 1); + }, + }; + + // Deferred installs (DOMContentLoaded) run outside the evaluate call, so + // every entry point that can execute page code installs the globals. + let win: any = {}; + const withGlobals = (cb: () => T): T => { + const prevDoc = (globalThis as any).document; + const prevWin = (globalThis as any).window; + (globalThis as any).document = document; + (globalThis as any).window = win; + try { return cb(); } finally { + (globalThis as any).document = prevDoc; + (globalThis as any).window = prevWin; + } + }; + + return { + document, + listeners, + withGlobals, + setWindow: (w: any) => { win = w; }, + attachBody: () => { body = documentElement.appendChild(new FakeEl('body')); }, + fire: (type: string, ev?: any) => withGlobals(() => { + for (const l of [...listeners].filter((l) => l.type === type)) l.fn(ev); + }), + ring: () => document.getElementById('argo-cursor-highlight') as FakeEl | null, + }; +} + +async function runInjection( + fn: (page: Page, opts?: any) => Promise, + dom: ReturnType, + win: any, + opts?: any, +) { + const page = createMockPage(); + await fn(page, opts); + const [browserFn, args] = (page.evaluate as any).mock.calls[0]; + dom.setWindow(win); + dom.withGlobals(() => browserFn(args)); +} + +describe('cursor injection (browser side)', () => { + it('installs the ring immediately when document.body exists', async () => { + const dom = createFakeDom({ withBody: true }); + await runInjection(cursorHighlight, dom, {}); + expect(dom.ring()).not.toBeNull(); + expect(dom.ring()!.parent!.tagName).toBe('body'); + }); + + it('defers installation until DOMContentLoaded when body is not parsed yet', async () => { + // `framenavigated` fires at navigation commit — the parser may not have + // produced . The old code called document.body.appendChild and threw. + const dom = createFakeDom({ withBody: false }); + const win: any = {}; + + await expect(runInjection(cursorHighlight, dom, win)).resolves.toBeUndefined(); + expect(dom.ring()).toBeNull(); + expect(win.__argoCursorPending).toBe(true); + expect(dom.listeners.some((l) => l.type === 'DOMContentLoaded')).toBe(true); + + dom.attachBody(); + dom.fire('DOMContentLoaded'); + expect(dom.ring()).not.toBeNull(); + expect(win.__argoCursorPending).toBe(false); + }); + + it('ensureCursorHighlight is a no-op when a ring is already installed', async () => { + const dom = createFakeDom({ withBody: true }); + const win: any = {}; + await runInjection(cursorHighlight, dom, win); + const first = dom.ring(); + const genAfterFirst = win.__argoCursorGen; + + await runInjection(ensureCursorHighlight, dom, win); + expect(dom.ring()).toBe(first); // same node, not rebuilt + expect(win.__argoCursorGen).toBe(genAfterFirst); + }); + + it('ensureCursorHighlight is a no-op while an install is queued', async () => { + const dom = createFakeDom({ withBody: false }); + const win: any = {}; + await runInjection(cursorHighlight, dom, win); + const queued = dom.listeners.filter((l) => l.type === 'DOMContentLoaded').length; + + await runInjection(ensureCursorHighlight, dom, win); + expect(dom.listeners.filter((l) => l.type === 'DOMContentLoaded')).toHaveLength(queued); + }); + + it('ensureCursorHighlight installs when no ring is present', async () => { + const dom = createFakeDom({ withBody: true }); + await runInjection(ensureCursorHighlight, dom, {}); + expect(dom.ring()).not.toBeNull(); + }); + + it('replacing a ring runs the previous cleanup so listeners do not leak', async () => { + const dom = createFakeDom({ withBody: true }); + const win: any = {}; + await runInjection(cursorHighlight, dom, win); + const before = dom.listeners.filter((l) => l.type === 'mousemove' || l.type === 'click').length; + expect(before).toBe(2); + + // cursorHighlight() explicitly replaces — the old document listeners must go. + await runInjection(cursorHighlight, dom, win); + const after = dom.listeners.filter((l) => l.type === 'mousemove' || l.type === 'click').length; + expect(after).toBe(2); + }); + + it('a superseded deferred install does not stack a second ring', async () => { + const dom = createFakeDom({ withBody: false }); + const win: any = {}; + await runInjection(cursorHighlight, dom, win); + await runInjection(cursorHighlight, dom, win); + + dom.attachBody(); + dom.fire('DOMContentLoaded'); + + const rings = dom.document.querySelectorAll('[data-argo-cursor]') + .filter((e: FakeEl) => e.id === 'argo-cursor-highlight'); + expect(rings).toHaveLength(1); + }); +}); diff --git a/tests/narration.test.ts b/tests/narration.test.ts index 25be0a4..1832b6c 100644 --- a/tests/narration.test.ts +++ b/tests/narration.test.ts @@ -303,6 +303,112 @@ describe('automatic cursor highlight', () => { expect(page.screencast.stop).toHaveBeenCalledTimes(1); expect(page.off).toHaveBeenCalledWith('framenavigated', navigationListener); }); + + it('nudges paint after navigation without waiting on the cursor injection', async () => { + // The framenavigated listener exists to unstick CDP after a navigation. + // The cursor evaluate blocks on the new document's execution context, so + // the paint nudge must not be chained behind it. + process.env.ARGO_SCREENCAST_PATH = 'cursor-paint.webm'; + process.env.ARGO_CURSOR_HIGHLIGHT = '{}'; + delete process.env.ARGO_USE_CDP_DIRECT; + delete process.env.ARGO_STREAM_OUT; + + let navigationListener: ((frame: { parentFrame: () => unknown }) => void) | undefined; + let cursorCallCount = 0; + const evaluate = vi.fn((_fn: unknown, arg?: any) => { + if (arg?.id === 'argo-cursor-highlight') { + cursorCallCount++; + // Let the initial install resolve; leave the post-nav one pending. + return cursorCallCount === 1 ? Promise.resolve() : new Promise(() => {}); + } + return Promise.resolve(); + }); + const page = { + screencast: { + start: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + showActions: vi.fn().mockResolvedValue(undefined), + }, + evaluate, + on: vi.fn((_event, listener) => { navigationListener = listener; }), + off: vi.fn(), + context: vi.fn(), + }; + + const timeline = new NarrationTimeline(); + await timeline.startRecording(page as never); + + const paintCalls = () => evaluate.mock.calls.filter((call) => call.length === 1); + const before = paintCalls().length; + + navigationListener?.({ parentFrame: () => null }); + + // Paint nudge fired even though the cursor evaluate never settles. + expect(paintCalls().length).toBe(before + 1); + expect(cursorCallCount).toBe(2); + + await timeline._closeRecording(); + }); + + it('ignores subframe navigations', async () => { + process.env.ARGO_SCREENCAST_PATH = 'cursor-subframe.webm'; + process.env.ARGO_CURSOR_HIGHLIGHT = '{}'; + delete process.env.ARGO_USE_CDP_DIRECT; + delete process.env.ARGO_STREAM_OUT; + + let navigationListener: ((frame: { parentFrame: () => unknown }) => void) | undefined; + const evaluate = vi.fn().mockResolvedValue(undefined); + const page = { + screencast: { + start: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + showActions: vi.fn().mockResolvedValue(undefined), + }, + evaluate, + on: vi.fn((_event, listener) => { navigationListener = listener; }), + off: vi.fn(), + context: vi.fn(), + }; + + const timeline = new NarrationTimeline(); + await timeline.startRecording(page as never); + const callsBefore = evaluate.mock.calls.length; + + navigationListener?.({ parentFrame: () => ({}) }); + expect(evaluate.mock.calls.length).toBe(callsBefore); + + await timeline._closeRecording(); + }); + + it('warns and stays disabled when ARGO_CURSOR_HIGHLIGHT is not a JSON object', async () => { + process.env.ARGO_SCREENCAST_PATH = 'cursor-bad-env.webm'; + process.env.ARGO_CURSOR_HIGHLIGHT = 'not-json'; + delete process.env.ARGO_USE_CDP_DIRECT; + delete process.env.ARGO_STREAM_OUT; + + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const evaluate = vi.fn().mockResolvedValue(undefined); + const page = { + screencast: { + start: vi.fn().mockResolvedValue(undefined), + stop: vi.fn().mockResolvedValue(undefined), + showActions: vi.fn().mockResolvedValue(undefined), + }, + evaluate, + on: vi.fn(), + off: vi.fn(), + context: vi.fn(), + }; + + const timeline = new NarrationTimeline(); + await timeline.startRecording(page as never); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('invalid ARGO_CURSOR_HIGHLIGHT')); + expect(evaluate.mock.calls.filter((c) => c[1]?.id === 'argo-cursor-highlight')).toHaveLength(0); + warn.mockRestore(); + + await timeline._closeRecording(); + }); }); describe('_closeRecording', () => {