diff --git a/.changeset/token-keyed-frame-registry.md b/.changeset/token-keyed-frame-registry.md new file mode 100644 index 0000000..68eeec5 --- /dev/null +++ b/.changeset/token-keyed-frame-registry.md @@ -0,0 +1,21 @@ +--- +"sideshow": patch +--- + +Fix surface iframes staying stuck at their seed height when a host mounts two +viewer instances into one JS realm at once (e.g. an embedding host that +cross-fades an outgoing viewer into an incoming one during an in-place +navigation). Because the engine loads as a single shared module, both instances +shared the module-level surface-frame registry (`cardEls`) and the single +`message` bridge listener. Two instances routing to the same URL rendered a +`Card` for the same post, so keying `cardEls` by post id let them clobber each +other's entry — and the outgoing instance's teardown deleted the entry the +still-visible instance needed, so its surface `resize` messages were dropped and +the frames never grew past their seed height. The shared listener had the same +hazard: `removeEventListener` on the first teardown tore it out from under the +survivor. + +`cardEls` is now keyed by a per-`Card` token (resolved by `contentWindow`, with a +new `cardForPost` for the scroll-to-card pill), and the bridge listener is +reference-counted so it lives while any viewer is mounted. Self-hosted single- +instance behavior is unchanged. diff --git a/viewer/src/App.tsx b/viewer/src/App.tsx index 945e773..cc98a46 100644 --- a/viewer/src/App.tsx +++ b/viewer/src/App.tsx @@ -13,7 +13,7 @@ import { type SessionRow, } from "./api.ts"; import { host, isShadow, navHostEl, root, SLOTS } from "./host.ts"; -import { applyFrameHeight, Card, cardEls, frameForSource } from "./Card.tsx"; +import { applyFrameHeight, Card, cardForPost, frameForSource } from "./Card.tsx"; import { ConnectInstructions } from "./Connect.tsx"; import { renderNotes } from "./notes.ts"; import { SessionTimeline } from "./SessionTimeline.tsx"; @@ -154,8 +154,8 @@ export default function App() { if (sessions.length > 0) refreshSessionsQuiet(); }, 45_000); onCleanup(() => clearInterval(timer)); - window.addEventListener("message", onBridgeMessage); - onCleanup(() => window.removeEventListener("message", onBridgeMessage)); + acquireBridgeListener(); + onCleanup(releaseBridgeListener); // returning to the tab counts as seeing the selected session const onVisibility = () => { const id = selected(); @@ -345,7 +345,7 @@ export default function App() { onClick={() => { const target = pillTarget(); if (target) - cardEls.get(target)?.card.scrollIntoView({ behavior: "smooth", block: "start" }); + cardForPost(target)?.scrollIntoView({ behavior: "smooth", block: "start" }); setPillTarget(null); }} > @@ -437,6 +437,25 @@ function WhatsNewCard() { ); } +// The bridge listener is a single module-level function, so `addEventListener` +// with it deduplicates to ONE registration no matter how many App instances mount +// — and a host that mounts two engines at once (e.g. the cloud's double-buffered +// in-place workspace switch keeps the outgoing engine mounted until the incoming +// one is ready) shares this one module realm. Adding/removing per-mount would then +// let the FIRST engine to unmount call removeEventListener and tear the shared +// listener out from under the still-mounted second engine — after which its +// sandboxed surfaces post `resize` to a window nobody is listening on, and every +// frame stays stuck at its seed height until a full reload. Reference-count instead: +// keep the one listener alive while ANY App is mounted, remove it only when the last +// unmounts. Single registration throughout, so no message is ever handled twice. +let bridgeListeners = 0; +function acquireBridgeListener(): void { + if (bridgeListeners++ === 0) window.addEventListener("message", onBridgeMessage); +} +function releaseBridgeListener(): void { + if (--bridgeListeners === 0) window.removeEventListener("message", onBridgeMessage); +} + // Messages from sandboxed post iframes (see server/surfacePage.ts bridge). async function onBridgeMessage(ev: MessageEvent) { const d = ev.data as { diff --git a/viewer/src/Card.tsx b/viewer/src/Card.tsx index 9cbf7de..0331f10 100644 --- a/viewer/src/Card.tsx +++ b/viewer/src/Card.tsx @@ -51,15 +51,30 @@ import { type ViewComment, } from "./state.ts"; -// Card registry keyed by post id: the "new post" pill scrolls to the -// card element, and each card tracks its sandboxed-surface iframes so the -// postMessage bridge in App can resolve the source post + iframe by -// contentWindow (a post may have several sandboxed surfaces → several frames). -export const cardEls = new Map }>(); +// Registry of live cards → their post id, card element, and sandboxed-surface +// iframes, so the "new post" pill can scroll to a card and the postMessage bridge +// in App can resolve the source post + iframe by contentWindow (a post may have +// several sandboxed surfaces → several frames). +// +// Keyed by a per-Card TOKEN, deliberately NOT by post id. This module is a +// singleton, and a HOST can mount two engine instances into ONE JS realm at once +// — the cloud's double-buffered in-place workspace switch keeps the outgoing +// engine mounted until the incoming one is ready. Both engines see the same URL, +// so both render a Card for the SAME post. Keying by post id let those two Cards +// clobber each other's entry, and when the outgoing engine tore down, its Card's +// onCleanup deleted the shared entry the still-visible engine's Card needed — so +// frameForSource could no longer resolve it and its surface iframes stayed stuck +// at their seed height until a full reload. A unique token per Card keeps the two +// registrations independent: frameForSource matches by contentWindow (key- +// agnostic) and cardForPost scans by id, so neither instance can evict the other. +export const cardEls = new Map< + object, + { id: string; card: HTMLDivElement; iframes: Set } +>(); // Resolve which post + iframe a postMessage came from, by contentWindow. export function frameForSource(source: unknown): { id: string; iframe: HTMLIFrameElement } | null { - for (const [id, { iframes }] of cardEls) { + for (const { id, iframes } of cardEls.values()) { for (const iframe of iframes) { if (iframe.contentWindow === source) return { id, iframe }; } @@ -67,6 +82,15 @@ export function frameForSource(source: unknown): { id: string; iframe: HTMLIFram return null; } +// The card element for a post, for scroll-into-view. Scans by id because the +// registry is token-keyed (see cardEls). If two engine instances are briefly both +// mounted, either card's position is equivalent (the visible one is what scrolls); +// once the switch settles only one remains. +export function cardForPost(id: string): HTMLDivElement | null { + for (const entry of cardEls.values()) if (entry.id === id) return entry.card; + return null; +} + // Size a post's surface iframe from a height the in-frame bridge reported. Min // one line, max generous enough for a long diff/markdown without runaway growth. const MIN_FRAME_H = 24; @@ -298,8 +322,10 @@ export function Card(props: { post: Post; standalone?: boolean }) { }); onMount(() => { - cardEls.set(props.post.id, { card, iframes }); - onCleanup(() => cardEls.delete(props.post.id)); + // Key by a token UNIQUE to this Card instance, never by post.id — see cardEls. + const token = {}; + cardEls.set(token, { id: props.post.id, card, iframes }); + onCleanup(() => cardEls.delete(token)); // Standalone is a single, full-page post — there is no feed to scroll // through and no session route to track, so skip the deep-link scroll and // the URL-syncing observer. The cardEls registration above still runs so the