diff --git a/packages/overlay/src/frame-agent.test.ts b/packages/overlay/src/frame-agent.test.ts new file mode 100644 index 0000000..bcbc894 --- /dev/null +++ b/packages/overlay/src/frame-agent.test.ts @@ -0,0 +1,94 @@ +import { describe, expect, it } from "vitest"; +import { type FrameHost, findHost } from "./frame-agent"; + +/* + * The host walk — the fix for the orphaned nested agent (#17). + * + * A frame agent used to reach exactly one hop up (`window.parent`), which for + * a document nested inside another app document — Storybook's preview inside + * its manager — is a window that exposes no shell hooks, so the agent + * registered with nobody and every forwarded event vanished. `findHost` walks + * upward past such windows to the first one carrying the ready hook. Pure + * function over fake window chains, so every shape is enumerable. + */ + +type FakeWindow = Window & FrameHost; + +const ready = () => { + // The walk only checks that a function is present. +}; + +/** A window whose parent is `parent`, or itself when omitted (a top window). */ +function fakeWin( + overrides: Partial = {}, + parent?: Window +): FakeWindow { + const win = { ...overrides } as FakeWindow; + (win as { parent: Window }).parent = parent ?? win; + return win; +} + +describe("findHost", () => { + it("finds a shell one hop up", () => { + const shell = fakeWin({ __airshipOnFrameReady: ready }); + const frame = fakeWin({}, shell); + + expect(findHost(frame)).toBe(shell); + }); + + it("walks past an app document that exposes no hooks", () => { + // shell → manager (plain app document) → nested preview. + const shell = fakeWin({ __airshipOnFrameReady: ready }); + const manager = fakeWin({}, shell); + const nested = fakeWin({}, manager); + + expect(findHost(nested)).toBe(shell); + }); + + it("answers null from a top window with no shell anywhere", () => { + expect(findHost(fakeWin())).toBeNull(); + }); + + it("stops at a cross-origin hop", () => { + const nested = {} as FakeWindow; + Object.defineProperty(nested, "parent", { + get() { + throw new DOMException("cross-origin"); + }, + }); + + expect(findHost(nested)).toBeNull(); + }); + + it("stops when reading the hook itself throws", () => { + const hostile = {} as FakeWindow; + Object.defineProperty(hostile, "__airshipOnFrameReady", { + get() { + throw new DOMException("cross-origin"); + }, + }); + const nested = fakeWin({}, hostile); + + expect(findHost(nested)).toBeNull(); + }); + + it("gives up past the depth cap rather than walking forever", () => { + const shell = fakeWin({ __airshipOnFrameReady: ready }); + let current: Window = shell; + for (let i = 0; i < 5; i += 1) { + current = fakeWin({}, current); + } + + expect(findHost(current)).toBeNull(); + }); + + it("ignores a hook that is not a function", () => { + const decoy = fakeWin({ + __airshipOnFrameReady: + "yes" as unknown as FrameHost["__airshipOnFrameReady"], + }); + const nested = fakeWin({}, decoy); + + expect(findHost(nested)).toBeNull(); + }); +}); diff --git a/packages/overlay/src/frame-agent.ts b/packages/overlay/src/frame-agent.ts index fc28106..22c0984 100644 --- a/packages/overlay/src/frame-agent.ts +++ b/packages/overlay/src/frame-agent.ts @@ -2,6 +2,7 @@ import type { ElementContext, SourceLocation } from "@airship/protocol"; import { AIRSHIP_FRAME_NAME } from "@airship/protocol"; import type { TokenScanResult } from "@airship/protocol/tokens"; import { extractElementInfo } from "@airship/source/browser"; +import { MAX_NEST_DEPTH } from "./canvas/space"; import { PREFIX } from "./dom"; import { SWALLOWED } from "./edit-guard"; import { css as portable, TEXT_EDIT_MARK } from "./styles/portable.css"; @@ -123,6 +124,62 @@ export interface FrameHost { __airshipOnFrameWheel?: (win: Window, e: FrameWheel) => boolean; } +/** + * The nearest ancestor window exposing the shell's hooks. + * + * `window.parent` is the wrong answer for a *nested* frame: its parent is the + * app document between it and the shell — Storybook's manager, say — which + * exposes no hooks, so the old one-hop reach silently stranded every agent + * booted below the first level (#17). The walk continues upward past such + * windows to the first one carrying `__airshipOnFrameReady`, capped at + * `MAX_NEST_DEPTH` hops; a cross-origin hop throws on property access and + * ends the search, which preserves the old "no reachable shell" behaviour + * exactly. Exported as a pure function so it can be tested against fake + * window chains. + */ +export function findHost(win: Window): FrameHost | null { + let current: Window = win; + for (let hops = 0; hops < MAX_NEST_DEPTH; hops += 1) { + let parent: Window | null; + try { + ({ parent } = current); + } catch { + return null; + } + if (!parent || parent === current) { + return null; + } + let ready = false; + try { + ready = + typeof (parent as unknown as FrameHost).__airshipOnFrameReady === + "function"; + } catch { + return null; + } + if (ready) { + return parent as unknown as FrameHost; + } + current = parent; + } + return null; +} + +/** + * `findHost`, memoised on success. The shell window never changes for the + * life of a realm, but a nested agent can boot before the shell has + * installed its hooks in pathological orders — so a null answer is asked + * again next time rather than cached. + */ +let cachedHost: FrameHost | null = null; + +function host(): FrameHost | null { + if (!cachedHost) { + cachedHost = findHost(window); + } + return cachedHost; +} + /** * Is this document inside an Airship frame, judged by its window name? * @@ -203,9 +260,7 @@ function createAgent(): FrameAgent { "pointerdown", () => { try { - (window.parent as unknown as FrameHost)?.__airshipOnFramePress?.( - window - ); + host()?.__airshipOnFramePress?.(window); } catch { // No reachable shell; nothing to select. } @@ -224,20 +279,17 @@ function createAgent(): FrameAgent { let handled = false; try { handled = - (window.parent as unknown as FrameHost)?.__airshipOnFrameWheel?.( - window, - { - altKey: e.altKey, - clientX: e.clientX, - clientY: e.clientY, - ctrlKey: e.ctrlKey, - deltaMode: e.deltaMode, - deltaX: e.deltaX, - deltaY: e.deltaY, - metaKey: e.metaKey, - shiftKey: e.shiftKey, - } - ) ?? false; + host()?.__airshipOnFrameWheel?.(window, { + altKey: e.altKey, + clientX: e.clientX, + clientY: e.clientY, + ctrlKey: e.ctrlKey, + deltaMode: e.deltaMode, + deltaX: e.deltaX, + deltaY: e.deltaY, + metaKey: e.metaKey, + shiftKey: e.shiftKey, + }) ?? false; } catch { // No reachable shell — leave the wheel to the app. } @@ -303,17 +355,14 @@ function onGuardedPress(e: Event): void { } const me = e as MouseEvent; try { - (window.parent as unknown as FrameHost)?.__airshipOnFrameTextPress?.( - window, - { - clientX: me.clientX, - clientY: me.clientY, - ctrlKey: me.ctrlKey, - dbl: e.type === "dblclick", - metaKey: me.metaKey, - shiftKey: me.shiftKey, - } - ); + host()?.__airshipOnFrameTextPress?.(window, { + clientX: me.clientX, + clientY: me.clientY, + ctrlKey: me.ctrlKey, + dbl: e.type === "dblclick", + metaKey: me.metaKey, + shiftKey: me.shiftKey, + }); } catch { // No reachable shell. The press is already swallowed, which is the part // that matters — the app stays inert either way. @@ -379,10 +428,10 @@ export function bootFrameAgent(): void { const agent = createAgent(); (window as unknown as { __airshipFrame?: FrameAgent }).__airshipFrame = agent; try { - (window.parent as unknown as FrameHost)?.__airshipOnFrameReady?.(agent); + host()?.__airshipOnFrameReady?.(agent); } catch { - // Cross-origin parent: nothing to register with, and nothing here is worth - // breaking the frame over. The frame still renders; the editor just cannot - // resolve source in it. + // Cross-origin ancestors only: nothing to register with, and nothing here + // is worth breaking the frame over. The frame still renders; the editor + // just cannot resolve source in it. } } diff --git a/packages/overlay/src/inspector/text-commit.test.ts b/packages/overlay/src/inspector/text-commit.test.ts index c8ae887..d697222 100644 --- a/packages/overlay/src/inspector/text-commit.test.ts +++ b/packages/overlay/src/inspector/text-commit.test.ts @@ -78,7 +78,7 @@ function harness(): Harness { document.body.append(host); const [a, b] = Array.from(host.children) as HTMLElement[]; - const surface = resolver.of(); + const surface = resolver.of(null); const meta = new Map([ [ a, diff --git a/packages/overlay/src/shell-app.ts b/packages/overlay/src/shell-app.ts index 6fca15b..8dccce2 100644 --- a/packages/overlay/src/shell-app.ts +++ b/packages/overlay/src/shell-app.ts @@ -4,7 +4,13 @@ import { FrameChrome } from "./canvas/frame-chrome"; import { type Frame, FrameManager } from "./canvas/frames"; import { FramesPanel } from "./canvas/frames-panel"; import { Minimap } from "./canvas/minimap"; -import { frameScreenRect, type Point, type Rect } from "./canvas/space"; +import { + frameChain, + frameToScreen, + nestOffset, + type Point, + type Rect, +} from "./canvas/space"; import { CanvasViewport, type SafeInset } from "./canvas/viewport"; import { axes, @@ -16,7 +22,7 @@ import { } from "./canvas/wheel"; import { ChromeLayer } from "./chrome-layer"; import { cls, PREFIX } from "./dom"; -import type { FrameHost, FrameWheel } from "./frame-agent"; +import type { FrameAgent, FrameHost, FrameWheel } from "./frame-agent"; import { keys } from "./keys/registry"; import type { Mods, Selection } from "./picker"; import { isElement } from "./realm"; @@ -46,8 +52,10 @@ class CanvasStage implements Stage { private readonly listeners: (() => void)[] = []; /** Subscribers to the trailing edge of a pan/zoom — see `isGesturing`. */ private readonly gestureEndListeners: (() => void)[] = []; - /** Per-frame unsubscribers for agent layout notifications. */ - private readonly frameUnsubs = new Map void>(); + /** Per frame id, one disposer per registered agent window — the frame's own + * realm and any nested same-origin document's. Keyed two-deep so a nested + * agent registering never tears down the root agent's subscriptions. */ + private readonly frameUnsubs = new Map void>>(); private getSelection: (() => Selection | null) | null = null; /** Where a press inside a live frame goes. Set by `bindFramePress`. */ private reportFramePress: @@ -79,7 +87,7 @@ class CanvasStage implements Stage { }); this.frames = new FrameManager({ onChanged: () => this.onFramesChanged(), - onFrameReady: (frame) => this.onFrameReady(frame), + onFrameReady: (frame, agent) => this.onFrameReady(frame, agent), pathname: config.pathname ?? "/", storageKey: `${key}:frames`, world: this.canvas.world, @@ -125,19 +133,17 @@ class CanvasStage implements Stage { * delivering the momentum events, exactly as the canvas pan relies on. */ private onFrameWheel(win: Window, e: FrameWheel): boolean { - const frame = this.frames.all.find((f) => f.win === win); + const frame = this.frames.frameOfWindow(win); if (!frame) { return false; } - // The point arrives in the frame's own client coordinates; `routeWheel` + // The point arrives in the agent's own client coordinates; `routeWheel` // takes shell screen space and maps back. Round-tripping it rather than // adding a second entry point is what stops the two routes disagreeing. - const origin = frameScreenRect(frame.el); - const { scale } = this.canvas; - const screen = { - x: origin.left + e.clientX * scale, - y: origin.top + e.clientY * scale, - }; + const screen = this.frameScreenPoint(frame, win, { + x: e.clientX, + y: e.clientY, + }); // Frames may overlap, so route only when *this* frame is the selected one. // Without it, a wheel in an unselected frame sitting over the selected one // would pass the geometry test and scroll the wrong frame. @@ -346,8 +352,14 @@ class CanvasStage implements Stage { setTextOwner(node: Element | null): void { const frame = node ? this.frames.frameOf(node) : null; this.frames.setTextFrame(frame); + // Every agent of the text frame, not just the root's: a text edit inside + // a nested document must be guarded in the realm the caret lives in. for (const f of this.frames.all) { - f.agent?.setTextGuard(f === frame); + const on = f === frame; + f.agent?.setTextGuard(on); + for (const agent of f.agents.values()) { + agent.setTextGuard(on); + } } } @@ -363,7 +375,7 @@ class CanvasStage implements Stage { if (this.editing) { return; } - const frame = this.frames.all.find((f) => f.win === win); + const frame = this.frames.frameOfWindow(win); if (frame) { this.frames.setActive(frame.id); } @@ -373,17 +385,12 @@ class CanvasStage implements Stage { // point lands back in the space `SelectionController.hitTest` expects and // both routes stay one code path. host.__airshipOnFrameTextPress = (win, e) => { - const frame = this.frames.all.find((f) => f.win === win); + const frame = this.frames.frameOfWindow(win); if (!frame) { return; } - const origin = frameScreenRect(frame.el); - const { scale } = this.canvas; this.reportFramePress?.( - { - x: origin.left + e.clientX * scale, - y: origin.top + e.clientY * scale, - }, + this.frameScreenPoint(frame, win, { x: e.clientX, y: e.clientY }), { meta: e.metaKey || e.ctrlKey, shift: e.shiftKey }, e.dbl ); @@ -492,8 +499,10 @@ class CanvasStage implements Stage { * gone would be pruning an empty list. */ destroy(): void { - for (const off of this.frameUnsubs.values()) { - off(); + for (const byWindow of this.frameUnsubs.values()) { + for (const off of byWindow.values()) { + off(); + } } this.frameUnsubs.clear(); this.listeners.length = 0; @@ -543,14 +552,32 @@ class CanvasStage implements Stage { */ private pruneFrameUnsubs(): void { const live = new Set(this.frames.all.map((f) => f.id)); - for (const [id, off] of this.frameUnsubs) { + for (const [id, byWindow] of this.frameUnsubs) { if (!live.has(id)) { - off(); + for (const off of byWindow.values()) { + off(); + } this.frameUnsubs.delete(id); } } } + /** + * A point in an agent's viewport coordinates → shell screen space, + * composing the nested chain when the agent's window sits below the frame's + * own document. The one transform every forwarded frame event shares. + */ + private frameScreenPoint(frame: Frame, win: Window, point: Point): Point { + const chain = frame.win ? frameChain(win, frame.win) : null; + const rect = frameToScreen( + frame.el, + { height: 0, left: point.x, top: point.y, width: 0 }, + this.canvas.scale, + chain ? nestOffset(chain) : undefined + ); + return { x: rect.left, y: rect.top }; + } + /** * A frame published its agent — on first load, and again after every HMR full * reload, which rebuilds the frame's realm from scratch. @@ -579,16 +606,34 @@ class CanvasStage implements Stage { * Disposal-first, because this fires again on every reload with a brand-new * document, and `pruneFrameUnsubs` takes the rest when a frame goes away. */ - private onFrameReady(frame: Frame): void { - this.frameUnsubs.get(frame.id)?.(); - const offLayout = frame.agent?.onLayoutChange(() => this.notify()); - const offKeys = frame.doc ? keys.observe(frame.doc) : null; - if (offLayout || offKeys) { - this.frameUnsubs.set(frame.id, () => { - offLayout?.(); - offKeys?.(); - }); + private onFrameReady(frame: Frame, agent: FrameAgent): void { + let byWindow = this.frameUnsubs.get(frame.id); + if (byWindow && agent.window === frame.win) { + // The root realm was rebuilt; the nested realms died with it, so every + // subscription for this frame goes together — a surviving disposer + // would hold a dead realm alive, and a nested agent that reboots will + // re-register on its own. + for (const off of byWindow.values()) { + off(); + } + byWindow.clear(); + } else { + byWindow?.get(agent.window)?.(); } + if (!byWindow) { + byWindow = new Map(); + this.frameUnsubs.set(frame.id, byWindow); + } + const offLayout = agent.onLayoutChange(() => this.notify()); + // The agent's own document, not `frame.doc`: for a nested agent that is + // the nested document, which is exactly where shortcuts would otherwise + // die the moment focus lands in it. + const doc = agent.window.document; + const offKeys = doc ? keys.observe(doc) : null; + byWindow.set(agent.window, () => { + offLayout(); + offKeys?.(); + }); this.notify(); }