fix(web): the split pane's Back returns to the previous item at the position it was read at (BUG-2182) (#1473)

* test(BUG-2182): reproduce pane Back returning to the top of the previous item

* fix(BUG-2182): the split pane's Back returns to the previous item at the position it was read at

* test(BUG-2182): the drill-after-Back race with a scrolled A, the case the saved position turns into a shallow entry

* test(BUG-2182): drop the scrolled race variant; the race it copies is flaky on main (filed separately)

web/e2e/bug-2182-pane-back-scroll.spec.ts

@@ -0,0 +1,97 @@
+import { test, expect } from './fixtures';
+import { browserLogin } from './lib/collab-helpers';
+import { deleteCollection } from './lib/attachment-viewer';
+
+/**
+ * BUG-2182 — the split pane's Back button (shown after following a link from
+ * item A to item B inside the pane) returned to A at the TOP, not at the scroll
+ * position A was read at.
+ */
+async function setup(page: import('@playwright/test').Page, fixture: import('./fixtures').SuiteFixture, request: import('@playwright/test').APIRequestContext) {
+	await page.setViewportSize({ width: 1400, height: 800 });
+	await browserLogin(page);
+	const headers = { Authorization: `Bearer ${fixture.apiToken}`, 'Content-Type': 'application/json' };
+	const stamp = Date.now();
+	const res = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/collections`, {
+		headers,
+		data: { name: `B2182 ${stamp}`, prefix: `BH${String(stamp).slice(-4)}` },
+	});
+	expect(res.ok(), await res.text()).toBeTruthy();
+	const coll = await res.json();
+	const titleB = `B2182 target ${stamp}`;
+	const b = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/collections/${coll.slug}/items`, {
+		headers, data: { title: titleB, content: Array.from({ length: 40 }, (_, i) => `Target paragraph ${i}.`).join('\n\n') },
+	});
+	expect(b.ok(), await b.text()).toBeTruthy();
+	const paras = Array.from({ length: 60 }, (_, i) => `Paragraph ${i} of a long item, long enough to scroll the pane.`).join('\n\n');
+	const a = await request.post(`/api/v1/workspaces/${fixture.workspaceSlug}/collections/${coll.slug}/items`, {
+		headers, data: { title: `B2182 source ${stamp}`, content: `${paras}\n\nSee [[${titleB}]].` },
+	});
+	expect(a.ok(), await a.text()).toBeTruthy();
+	const itemA = await a.json();
+
+	await page.goto(`/${fixture.adminUsername}/${fixture.workspaceSlug}/${coll.slug}?item=${itemA.slug}`);
+	// Scoped to the pane: an unscoped match finds B's CARD on the board behind it.
+	const pane = page.locator('.item-pane');
+	const link = pane.locator('a', { hasText: titleB }).last();
+	await expect(link).toBeAttached({ timeout: 15_000 });
+	await link.scrollIntoViewIfNeeded();
+	await page.waitForTimeout(300);
+	const scrollTop = () => pane.evaluate((el) => el.scrollTop);
+	const before = await scrollTop();
+	expect(before, 'precondition: the pane scrolled').toBeGreaterThan(300);
+
+	// A click on an editor link opens its popover; the popover's href drills.
+	await link.click();
+	await page.locator('.link-href').first().click();
+	await expect(page.locator('.pane-back-btn')).toBeVisible({ timeout: 10_000 });
+	await expect(pane.getByText('Target paragraph 0.')).toBeVisible();
+	await page.waitForTimeout(300);
+	return { coll, itemA, titleB, pane, before, scrollTop };
+}
+
+test('BUG-2182: pane Back returns to the previous item at its scroll position; the new item starts at the top', async ({ page, fixture, request }, testInfo) => {
+	test.skip(testInfo.project.name !== 'desktop-chromium', 'the split pane is a desktop layout');
+	test.setTimeout(60_000);
+	const ctx = await setup(page, fixture, request);
+	try {
+		// A cold entry (B, just drilled to, saved nothing) opens at the top.
+		expect(await ctx.scrollTop(), 'the drilled-to item did not open at the top').toBeLessThan(40);
+
+		await page.locator('.pane-back-btn').click();
+		await expect(ctx.pane.locator('a', { hasText: ctx.titleB }).last()).toBeAttached({ timeout: 10_000 });
+		await expect.poll(ctx.scrollTop, { timeout: 3000, message: 'pane Back did not restore the scroll position' })
+			.toBeGreaterThan(ctx.before - 40);
+		expect(await ctx.scrollTop()).toBeLessThan(ctx.before + 40);
+	} finally {
+		await deleteCollection(fixture, request, ctx.coll.slug);
+	}
+});
+
+test('BUG-2182: a reader gesture during the restore wait wins', async ({ page, fixture, request }, testInfo) => {
+	test.skip(testInfo.project.name !== 'desktop-chromium', 'the split pane is a desktop layout');
+	test.setTimeout(60_000);
+	const ctx = await setup(page, fixture, request);
+	try {
+		// Hold A's item read on the way back, so the restore is still WAITING
+		// for content when the reader acts.
+		let release!: () => void;
+		const gate = new Promise<void>((r) => (release = r));
+		await page.route(`**/items/${ctx.itemA.slug}**`, async (route) => {
+			await gate;
+			await route.continue();
+		});
+		await page.locator('.pane-back-btn').click();
+		await page.waitForTimeout(200);
+		const box = (await ctx.pane.boundingBox())!;
+		await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2);
+		await page.mouse.wheel(0, 100); // the reader's gesture, during the wait
+		release();
+		await expect(ctx.pane.locator('a', { hasText: ctx.titleB }).last()).toBeAttached({ timeout: 10_000 });
+		await page.waitForTimeout(1500); // past the restore cap
+		expect(await ctx.scrollTop(), 'the restore overrode the reader').toBeLessThan(ctx.before / 2);
+	} finally {
+		await page.unrouteAll({ behavior: 'ignoreErrors' });
+		await deleteCollection(fixture, request, ctx.coll.slug);
+	}
+});

