diff --git a/apps/desktop/src/renderer/src/platform/tab-coordinator.test.ts b/apps/desktop/src/renderer/src/platform/tab-coordinator.test.ts new file mode 100644 index 000000000..416be546e --- /dev/null +++ b/apps/desktop/src/renderer/src/platform/tab-coordinator.test.ts @@ -0,0 +1,160 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { + __resetTabCoordinatorForTests, + createScrollRestorationAdapter, + initTabCoordinator, + registerActiveHostElement, +} from "./tab-coordinator"; +import { getActiveTab, useTabStore } from "@/stores/tab-store"; + +describe("tab-coordinator external scroll sources", () => { + beforeEach(() => { + __resetTabCoordinatorForTests(); + useTabStore.getState().reset(); + useTabStore.getState().switchWorkspace("acme"); + initTabCoordinator(); + }); + + function seedActiveTab(path: string, title: string): string { + return useTabStore.getState().openTab(path, title, { activate: true }); + } + + it("captures external sources registered by the outgoing tab alongside DOM scroll roots", () => { + const tabId = seedActiveTab("/acme/attachments/abc/preview", "Report"); + const adapter = createScrollRestorationAdapter(tabId); + + // Set up an outer DOM scroll root to confirm both paths merge. + const host = document.createElement("div"); + const list = document.createElement("div"); + list.setAttribute("data-tab-scroll-root", "list"); + Object.defineProperty(list, "scrollTop", { value: 250, configurable: true }); + Object.defineProperty(list, "scrollHeight", { value: 1000, configurable: true }); + host.appendChild(list); + registerActiveHostElement(host); + + // Register an iframe-style external source with a contentKey. + adapter.registerExternalSource?.("html-iframe", { + capture: () => ({ top: 512, height: 4000, contentKey: "v1hash" }), + }); + + // Switch to another tab — the Coordinator's store subscription fires + // synchronously during the set() and captures the outgoing tab's state. + const otherId = useTabStore + .getState() + .openTab("/acme/issues", "Issues", { activate: true }); + void otherId; + + const outgoing = useTabStore + .getState() + .byWorkspace.acme.tabs.find((t) => t.id === tabId)!; + const scroll = outgoing.memento.scroll; + // DOM root was captured (top > 0 rule). + expect(scroll["/acme/attachments/abc/preview::list"]).toEqual({ + top: 250, + height: 1000, + }); + // External source was captured with its contentKey. + expect(scroll["/acme/attachments/abc/preview::html-iframe"]).toEqual({ + top: 512, + height: 4000, + contentKey: "v1hash", + }); + + registerActiveHostElement(null); + }); + + it("writes external-source entries even at top:0 so a contentKey change clears stale offsets", () => { + const tabId = seedActiveTab("/acme/attachments/abc/preview", "Report"); + const adapter = createScrollRestorationAdapter(tabId); + registerActiveHostElement(document.createElement("div")); + + // Positive offset first. + adapter.registerExternalSource?.("html-iframe", { + capture: () => ({ top: 800, height: 5000, contentKey: "old" }), + }); + useTabStore + .getState() + .openTab("/acme/issues", "Issues", { activate: true }); + + // Re-activate the original tab, then switch again with new content but + // no scroll yet (top:0). capture() should produce top:0 with the new + // contentKey and the old offset must be replaced, not left behind. + useTabStore.getState().setActiveTab(tabId); + const adapter2 = createScrollRestorationAdapter(tabId); + adapter2.registerExternalSource?.("html-iframe", { + capture: () => ({ top: 0, height: 0, contentKey: "new" }), + }); + useTabStore + .getState() + .openTab("/acme/issues", "Issues", { activate: true }); + + const entry = useTabStore + .getState() + .byWorkspace.acme.tabs.find((t) => t.id === tabId)!.memento.scroll[ + "/acme/attachments/abc/preview::html-iframe" + ]; + expect(entry).toEqual({ top: 0, height: 0, contentKey: "new" }); + + registerActiveHostElement(null); + }); + + it("isolates external sources per tab: an incoming tab cannot read another tab's source", () => { + const tabA = seedActiveTab("/acme/attachments/a/preview", "A"); + const adapterA = createScrollRestorationAdapter(tabA); + adapterA.registerExternalSource?.("html-iframe", { + capture: () => ({ top: 111, height: 1000, contentKey: "a" }), + }); + + const tabB = useTabStore + .getState() + .openTab("/acme/attachments/b/preview", "B", { activate: true }); + // tab B is now incoming; its adapter registers its own source. tab A's + // capture must NOT be attributed to B when B leaves. + const adapterB = createScrollRestorationAdapter(tabB); + adapterB.registerExternalSource?.("html-iframe", { + capture: () => ({ top: 222, height: 2000, contentKey: "b" }), + }); + + useTabStore + .getState() + .openTab("/acme/issues", "Issues", { activate: true }); + + const tabs = useTabStore.getState().byWorkspace.acme.tabs; + const aEntry = tabs.find((t) => t.id === tabA)!.memento.scroll[ + "/acme/attachments/a/preview::html-iframe" + ]; + const bEntry = tabs.find((t) => t.id === tabB)!.memento.scroll[ + "/acme/attachments/b/preview::html-iframe" + ]; + expect(aEntry).toEqual({ top: 111, height: 1000, contentKey: "a" }); + expect(bEntry).toEqual({ top: 222, height: 2000, contentKey: "b" }); + }); + + it("get() on the per-tab adapter only returns entries when that tab is active", () => { + const tabId = seedActiveTab("/acme/attachments/abc/preview", "Report"); + const adapter = createScrollRestorationAdapter(tabId); + registerActiveHostElement(document.createElement("div")); + adapter.registerExternalSource?.("html-iframe", { + capture: () => ({ top: 321, height: 900, contentKey: "v" }), + }); + useTabStore + .getState() + .openTab("/acme/issues", "Issues", { activate: true }); + + // While another tab is active, get() returns undefined even though the + // entry exists in the memento (prevents a stale view pulling another + // tab's offset). + expect(adapter.get("html-iframe")).toBeUndefined(); + + useTabStore.getState().setActiveTab(tabId); + expect(getActiveTab(useTabStore.getState())?.id).toBe(tabId); + // A fresh adapter for the now-active tab sees the memento. + const adapterAgain = createScrollRestorationAdapter(tabId); + expect(adapterAgain.get("html-iframe")).toEqual({ + top: 321, + height: 900, + contentKey: "v", + }); + registerActiveHostElement(null); + }); +}); diff --git a/apps/desktop/src/renderer/src/platform/tab-coordinator.ts b/apps/desktop/src/renderer/src/platform/tab-coordinator.ts index 53235f53c..7df5b666f 100644 --- a/apps/desktop/src/renderer/src/platform/tab-coordinator.ts +++ b/apps/desktop/src/renderer/src/platform/tab-coordinator.ts @@ -1,6 +1,10 @@ import type { DataRouter } from "react-router-dom"; import type { QueryClient } from "@tanstack/react-query"; -import type { ScrollRestorationAdapter } from "@multica/views/platform"; +import type { + ExternalScrollSource, + ScrollRestorationAdapter, + ScrollRestorationEntry, +} from "@multica/views/platform"; import { createAppRouter } from "@/routes"; import { useTabStore, @@ -46,6 +50,45 @@ let activeHostElement: HTMLElement | null = null; /** Query client for reload()'s current-page-scope invalidation. */ let queryClient: QueryClient | null = null; +/** + * Scroll sources outside the outer DOM that the Coordinator cannot scan for + * itself — currently the document inside a sandboxed HTML-attachment iframe + * (opaque origin → `contentWindow.scrollY` throws). Keyed by tab id so each + * tab's sources are captured only when that tab is outgoing; sources are + * registered/unregistered by the per-tab view via the adapter's + * `registerExternalSource` and vanish naturally when the tab's ActiveTabHost + * unmounts. We still prune on capture so a tab id is never leaked after its + * subtree is gone. + */ +const externalScrollSources = new Map< + string, + Map +>(); + +/** + * Register (or unregister, when `source` is null) an external scroll source + * for a tab. The view's ScrollRestorationAdapter is per-tab, so it passes its + * own tabId here. Safe to call with null on cleanup. + */ +export function registerExternalScrollSource( + tabId: string, + containerKey: string, + source: ExternalScrollSource | null, +): void { + let perTab = externalScrollSources.get(tabId); + if (!source) { + if (!perTab) return; + perTab.delete(containerKey); + if (perTab.size === 0) externalScrollSources.delete(tabId); + return; + } + if (!perTab) { + perTab = new Map(); + externalScrollSources.set(tabId, perTab); + } + perTab.set(containerKey, source); +} + /** Identity of what the host currently shows: slug:tabId:generation. */ let lastIdentity: string | null = null; let lastActiveTabId: string | null = null; @@ -74,6 +117,11 @@ export function registerCoordinatorQueryClient(qc: QueryClient): void { * The generic view-state entries ride the same channel: reads resolve * against the memento's `view` map, writes commit through the store — no * capture pass, the view pushes at the moment its restorable state changes. + * + * The adapter is constructed per ActiveTabHost (memoized on tabId in + * tab-content.tsx), so `registerExternalSource` records this tab's + * out-of-DOM scroll sources (iframe documents) under that tabId; the + * Coordinator captures them when this tab is the outgoing one. */ export function createScrollRestorationAdapter( tabId: string, @@ -102,6 +150,9 @@ export function createScrollRestorationAdapter( if (routeKey === null) return; useTabStore.getState().commitViewState(tabId, routeKey, entryKey, value); }, + registerExternalSource(containerKey, source) { + registerExternalScrollSource(tabId, containerKey, source); + }, }; } @@ -136,22 +187,40 @@ function reconcile(): void { * React re-renders — so the outgoing DOM (previous tab, or previous route * of the same tab) is still mounted. * - * Scroll containers self-mark with `data-tab-scroll-root` (the attribute - * value is the container key, "main" when bare). Containers sitting at 0 - * are simply absent from the result — the store's per-route REPLACE + * Plain scroll containers self-mark with `data-tab-scroll-root` (the + * attribute value is the container key, "main" when bare). Containers sitting + * at 0 are simply absent from the result — the store's per-route REPLACE * semantics turn that absence into "clear the stale offset". + * + * External sources (sandboxed iframe documents) are merged in for the + * outgoing tab ONLY; unlike plain containers they are ALWAYS written, + * including at top 0, because their entry carries a `contentKey` and a + * top:0 write with the new key is how a content change replaces a stale + * positive offset rather than resurrecting it. */ -function captureScrollEntries(): Record { - const entries: Record = {}; - if (!activeHostElement) return entries; - const els = activeHostElement.querySelectorAll( - "[data-tab-scroll-root]", - ); - els.forEach((el) => { - if (el.scrollTop <= 0) return; - const key = el.getAttribute("data-tab-scroll-root") || "main"; - entries[key] = { top: el.scrollTop, height: el.scrollHeight }; - }); +function captureScrollEntries( + outgoingTabId: string | null, +): Record { + const entries: Record = {}; + if (activeHostElement) { + const els = activeHostElement.querySelectorAll( + "[data-tab-scroll-root]", + ); + els.forEach((el) => { + if (el.scrollTop <= 0) return; + const key = el.getAttribute("data-tab-scroll-root") || "main"; + entries[key] = { top: el.scrollTop, height: el.scrollHeight }; + }); + } + if (outgoingTabId) { + const sources = externalScrollSources.get(outgoingTabId); + if (sources) { + for (const [key, source] of sources) { + const entry = source.capture(); + if (entry) entries[key] = entry; + } + } + } return entries; } @@ -182,7 +251,19 @@ function handleStoreChange(): void { // scrolled back to 0; the store skips the write when nothing changed. useTabStore .getState() - .commitScrollMemento(outgoingTabId, routeKey, captureScrollEntries()); + .commitScrollMemento( + outgoingTabId, + routeKey, + captureScrollEntries(outgoingTabId), + ); + } + // The outgoing tab's ActiveTabHost is about to unmount; its views' + // cleanup will unregister their sources, but a host switch that tears + // down the whole tree can drop those cleanups without running them. + // Prune proactively so a future re-mount starts clean and a closed tab + // never leaks sources. + if (outgoingTabId && hostSwitching) { + externalScrollSources.delete(outgoingTabId); } } @@ -270,4 +351,5 @@ export function __resetTabCoordinatorForTests(): void { lastIdentity = null; lastActiveTabId = null; lastActiveUrl = null; + externalScrollSources.clear(); } diff --git a/apps/desktop/src/renderer/src/stores/tab-store.test.ts b/apps/desktop/src/renderer/src/stores/tab-store.test.ts index 2cb19d29d..8058b947b 100644 --- a/apps/desktop/src/renderer/src/stores/tab-store.test.ts +++ b/apps/desktop/src/renderer/src/stores/tab-store.test.ts @@ -577,6 +577,64 @@ describe("commitScrollMemento", () => { view: { "/acme/inbox::highlight:i1": "c1" }, }); }); + + it("persists contentKey on external-source entries (iframe scroll)", () => { + const store = useTabStore.getState(); + store.switchWorkspace("acme"); + const tabId = useTabStore.getState().byWorkspace.acme.tabs[0].id; + store.commitScrollMemento(tabId, "/acme/attachments/x/preview", { + "html-iframe": { top: 400, height: 9000, contentKey: "abc123" }, + }); + + expect( + useTabStore.getState().byWorkspace.acme.tabs[0].memento.scroll[ + "/acme/attachments/x/preview::html-iframe" + ], + ).toEqual({ top: 400, height: 9000, contentKey: "abc123" }); + }); + + it("treats top:0 with a new contentKey as a change so it REPLACES a stale positive offset", () => { + const store = useTabStore.getState(); + store.switchWorkspace("acme"); + const tabId = useTabStore.getState().byWorkspace.acme.tabs[0].id; + const route = "/acme/attachments/x/preview"; + + store.commitScrollMemento(tabId, route, { + "html-iframe": { top: 800, height: 9000, contentKey: "v1" }, + }); + // Content changed (re-upload): the new document hasn't scrolled yet so + // capture reports top:0 with the NEW key. This must overwrite the old + // positive offset, otherwise the next visit would wrongly restore 800. + store.commitScrollMemento(tabId, route, { + "html-iframe": { top: 0, height: 0, contentKey: "v2" }, + }); + + expect( + useTabStore.getState().byWorkspace.acme.tabs[0].memento.scroll[ + `${route}::html-iframe` + ], + ).toEqual({ top: 0, height: 0, contentKey: "v2" }); + }); + + it("does NOT skip the write when contentKey changes even with identical top/height", () => { + const store = useTabStore.getState(); + store.switchWorkspace("acme"); + const tabId = useTabStore.getState().byWorkspace.acme.tabs[0].id; + const route = "/acme/attachments/x/preview"; + + store.commitScrollMemento(tabId, route, { + "html-iframe": { top: 100, height: 5000, contentKey: "v1" }, + }); + const before = useTabStore.getState().byWorkspace.acme.tabs[0].memento; + + store.commitScrollMemento(tabId, route, { + "html-iframe": { top: 100, height: 5000, contentKey: "v2" }, + }); + + const after = useTabStore.getState().byWorkspace.acme.tabs[0].memento; + expect(after).not.toBe(before); + expect(after.scroll[`${route}::html-iframe`].contentKey).toBe("v2"); + }); }); describe("commitViewState", () => { diff --git a/apps/desktop/src/renderer/src/stores/tab-store.ts b/apps/desktop/src/renderer/src/stores/tab-store.ts index 0ab3eb8d0..02eee0b81 100644 --- a/apps/desktop/src/renderer/src/stores/tab-store.ts +++ b/apps/desktop/src/renderer/src/stores/tab-store.ts @@ -17,6 +17,22 @@ import { isReservedSlug } from "@multica/core/paths"; // router to `activeSession.url` with a navigation token. Nothing in this // file touches react-router. +/** + * One captured scroll position. + * + * `contentKey` is an optional fingerprint of the rendered content, used by + * out-of-DOM scroll sources (the sandboxed HTML-attachment iframe — see + * multica-ai#6405) to invalidate restoration when the content changed + * (re-upload to the same attachment id). Plain `[data-tab-scroll-root]` + * containers leave it undefined; identity comparison (undefined === + * undefined) means their behavior is unchanged. + */ +export interface ScrollMementoEntry { + top: number; + height: number; + contentKey?: string; +} + export interface TabMemento { /** * Scroll offsets keyed `${routePathname}::${containerKey}` where the @@ -27,7 +43,7 @@ export interface TabMemento { * tab). `height` is the container's scrollHeight at capture time, kept * for diagnostics and future pre-sizing heuristics. */ - scroll: Record; + scroll: Record; /** * Generic restorable view-state entries, keyed like `scroll` * (`${routePathname}::${entryKey}`). The memento's second kind of cargo: @@ -196,12 +212,15 @@ interface TabStore { * deactivate / before in-tab navigation). REPLACE semantics per route: all * of the route's previous entries are dropped and the given ones written — * so a container scrolled back to 0 (captured as "no entry") clears its - * stale offset instead of resurrecting it on the next visit. + * stale offset instead of resurrecting it on the next visit. External + * sources (iframe documents) always write an entry, including top:0, so a + * contentKey change can replace a stale positive offset rather than leave + * it behind. */ commitScrollMemento: ( tabId: string, routeKey: string, - entries: Record, + entries: Record, ) => void; /** * Write (string) or clear (undefined) one generic view-state entry for one @@ -773,7 +792,12 @@ export const useTabStore = create()( nextKeys.every((k) => { const prev = current.memento.scroll[k]; const next = nextScroll[k]; - return prev !== undefined && prev.top === next.top && prev.height === next.height; + return ( + prev !== undefined && + prev.top === next.top && + prev.height === next.height && + prev.contentKey === next.contentKey + ); }); if (unchanged) return; diff --git a/e2e/iframe-scroll-bridge.spec.ts b/e2e/iframe-scroll-bridge.spec.ts new file mode 100644 index 000000000..52ed9d129 --- /dev/null +++ b/e2e/iframe-scroll-bridge.spec.ts @@ -0,0 +1,257 @@ +import { expect, test, type Frame } from "@playwright/test"; +import { buildScrollBridge } from "../packages/views/editor/utils/iframe-scroll-bridge"; + +/** + * Self-contained Chromium tests for the in-iframe scroll bridge + * (multica-ai#6405). No app server: `page.setContent` with a real + * `sandbox="allow-scripts"` srcdoc iframe, injecting the exact bridge + * script that ships in the bundle. These pin facts jsdom cannot verify: + * sandboxed srcdoc throws on sessionStorage, and a real ResizeObserver + * re-positions after a delayed content append. + */ + +const TOKEN = "chrometok"; + +function pageWithIframe(bodyHtml: string): string { + // p{height:1200px} plus a 5000px bottom spacer so scroll targets like 600 + // are reachable regardless of the test viewport's height. + const userHtml = `${bodyHtml}
`; + const srcDoc = userHtml + buildScrollBridge(TOKEN); + const attr = srcDoc.replace(/&/g, "&").replace(/"/g, """); + return ` + +`; +} + +async function attachPage(page: import("@playwright/test").Page): Promise { + await page.setContent(pageWithIframe("

loaded

")); + // Wait until the iframe's document is parsed (the bridge announces ready). + await expect + .poll(async () => { + const msgs = await page.evaluate(() => + (window as unknown as { __messages: unknown[] }).__messages.filter( + (m) => (m as { kind?: string }).kind === "ready", + ), + ); + return msgs.length; + }) + .toBeGreaterThan(0); + const handle = await page.$("#f"); + const frame = await handle!.contentFrame(); + expect(frame).toBeTruthy(); + return frame!; +} + +test.describe("iframe scroll bridge (real Chromium, sandboxed srcdoc)", () => { + test("sandbox without allow-same-origin blocks sessionStorage (documents why we cannot use it)", async ({ + page, + }) => { + await page.setContent( + ``, + ); + await expect + .poll(async () => page.evaluate(() => (window as unknown as { __result: unknown }).__result), { + timeout: 10000, + }) + .toBe("SECURITY_ERROR"); + }); + + test("reports scroll position to the parent with the correct token", async ({ + page, + }) => { + const frame = await attachPage(page); + await frame.evaluate(() => window.scrollTo(0, 400)); + await expect + .poll( + async () => { + const all = await page.evaluate(() => + (window as unknown as { __messages: Record[] }).__messages, + ); + return all.find((m) => m.kind === "scroll" && (m.y as number) >= 400); + }, + { timeout: 5000 }, + ) + .toMatchObject({ y: 400, token: TOKEN, kind: "scroll" }); + }); + + test("restores scroll position on a parent restore message", async ({ + page, + }) => { + const frame = await attachPage(page); + await page.evaluate( + ([token]) => { + const w = document.querySelector("#f")!.contentWindow!; + w.postMessage( + { __multica: "__multica", kind: "restore", y: 600, token }, + "*", + ); + }, + [TOKEN], + ); + await expect + .poll(async () => frame.evaluate(() => window.scrollY), { timeout: 5000 }) + .toBe(600); + }); + + test("holds position across a 1s-delayed content append, then yields on user wheel (v4 guarantee)", async ({ + page, + }) => { + const frame = await attachPage(page); + + await page.evaluate( + ([token]) => { + const w = document.querySelector("#f")!.contentWindow!; + w.postMessage( + { __multica: "__multica", kind: "restore", y: 500, token }, + "*", + ); + }, + [TOKEN], + ); + // Wait > 33ms so a "2 stable frames then end session" (v3 bug) would + // already have terminated the restoring session. + await page.waitForTimeout(250); + + // Append content 1s after restore; ResizeObserver must re-apply target. + await frame.evaluate(() => + window.setTimeout(() => { + const div = document.createElement("div"); + div.style.height = "3000px"; + document.body.appendChild(div); + }, 1000), + ); + // Allow the append + RO callback to run. + await page.waitForTimeout(1500); + expect(await frame.evaluate(() => window.scrollY)).toBe(500); + + // User takeover: dispatch a real wheel event over the iframe. + await page.mouse.move(50, 50); + await page.mouse.wheel(0, 120); + await page.waitForTimeout(150); + + // After takeover, further growth + programmatic scroll must not be + // forced back to 500. + await frame.evaluate(() => { + const div = document.createElement("div"); + div.style.height = "2000px"; + document.body.appendChild(div); + window.scrollTo(0, 900); + }); + await page.waitForTimeout(200); + expect(await frame.evaluate(() => window.scrollY)).toBe(900); + }); + + test("ignores restore messages with a mismatched token", async ({ + page, + }) => { + const frame = await attachPage(page); + await page.evaluate(() => { + const w = document.querySelector("#f")!.contentWindow!; + w.postMessage( + { __multica: "__multica", kind: "restore", y: 777, token: "old" }, + "*", + ); + }); + await page.waitForTimeout(200); + expect(await frame.evaluate(() => window.scrollY)).toBe(0); + }); + + test("parent handshake is bounded (no ready→request-sync→ready loop) and restores once", async ({ + page, + }) => { + // Faithful in-page port of the PARENT side of useHtmlPreviewScrollRestore: + // - request-sync is sent only from the ref/onLoad handshake points; + // - on `ready` the parent records position and issues restore AT MOST + // ONCE per token — it never replies with another request-sync. + // Before the fix, ready → request-sync → ready looped forever. + const targetTop = 540; + await page.setContent(pageWithIframe("

loaded

")); + const counts = await page.evaluate( + ([token, target]) => + new Promise<{ + ready: number; + requestSync: number; + restore: number; + scroll: number; + }>((resolve) => { + const MARK = "__multica"; + const iframe = document.querySelector("#f")!; + let restoreIssued = false; + let requestSync = 0; + let restore = 0; + let ready = 0; + let scroll = 0; + + function post(msg: Record) { + iframe.contentWindow!.postMessage({ [MARK]: MARK, token, ...msg }, "*"); + } + function sendRequestSync() { + requestSync++; + post({ kind: "request-sync" }); + } + function sendRestore() { + if (restoreIssued) return; + if (target <= 0) return; + restoreIssued = true; + restore++; + post({ kind: "restore", y: target }); + } + + window.addEventListener("message", (e) => { + if (e.source !== iframe.contentWindow) return; + const d = e.data as + | { __multica?: string; token?: string; kind?: string } + | undefined; + if (!d || d.__multica !== MARK || d.token !== token) return; + if (d.kind === "ready") { + ready++; + // THE FIX: record + restore, but do NOT send request-sync here. + sendRestore(); + } else if (d.kind === "scroll") { + scroll++; + } + }); + + // Ref callback (element attached) and onLoad each fire exactly one + // request-sync — the only two legitimate request-sync moments. + sendRequestSync(); + iframe.addEventListener("load", () => { + sendRequestSync(); + sendRestore(); + }); + + // Let any would-be loop run; then snapshot the counts twice. + window.setTimeout(() => { + const first = { ready, requestSync, restore, scroll }; + window.setTimeout(() => { + resolve({ + ready: ready - first.ready, + requestSync: requestSync - first.requestSync, + restore: restore - first.restore, + scroll: scroll - first.scroll, + }); + // We resolve with the DELTA — it must be all zeros (no loop). + }, 400); + }, 600); + }), + [TOKEN, targetTop], + ); + + // No messages were added in the 400ms observation window → no loop. + expect(counts).toEqual({ ready: 0, requestSync: 0, restore: 0, scroll: 0 }); + + // Restore actually happened in the iframe (proves the handshake still + // completes, not just that it went quiet). + const handle = await page.$("#f"); + const frame = await handle!.contentFrame(); + await expect + .poll(async () => frame!.evaluate(() => window.scrollY), { timeout: 5000 }) + .toBe(targetTop); + }); +}); diff --git a/packages/views/attachments/attachment-preview-page.test.tsx b/packages/views/attachments/attachment-preview-page.test.tsx new file mode 100644 index 000000000..617e6650e --- /dev/null +++ b/packages/views/attachments/attachment-preview-page.test.tsx @@ -0,0 +1,62 @@ +import { describe, expect, it, vi } from "vitest"; +import { render } from "@testing-library/react"; +import type { ScrollRestorationAdapter } from "../platform"; +import { ScrollRestorationProvider } from "../platform"; +import { AttachmentPreviewPage } from "./attachment-preview-page"; +import { hashString } from "../editor/utils/hash-string"; +import { buildScrollBridge } from "../editor/utils/iframe-scroll-bridge"; + +const htmlText = "

Report

"; + +vi.mock("../editor/hooks/use-attachment-html-text", () => ({ + useAttachmentHtmlText: () => ({ + data: { text: htmlText }, + isLoading: false, + error: null, + }), +})); + +vi.mock("../i18n", () => ({ + useT: () => (fn: (d: Record) => string) => + fn({ attachment: { preview_loading: "l", preview_failed: "f" } }), +})); + +describe("AttachmentPreviewPage", () => { + it("injects the scroll bridge under the desktop adapter and keys the iframe on contentKey", () => { + const adapter: ScrollRestorationAdapter = { + get: () => undefined, + registerExternalSource: vi.fn(), + }; + const { container } = render( + + + , + ); + const iframe = container.querySelector("iframe")!; + expect(iframe).toBeTruthy(); + expect(iframe.getAttribute("srcdoc")).toContain( + buildScrollBridge(hashString(htmlText)), + ); + // React only exposes `key` via the reconciled element, but we can assert + // the bridge was built for the expected token — that's the signal the + // component derived contentKey from the text. + expect(adapter.registerExternalSource).toHaveBeenCalledWith( + "html-iframe", + expect.objectContaining({ capture: expect.any(Function) }), + ); + }); + + it("does NOT inject the bridge or register a source on web (adapter without registerExternalSource)", () => { + const webAdapter: ScrollRestorationAdapter = { get: () => undefined }; + const { container } = render( + + + , + ); + const iframe = container.querySelector("iframe")!; + expect(iframe.getAttribute("srcdoc")).not.toContain("__multica"); + expect(iframe.getAttribute("srcdoc")).not.toContain(buildScrollBridge("x")); + // Still applies the fragment-nav shim on both surfaces. + expect(iframe.getAttribute("srcdoc")).toContain("scrollIntoView"); + }); +}); diff --git a/packages/views/attachments/attachment-preview-page.tsx b/packages/views/attachments/attachment-preview-page.tsx index cf97b4ef3..aea928637 100644 --- a/packages/views/attachments/attachment-preview-page.tsx +++ b/packages/views/attachments/attachment-preview-page.tsx @@ -21,7 +21,7 @@ import { useEffect } from "react"; import { useT } from "../i18n"; import { useAttachmentHtmlText } from "../editor/hooks/use-attachment-html-text"; -import { withFragmentNavShim } from "../editor/utils/iframe-fragment-nav"; +import { useHtmlPreviewScrollRestore } from "./use-html-preview-scroll-restore"; interface AttachmentPreviewPageProps { attachmentId: string; @@ -37,13 +37,22 @@ export function AttachmentPreviewPage({ const { t } = useT("editor"); const query = useAttachmentHtmlText(attachmentId); + const text = query.data?.text; + + // Scroll-position restoration across desktop tab switches (multica-ai#6405). + // No-op on web (no desktop adapter). The iframe is keyed on contentKey so a + // content change (re-upload) structurally remounts a fresh document; the + // hook reports y=0 with the new key until that document scrolls, and + // messages from the previous document are dropped by token. + const { contentKey, buildSrcDoc, iframeRef, onLoad } = + useHtmlPreviewScrollRestore(text); + // Set document.title so desktop's MutationObserver-based tab title picks // up the filename. Web shows the same string in the browser tab. useEffect(() => { if (filename) document.title = filename; }, [filename]); - const text = query.data?.text; const isLoading = query.isLoading; const isError = !isLoading && (!!query.error || !text); @@ -62,7 +71,10 @@ export function AttachmentPreviewPage({ ) : (