web/src/app.d.ts

@@ -13,6 +13,8 @@ declare global {
 		interface PageState {
 			paneDepth?: number;
 			paneOwned?: boolean;
+			/** Pane scroll offset saved when drilling forward from this entry (BUG-2182). */
+			paneScrollTop?: number;
 		}
 		// interface Platform {}
 	}

web/src/lib/collections/paneController.ts

@@ -37,6 +37,19 @@ export interface PaneHistoryState {
 	 * shared `?item=` URL. Drills copy (inherit) this from the current entry.
 	 */
 	paneOwned?: boolean;
+	/**
+	 * The pane's scroll position when the user drilled FORWARD from this entry
+	 * (BUG-2182). Written into the entry being left, never into the new one, so
+	 * a Back/Forward traversal can put the reader where they were; an entry
+	 * without it (a cold load, a first open) stays at the top.
+	 */
+	paneScrollTop?: number;
+}
+
+/** The saved pane scroll position of an entry, or null when it has none. */
+export function readPaneScrollTop(state: PaneHistoryState | null | undefined): number | null {
+	const v = state?.paneScrollTop;
+	return typeof v === 'number' && Number.isFinite(v) && v > 0 ? Math.floor(v) : null;
 }
 
 /** The normalized, always-present form after {@link readPaneState}. */

web/src/lib/collections/paneHostController.ts

@@ -22,7 +22,7 @@
 // pane-follow, the list-row focus-return target, the unsaved-draft leave guard)
 // stays in the host and is handed in as a small set of injected callbacks.
 
-import { goto, afterNavigate } from '$app/navigation';
+import { goto, afterNavigate, replaceState } from '$app/navigation';
 import { page } from '$app/state';
 import { browser } from '$app/environment';
 import { itemUrlId, type Item, type PaneTarget } from '$lib/types';
@@ -32,6 +32,7 @@ import {
 	planLateralOpen,
 	planPaneClose,
 	type ResolvedPaneState,
+	type PaneHistoryState,
 } from '$lib/collections/paneController';
 import { resolvePaneTarget } from '$lib/collections/paneTarget';
 
@@ -57,6 +58,12 @@ export interface PaneControllerDeps {
 	 * `{#key itemSlug}` remount destroys the just-activated control (R1).
 	 */
 	focusPaneRegion: () => void;
+	/**
+	 * The pane's current scroll offset, or null with no pane (BUG-2182). Read
+	 * just before a forward drill and saved into the entry being left, so Back
+	 * can restore it. Optional: a host without a pane has nothing to report.
+	 */
+	getPaneScrollTop?: () => number | null;
 	/**
 	 * Capture the element that opened the pane, so an eventual close can return
 	 * focus to it (list-row fallback; collection page only). Called ONLY on a
@@ -295,6 +302,17 @@ export function createPaneController(deps: PaneControllerDeps): PaneController {
 		deps.cancelFollow();
 		const plan = planPaneDrill(deps.getOpenItemRef(), target, currentPaneState());
 		if (plan.kind === 'noop') return;
+		// BUG-2182: remember where the reader was in THIS item before leaving it,
+		// merged into the current entry's state through SvelteKit's own shallow
+		// routing (never a raw history.replaceState, which would clobber Kit's
+		// sveltekit:* keys). Only for a push: a capped drill REPLACES the entry,
+		// and the entry it would be saved on is the one being overwritten.
+		if (plan.kind === 'push') {
+			const top = deps.getPaneScrollTop?.() ?? null;
+			if (top !== null && top > 0) {
+				replaceState('', { ...(page.state as PaneHistoryState), paneScrollTop: top });
+			}
+		}
 		const url = new URL(page.url);
 		url.searchParams.set('item', target);
 		goto(`${url.pathname}${url.search}`, {

web/src/lib/components/collections/PaneHost.svelte

@@ -24,6 +24,8 @@
 	// openItemRef}` gate, which mounts/unmounts THIS whole component.
 	import { browser } from '$app/environment';
 	import { onDestroy, untrack } from 'svelte';
+	import { page } from '$app/state';
+	import { readPaneScrollTop } from '$lib/collections/paneController';
 	import ItemDetail from '$lib/components/items/ItemDetail.svelte';
 	import { viewport } from '$lib/stores/breakpoint.svelte';
 	import { paneOverlay } from '$lib/stores/paneOverlay.svelte';
@@ -111,6 +113,103 @@
 	// Exposed as a Svelte 5 instance export (PLAN-2154 Architecture E / TASK-2170)
 	// so the host can wire it as the controller's `focusPaneRegion` dep AND its
 	// list→pane Tab bridge, both via `paneHostEl?.focusPaneRegion()`.
+	/** The pane's scroll offset, for the controller's forward-drill save (BUG-2182). */
+	export function getPaneScrollTop(): number | null {
+		return paneEl ? paneEl.scrollTop : null;
+	}
+
+	// BUG-2182: a Back/Forward traversal onto an entry that saved a scroll
+	// position (see PaneHistoryState.paneScrollTop) puts the reader back there.
+	// Only on a POPSTATE (a traversal): page.state also changes on unrelated replaceState
+	// writes to the same entry, and restoring on those would yank the reader
+	// back to a position they have since scrolled away from.
+	//
+	// The item's content renders after the navigation, so the position is
+	// applied once the pane can scroll that far — waited for with a
+	// ResizeObserver on the pane's content, capped at ~1s (lead ruling). The
+	// reader wins: any scroll during the wait that is not our own, or a
+	// wheel/touch/key gesture, cancels it. Our own assignment happens only at
+	// the moment it succeeds, so the scroll event it raises is never mistaken
+	// for the reader's.
+	const RESTORE_CAP_MS = 1000;
+	let cancelRestore: (() => void) | null = null;
+
+	function restorePaneScroll(el: HTMLElement, target: number): () => void {
+		let done = false;
+		const finish = () => {
+			if (done) return;
+			done = true;
+			ro.disconnect();
+			clearTimeout(timer);
+			el.removeEventListener('scroll', onScroll);
+			for (const ev of GESTURES) el.removeEventListener(ev, finish);
+		};
+		const apply = () => {
+			if (done) return;
+			if (el.scrollHeight - el.clientHeight < target) return;
+			el.scrollTop = target;
+			finish();
+		};
+		// Before the restore lands, the pane sits at 0 while content grows, which
+		// raises no scroll event; a scroll now can only be the reader's.
+		const onScroll = () => {
+			if (!done && el.scrollTop > 2) finish();
+		};
+		const ro = new ResizeObserver(apply);
+		for (const child of Array.from(el.children)) ro.observe(child);
+		const timer = setTimeout(finish, RESTORE_CAP_MS);
+		el.addEventListener('scroll', onScroll, { passive: true });
+		for (const ev of GESTURES) el.addEventListener(ev, finish, { passive: true });
+		apply();
+		return finish;
+	}
+
+	// The window's popstate, not afterNavigate: saving the position makes the
+	// entry being left a SvelteKit SHALLOW entry (replaceState), and traversing
+	// back to a shallow entry updates page.state without running a navigation,
+	// so afterNavigate never fires for it (measured). SvelteKit applies the
+	// entry's state in its own popstate handler; the read waits a task for it.
+	//
+	// The restore then waits for ItemDetail's onReady(true) — the LOADED item
+	// matching the requested ref — before it starts. Starting at the popstate
+	// measured against the PREVIOUS item still on screen (it read B's height,
+	// then the swap collapsed it) and let the 1s cap run out on the network
+	// fetch. onReady is the same switch boundary the full-page scroll restore
+	// uses for this A→B→A case (BUG-1425).
+	let readyGen = 0;
+	let pendingRestore: { target: number; gen: number } | null = null;
+	const GESTURES = ['wheel', 'touchstart', 'keydown'] as const;
+
+	// A reader gesture while the item is still loading cancels the pending
+	// restore too, not only one during the height wait: the reader has already
+	// started moving, and jumping them afterwards is the same override.
+	function dropPending() {
+		pendingRestore = null;
+		for (const ev of GESTURES) paneEl?.removeEventListener(ev, dropPending);
+	}
+
+	function onPopState() {
+		cancelRestore?.();
+		cancelRestore = null;
+		dropPending();
+		setTimeout(() => {
+			const target = readPaneScrollTop(page.state);
+			if (target === null || !paneEl) return;
+			pendingRestore = { target, gen: readyGen };
+			for (const ev of GESTURES) paneEl.addEventListener(ev, dropPending, { passive: true });
+		}, 0);
+	}
+
+	function handleItemReady(ready: boolean) {
+		if (!ready) return;
+		readyGen++;
+		const p = pendingRestore;
+		if (!p || readyGen <= p.gen || !paneEl) return;
+		dropPending();
+		cancelRestore = restorePaneScroll(paneEl, p.target);
+	}
+	if (browser) window.addEventListener('popstate', onPopState);
+
 	export function focusPaneRegion() {
 		if (!browser) return;
 		if (!openItemRef || !paneEl) return;
@@ -481,6 +580,9 @@
 	// this from an `openItemRef`→null effect — the shell's unmount IS that
 	// transition now).
 	onDestroy(() => {
+		cancelRestore?.();
+		dropPending();
+		if (browser) window.removeEventListener('popstate', onPopState);
 		if (resizingPane && browser) {
 			document.body.style.userSelect = '';
 			document.body.style.cursor = '';
@@ -656,6 +758,7 @@
 		<ItemDetail
 			ref={paneMintForRoute}
 			embedded
+			onReady={handleItemReady}
 			peeking={activePane === 'master'}
 			{username}
 			{wsSlug}

web/src/routes/[username]/[workspace]/[collection]/+page.svelte

@@ -683,6 +683,7 @@
 		getOpenItemRef: () => openItemRef,
 		cancelFollow: () => cancelPaneFollow(),
 		focusPaneRegion: () => paneHostEl?.focusPaneRegion(),
+		getPaneScrollTop: () => paneHostEl?.getPaneScrollTop() ?? null,
 		captureReturnFocus,
 		setBypassNavGuard: (bypass) => {
 			bypassNavGuard = bypass;

web/src/routes/[username]/[workspace]/[collection]/[slug]/+page.svelte

@@ -168,6 +168,7 @@
 		},
 		captureReturnFocus: () => {},
 		setBypassNavGuard: () => {},
+		getPaneScrollTop: () => paneHostEl?.getPaneScrollTop() ?? null,
 	});
 	const {
 		openItemPaneByRef,