diff --git a/web/e2e/attachment-viewer-zoom.spec.ts b/web/e2e/attachment-viewer-zoom.spec.ts new file mode 100644 index 00000000..38026fd7 --- /dev/null +++ b/web/e2e/attachment-viewer-zoom.spec.ts @@ -0,0 +1,628 @@ +import { test, expect } from './fixtures'; +import type { SuiteFixture } from './fixtures'; +import type { Page, APIRequestContext } from '@playwright/test'; +import { browserLogin, seedDoc } from './lib/collab-helpers'; +import { + BIG_PNG, + DESKTOP, + HUGE_PNG, + MID_PNG, + TILE, + VIEWER, + VIEWER_IMAGE, + VIEWER_STAGE, + WIDE_PNG, + imageRect, + itemUrl, + renderedScale, + uploadAttachment, + viewerClose, + viewerTapLoad +} from './lib/attachment-viewer'; + +/** + * THE VIEWER'S ZOOM / PAN / LOADING BEHAVIOUR, IN A REAL BROWSER + * (PLAN-2392 phase 3b / TASK-2461, DR-9). + * + * jsdom has no layout, no CSS and no gestures, so the unit suite proves the + * zoom/pan MATH (zoom.test.ts) and the loader's request POLICY + * (viewerImageLoader.svelte.test.ts) but never that a wheel/key/drag reaches + * that math, that the transform actually paints, that a clamp holds against real + * geometry, or that a control stays clickable under a scaled image. Each test + * here is written against "what mutation would this catch that a jsdom-equivalent + * would survive?" — named at each test. + */ + +const BIG_W = 1600; + +/** + * Open the viewer on a >1024px image and wait until the ORIGINAL has painted + * (naturalWidth === 1600), so geometry is settled past the thumb→original + * upgrade and every zoom/pan measurement below is deterministic. + */ +async function openBig( + page: Page, + fixture: SuiteFixture, + request: APIRequestContext, + title: string, + names: string[] = ['big.png'], + buffer: Buffer = BIG_PNG, + naturalWidth = BIG_W +): Promise { + await browserLogin(page); + const doc = await seedDoc(fixture, request, title); + for (const name of names) { + await uploadAttachment(fixture, request, doc.id, name, 'image/png', buffer); + } + await page.goto(itemUrl(fixture, doc.slug)); + await expect(page.locator(TILE).first()).toBeVisible(); + await page.locator(TILE).first().click(); + await expect(page.locator(VIEWER_IMAGE)).toBeVisible(); + // Settle on the original — the final, stable geometry. + await expect + .poll(() => page.locator(VIEWER_IMAGE).evaluate((el) => (el as HTMLImageElement).naturalWidth)) + .toBe(naturalWidth); +} + +/** Read the RENDERED scale once the CSS transition has settled (two equal reads). */ +async function settledScale(page: Page): Promise { + let last = Number.NaN; + await expect + .poll(async () => { + const s = await renderedScale(page); + const stable = Math.abs(s - last) < 1e-3; + last = s; + return stable; + }) + .toBe(true); + return renderedScale(page); +} + +/** + * Zoom to maximum by pressing '+' ONE step at a time, settling each transition, + * until the scale stops climbing (the clamp). Per-step settling avoids the + * rapid-press race where two mid-animation reads look equal and stop early. + */ +async function zoomToMax(page: Page): Promise { + let prev = await settledScale(page); + for (let i = 0; i < 20; i++) { + await page.keyboard.press('+'); + // Wait for THIS press to take effect (the scale climbs past `prev`), up to a + // deadline; if it never climbs, we are at the clamp. This avoids the race + // where the press hasn't been processed yet and two equal reads look settled. + const deadline = Date.now() + 1500; + let climbed = false; + while (Date.now() < deadline) { + if ((await renderedScale(page)) > prev + 1e-3) { + climbed = true; + break; + } + } + if (!climbed) return prev; // clamp reached + prev = await settledScale(page); + } + return prev; +} + +test.describe('attachment viewer — desktop zoom & pan (TASK-2461)', () => { + test.beforeEach(async ({ page }, testInfo) => { + test.skip( + testInfo.project.name !== 'desktop-chromium', + 'desktop zoom/pan is viewport-driven; one project is enough' + ); + await page.setViewportSize(DESKTOP); + }); + + test('wheel, ctrl-wheel, keyboard and double-click each transform the RENDERED image', async ({ + page, + fixture, + request + }) => { + // jsdom has no layout, so `getComputedStyle(img).transform` is never a real + // matrix there — a handler wired to nothing (or to a no-op `zoomTo`) passes + // every unit test. Here the proof is the painted matrix moving off identity. + await openBig(page, fixture, request, 'Zoom transforms'); + const stage = page.locator(VIEWER_STAGE); + const box = (await stage.boundingBox())!; + const cx = box.x + box.width / 2; + const cy = box.y + box.height / 2; + + expect(await renderedScale(page), 'opens at fit (scale 1)').toBeCloseTo(1, 1); + + // WHEEL (plain) in. + await page.mouse.move(cx, cy); + await page.mouse.wheel(0, -120); + await expect.poll(() => renderedScale(page)).toBeGreaterThan(1.05); + const afterWheel = await renderedScale(page); + + // KEYBOARD '0' resets to fit. + await page.keyboard.press('0'); + await expect.poll(() => renderedScale(page)).toBeCloseTo(1, 1); + + // KEYBOARD '+' in. + await page.keyboard.press('+'); + await expect.poll(() => renderedScale(page)).toBeGreaterThan(1.05); + await page.keyboard.press('0'); + + // CTRL+WHEEL in — a genuinely separate gesture path (Control HELD). We assert + // it zooms the IMAGE; that the viewer's `preventDefault` also suppresses the + // browser's own ctrl-wheel page-zoom is not reliably observable through + // Playwright (page zoom does not move visualViewport.scale), so it is left to + // the unit coverage of the non-passive listener. + expect(afterWheel, 'the plain-wheel leg zoomed').toBeGreaterThan(1.05); + await page.mouse.move(cx, cy); + await page.keyboard.down('Control'); + await page.mouse.wheel(0, -120); + await page.keyboard.up('Control'); + await expect.poll(() => renderedScale(page)).toBeGreaterThan(1.05); + await page.keyboard.press('0'); + + // DOUBLE-CLICK toggles fit → actual (a jump well past 1). + await page.mouse.dblclick(cx, cy); + await expect.poll(() => renderedScale(page)).toBeGreaterThan(1.05); + }); + + test('the close and nav controls stay clickable and focus-visible AT MAXIMUM ZOOM', async ({ + page, + fixture, + request + }) => { + // The state where a transformed image can paint over the chrome and swallow + // its clicks. jsdom cannot see the stacking or the hit test, so only a real + // engine proves the controls survive a full-scale image on top of the stage. + // Two images so the nav buttons exist alongside close. + await openBig(page, fixture, request, 'Controls at max zoom', ['ctrl-a.png', 'ctrl-b.png']); + const scale = await zoomToMax(page); + expect(scale, 'reached a real maximum well past fit').toBeGreaterThan(2); + + // HIT TEST — the point proof that the scaled image does not paint over the + // controls: `elementFromPoint` at each control's centre must return that + // control, not the IMG. jsdom cannot do this (no layout, no stacking). + const topAt = (sel: string) => + page.evaluate((s) => { + const front = [...document.querySelectorAll('.attachment-viewer')].at(-1); + const el = front?.querySelector(s.replace('.attachment-viewer[role="dialog"] ', '')); + if (!el) return 'missing'; + const r = el.getBoundingClientRect(); + const hit = document.elementFromPoint(r.x + r.width / 2, r.y + r.height / 2); + return hit ? `${hit.tagName}.${(hit.closest('button')?.className ?? hit.className).split(' ').find((c) => c.startsWith('lightbox-')) ?? ''}` : 'none'; + }, sel); + expect(await topAt(`${VIEWER} .lightbox-close`), 'close is the top surface at its centre').toContain('lightbox-close'); + expect(await topAt(`${VIEWER} .lightbox-nav.next`), 'next is the top surface at its centre').toContain('lightbox-nav'); + + // TAB CYCLES at max zoom — focus never escapes the viewer. + const close = viewerClose(page); + await close.focus(); + await expect(close).toBeFocused(); + for (let i = 0; i < 4; i++) { + await page.keyboard.press('Tab'); + expect( + await page.evaluate(() => !!document.activeElement?.closest('.attachment-viewer')), + `Tab ${i + 1} kept focus inside the viewer at max zoom` + ).toBe(true); + } + + // FOCUS-VISIBLE: the focused control shows an indicator, not `none`. + await close.focus(); + const outline = await close.evaluate((el) => { + const cs = getComputedStyle(el); + return { outlineWidth: cs.outlineWidth, boxShadow: cs.boxShadow }; + }); + expect( + outline.outlineWidth !== '0px' || outline.boxShadow !== 'none', + 'the focused control must show a visible focus ring at max zoom' + ).toBe(true); + + await close.click(); + await expect(page.locator(VIEWER)).toHaveCount(0); + }); + + test('a wheel-zoom keeps the ANCHORED point under the cursor', async ({ page, fixture, request }) => { + // The property that separates pointer-anchored zoom (TASK-2457) from + // centre-anchored: the image content under the cursor stays under it. jsdom + // has no cursor and no rect, so the anchor math has never been exercised + // against real geometry. Mutating the anchor to the stage centre drifts the + // point away and fails this. + await openBig(page, fixture, request, 'Zoom anchor'); + const stage = (await page.locator(VIEWER_STAGE).boundingBox())!; + // Off-centre, so a centre-anchored zoom would visibly move this point. + const px = stage.x + stage.width * 0.35; + const py = stage.y + stage.height * 0.4; + const before = await imageRect(page); + const fx = (px - before.x) / before.width; + const fy = (py - before.y) / before.height; + + await page.mouse.move(px, py); + await page.mouse.wheel(0, -120); // one step in + await settledScale(page); + const after = await imageRect(page); + // The same image-fractional point, mapped through the NEW rect, is still + // under the cursor (uniform scale ⇒ linear map). + expect(Math.abs(after.x + fx * after.width - px), 'anchored X held').toBeLessThan(3); + expect(Math.abs(after.y + fy * after.height - py), 'anchored Y held').toBeLessThan(3); + }); + + test('pan CLAMPS: an in-bounds drag moves by the delta; an over-drag stops at the edge', async ({ + page, + fixture, + request + }) => { + // TWO legs on purpose (TASK-2461): a one-legged "it panned" passes with the + // clamp disabled, and a one-legged "it stopped" passes with pan disabled + // entirely. jsdom clamps against all-zero geometry, so neither leg has run + // against real bounds. + await openBig(page, fixture, request, 'Pan clamp'); + await zoomToMax(page); // heavy overflow ⇒ real pan room + const stage = (await page.locator(VIEWER_STAGE).boundingBox())!; + const cx = stage.x + stage.width / 2; + const cy = stage.y + stage.height / 2; + + // Settle at the centred max-zoom position before measuring. + const r0 = await imageRect(page); + // POSITIVE leg — an 80px in-bounds drag moves the image ~80px to the RIGHT + // (the pan follows the pointer). A small hold at the start engages the drag + // cleanly past the 4px threshold. + await page.mouse.move(cx, cy); + await page.mouse.down(); + await page.mouse.move(cx + 20, cy, { steps: 4 }); // engage past threshold + await page.mouse.move(cx + 80, cy, { steps: 8 }); + await page.mouse.up(); + const r1 = await imageRect(page); + expect(r1.x - r0.x, 'an in-bounds pan moved the image right by ~the drag delta').toBeGreaterThan(60); + expect(r1.x - r0.x).toBeLessThan(100); + + // EDGE leg — a huge drag clamps at the stage edge, and dragging FURTHER in + // the same direction moves it no more. (Two drags, because "it stopped at the + // edge" is only proof of a clamp if a further push is a no-op.) + await page.mouse.move(cx, cy); + await page.mouse.down(); + await page.mouse.move(cx + 6000, cy, { steps: 12 }); + await page.mouse.up(); + const r2 = await imageRect(page); + expect(Math.abs(r2.x - stage.x), 'the image left edge clamps to the stage left').toBeLessThan(3); + await page.mouse.move(cx, cy); + await page.mouse.down(); + await page.mouse.move(cx + 3000, cy, { steps: 8 }); + await page.mouse.up(); + const r3 = await imageRect(page); + expect(Math.abs(r3.x - r2.x), 'at the clamp, more drag does not move the image').toBeLessThan(2); + }); + + test('dismissal: a plain backdrop click closes; a drag that releases over the backdrop does NOT', async ({ + page, + fixture, + request + }) => { + // TASK-2458's "a drag that ends where it started is a click": a press that + // MOVES past threshold and RETURNS to its start on the backdrop synthesizes a + // real click at that point — and must be SUPPRESSED (a drag is not a + // dismissal). A drag that merely ended elsewhere would produce no click at + // all, so it would prove nothing about the suppress path; returning to the + // start is what makes leg 2 depend on the viewer's own logic. jsdom has no + // drag. Both legs, because the no-close leg alone passes against a viewer that + // never closes on any click. + await openBig(page, fixture, request, 'Dismiss drag'); + const stage = (await page.locator(VIEWER_STAGE).boundingBox())!; + // A point in the left backdrop margin, clear of the fitted image (openBig + // uploads one image, so there are no nav buttons to hit here). + const px = Math.max(2, Math.round(stage.x / 2)); + const py = Math.round(stage.y + stage.height / 2); + + // LEG 2 first (it must NOT close): press, drag past threshold, return to the + // SAME backdrop point, release → a synthesized backdrop click that the drag + // must suppress. + await page.mouse.move(px, py); + await page.mouse.down(); + await page.mouse.move(px + 140, py, { steps: 6 }); // past the 4px threshold + await page.mouse.move(px, py, { steps: 6 }); // ...back to the start + await page.mouse.up(); + await expect( + page.locator(VIEWER), + 'a drag that returns to its start is not a dismissal' + ).toHaveCount(1); + + // LEG 1 — a PLAIN backdrop click at the same point DOES dismiss. + await page.mouse.click(px, py); + await expect(page.locator(VIEWER), 'a plain backdrop click dismisses').toHaveCount(0); + }); + + test('a click on BLANK STAGE SPACE inside the stage box but outside the image dismisses', async ({ + page, + fixture, + request + }) => { + // THE UNCOVERED POINT (TASK-2461). The existing suite clicks (4,4) — outside + // the centred stage — so a stage that swallowed letterbox clicks would stay + // green. A wide image is letterboxed top/bottom; a click in that band is + // INSIDE the stage box, off the image, and must reach the backdrop and close + // (the stage letterbox is `pointer-events: none`). + await openBig(page, fixture, request, 'Dismiss letterbox', ['wide.png'], WIDE_PNG, 1600); + const stage = (await page.locator(VIEWER_STAGE).boundingBox())!; + const img = await imageRect(page); + // A point inside the stage box, above the letterboxed image. + const bandY = (stage.y + img.top) / 2; + expect(bandY, 'there is a real letterbox band above the image').toBeLessThan(img.top - 5); + await page.mouse.click(stage.x + stage.width / 2, bandY); + await expect(page.locator(VIEWER)).toHaveCount(0); + }); + + test('resizing the window while zoomed to MAXIMUM clamps the scale down to the new bound', async ({ + page, + fixture, + request + }) => { + // `maxScale` is geometry-dependent: ENLARGING the window lowers `actualScale` + // and with it the ceiling, stranding a previously-valid scale above it + // (TASK-2455). jsdom fires no resize with real geometry, so the re-clamp + // effect has never run against a live layout change. + await openBig(page, fixture, request, 'Resize clamp'); + const maxBefore = await zoomToMax(page); + await page.setViewportSize({ width: DESKTOP.width + 500, height: DESKTOP.height + 400 }); + // The ResizeObserver re-clamp fires async on layout — poll until the stranded + // scale is pulled down to the new, lower ceiling. + await expect.poll(() => renderedScale(page)).toBeLessThan(maxBefore - 0.2); + const after = await settledScale(page); + // Clamped DOWN to the new ceiling — but NOT reset to fit: a handler that + // snapped to `resetZoom()` would also be below the old max, so the "still + // zoomed" leg is what makes this a clamp and not a reset. + expect(after, 'the image is still zoomed in, not reset to fit').toBeGreaterThan(1.5); + // And it sits at the NEW maximum, not some arbitrary lower value: pressing '+' + // again does not climb (already at the clamp). + await page.keyboard.press('+'); + expect(await settledScale(page), 'the clamped scale IS the new maximum').toBeCloseTo(after, 1); + }); + + test('reduced-motion suppresses the zoom ANIMATION, and normal mode keeps it', async ({ + page, + fixture, + request + }) => { + // TWO legs (TASK-2461): the one-legged "reduced-motion has no transition" + // passes against a viewer that never animates at all. `Modal.svelte:207` is + // the precedent this follows. + await openBig(page, fixture, request, 'Reduced motion'); + const transitionDuration = () => + page.locator(VIEWER_IMAGE).evaluate((el) => getComputedStyle(el).transitionDuration); + + await page.emulateMedia({ reducedMotion: 'reduce' }); + expect(await transitionDuration(), 'reduced-motion: no zoom animation').toBe('0s'); + + await page.emulateMedia({ reducedMotion: 'no-preference' }); + expect( + Number.parseFloat(await transitionDuration()), + 'normal mode: the zoom DOES animate' + ).toBeGreaterThan(0); + }); + + test('forced-colors keeps the controls and the image BOUNDARY visible (computed style)', async ({ + page, + fixture, + request + }) => { + // DR-4. Under forced-colors the custom palette is discarded and the image's + // box-shadow BOUNDARY vanishes; the media block's explicit border is what + // keeps it visible. Asserted on COMPUTED STYLE (a border with real width), + // not DOM presence. jsdom has no forced-colors media and computes no border. + // + // The IMAGE border is the mutation-load-bearing proof: the base `.lightbox- + // image` has NO border, so its width>0 here comes ONLY from the media block + // (removing that rule fails this — verified). The CONTROLS already carry a + // base 1px border that forced-colors re-colours to a system colour, so they + // are visible regardless; the control leg is a genuine "still visible" check, + // not a proof of the media rule (border COLOUR under forced-colors resolves + // to opaque system values that are not worth pinning). + await openBig(page, fixture, request, 'Forced colors', ['fc-a.png', 'fc-b.png']); + await page.emulateMedia({ forcedColors: 'active' }); + const styles = await page.evaluate(() => { + const front = [...document.querySelectorAll('.attachment-viewer')].at(-1); + const get = (sel: string) => { + const el = front?.querySelector(sel); + if (!el) return null; + const cs = getComputedStyle(el); + return { w: Number.parseFloat(cs.borderTopWidth), style: cs.borderTopStyle }; + }; + return { img: get('.lightbox-image'), close: get('.lightbox-close'), next: get('.lightbox-nav.next') }; + }); + expect(styles.img, 'the image element exists').not.toBeNull(); + expect(styles.img!.w, 'the image keeps a visible boundary under forced-colors').toBeGreaterThan(0); + expect(styles.img!.style).toBe('solid'); + expect(styles.close!.w, 'the close control stays visible (bordered)').toBeGreaterThan(0); + expect(styles.next!.w, 'the nav control stays visible (bordered)').toBeGreaterThan(0); + }); +}); + +/** The viewer image's current `src` attribute (the URL the browser is loading). */ +function imageSrc(page: Page): Promise { + return page.locator(VIEWER_IMAGE).evaluate((el) => el.getAttribute('src') ?? ''); +} +/** The viewer image's decoded natural width — the observable of which variant painted. */ +function imageNat(page: Page): Promise { + return page.locator(VIEWER_IMAGE).evaluate((el) => (el as HTMLImageElement).naturalWidth); +} + +/** + * Intercept the attachment DOWNLOAD requests so the thumb→original swap is + * deterministic: `?variant=thumb-md` → a bounded 800px thumb, the no-variant + * original → the 1600px BIG_PNG (optionally delayed). Other variants — the + * strip's own tile thumbnail — fall through to the real server. The upload + * itself goes through the Node API context, not the page, so it is never + * intercepted and the server records the real 1600×1200 metadata. + */ +async function stubVariants(page: Page, originalDelayMs = 0): Promise { + await page.route('**/attachments/*', async (route) => { + const url = new URL(route.request().url()); + if (!/\/attachments\/[^/?]+$/.test(url.pathname)) return route.continue(); + const variant = url.searchParams.get('variant'); + if (variant === 'thumb-md') return route.fulfill({ contentType: 'image/png', body: MID_PNG }); + if (!variant) { + if (originalDelayMs) await new Promise((r) => setTimeout(r, originalDelayMs)); + return route.fulfill({ contentType: 'image/png', body: BIG_PNG }); + } + return route.continue(); + }); +} + +test.describe('attachment viewer — desktop loading policy (TASK-2461)', () => { + test.beforeEach(async ({ page }, testInfo) => { + test.skip(testInfo.project.name !== 'desktop-chromium', 'desktop policy; one project is enough'); + await page.setViewportSize(DESKTOP); + }); + + test('the thumb→original swap is observable against a >1024px fixture', async ({ page, fixture, request }) => { + // DR-5b: a >1024px image paints the bounded thumb first, then upgrades to the + // original in the background. jsdom proves the request POLICY but never that + // the browser actually swaps the painted bitmap. A small original delay makes + // the thumb phase observable rather than a race. + // Record the viewer's OWN download requests (the tile's thumb-sm is filtered + // out). The swap is the request sequence thumb-md → original; the bitmap + // naturalWidth window at 800 is too brief to sample (the upgrade swaps `src` + // the instant the thumb decodes), so the network order is the reliable proof. + // An ordered timeline of the viewer's own attachment events (the tile's + // thumb-sm is filtered out). The swap PROOF is: the thumb-md response + // FINISHED before the original request STARTED — because the loader only + // requests the original from `decoded()`, i.e. after the thumb's bytes + // arrived AND painted. A mere request-order check would pass an impl that + // fired both immediately; the finish→start ordering rules that out. + const timeline: string[] = []; + const isViewerReq = (u: string) => + /\/attachments\/[^/?]+(\?variant=(thumb-md|original))?$/.test(u) && !u.includes('thumb-sm'); + page.on('request', (r) => { + if (isViewerReq(r.url())) timeline.push((r.url().includes('variant=thumb-md') ? 'thumb-md' : 'original') + ':start'); + }); + page.on('requestfinished', (r) => { + if (isViewerReq(r.url())) timeline.push((r.url().includes('variant=thumb-md') ? 'thumb-md' : 'original') + ':finish'); + }); + await stubVariants(page, 400); + await browserLogin(page); + const doc = await seedDoc(fixture, request, 'Swap'); + await uploadAttachment(fixture, request, doc.id, 'swap.png', 'image/png', BIG_PNG); + await page.goto(itemUrl(fixture, doc.slug)); + await page.locator(TILE).first().click(); + await expect(page.locator(VIEWER_IMAGE)).toBeVisible(); + + // The final painted bitmap is the ORIGINAL (naturalWidth 1600, no variant). + await expect.poll(() => imageNat(page)).toBe(1600); + expect(await imageSrc(page)).not.toContain('variant='); + // The thumb was requested first, its response FINISHED (bytes arrived, the + // bitmap painted), and ONLY THEN the original was requested — the upgrade. + const thumbStart = timeline.indexOf('thumb-md:start'); + const thumbFinish = timeline.indexOf('thumb-md:finish'); + const origStart = timeline.indexOf('original:start'); + expect(thumbStart, 'the viewer requested the bounded thumb').toBeGreaterThanOrEqual(0); + expect(thumbFinish, 'the thumb response arrived (the bitmap could paint)').toBeGreaterThan(thumbStart); + expect(origStart, 'the original was requested only AFTER the thumb painted').toBeGreaterThan(thumbFinish); + }); + + test('a rapid A→B→A with a slow original leaves the LIVE image correct (switch-safety / generation fence)', async ({ + page, + fixture, + request + }) => { + // TASK-2459's carried-forward concern, proven in a real engine. The originals + // are SLOW, so a rapid A→B→A navigates away while A's first original is still + // in flight: the first A element is torn down (the `{#key loadToken}` remount) + // with a pending load, and a fresh element takes over for the third visit. The + // live A must end on ITS OWN original — not B's content, not a broken image, + // not an error — which is what the per-mount generation fence + load key + // guarantee. jsdom has no real network timing to stage this. + // + // NOTE (honest scope): Chromium ABORTS a detached 's in-flight request, + // so the exact "detached element fires a late ERROR that clobbers the live + // one" case the data-gen fence guards is not reachable in a browser — the + // engine cancels first. That case is covered by the unit tests + // (viewerImageLoader / Lightbox); this leg proves the rapid-nav END-STATE + // integrity, the practical half of the concern. + await browserLogin(page); + const doc = await seedDoc(fixture, request, 'Gen fence'); + const aId = await uploadAttachment(fixture, request, doc.id, 'gen-a.png', 'image/png', BIG_PNG); + const bId = await uploadAttachment(fixture, request, doc.id, 'gen-b.png', 'image/png', BIG_PNG); + + // thumb-md → the bounded thumb (fast); the original → BIG, but SLOW, so the + // nav races the in-flight upgrade. + await stubVariants(page, 1500); + + // Persistent listener (set up BEFORE navigation, so it can't miss the fast + // upgrade request the way a late `waitForRequest` would). + let aOriginalRequested = false; + page.on('request', (r) => { + if (r.url().includes(aId) && !r.url().includes('variant=')) aOriginalRequested = true; + }); + await page.goto(itemUrl(fixture, doc.slug)); + await expect(page.locator(TILE)).toHaveCount(2); + await page.locator(`${TILE}[aria-label*="gen-a.png"]`).click(); + await expect(page.locator(VIEWER)).toHaveCount(1); + // A's (slow) original request is in flight before we navigate. + await expect.poll(() => aOriginalRequested).toBe(true); + + // A→B→A: the first A element is torn down mid-load; a fresh one takes over. + await page.keyboard.press('ArrowRight'); + await expect.poll(() => imageSrc(page)).toContain(bId); // showing B + await page.keyboard.press('ArrowLeft'); + await expect.poll(() => imageSrc(page)).toContain(aId); // back to A (fresh element) + + // The LIVE A resolves to ITS OWN original — 1600px, A's URL, no error, no + // stale B content and no broken bitmap. + await expect.poll(() => imageNat(page)).toBe(1600); + expect(await imageSrc(page), 'the live image is A, not the B we passed through').toContain(aId); + expect(await imageSrc(page)).not.toContain(bId); + expect(await imageSrc(page)).not.toContain('variant='); + await expect(page.locator(`${VIEWER} .lightbox-error`), 'no stale load clobbered the live image').toHaveCount(0); + }); +}); + +test.describe('attachment viewer — mobile DR-5b (TASK-2461)', () => { + test.beforeEach(async ({}, testInfo) => { + test.skip( + testInfo.project.name !== 'mobile-chromium', + 'the DR-5b mobile policy needs a real mobile (Pixel 7) project' + ); + }); + + test('a large image issues NO automatic request until the tap affordance is used', async ({ + page, + fixture, + request + }) => { + // DR-5b mobile (TASK-2460): a large image auto-fetches NOTHING — a tap-to-load + // affordance stands in, and only a real tap issues the original request. The + // no-request half is the assertion that fails if the deferral is dropped; the + // tap half proves the affordance is a live, hit-testable control (not pointer- + // dead under the stage's `pointer-events: none`). Needs a real mobile project: + // the policy keys on `viewport.isMobile`. + let originalRequests = 0; + await page.route('**/attachments/*', async (route) => { + const url = new URL(route.request().url()); + if (/\/attachments\/[^/?]+$/.test(url.pathname) && !url.searchParams.get('variant')) { + originalRequests++; + return route.fulfill({ contentType: 'image/png', body: BIG_PNG }); + } + if (url.searchParams.get('variant') === 'thumb-md') + return route.fulfill({ contentType: 'image/png', body: MID_PNG }); + return route.continue(); + }); + await browserLogin(page); + const doc = await seedDoc(fixture, request, 'Mobile deferred'); + // A genuinely large image (>8 MP) so mobile classifies it as the DEFERRED + // cell — a 1.9 MP image is only the mobile thumb cell. + await uploadAttachment(fixture, request, doc.id, 'mobile-huge.png', 'image/png', HUGE_PNG); + await page.goto(itemUrl(fixture, doc.slug)); + await page.locator(TILE).first().click(); + await expect(page.locator(VIEWER)).toHaveCount(1); + + // The deferred cell: the tap affordance is shown, NO image, NO original fetch. + await expect(viewerTapLoad(page)).toBeVisible(); + await expect(page.locator(VIEWER_IMAGE)).toHaveCount(0); + expect(originalRequests, 'no original fetched before the tap').toBe(0); + + // A REAL tap loads the original — the request fires, the bitmap DECODES + // (naturalWidth settles), and no error state appears. + await viewerTapLoad(page).tap(); + await expect(page.locator(VIEWER_IMAGE)).toBeVisible(); + await expect.poll(() => imageNat(page)).toBe(1600); // the stubbed original decoded + expect(await imageSrc(page)).not.toContain('variant='); + await expect(page.locator(`${VIEWER} .lightbox-error`)).toHaveCount(0); + // EXACTLY one original request — the tap loads it once (dedup), not zero and + // not a double-fetch. + expect(originalRequests, 'the tap issued exactly one original request').toBe(1); + }); +}); diff --git a/web/e2e/lib/attachment-viewer.ts b/web/e2e/lib/attachment-viewer.ts index ce5ed1d6..8c53cea2 100644 --- a/web/e2e/lib/attachment-viewer.ts +++ b/web/e2e/lib/attachment-viewer.ts @@ -56,6 +56,40 @@ function buildPng(width: number, height: number, rgb: [number, number, number]): export const REAL_PNG = buildPng(200, 150, [200, 120, 60]); +/** + * A decodable PNG whose LONG EDGE EXCEEDS 1024 px (TASK-2461). The server derives + * a `thumb-md` (1024 px long edge) only when the source is bigger, and the DR-5b + * loader paints that thumb first and upgrades to this original — so the swap is + * observable only against a fixture this size. Kept flat-colour so `deflate` + * keeps it small despite the pixel count. + */ +export const BIG_PNG = buildPng(1600, 1200, [40, 110, 190]); + +/** + * A >1024px WIDE fixture (4:1). Fitted into the 4:3 desktop stage it is + * letterboxed top and bottom, so there is blank stage space INSIDE the stage box + * but outside the image — the click target the dismiss test needs (TASK-2461). + */ +export const WIDE_PNG = buildPng(1600, 400, [190, 90, 40]); + +/** + * A bounded (<= 1024px long edge) PNG to stand in for the server's `thumb-md` + * variant under route interception (TASK-2461): the loader's fallback detector + * treats a decode this size as a real thumbnail and upgrades, so serving it for + * `?variant=thumb-md` and {@link BIG_PNG} for the original makes the swap + * deterministic and observable (a 800→1600 naturalWidth jump). + */ +export const MID_PNG = buildPng(800, 600, [90, 170, 90]); + +/** + * A genuinely LARGE fixture — 4000×2500 = 10 MP, OVER the 8 MP + * `AUTO_LOAD_MAX_PIXELS` — so the DR-5b classifier calls it `large`, not just + * `small`-but-long. On mobile that is the DEFERRED cell (tap-to-load, no + * automatic request); a 1.9 MP image is only the mobile thumb cell (TASK-2461). + * Flat colour keeps the deflated bytes tiny despite the pixel count. + */ +export const HUGE_PNG = buildPng(4000, 2500, [150, 60, 150]); + export const DESKTOP = { width: 1200, height: 900 }; /** Below the 639.98px mobile breakpoint — BottomNav / DockedSheet branch. */ export const MOBILE = { width: 390, height: 844 }; @@ -71,6 +105,8 @@ export const TILE = `${STRIP} .att-tile`; export const VIEWER = '.attachment-viewer[role="dialog"]'; export const VIEWER_IMAGE = `${VIEWER} .lightbox-image`; export const VIEWER_COUNTER = `${VIEWER} .lightbox-counter`; +/** The zoom/pan stage (the letterbox around the image). */ +export const VIEWER_STAGE = `${VIEWER} .lightbox-stage`; /** The viewer's controls, addressed by the accessible names TASK-2429 gave them. */ export const viewerClose = (page: Page) => page.locator(VIEWER).getByRole('button', { name: 'Close' }); @@ -78,6 +114,40 @@ export const viewerNext = (page: Page) => page.locator(VIEWER).getByRole('button', { name: 'Next image' }); export const viewerPrev = (page: Page) => page.locator(VIEWER).getByRole('button', { name: 'Previous image' }); +/** The DR-5b mobile tap-to-load affordance (TASK-2460 / TASK-2461). */ +export const viewerTapLoad = (page: Page) => + page.locator(VIEWER).getByRole('button', { name: 'Tap to load full image' }); + +/** + * The `scale(...)` factor the browser has actually applied to the viewer image, + * read from its COMPUTED transform matrix (`matrix(a, …)`, a === scale). NaN when + * there is no image or no transform — the thing jsdom cannot produce, since it + * has no layout and computes no matrix. + */ +export function renderedScale(page: Page, selector = VIEWER_IMAGE): Promise { + return page.evaluate((sel) => { + // Last match = the FRONTMOST viewer, in case two are stacked (the shared lib + // is used by specs that stack viewers). + const all = document.querySelectorAll(sel); + const el = all[all.length - 1]; + if (!el) return NaN; + const t = getComputedStyle(el).transform; + if (!t || t === 'none') return 1; // identity — fit + const m = /matrix\(([^)]+)\)/.exec(t); + return m ? Number(m[1].split(',')[0]) : NaN; + }, selector); +} + +/** The viewer image's on-screen rectangle (post-transform), for anchor/pan math. */ +export function imageRect(page: Page, selector = VIEWER_IMAGE): Promise { + return page.evaluate((sel) => { + const all = document.querySelectorAll(sel); + const el = all[all.length - 1]; // frontmost viewer (see renderedScale) + if (!el) throw new Error(`imageRect: no element for ${sel}`); + const r = el.getBoundingClientRect(); + return { x: r.x, y: r.y, width: r.width, height: r.height, top: r.top, left: r.left, right: r.right, bottom: r.bottom } as DOMRect; + }, selector); +} export function itemUrl(fixture: SuiteFixture, slug: string): string { return `/${fixture.adminUsername}/${fixture.workspaceSlug}/docs/${slug}`; diff --git a/web/src/lib/attachments/events.test.ts b/web/src/lib/attachments/events.test.ts index b48f1420..81d82c2a 100644 --- a/web/src/lib/attachments/events.test.ts +++ b/web/src/lib/attachments/events.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from 'vitest'; +import type { AttachmentUploadResult } from '$lib/types'; import { createAttachmentHostToken, isAttachmentPanelEventForHost, @@ -7,6 +8,7 @@ import { notifyViewerOpen, registerAttachmentPanelListener, registerAttachmentViewerListener, + toUploadedAttachment, type AttachmentPanelOpenEvent, type AttachmentViewerOpenEvent, type LightboxImage, @@ -524,3 +526,29 @@ describe('viewer open channel', () => { expect(viewerSeen[0].attachmentId).toBe('att-1'); }); }); + +describe('toUploadedAttachment (TASK-2459)', () => { + it('threads the dimensions the narrowing used to drop', () => { + const out = toUploadedAttachment({ + id: 'a1', + filename: 'big.png', + mime: 'image/png', + size: 4096, + width: 4000, + height: 3000, + } as AttachmentUploadResult); + expect(out.width).toBe(4000); + expect(out.height).toBe(3000); + }); + + it('leaves dimensions null when the response omits them', () => { + const out = toUploadedAttachment({ + id: 'a1', + filename: 'f.bin', + mime: 'application/octet-stream', + size: 10, + } as AttachmentUploadResult); + expect(out.width).toBeNull(); + expect(out.height).toBeNull(); + }); +}); diff --git a/web/src/lib/attachments/events.ts b/web/src/lib/attachments/events.ts index eae88316..7d947cdf 100644 --- a/web/src/lib/attachments/events.ts +++ b/web/src/lib/attachments/events.ts @@ -84,12 +84,21 @@ export interface UploadedAttachment { filename: string; mime_type: string; size_bytes: number; + /** + * Pixel dimensions, when the server returned them (nullable — a non-image, or + * an image whose dimensions it couldn't read). Carried so a freshly uploaded + * image opened in the viewer can classify for the DR-5b loading policy + * (TASK-2459) instead of falling to `unknown` and pulling the original + * outright; the upload response has them, this narrowing used to DROP them. + */ + width: number | null; + height: number | null; } /** * Narrow an upload response to what subscribers need. Both upload paths (body - * editor, comment composer) were hand-mapping the same four fields, which is - * how the two drift apart. + * editor, comment composer) were hand-mapping the same fields, which is how the + * two drift apart. */ export function toUploadedAttachment(result: AttachmentUploadResult): UploadedAttachment { return { @@ -97,6 +106,8 @@ export function toUploadedAttachment(result: AttachmentUploadResult): UploadedAt filename: result.filename, mime_type: result.mime, size_bytes: result.size, + width: result.width ?? null, + height: result.height ?? null, }; } @@ -282,8 +293,9 @@ export interface LightboxImage { * Metadata the viewer may caption with, all NULLABLE for the same reason * the panel's three are: an emitter knows only what its own surface gives * it, and an inline image's HEAD probe may not have completed or may have - * failed, while an upload event carries only four fields - * (`UploadedAttachment`). + * failed, while an upload event carries only the `UploadedAttachment` fields + * (which now include the pixel dimensions, threaded for the DR-5b policy — + * TASK-2459). * * `mime_type` is not decoration: it is what lets a CONSUMER re-state the * DR-16 open gate over a whole set rather than trusting the one element diff --git a/web/src/lib/attachments/viewerImageLoader.svelte.test.ts b/web/src/lib/attachments/viewerImageLoader.svelte.test.ts new file mode 100644 index 00000000..3657f533 --- /dev/null +++ b/web/src/lib/attachments/viewerImageLoader.svelte.test.ts @@ -0,0 +1,342 @@ +import { describe, it, expect } from 'vitest'; +import { createViewerImageLoader } from './viewerImageLoader.svelte'; +import type { LightboxImage } from './events'; + +// TASK-2459 — the DR-5b loader. `displaySrc` IS the request: the canonical +// attachment URL the viewer's loads natively (a `?variant=thumb-md` first, +// the plain original second). The acceptance is phrased in requests — a DR-16 +// gate is "no request issued", the fallback detector is "no SECOND request", the +// upgrade is "the second request is the original" — and each is a `displaySrc` +// transition here. +// +// `decoded`/`errored` carry the `gen` (the `loadToken` the reporting element was +// mounted under); the current generation is `loader.loadToken`, so a live decode +// passes `loader.loadToken` and a DETACHED element's stale decode passes the +// token captured when it loaded. + +function image(id: string, over: Partial = {}): LightboxImage { + return { + id, + alt: id, + filename: null, + mime_type: 'image/png', + size_bytes: null, + width: null, + height: null, + ...over, + }; +} + +const THUMB = (id: string) => `/api/v1/workspaces/ws/attachments/${id}?variant=thumb-md`; +const ORIGINAL = (id: string) => `/api/v1/workspaces/ws/attachments/${id}`; + +describe('viewerImageLoader — the decision table as requests (TASK-2459)', () => { + it('small, long edge <= 1024: ONE request, the original directly (no variant)', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 800, height: 600 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(ORIGINAL('A')); + expect(loader.phase).toBe('loading'); + loader.decoded(800, 600, loader.displaySrc, loader.loadToken); + // No upgrade — it IS the original. + expect(loader.displaySrc).toBe(ORIGINAL('A')); + expect(loader.phase).toBe('ready'); + }); + + it('unknown dims on desktop: the original directly (one request)', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: null, height: 900 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(ORIGINAL('A')); + loader.decoded(600, 900, loader.displaySrc, loader.loadToken); + expect(loader.displaySrc).toBe(ORIGINAL('A')); + }); + + it('large / unknown on mobile: NO request — `deferred`, the tap affordance (TASK-2460)', () => { + const large = createViewerImageLoader(); + large.load(image('A', { width: 5000, height: 5000 }), 'ws', 'mobile'); + expect(large.displaySrc).toBe(''); // no request issued + expect(large.phase).toBe('deferred'); // NOT 'idle' — the original is on tap + + const unknown = createViewerImageLoader(); + unknown.load(image('B', { width: null, height: 900 }), 'ws', 'mobile'); + expect(unknown.displaySrc).toBe(''); + expect(unknown.phase).toBe('deferred'); + }); +}); + +describe('viewerImageLoader — mobile on-demand original (TASK-2460)', () => { + it('deferred cell: loadOriginal (tap) requests the ORIGINAL directly, once', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'mobile'); + expect(loader.phase).toBe('deferred'); + expect(loader.displaySrc).toBe(''); + + loader.loadOriginal(); // the tap + expect(loader.displaySrc).toBe(ORIGINAL('A')); + expect(loader.phase).toBe('loading'); + + // A second trigger (a second tap, or a zoom-past-fit racing it) is a no-op. + const t = loader.loadToken; + loader.loadOriginal(); + expect(loader.displaySrc).toBe(ORIGINAL('A')); + expect(loader.loadToken).toBe(t); + + loader.decoded(5000, 5000, loader.displaySrc, loader.loadToken); + expect(loader.phase).toBe('ready'); + }); + + it('mobile thumb cell: thumb paints, then loadOriginal (zoom-past-fit) upgrades once', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 2000, height: 100 }), 'ws', 'mobile'); + expect(loader.displaySrc).toBe(THUMB('A')); // thumb painted, no auto-upgrade + const thumbToken = loader.loadToken; + loader.decoded(1024, 51, loader.displaySrc, loader.loadToken); + expect(loader.displaySrc).toBe(THUMB('A')); // mobile: still the thumb + + loader.loadOriginal(); // zoom past fit + expect(loader.displaySrc).toBe(ORIGINAL('A')); + // SAME element reused (token unchanged) so the thumb stays until the original + // decodes — no flash. + expect(loader.loadToken).toBe(thumbToken); + + // A second zoom step past fit does not re-request. + loader.loadOriginal(); + expect(loader.displaySrc).toBe(ORIGINAL('A')); + expect(loader.loadToken).toBe(thumbToken); + }); + + it('mobile thumb cell whose thumb-md SERVED THE ORIGINAL issues no zoom re-fetch', () => { + // The fallback detector must run on the mobile path too: a fresh upload / + // WebP / AVIF has no derived thumb, so `?variant=thumb-md` returns the + // ORIGINAL. Without clearing `originalDeferred`, a later zoom-past-fit would + // download and decode the original a SECOND time. + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 2000, height: 100 }), 'ws', 'mobile'); + expect(loader.displaySrc).toBe(THUMB('A')); + // The "thumb" decoded ABOVE the bound → it WAS the original. + loader.decoded(2000, 100, loader.displaySrc, loader.loadToken); + expect(loader.phase).toBe('ready'); + // Zoom past fit now finds nothing deferred — no second request. + loader.loadOriginal(); + expect(loader.displaySrc).toBe(THUMB('A')); // unchanged + }); + + it('desktop never defers: loadOriginal is a no-op (auto-upgrade owns the original)', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(THUMB('A')); + const before = loader.displaySrc; + loader.loadOriginal(); // no-op on desktop + expect(loader.displaySrc).toBe(before); + }); + + it('retry after an on-demand original FAILS re-requests the original, not the affordance', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'mobile'); + loader.loadOriginal(); // tap + expect(loader.displaySrc).toBe(ORIGINAL('A')); + loader.errored(loader.displaySrc, loader.loadToken); + expect(loader.phase).toBe('error'); + + const t = loader.loadToken; + loader.retry(); + // Back to the ORIGINAL (a fresh token remounts to refetch) — NOT reverted to + // the 'deferred' tap affordance. + expect(loader.phase).toBe('loading'); + expect(loader.displaySrc).toBe(ORIGINAL('A')); + expect(loader.loadToken).toBeGreaterThan(t); + }); + + it('retry after the INITIAL thumb fails re-requests the thumb (start path)', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 2000, height: 100 }), 'ws', 'mobile'); + expect(loader.displaySrc).toBe(THUMB('A')); + loader.errored(loader.displaySrc, loader.loadToken); + expect(loader.phase).toBe('error'); + loader.retry(); + expect(loader.displaySrc).toBe(THUMB('A')); // the initial policy, re-run + }); +}); + +describe('viewerImageLoader — thumb then original, the four bound ways (TASK-2459)', () => { + it('large on desktop: first request bounded, second the original, the bitmap CHANGES', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'desktop'); + // (1) first response bounded. + expect(loader.displaySrc).toBe(THUMB('A')); + + // The thumb decoded at a bounded size → the background upgrade fires. + loader.decoded(1024, 768, loader.displaySrc, loader.loadToken); + // (2) second request is explicitly the original (canonical, no variant). + expect(loader.displaySrc).toBe(ORIGINAL('A')); + // (3) the displayed bitmap visibly CHANGES (thumb URL → original URL). + expect(loader.displaySrc).not.toBe(THUMB('A')); + + loader.decoded(5000, 5000, loader.displaySrc, loader.loadToken); + expect(loader.phase).toBe('ready'); + }); + + it('small, long edge > 1024 on desktop: thumb-md then upgrade; on mobile: thumb-md, no upgrade', () => { + const desktop = createViewerImageLoader(); + desktop.load(image('A', { width: 2000, height: 100 }), 'ws', 'desktop'); + expect(desktop.displaySrc).toBe(THUMB('A')); + desktop.decoded(1024, 51, desktop.displaySrc, desktop.loadToken); + expect(desktop.displaySrc).toBe(ORIGINAL('A')); + + const mobile = createViewerImageLoader(); + mobile.load(image('A', { width: 2000, height: 100 }), 'ws', 'mobile'); + expect(mobile.displaySrc).toBe(THUMB('A')); + mobile.decoded(1024, 51, mobile.displaySrc, mobile.loadToken); + expect(mobile.displaySrc).toBe(THUMB('A')); // NO auto upgrade on mobile + }); + + it('(4) the FALLBACK case issues NO second request: a thumb-md served the original', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(THUMB('A')); + // The "thumb" decoded ABOVE the thumbnail bound → it WAS the original. + loader.decoded(5000, 5000, loader.displaySrc, loader.loadToken); + expect(loader.displaySrc).toBe(THUMB('A')); // never upgraded — no double decode + expect(loader.phase).toBe('ready'); + }); +}); + +describe('viewerImageLoader — DR-16 as a LOADING gate (TASK-2459)', () => { + it('issues NO request for an unsafe MIME', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { mime_type: 'image/svg+xml', width: 800, height: 600 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(''); + expect(loader.phase).toBe('idle'); + }); + + it('issues NO request for an unresolved MIME', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { mime_type: null, width: 800, height: 600 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(''); + }); + + it('issues NO request for no image', () => { + const loader = createViewerImageLoader(); + loader.load(undefined, 'ws', 'desktop'); + expect(loader.displaySrc).toBe(''); + }); +}); + +describe('viewerImageLoader — staleness / abort on navigate + shrink (TASK-2459)', () => { + it('repointing DROPS the old URL immediately (abort by src reassignment)', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(THUMB('A')); + // Navigate to B before A finishes: the old URL is gone at once. + loader.load(image('B', { width: 800, height: 600 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(ORIGINAL('B')); + expect(loader.displaySrc).not.toContain('/A'); + }); + + it('a LATE decode for a navigated-away image does NOT drive the new image', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'desktop'); + const staleSrc = loader.displaySrc; // A's thumb + const staleGen = loader.loadToken; // A's generation + loader.load(image('B', { width: 800, height: 600 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(ORIGINAL('B')); + + // A's thumbnail finishes decoding LATE, at a fallback size (>1024). Without + // the src fence this would flip B into an unexpected upgrade / phase. + loader.decoded(5000, 5000, staleSrc, staleGen); + expect(loader.displaySrc).toBe(ORIGINAL('B')); // untouched + }); + + it('the GENERATION fence rejects an A→B→A same-URL stale decode', () => { + // The URL fence alone is insufficient: navigating A→B→A reuses A's exact + // URL, so the detached first A element's late decode has the SAME src as the + // live third request. Only the captured generation tells them apart. + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'desktop'); + const firstA = loader.loadToken; // the detached A element's generation + loader.load(image('B', { width: 5000, height: 5000 }), 'ws', 'desktop'); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'desktop'); + expect(loader.displaySrc).toBe(THUMB('A')); // the live third request + + // The FIRST A element decodes late at a fallback size (>1024). If accepted it + // would call `servedOriginal` true and SUPPRESS the live A's upgrade. + loader.decoded(5000, 5000, THUMB('A'), firstA); + // Live A is untouched — its own decode still drives the upgrade. + loader.decoded(1024, 768, loader.displaySrc, loader.loadToken); + expect(loader.displaySrc).toBe(ORIGINAL('A')); // upgraded, not suppressed + }); + + it('the GENERATION fence rejects an A→B→A same-URL stale ERROR', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 800, height: 600 }), 'ws', 'desktop'); + const firstA = loader.loadToken; + loader.load(image('B', { width: 800, height: 600 }), 'ws', 'desktop'); + loader.load(image('A', { width: 800, height: 600 }), 'ws', 'desktop'); + // The live third A decodes successfully. + loader.decoded(800, 600, loader.displaySrc, loader.loadToken); + expect(loader.phase).toBe('ready'); + // The detached first A element errors LATE at the same URL — it must NOT + // flip the live, ready image into 'error'. + loader.errored(ORIGINAL('A'), firstA); + expect(loader.phase).toBe('ready'); + }); + + it('dispose (close / shrink to empty) drops the load', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 800, height: 600 }), 'ws', 'desktop'); + expect(loader.displaySrc).not.toBe(''); + loader.dispose(); + expect(loader.displaySrc).toBe(''); + expect(loader.phase).toBe('idle'); + // A stale decode after dispose is inert. + loader.decoded(800, 600, ORIGINAL('A'), loader.loadToken); + expect(loader.phase).toBe('idle'); + }); +}); + +describe('viewerImageLoader — error + retry (TASK-2459)', () => { + it('a load failure shows a retryable error; retry RE-REQUESTS (never replays)', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 800, height: 600 }), 'ws', 'desktop'); + const url = loader.displaySrc; + loader.errored(url, loader.loadToken); + expect(loader.phase).toBe('error'); + + loader.retry(); + // Re-issued: the src is reset then set again (a real re-request, not a + // replay of the failed one). + expect(loader.displaySrc).toBe(url); + expect(loader.phase).toBe('loading'); + loader.decoded(800, 600, loader.displaySrc, loader.loadToken); + expect(loader.phase).toBe('ready'); + }); + + it('retry bumps the load token so the viewer re-requests a same-URL failure', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 800, height: 600 }), 'ws', 'desktop'); + const t1 = loader.loadToken; + loader.errored(loader.displaySrc, loader.loadToken); + loader.retry(); + // Same URL, but a NEW token — the viewer re-mounts the and re-fetches. + expect(loader.displaySrc).toBe(ORIGINAL('A')); + expect(loader.loadToken).toBeGreaterThan(t1); + }); + + it('the thumb→original UPGRADE does NOT bump the load token (element reused)', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 5000, height: 5000 }), 'ws', 'desktop'); + const t1 = loader.loadToken; + loader.decoded(1024, 768, loader.displaySrc, loader.loadToken); // upgrade + expect(loader.displaySrc).toBe(ORIGINAL('A')); + expect(loader.loadToken).toBe(t1); // unchanged — same element, no flash + }); + + it('ignores an error for a stale (already-navigated-away) src', () => { + const loader = createViewerImageLoader(); + loader.load(image('A', { width: 800, height: 600 }), 'ws', 'desktop'); + const staleSrc = loader.displaySrc; + const staleGen = loader.loadToken; + loader.load(image('B', { width: 800, height: 600 }), 'ws', 'desktop'); + loader.errored(staleSrc, staleGen); + expect(loader.phase).toBe('loading'); // B is unaffected + }); +}); diff --git a/web/src/lib/attachments/viewerImageLoader.svelte.ts b/web/src/lib/attachments/viewerImageLoader.svelte.ts new file mode 100644 index 00000000..388eb98c --- /dev/null +++ b/web/src/lib/attachments/viewerImageLoader.svelte.ts @@ -0,0 +1,357 @@ +/** + * The attachment viewer's image LOADER (PLAN-2392 / TASK-2459) — the desktop + * thumb-then-original path with the DR-5b memory policy from + * {@link ./viewerLoading}. + * + * MODEL. The viewer's `` is loaded NATIVELY — `displaySrc` is the canonical + * attachment URL (a `?variant=thumb-md` first, the plain original second), and + * the browser fetches, decodes, caches and (on `src` reassignment) CANCELS each + * request. That reassignment is the abort: repointing the loader immediately + * drops the URL the user navigated away from, and no cross-image reference — + * off-DOM `Image`, object URL, fetch controller — is ever retained, so arrowing + * through twenty images can leak nothing. The bytes stay under the browser's own + * cache, not a hand-rolled blob store. + * + * STALENESS (the no-`{#key}` switch-safety class). The `` persists across + * navigation, so a bitmap that finishes decoding LATE could report the wrong + * image's pixels. Every `decoded()` therefore carries the src that decoded and + * is ignored unless it still equals `displaySrc` — the fence that keeps a stale + * completion from driving the fallback detector / upgrade for the image the user + * has already left. The workspace slug is the CAPTURED one the caller passes + * (the pane can switch workspace under a mounted viewer). + * + * The FALLBACK DETECTOR ({@link servedOriginal}) is the load-bearing piece: when + * a `thumb-md` request is served the original (fresh upload mid-derivation, or a + * WebP/AVIF the server can't derive), the decoded long edge exceeds the + * thumbnail bound and the background original request is SKIPPED — the double + * decode the whole policy exists to prevent. + */ +import { attachmentDownloadUrl } from '$lib/markdown/attachments'; +import { canOpenInViewer } from '$lib/attachments/display'; +import { decideFirstRequest, servedOriginal, type Platform } from './viewerLoading'; +import type { LightboxImage } from './events'; + +export type LoadPhase = + /** Nothing to load and nothing to offer — no image, or an unsafe/unresolved + * entry (DR-16). No ``, no affordance. */ + | 'idle' + /** A mobile large/unknown cell: nothing auto-loads, but the ORIGINAL is + * available on demand — the viewer shows the tap-to-load affordance, and a + * tap (or zoom-past-fit once a bitmap exists) calls {@link + * ViewerImageLoader.loadOriginal} (TASK-2460). Distinct from `idle`, which + * offers nothing. */ + | 'deferred' + /** A request is in flight (first paint, or the on-demand / background + * original). */ + | 'loading' + /** A bitmap is displayed. A background/on-demand original may still be + * running. */ + | 'ready' + /** The image failed to load; retryable. */ + | 'error'; + +export interface ViewerImageLoader { + /** The URL the viewer's `` should show (canonical attachment URL), or `''`. */ + readonly displaySrc: string; + readonly phase: LoadPhase; + /** + * A token that changes on every fresh REQUEST — a load or a retry, but NOT the + * thumb→original upgrade. The viewer keys its `` on this so a retry (whose + * URL is often unchanged) actually re-mounts and re-requests, and so a nav + * gets a fresh element (tearing down the previous one's stale load listener), + * while the upgrade reuses the element and does not flash. + */ + readonly loadToken: number; + /** + * Whether a bitmap has DECODED for the current image and is on screen (a thumb + * or the original). False from `load` until the first decode, and in `idle` / + * `deferred`; stays true across the thumb→original upgrade. The viewer's + * zoom-past-fit trigger reads it so a zoom made BEFORE the thumb paints still + * upgrades the moment it does — and, because `loadOriginal` never writes it, the + * trigger effect can track it without self-invalidating (CONVE-1688). + */ + readonly painted: boolean; + /** + * Point the loader at `img` in workspace `wsSlug` on `platform`, superseding + * any prior load (its in-flight request is dropped by the `src` change). An + * `idle` no-op for no image, an unsafe/unresolved MIME (DR-16), or a cell that + * loads nothing now. + */ + load(img: LightboxImage | undefined, wsSlug: string, platform: Platform): void; + /** + * Request the ORIGINAL on demand — the mobile fetch DR-5b defers. Called by + * BOTH the tap affordance (the `deferred` cell, where nothing painted) and by + * zoom-past-fit (the mobile thumb cell, where a thumbnail painted). The two are + * ONE deduplicated fetch: a call while the original is already requested or + * in flight is a no-op, so a tap racing a zoom, 3d's pinch, or a retry can + * never issue a second request (TASK-2460). A no-op on desktop and on any cell + * that already loaded the original directly, where nothing is deferred. + */ + loadOriginal(): void; + /** + * The `` decoded `decodedSrc` at `naturalWidth x naturalHeight`. `gen` is + * the `loadToken` the reporting element was mounted under. Ignored unless BOTH + * `gen` is the current `loadToken` (the generation fence) AND `decodedSrc` is + * still the current `displaySrc`. The generation fence is load-bearing where + * the URL fence is not: an A→B→A navigation reuses A's exact URL, so a detached + * first element's late completion has the same `decodedSrc` as the live request + * — only the per-mount `gen` tells them apart (the no-`{#key}` switch-safety + * class). Drives the fallback detector and the thumb→original upgrade. + */ + decoded(naturalWidth: number, naturalHeight: number, decodedSrc: string, gen: number): void; + /** + * The `` failed to load `erroredSrc`. `gen` is the reporting element's + * mount `loadToken`. Ignored unless it is the current generation AND src — + * without the generation fence a detached same-URL element's late error would + * clobber the live element's success (A→B→A). + */ + errored(erroredSrc: string, gen: number): void; + /** Retry a failed load — RE-REQUESTS from scratch, never replays a failure. */ + retry(): void; + /** Drop the current load (on close / set-shrink to empty). */ + dispose(): void; +} + +interface ActiveLoad { + img: LightboxImage; + wsSlug: string; + platform: Platform; + /** Whether a background original upgrade is still owed (desktop thumb-md). */ + upgradePending: boolean; + /** True once the original request has been issued. */ + upgrading: boolean; + /** + * Whether the ORIGINAL is available to fetch ON DEMAND and has not been + * requested yet — the mobile deferral (TASK-2460). True for a mobile + * large/unknown cell (fetched on tap) AND a mobile thumb cell (fetched on + * zoom-past-fit); false on desktop (auto-upgrades) and where the original was + * loaded directly. {@link loadOriginal}'s dedup guard reads it: it flips false + * on the first fetch, so a second trigger is a no-op. + */ + originalDeferred: boolean; +} + +/** + * The URL for one variant. `original` is the CANONICAL, no-`?variant` download — + * that is what "the original" IS on the server (it serves the original when no + * derived row exists); the thumb↔original distinction IS the query param. + */ +function variantUrl(wsSlug: string, id: string, variant: 'thumb-md' | 'original'): string { + return variant === 'original' + ? attachmentDownloadUrl(wsSlug, id) + : attachmentDownloadUrl(wsSlug, id, 'thumb-md'); +} + +export function createViewerImageLoader(): ViewerImageLoader { + let displaySrc = $state(''); + let phase = $state('idle'); + // Bumped on each fresh request so the viewer can key its and force a + // re-request even when the URL is unchanged (a retry) — see `loadToken`. + let loadToken = $state(0); + // Whether a bitmap has decoded for the current image (see the interface). The + // zoom-past-fit trigger TRACKS this; `loadOriginal` deliberately never writes + // it, so that tracking cannot self-invalidate the trigger's flush (CONVE-1688). + let painted = $state(false); + // Non-reactive: never read in an $effect's tracked scope, so writing it can't + // self-invalidate a flush (CONVE-1688). + let active: ActiveLoad | null = null; + + // Retire to the neutral state — no active load, no URL, nothing decoded, no + // affordance. ONE cleanup so every "give up on this entry" site keeps the + // `painted === false in idle` contract; sprinkling the resets inline is how one + // gets left stale. + function toIdle(): void { + active = null; + displaySrc = ''; + painted = false; + phase = 'idle'; + } + + function start(): void { + const a = active; + if (!a) return; + // DR-16 as a LOADING gate, at the request CHOKEPOINT: both load() and + // retry() funnel through here, so an unsafe or unresolved MIME never issues + // a request from either — even were a prior state to leave `active` set. The + // gate is restated where the request is actually made (not only at the + // renderer), because the renderer showing nothing is not the same as the + // loader asking for nothing. + if (!canOpenInViewer(a.img.mime_type)) { + toIdle(); + return; + } + // A NEW request (load / retry). The upgrade in `decoded` and the on-demand + // `loadOriginal` do NOT call `start`, so neither bumps this — the element + // (and its bitmap) is reused. + loadToken++; + const decision = decideFirstRequest(a.img.width, a.img.height, a.platform); + a.upgrading = false; + // The original is DEFERRED (fetched on tap / zoom-past-fit) precisely when + // the platform is mobile and the first request is not already the original: + // the mobile large/unknown cell (nothing painted) and the mobile thumb cell + // (a thumbnail painted, no auto-upgrade). Desktop auto-upgrades; a + // direct-original cell has nothing left to fetch (TASK-2460). + a.originalDeferred = a.platform === 'mobile' && decision.variant !== 'original'; + a.upgradePending = decision.variant === 'thumb-md' && decision.upgrade === true; + if (decision.variant === null) { + // Mobile large/unknown: nothing auto-loads. `deferred` (NOT `idle`) so the + // viewer shows the tap-to-load affordance; the original arrives via + // `loadOriginal` on tap. + phase = 'deferred'; + displaySrc = ''; + return; + } + phase = 'loading'; + displaySrc = variantUrl(a.wsSlug, a.img.id, decision.variant); + } + + function load(img: LightboxImage | undefined, wsSlug: string, platform: Platform): void { + // Repoint: the `src` reassignment below (or the '' here) drops the old + // request. No reference to release — nothing is retained across images. + active = null; + displaySrc = ''; + painted = false; // a new image has nothing decoded yet + if (!img) { + // No image to show. The DR-16 MIME gate lives in `start()` — the request + // chokepoint, so retry is gated too — leaving only "no image" here. + phase = 'idle'; + return; + } + active = { img, wsSlug, platform, upgradePending: false, upgrading: false, originalDeferred: false }; + start(); + } + + function loadOriginal(): void { + const a = active; + // Dedup: only fetch when an original is DEFERRED and none is already in + // flight. This single guard makes the two triggers (tap, zoom-past-fit) one + // fetch and closes every race — a second tap, 3d's pinch reaching + // zoom-past-fit again, or a retry that overlaps a zoom all find + // `originalDeferred` already false (or `upgrading` true) and no-op. + if (!a || !a.originalDeferred || a.upgrading) return; + // DR-16 restated at this request edge too (defensive: an entry only becomes + // `originalDeferred` in `start()` AFTER its gate, so `a.img` is already safe — + // but every place that issues a request re-checks, so the invariant is local + // rather than argued). RETIRE the entry to `idle` — the same cleanup `start` + // and `retry` do — rather than only clearing the flag, which would leave the + // tap affordance rendered (`phase` stuck `'deferred'`) over a dead, no-op + // button. + if (!canOpenInViewer(a.img.mime_type)) { + toIdle(); + return; + } + a.originalDeferred = false; + a.upgrading = true; + // No `loadToken` bump: the mobile THUMB cell reuses its painted element so + // the thumbnail stays visible until the original decodes (no flash, like the + // desktop upgrade); the DEFERRED cell has no element yet, so one mounts fresh + // at the current token. `phase` returns to 'loading' while it is in flight. + phase = 'loading'; + displaySrc = variantUrl(a.wsSlug, a.img.id, 'original'); + } + + function decoded(naturalWidth: number, naturalHeight: number, decodedSrc: string, gen: number): void { + const a = active; + // Staleness fence, two parts. The generation fence (`gen !== loadToken`) + // rejects a DETACHED element's late completion even when it carries the + // live URL — an A→B→A nav reuses A's exact URL, so the src check alone would + // let the old A element's late decode drive the new A load. The src check + // then handles the within-element thumb→original sequencing. + if (!a || gen !== loadToken || decodedSrc === '' || decodedSrc !== displaySrc) return; + phase = 'ready'; + // A bitmap is now on screen (thumb or original) — this is the paint the + // zoom-past-fit trigger waits for, so a pre-paint zoom upgrades the instant + // it becomes true. + painted = true; + // Decoding the ORIGINAL (a desktop upgrade or a mobile on-demand fetch is in + // flight): nothing left to decide. + if (a.upgrading) return; + // Decoding the FIRST request (thumb-md, or an original-direct cell). The + // fallback detector runs on EVERY first decode, desktop or mobile: if the + // long edge exceeds the thumbnail bound the server served the ORIGINAL + // (thumb-md absent — fresh upload, or a WebP/AVIF it can't derive). There is + // then nothing more to fetch on EITHER path — clear the desktop upgrade AND + // the mobile deferred original, so neither a background upgrade nor a mobile + // zoom-past-fit re-requests bytes we already have (the double decode the + // whole policy exists to prevent). + if (servedOriginal(naturalWidth, naturalHeight)) { + a.upgradePending = false; + a.originalDeferred = false; + return; + } + // A real thumbnail (<= bound) decoded. The mobile thumb cell leaves + // `originalDeferred` true and waits for the zoom-past-fit trigger; only the + // DESKTOP cell background-upgrades now. + if (!a.upgradePending) return; + // DR-16 restated at the upgrade request edge too. Defensive: `a.img` is the + // same entry `start()` already gated, but every place that issues a request + // re-checks, so the invariant is local rather than argued. + if (!canOpenInViewer(a.img.mime_type)) { + a.upgradePending = false; + return; + } + // Background-request the original. `phase` returns to 'loading' while it is + // in flight; the thumbnail stays displayed until the original decodes. + a.upgrading = true; + a.upgradePending = false; + phase = 'loading'; + displaySrc = variantUrl(a.wsSlug, a.img.id, 'original'); + } + + function errored(erroredSrc: string, gen: number): void { + // Generation + src fence (see `decoded`): a detached same-URL element's late + // error must not flip the live element's success into 'error'. + if (!active || gen !== loadToken || erroredSrc === '' || erroredSrc !== displaySrc) return; + phase = 'error'; + } + + function retry(): void { + const a = active; + if (!a) return; + // A fresh request, never a replay: the `src` is cleared and reissued, and + // the server may now have the variant (derivation completes async). + displaySrc = ''; + if (a.upgrading) { + // The failed request was the on-demand / background ORIGINAL (a mobile tap + // or zoom-past-fit, or a desktop upgrade). Re-request the ORIGINAL directly + // — NOT the initial policy, which for a mobile deferred cell would revert + // to the tap affordance and drop the user's committed load intent. A fresh + // token remounts the element so the same URL actually refetches. + // DR-16 restated at this request edge too (like `start`). + if (!canOpenInViewer(a.img.mime_type)) { + toIdle(); + return; + } + loadToken++; + phase = 'loading'; + displaySrc = variantUrl(a.wsSlug, a.img.id, 'original'); + return; + } + start(); + } + + function dispose(): void { + toIdle(); + } + + return { + get displaySrc() { + return displaySrc; + }, + get phase() { + return phase; + }, + get loadToken() { + return loadToken; + }, + get painted() { + return painted; + }, + load, + loadOriginal, + decoded, + errored, + retry, + dispose, + }; +} diff --git a/web/src/lib/attachments/viewerLoading.test.ts b/web/src/lib/attachments/viewerLoading.test.ts new file mode 100644 index 00000000..6e916e52 --- /dev/null +++ b/web/src/lib/attachments/viewerLoading.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect } from 'vitest'; +import { + THUMB_LONG_EDGE, + AUTO_LOAD_MAX_PIXELS, + classify, + decideFirstRequest, + servedOriginal, +} from './viewerLoading'; + +describe('viewerLoading constants', () => { + it('mirror the server bounds by value', () => { + expect(THUMB_LONG_EDGE).toBe(1024); + expect(AUTO_LOAD_MAX_PIXELS).toBe(8_000_000); + }); +}); + +describe('classify — pixels only, never bytes', () => { + it('is small at or under the pixel ceiling, large strictly above', () => { + expect(classify(2000, 2000)).toBe('small'); // 4 MP + expect(classify(4000, 2000)).toBe('small'); // exactly 8 MP — still small + expect(classify(4000, 2001)).toBe('large'); // just over + expect(classify(5000, 5000)).toBe('large'); // 25 MP + }); + + it('classifies a SMALL-byte, LARGE-pixel image as large (bytes are never consulted)', () => { + // A heavily-compressed 50 MP JPEG is a few MB on the wire but 200 MiB + // decoded — `large` is about the decoded cost, not the download. + expect(classify(10000, 5000)).toBe('large'); // 50 MP + }); + + it('yields `unknown` (a third value, NOT `large`) when a dimension is missing', () => { + expect(classify(null, 900)).toBe('unknown'); + expect(classify(900, null)).toBe('unknown'); + expect(classify(null, null)).toBe('unknown'); + expect(classify(undefined, 900)).toBe('unknown'); + // Explicitly distinct from `large`. + expect(classify(null, 900)).not.toBe('large'); + }); + + it('treats non-finite / non-positive dimensions as `unknown`', () => { + expect(classify(0, 900)).toBe('unknown'); + expect(classify(-1, 900)).toBe('unknown'); + expect(classify(NaN, 900)).toBe('unknown'); + expect(classify(Infinity, 900)).toBe('unknown'); + }); +}); + +describe('decideFirstRequest — the DR-5b decision table', () => { + it('small, long edge <= 1024: the original directly, either platform', () => { + expect(decideFirstRequest(800, 600, 'desktop')).toEqual({ variant: 'original' }); + expect(decideFirstRequest(800, 600, 'mobile')).toEqual({ variant: 'original' }); + expect(decideFirstRequest(1024, 700, 'desktop')).toEqual({ variant: 'original' }); // exactly 1024 + }); + + it('small, long edge > 1024: thumb-md — upgrade on desktop, not on mobile', () => { + // 2000x100 = 200k px (small) but long edge 2000 > 1024. + expect(decideFirstRequest(2000, 100, 'desktop')).toEqual({ variant: 'thumb-md', upgrade: true }); + expect(decideFirstRequest(2000, 100, 'mobile')).toEqual({ variant: 'thumb-md', upgrade: false }); + }); + + it('large (dims known): desktop thumb-md+upgrade, mobile nothing (tap)', () => { + expect(decideFirstRequest(5000, 5000, 'desktop')).toEqual({ variant: 'thumb-md', upgrade: true }); + expect(decideFirstRequest(5000, 5000, 'mobile')).toEqual({ variant: null }); + }); + + it('unknown dims: desktop the original DIRECTLY (one request), mobile nothing (tap)', () => { + // The unknown-desktop cell must not request a thumb — a bounded fallback + // original would defeat the detector and cause a second request. + expect(decideFirstRequest(null, 900, 'desktop')).toEqual({ variant: 'original' }); + expect(decideFirstRequest(null, null, 'desktop')).toEqual({ variant: 'original' }); + expect(decideFirstRequest(null, 900, 'mobile')).toEqual({ variant: null }); + }); +}); + +describe('servedOriginal — the fallback detector', () => { + it('is true when the decoded long edge exceeds the thumbnail bound', () => { + // The thumb request was served the original (variant absent). + expect(servedOriginal(2048, 1200)).toBe(true); + expect(servedOriginal(1200, 2048)).toBe(true); // long edge on either axis + expect(servedOriginal(1025, 10)).toBe(true); + }); + + it('is false when the decoded bitmap fits the thumbnail bound (a real thumb)', () => { + expect(servedOriginal(1024, 768)).toBe(false); // exactly at the bound + expect(servedOriginal(1000, 500)).toBe(false); + }); + + it('is false (never crashes) on missing / degenerate dimensions', () => { + expect(servedOriginal(null, null)).toBe(false); + expect(servedOriginal(0, 0)).toBe(false); + expect(servedOriginal(NaN, undefined)).toBe(false); + }); +}); diff --git a/web/src/lib/attachments/viewerLoading.ts b/web/src/lib/attachments/viewerLoading.ts new file mode 100644 index 00000000..de0c4790 --- /dev/null +++ b/web/src/lib/attachments/viewerLoading.ts @@ -0,0 +1,152 @@ +/** + * DR-5b image-loading policy for the attachment viewer (PLAN-2392 / TASK-2459). + * + * The memory-safety half of DR-5b: decide, from an image's PIXEL dimensions and + * the platform, what the viewer requests first and whether it upgrades to the + * original in the background — so a desktop promised a bounded first paint never + * silently decodes a 50 MP original twice, and a phone never auto-pulls one at + * all. + * + * PIXELS, NEVER BYTES. `size_bytes` is COMPRESSED size; a 3 MB JPEG can decode + * to 50 MP. The only honest ceiling for "how much RAM will this bitmap cost" is + * `width x height x 4` (RGBA), so classification reads dimensions and nothing + * else. When a dimension is missing the class is {@link SizeClass.unknown} — a + * genuine third value, NOT an alias for `large`: only the mobile + * no-auto-request decision treats the two alike; the desktop paths differ + * (unknown asks for the original directly, large asks for the thumbnail). + * + * This is a MEMORY POLICY, not a security boundary — the server authorizes the + * parent before it resolves any variant (`handlers_storage.go`). It is also + * FORMAT-BLIND on purpose: the load-bearing piece, {@link servedOriginal}, reads + * the decoded bitmap's own long edge rather than mirroring the server's decoder + * set, which would be a silently-rotting copy (the mirror DR-3a's shared fixture + * exists to avoid). + */ + +/** + * The `thumb-md` variant's long-edge bound, in CSS px — it mirrors the server's + * (`handlers_attachments.go`: a 1024 px long-edge derivation). A decoded bitmap + * whose long edge EXCEEDS this cannot be that thumbnail, which is exactly what + * {@link servedOriginal} keys off. + */ +export const THUMB_LONG_EDGE = 1024; + +/** + * The pixel ceiling for auto-fetching an ORIGINAL without asking: ~8 MP, about + * 32 MiB decoded RGBA. At or below it a dimensioned image is `small`; above it + * `large`. This is a CLIENT policy threshold, distinct from the server's 64 MP + * upload ceiling (`MaxPixelsDefault`). + */ +export const AUTO_LOAD_MAX_PIXELS = 8_000_000; + +/** + * The size class of an image, from its pixels alone. + * + * - `small` — dimensioned, at or under {@link AUTO_LOAD_MAX_PIXELS}. + * - `large` — dimensioned, over it. + * - `unknown` — a dimension is missing / non-finite / non-positive. A THIRD + * value, never folded into `large`. + */ +export type SizeClass = 'small' | 'large' | 'unknown'; + +export type Platform = 'desktop' | 'mobile'; + +function usableDimension(value: number | null | undefined): value is number { + return typeof value === 'number' && Number.isFinite(value) && value > 0; +} + +/** + * Classify on `width x height` and NOTHING else (never `size_bytes`). + * + * `classify(null, 900) === 'unknown'` — one missing dimension is enough, because + * a policy that can't see one axis can't bound the pixel count. A dimensioned + * image is `large` strictly ABOVE the ceiling, so an exactly-ceiling image is + * still `small`. + */ +export function classify( + width: number | null | undefined, + height: number | null | undefined +): SizeClass { + if (!usableDimension(width) || !usableDimension(height)) return 'unknown'; + return width * height > AUTO_LOAD_MAX_PIXELS ? 'large' : 'small'; +} + +/** + * True when the image's OWN pixels already fit inside the thumbnail bound, so + * the server would skip derivation and the download IS the original. Only + * meaningful for a dimensioned (`small`) image; `unknown` can't answer it. + */ +function withinThumbBound(width: number, height: number): boolean { + return Math.max(width, height) <= THUMB_LONG_EDGE; +} + +/** Which variant the viewer requests for FIRST paint, and whether it upgrades. */ +export type FirstRequest = + /** Request the original directly; there is no separate upgrade. */ + | { variant: 'original' } + /** + * Request the bounded thumbnail. `upgrade` is whether to background-fetch the + * original once it paints (desktop yes; mobile no — the original arrives on + * zoom past fit, TASK-2460). Suppressed anyway if {@link servedOriginal}. + */ + | { variant: 'thumb-md'; upgrade: boolean } + /** Request NOTHING now — the mobile tap affordance loads it (TASK-2460). */ + | { variant: null }; + +/** + * The DR-5b decision table, as one pure function. + * + * | dims | desktop | mobile | + * | -------------------------- | -------------------------- | ---------------------- | + * | small, long edge <= 1024 | original directly | original directly | + * | small, long edge > 1024 | thumb-md, upgrade | thumb-md, no upgrade | + * | large (dims known) | thumb-md, upgrade | nothing (tap) | + * | unknown dims | original directly | nothing (tap) | + * + * The `unknown`-desktop cell asks for the ORIGINAL, not a thumb: an unknown + * image whose original is <= 1024 px would defeat {@link servedOriginal} (the + * fallback serves that bounded original and the detector, seeing <= 1024, + * wrongly concludes it got a real thumbnail and fetches again). Requesting the + * original outright is exactly one request, always. + */ +export function decideFirstRequest( + width: number | null | undefined, + height: number | null | undefined, + platform: Platform +): FirstRequest { + const cls = classify(width, height); + if (cls === 'unknown') { + return platform === 'desktop' ? { variant: 'original' } : { variant: null }; + } + if (cls === 'small' && withinThumbBound(width as number, height as number)) { + // The image IS within the thumbnail bound — the download is the original. + return { variant: 'original' }; + } + // small-but-long (> 1024 px long edge) OR large, both with known dims. + if (platform === 'mobile') { + // large → tap affordance (nothing now); small-but-long → thumb, no auto original. + return cls === 'large' ? { variant: null } : { variant: 'thumb-md', upgrade: false }; + } + return { variant: 'thumb-md', upgrade: true }; +} + +/** + * THE FALLBACK DETECTOR — the load-bearing piece. + * + * A cell that requested `thumb-md` but got a bitmap whose long edge exceeds the + * thumbnail bound was served the ORIGINAL (the server falls back to it when the + * variant is absent — a fresh upload mid-derivation, or any format the pure-Go + * processor doesn't derive: WebP / AVIF). Two distinct URLs would then decode + * the original TWICE, so the background upgrade is skipped. + * + * Format-blind and dimension-free: it reads only the decoded bitmap, so it can't + * rot when the server's decoder set changes. + */ +export function servedOriginal( + naturalWidth: number | null | undefined, + naturalHeight: number | null | undefined +): boolean { + const w = usableDimension(naturalWidth) ? naturalWidth : 0; + const h = usableDimension(naturalHeight) ? naturalHeight : 0; + return Math.max(w, h) > THUMB_LONG_EDGE; +} diff --git a/web/src/lib/attachments/zoom.test.ts b/web/src/lib/attachments/zoom.test.ts new file mode 100644 index 00000000..4f2c6ac8 --- /dev/null +++ b/web/src/lib/attachments/zoom.test.ts @@ -0,0 +1,836 @@ +import { describe, it, expect } from 'vitest'; +import { + FIT, + FIT_EPSILON, + MAX_SCALE_FACTOR, + TOGGLE_SMALL_SCALE, + ZOOM_STEP, + actualScale, + clampPan, + clampScale, + clampState, + isAtFit, + maxScale, + reset, + stageCenter, + toggleFitOrActual, + zoomTo, + type Geometry, + type Point, + type ZoomState, +} from './zoom'; + +// --------------------------------------------------------------------------- +// Helpers +// +// Geometry is built the way the BROWSER builds it — `object-fit: contain`, +// which scales down to fit and never up — so the fixtures cannot drift from the +// coordinate system the module documents. Re-typing fitted sizes by hand is how +// a test ends up asserting against a geometry the CSS never produces. +// --------------------------------------------------------------------------- + +function containScale(stageW: number, stageH: number, natW: number, natH: number): number { + return Math.min(stageW / natW, stageH / natH, 1); +} + +function geom(stageW: number, stageH: number, natW: number, natH: number): Geometry { + const k = containScale(stageW, stageH, natW, natH); + return { stageW, stageH, fittedW: natW * k, fittedH: natH * k, naturalW: natW, naturalH: natH }; +} + +/** + * The image-space point currently painted under `anchor`, measured in UNSCALED + * fitted-box px from the image's centre. This is the quantity an anchored zoom + * must leave unchanged; computing it from the documented transform rather than + * from the implementation is what makes the invariance assertions independent. + */ +function imagePointUnder(state: ZoomState, g: Geometry, anchor: Point): Point { + return { + x: (anchor.x - g.stageW / 2 - state.x) / state.scale, + y: (anchor.y - g.stageH / 2 - state.y) / state.scale, + }; +} + +/** Where an image-space point paints, in stage-local px. Inverse of the above. */ +function stagePointOf(state: ZoomState, g: Geometry, d: Point): Point { + return { + x: g.stageW / 2 + state.x + state.scale * d.x, + y: g.stageH / 2 + state.y + state.scale * d.y, + }; +} + +function expectClose(actual: number, expected: number, tol = 1e-9): void { + expect(Math.abs(actual - expected)).toBeLessThanOrEqual(tol); +} + +/** The pan bound the module promises for one axis, recomputed independently. */ +function bound(stageExtent: number, fittedExtent: number, scale: number): number { + return Math.max(0, (fittedExtent * scale - stageExtent) / 2); +} + +function expectWithinBounds(state: ZoomState, g: Geometry, tol = 1e-9): void { + const bx = bound(g.stageW, g.fittedW, state.scale); + const by = bound(g.stageH, g.fittedH, state.scale); + expect(state.x).toBeLessThanOrEqual(bx + tol); + expect(state.x).toBeGreaterThanOrEqual(-bx - tol); + expect(state.y).toBeLessThanOrEqual(by + tol); + expect(state.y).toBeGreaterThanOrEqual(-by - tol); + // The centring rule: an axis that does not overflow is pinned to 0, not + // merely "within a zero-width bound" — assert it as its own property so a + // bound-only implementation that leaves 1e-16 drift is still caught. + if (bx === 0) expect(state.x).toBe(0); + if (by === 0) expect(state.y).toBe(0); +} + +// A square-ish image whose fitted box exactly fills the stage: the only shape +// for which a CORNER-anchored zoom lands exactly on the pan bound rather than +// outside it, so anchor invariance is testable there without the bounds +// legitimately overriding it. +const EXACT = geom(1000, 800, 2000, 1600); +// Landscape image in a landscape stage: fills the width, letterboxed vertically. +const WIDE = geom(1000, 800, 4000, 1000); +// Portrait image in a landscape stage: fills the height, pillarboxed. +const TALL = geom(1000, 800, 1000, 4000); +// Landscape image in a PORTRAIT stage. +const WIDE_IN_PORTRAIT = geom(800, 1000, 4000, 1000); +// An image smaller than the stage on both axes: `contain` does not upscale, so +// fit already is 1:1. +const SMALL = geom(1000, 800, 400, 300); + +describe('geometry fixtures', () => { + it('are built by the same contain rule the CSS applies', () => { + expect(EXACT.fittedW).toBe(1000); + expect(EXACT.fittedH).toBe(800); + expect(WIDE.fittedW).toBe(1000); + expect(WIDE.fittedH).toBe(250); + expect(TALL.fittedW).toBe(200); + expect(TALL.fittedH).toBe(800); + expect(SMALL.fittedW).toBe(400); + expect(SMALL.fittedH).toBe(300); + }); +}); + +describe('the contract constants', () => { + // THE ONE PLACE a literal from the spec is written down. Every other test + // references the exported constant, so behaviour tests cannot drift into + // re-typed magic numbers — but without this, renumbering MAX_SCALE_FACTOR to + // 5 or TOGGLE_SMALL_SCALE to 3 would silently change the product and keep the + // whole suite green, because every assertion would move with it. + it('carry the values PLAN-2392 specifies', () => { + expect(FIT).toBe(1); + expect(MAX_SCALE_FACTOR).toBe(4); + expect(TOGGLE_SMALL_SCALE).toBe(2); + expect(FIT_EPSILON).toBe(0.001); + expect(ZOOM_STEP).toBe(1.25); + }); +}); + +describe('reset', () => { + it('is fit, centred', () => { + expect(reset()).toEqual({ scale: FIT, x: 0, y: 0 }); + }); + + it('returns a fresh object each call so callers cannot alias state', () => { + const a = reset(); + a.x = 99; + expect(reset().x).toBe(0); + }); +}); + +describe('actualScale', () => { + it('is the painted bitmap over the fitted box', () => { + expect(actualScale(WIDE)).toBe(4000 / 1000); + expect(actualScale(TALL)).toBe(1000 / 200); + }); + + it('is exactly fit for an image smaller than the stage — contain never upscales', () => { + expect(actualScale(SMALL)).toBe(FIT); + }); + + it('is the WIDTH pair, not the height pair', () => { + // `contain` preserves the aspect ratio, so on every realistic fixture the + // two ratios agree and a height-based implementation is indistinguishable. + // Pin the axis the contract names with a deliberately non-uniform + // geometry — measurements DO diverge slightly per-axis once layout is + // snapped to a fractional device-pixel grid. + const nonUniform: Geometry = { + stageW: 1000, + stageH: 800, + fittedW: 1000, + fittedH: 100, + naturalW: 3000, + naturalH: 900, + }; + expect(actualScale(nonUniform)).toBe(3); + expect(actualScale(nonUniform)).not.toBe(nonUniform.naturalH / nonUniform.fittedH); + }); + + it('falls back to fit rather than dividing by a degenerate measurement', () => { + expect(actualScale({ ...WIDE, fittedW: 0 })).toBe(FIT); + expect(actualScale({ ...WIDE, fittedW: Number.NaN })).toBe(FIT); + expect(actualScale({ ...WIDE, naturalW: 0 })).toBe(FIT); + }); + + it('grows when a higher-resolution bitmap swaps in under the same layout', () => { + // The thumb-then-original path (TASK-2459): same fitted box, bigger + // bitmap, so 1:1 — and with it the zoom ceiling — moves. + const thumb = { ...WIDE, naturalW: 1024, naturalH: 256 }; + const original = { ...WIDE, naturalW: 4000, naturalH: 1000 }; + expect(maxScale(original)).toBeGreaterThan(maxScale(thumb)); + }); +}); + +describe('maxScale', () => { + it('is MAX_SCALE_FACTOR times 1:1', () => { + expect(maxScale(WIDE)).toBe(actualScale(WIDE) * MAX_SCALE_FACTOR); + }); + + it('stays finite when a nonsensical bitmap size overflows the multiplication', () => { + // A finite ratio can still overflow: an infinite ceiling would disable the + // scale clamp altogether rather than merely raising it. + // fittedW must be 1 for the RATIO itself to reach MAX_VALUE — with a + // larger fitted box the ratio is small enough that x4 stays finite and the + // guard is never exercised. + const absurd: Geometry = { + stageW: 1, + stageH: 1, + fittedW: 1, + fittedH: 1, + naturalW: Number.MAX_VALUE, + naturalH: Number.MAX_VALUE, + }; + expect(actualScale(absurd)).toBe(Number.MAX_VALUE); + expect(Number.isFinite(actualScale(absurd))).toBe(true); + expect(Number.isFinite(maxScale(absurd))).toBe(true); + expect(clampScale(Number.MAX_VALUE, absurd)).toBeLessThanOrEqual(maxScale(absurd)); + }); + + it('floors at fit so a small image still has a zoom range', () => { + expect(maxScale(SMALL)).toBe(MAX_SCALE_FACTOR); + // A nonsensical sub-1 actual scale must not invert the bounds. + const upscaled: Geometry = { ...SMALL, fittedW: 800, fittedH: 600 }; + expect(actualScale(upscaled)).toBeLessThan(FIT); + expect(maxScale(upscaled)).toBe(MAX_SCALE_FACTOR); + }); +}); + +describe('clampScale', () => { + it('clamps to the fit floor and the maxScale ceiling', () => { + expect(clampScale(0.25, WIDE)).toBe(FIT); + expect(clampScale(1e6, WIDE)).toBe(maxScale(WIDE)); + expect(clampScale(2.5, WIDE)).toBe(2.5); + }); + + it('coerces a non-finite scale to fit rather than emitting NaN into a CSS string', () => { + // Infinity falls back rather than saturating: it only ever arrives from a + // caller-side arithmetic bug, and snapping it to maximum zoom would look + // like a deliberate gesture. + expect(clampScale(Number.NaN, WIDE)).toBe(FIT); + expect(clampScale(Number.POSITIVE_INFINITY, WIDE)).toBe(FIT); + expect(clampScale(Number.NEGATIVE_INFINITY, WIDE)).toBe(FIT); + }); + + it('normalises negative zero away — a centred axis is exactly +0', () => { + expect(Object.is(clampPan({ scale: 2, x: -5, y: -5 }, WIDE).y, 0)).toBe(true); + expect(Object.is(clampPan({ scale: 2, x: -0, y: -0 }, WIDE).x, 0)).toBe(true); + }); +}); + +describe('clampPan', () => { + it('centres an axis whose scaled extent fits inside the stage', () => { + // WIDE at 2x: the width overflows, the height still does not. + const clamped = clampPan({ scale: 2, x: 300, y: 300 }, WIDE); + expect(clamped.y).toBe(0); + expect(clamped.x).toBe(300); + }); + + it('centres BOTH axes at fit — the parity property', () => { + for (const g of [EXACT, WIDE, TALL, WIDE_IN_PORTRAIT, SMALL]) { + expect(clampPan({ scale: FIT, x: 500, y: -500 }, g)).toEqual({ scale: FIT, x: 0, y: 0 }); + } + }); + + it('bounds all four edges of a wider-than-stage image', () => { + const g = WIDE; + const s = 2; + const bx = bound(g.stageW, g.fittedW, s); // 500 + expect(bx).toBe(500); + expect(clampPan({ scale: s, x: 10_000, y: 0 }, g).x).toBe(bx); + expect(clampPan({ scale: s, x: -10_000, y: 0 }, g).x).toBe(-bx); + // The vertical axis is the non-overflowing one here: both directions pin to 0. + expect(clampPan({ scale: s, x: 0, y: 10_000 }, g).y).toBe(0); + expect(clampPan({ scale: s, x: 0, y: -10_000 }, g).y).toBe(0); + }); + + it('bounds all four edges of a taller-than-stage image', () => { + const g = TALL; + const s = 3; + const bx = bound(g.stageW, g.fittedW, s); // 200*3=600 > 1000? no -> 0 + const by = bound(g.stageH, g.fittedH, s); // 800*3=2400 -> 800 + expect(bx).toBe(0); + expect(by).toBe(800); + expect(clampPan({ scale: s, x: 10_000, y: 0 }, g).x).toBe(0); + expect(clampPan({ scale: s, x: -10_000, y: 0 }, g).x).toBe(0); + expect(clampPan({ scale: s, x: 0, y: 10_000 }, g).y).toBe(by); + expect(clampPan({ scale: s, x: 0, y: -10_000 }, g).y).toBe(-by); + }); + + it('bounds a corner — both axes at once, in both orientations', () => { + for (const g of [EXACT, WIDE_IN_PORTRAIT]) { + const s = 4; + const bx = bound(g.stageW, g.fittedW, s); + const by = bound(g.stageH, g.fittedH, s); + for (const [sx, sy] of [ + [1, 1], + [1, -1], + [-1, 1], + [-1, -1], + ]) { + const c = clampPan({ scale: s, x: sx * 10_000, y: sy * 10_000 }, g); + // `|| 0` only normalises the -0 a zero bound produces on the test + // side; a real bound is never zero, so no expectation is weakened. + expect(c.x).toBe(sx * bx || 0); + expect(c.y).toBe(sy * by || 0); + } + } + }); + + it('leaves an in-bounds offset untouched', () => { + expect(clampPan({ scale: 2, x: 123.5, y: 0 }, WIDE)).toEqual({ scale: 2, x: 123.5, y: 0 }); + }); + + it('does not mutate its argument', () => { + const state = { scale: 2, x: 10_000, y: 10_000 }; + clampPan(state, WIDE); + expect(state).toEqual({ scale: 2, x: 10_000, y: 10_000 }); + }); + + it('passes scale through — clamping it is clampScale/clampState work', () => { + expect(clampPan({ scale: 99, x: 0, y: 0 }, WIDE).scale).toBe(99); + expect(clampPan({ scale: Number.NaN, x: 0, y: 0 }, WIDE).scale).toBe(FIT); + }); +}); + +describe('clampState (the resize path)', () => { + it('clamps scale FIRST, then bounds pan against the NEW scale', () => { + // Enlarging the window lowers actualScale and with it maxScale, stranding + // a scale above the new ceiling (TASK-2455). + const before = geom(1000, 800, 4000, 1000); // actual 4 -> max 16 + const after = geom(2000, 1600, 4000, 1000); // fitted 2000x500, actual 2 -> max 8 + const zoomed = { scale: 16, x: bound(before.stageW, before.fittedW, 16), y: 0 }; + const next = clampState(zoomed, after); + expect(next.scale).toBe(maxScale(after)); + expectWithinBounds(next, after); + // Pan-first would have kept the OLD scale's much larger bound and left x + // far outside the new one; assert the value, not just the invariant. + expect(next.x).toBe(bound(after.stageW, after.fittedW, maxScale(after))); + }); + + it('preserves a scale that is still inside the new bounds', () => { + const after = geom(2000, 1600, 4000, 1000); + expect(clampState({ scale: 3, x: 0, y: 0 }, after).scale).toBe(3); + }); +}); + +describe('zoomTo anchor invariance', () => { + const anchors: Array<[string, Point]> = [ + ['stage centre', { x: 500, y: 400 }], + ['an off-centre interior point', { x: 250, y: 600 }], + ['the top-left corner', { x: 0, y: 0 }], + ]; + + for (const [label, anchor] of anchors) { + it(`keeps the image point under ${label} under it`, () => { + const from: ZoomState = { scale: 1, x: 0, y: 0 }; + const before = imagePointUnder(from, EXACT, anchor); + const after = zoomTo(from, 2, anchor, EXACT); + const painted = stagePointOf(after, EXACT, before); + expectClose(painted.x, anchor.x); + expectClose(painted.y, anchor.y); + }); + } + + it('holds across a chain of steps from an already-panned state', () => { + const anchor = { x: 812.5, y: 137.25 }; + let state: ZoomState = clampPan({ scale: 2, x: -200, y: -150 }, EXACT); + for (let i = 0; i < 6; i++) { + const before = imagePointUnder(state, EXACT, anchor); + const next = zoomTo(state, state.scale * ZOOM_STEP, anchor, EXACT); + // Once the chain reaches the ceiling the scale stops changing, and an + // unchanged scale trivially preserves the anchor — assert only while + // the step is real. + if (next.scale !== state.scale) { + const painted = stagePointOf(next, EXACT, before); + expectClose(painted.x, anchor.x, 1e-8); + expectClose(painted.y, anchor.y, 1e-8); + } + state = next; + } + expect(state.scale).toBeGreaterThan(2); + }); + + it('is not a no-op dressed up as invariance — the transform actually moves', () => { + // The invariance assertions above all pass for an implementation that + // refuses to zoom at all. This is the control leg. + const out = zoomTo({ scale: 1, x: 0, y: 0 }, 2, { x: 0, y: 0 }, EXACT); + expect(out.scale).toBe(2); + expect(out.x).not.toBe(0); + expect(out.y).not.toBe(0); + }); + + it('lets the pan bounds win over the anchor when they conflict', () => { + // A corner anchor on a letterboxed image would need an offset outside the + // bound; showing blank stage past an image edge is worse than losing the + // anchor, so the bound wins — and the non-overflowing axis stays centred. + const out = zoomTo({ scale: 1, x: 0, y: 0 }, 2, { x: 0, y: 0 }, WIDE); + expectWithinBounds(out, WIDE); + expect(out.x).toBe(bound(WIDE.stageW, WIDE.fittedW, 2)); + expect(out.y).toBe(0); + }); + + it('clamps the target scale into [FIT, maxScale]', () => { + const g = WIDE; + expect(zoomTo({ scale: 2, x: 0, y: 0 }, 0.1, { x: 0, y: 0 }, g).scale).toBe(FIT); + expect(zoomTo({ scale: 2, x: 0, y: 0 }, 1e6, { x: 0, y: 0 }, g).scale).toBe(maxScale(g)); + }); + + it('zooming back out to fit re-centres both axes', () => { + const zoomed = zoomTo({ scale: 1, x: 0, y: 0 }, 3, { x: 0, y: 0 }, EXACT); + const back = zoomTo(zoomed, FIT, { x: 0, y: 0 }, EXACT); + expect(back).toEqual({ scale: FIT, x: 0, y: 0 }); + }); + + it('falls back to the stage centre for a non-finite anchor', () => { + const centred = zoomTo({ scale: 1, x: 0, y: 0 }, 2, stageCenter(EXACT), EXACT); + const nan = zoomTo({ scale: 1, x: 0, y: 0 }, 2, { x: Number.NaN, y: Number.NaN }, EXACT); + expect(nan).toEqual(centred); + }); + + it('does not mutate its argument', () => { + const state = { scale: 1, x: 0, y: 0 }; + zoomTo(state, 3, { x: 0, y: 0 }, EXACT); + expect(state).toEqual({ scale: 1, x: 0, y: 0 }); + }); + + it('normalises an already-invalid incoming state before anchoring', () => { + // The shape a consumer hits after a resize lowers the ceiling under a state + // it has not re-clamped yet. Clamping the incoming SCALE while trusting its + // PAN would apply a pan belonging to the old scale, so the invariant must + // be stated against the normalised state — assert it exactly that way. + const g = WIDE; + // Pan out of bounds for its own scale, which is what a shrinking resize + // leaves behind. Bound at scale 2 is 500, so 700 is 200px past it. + const stale: ZoomState = { scale: 2, x: 700, y: 0 }; + const normalised = clampState(stale, g); + expect(normalised.x).toBe(500); + + const out = zoomTo(stale, 2.2, stageCenter(g), g); + expect(out).toEqual(zoomTo(normalised, 2.2, stageCenter(g), g)); + expectWithinBounds(out, g); + + // Not vacuous: the rejected alternative — clamp the incoming SCALE, trust + // the incoming PAN — is written out here and lands somewhere else, so the + // assertion above has something to catch. A centre anchor scales the pan + // by k, and 700 * 1.1 = 770 exceeds the new bound while 500 * 1.1 does not. + expect(out.x).toBe(550); + expect(clampPan({ scale: 2.2, x: 700 * 1.1, y: 0 }, g).x).toBe(600); + }); +}); + +describe('the drag contract (TASK-2458 consumes this)', () => { + // A drag MUST be computed from the state captured at pointer-down plus the + // total pointer delta. These two legs pin why: clamping is lossy, so feeding + // each move delta into the previous clamped result silently changes the + // gesture's feel at an edge. + const g = WIDE; + const scale = 2; + const bx = bound(g.stageW, g.fittedW, scale); // 500 + + it('takes up the slack when the drag is computed from the pointer-down baseline', () => { + const start: ZoomState = { scale, x: 0, y: 0 }; + const overshoot = clampPan({ ...start, x: bx + 150 }, g); + expect(overshoot.x).toBe(bx); + // Pointer comes back 30px: total delta is +bx+120, still inside the bound. + const back = clampPan({ ...start, x: bx + 150 - 30 }, g); + expect(back.x).toBe(bx); + }); + + it('moves immediately — the wrong feel — if deltas are accumulated instead', () => { + // The control leg. Same pointer path, incremental composition: the image + // starts moving the instant the pointer reverses, 30px early. + const wrong = clampPan({ scale, x: clampPan({ scale, x: bx + 150, y: 0 }, g).x - 30, y: 0 }, g); + expect(wrong.x).toBe(bx - 30); + expect(wrong.x).not.toBe(bx); + }); + + it('an in-bounds drag moves by exactly the pointer delta', () => { + // The positive leg: "pan is clamped" alone passes with pan disabled. + const moved = clampPan({ scale, x: 0 + 123.75, y: 0 }, g); + expect(moved.x).toBe(123.75); + }); +}); + +describe('stageCenter', () => { + it('is half the stage box', () => { + expect(stageCenter(WIDE)).toEqual({ x: 500, y: 400 }); + }); +}); + +describe('isAtFit / toggleFitOrActual', () => { + it('at fit, goes to actual size anchored at the pointer', () => { + const anchor = { x: 700, y: 300 }; + const before = imagePointUnder(reset(), WIDE, anchor); + const out = toggleFitOrActual(reset(), anchor, WIDE); + expect(out.scale).toBe(actualScale(WIDE)); + // Anchored, to the extent the bounds allow: the overflowing axis keeps the + // point, the centred axis cannot. + const painted = stagePointOf(out, WIDE, before); + expectClose(painted.x, anchor.x); + expectWithinBounds(out, WIDE); + }); + + it('away from fit, goes back to fit CENTRED regardless of the anchor', () => { + const zoomed = zoomTo(reset(), 3, { x: 0, y: 0 }, WIDE); + expect(toggleFitOrActual(zoomed, { x: 900, y: 700 }, WIDE)).toEqual(reset()); + }); + + it('returns on a second toggle — the gesture is two-way', () => { + const anchor = { x: 700, y: 300 }; + const once = toggleFitOrActual(reset(), anchor, WIDE); + const twice = toggleFitOrActual(once, anchor, WIDE); + expect(once.scale).toBeGreaterThan(FIT); + expect(twice).toEqual(reset()); + }); + + it('treats the epsilon boundary as at-fit, and just past it as not', () => { + const inside: ZoomState = { scale: FIT + FIT_EPSILON, x: 0, y: 0 }; + const outside: ZoomState = { scale: FIT + FIT_EPSILON * 2, x: 0, y: 0 }; + expect(isAtFit(inside)).toBe(true); + expect(isAtFit(outside)).toBe(false); + expect(toggleFitOrActual(inside, { x: 500, y: 400 }, WIDE).scale).toBe(actualScale(WIDE)); + expect(toggleFitOrActual(outside, { x: 500, y: 400 }, WIDE)).toEqual(reset()); + }); + + it('is never a no-op on an image whose actual size IS fit', () => { + const out = toggleFitOrActual(reset(), { x: 500, y: 400 }, SMALL); + expect(actualScale(SMALL)).toBe(FIT); + expect(out.scale).toBe(TOGGLE_SMALL_SCALE); + // And it still toggles back. + expect(toggleFitOrActual(out, { x: 500, y: 400 }, SMALL)).toEqual(reset()); + }); + + it('is never a no-op when fractional geometry puts actual size a hair above fit', () => { + // A 1:1 image measured at dpr 2.75: the ratio is 1.0000000004, not 1, so an + // `=== 1` test would toggle to a scale the user cannot see. + const hair: Geometry = { ...SMALL, naturalW: 400.0000001, naturalH: 300.0000001 }; + expect(actualScale(hair)).toBeGreaterThan(FIT); + expect(toggleFitOrActual(reset(), { x: 500, y: 400 }, hair).scale).toBe(TOGGLE_SMALL_SCALE); + }); +}); + +// --------------------------------------------------------------------------- +// Fractional geometry / non-integer devicePixelRatio. +// +// The browser lays out on DEVICE-pixel boundaries, which are not CSS-pixel +// boundaries at dpr 1.25 / 1.5 / 2.75 — so every measurement the viewer reads +// is fractional there. An implementation that rounds looks correct on the +// integer fixtures above and drifts the anchored point visibly across a +// sequence of steps. +// --------------------------------------------------------------------------- + +describe('fractional geometry and non-integer devicePixelRatio', () => { + function snap(css: number, dpr: number): number { + return Math.round(css * dpr) / dpr; + } + + const dprs = [1.25, 1.5, 1.75, 2.5, 2.75]; + + for (const dpr of dprs) { + describe(`dpr ${dpr}`, () => { + // 92vw x 92vh of an odd viewport, snapped to the device grid. + const stageW = snap(0.92 * 1367, dpr); + const stageH = snap(0.92 * 769, dpr); + const g = geom(stageW, stageH, 3021, 2013); + + it('produces non-integer geometry (otherwise the case proves nothing)', () => { + expect(Number.isInteger(g.fittedW) && Number.isInteger(g.fittedH)).toBe(false); + }); + + it('keeps the anchored point under the cursor to well under one device pixel', () => { + const tol = 1 / dpr / 1000; + const anchor = { x: stageW * 0.317, y: stageH * 0.733 }; + let state = reset(); + for (let i = 0; i < 8; i++) { + const before = imagePointUnder(state, g, anchor); + const next = zoomTo(state, state.scale * ZOOM_STEP, anchor, g); + const bx = bound(g.stageW, g.fittedW, next.scale); + const by = bound(g.stageH, g.fittedH, next.scale); + const painted = stagePointOf(next, g, before); + // Only where the bounds did not legitimately override the anchor. + if (Math.abs(next.x) < bx - 1 && next.scale !== state.scale) { + expectClose(painted.x, anchor.x, tol); + } + if (Math.abs(next.y) < by - 1 && next.scale !== state.scale) { + expectClose(painted.y, anchor.y, tol); + } + expectWithinBounds(next, g); + state = next; + } + }); + + it('does not round the transform to whole CSS pixels', () => { + const out = zoomTo(reset(), 2.5, { x: stageW * 0.13, y: stageH * 0.87 }, g); + expect(Number.isInteger(out.x) && Number.isInteger(out.y)).toBe(false); + }); + + it('bounds a fractional geometry exactly, with no gap or overshoot', () => { + const s = maxScale(g); + const far = clampPan({ scale: s, x: 1e9, y: 1e9 }, g); + expectClose(far.x, bound(g.stageW, g.fittedW, s), 0); + expectClose(far.y, bound(g.stageH, g.fittedH, s), 0); + }); + }); + } +}); + +// --------------------------------------------------------------------------- +// RTL. +// +// The module's coordinates are STAGE-LOCAL — measured from the stage box's +// physical left edge, which is what `getBoundingClientRect()` reports in either +// direction. `direction: rtl` mirrors the LAYOUT, not that measurement, so the +// math must be exactly direction-symmetric: mirror the anchor and the pan, and +// the result must be the mirror of the original. Any place the implementation +// leaked a signed horizontal direction (an inline-start assumption, a +// `stageW - x`) breaks this and nothing else does. +// --------------------------------------------------------------------------- + +describe('RTL / horizontal direction symmetry', () => { + function mirrorPoint(p: Point, g: Geometry): Point { + return { x: g.stageW - p.x, y: p.y }; + } + function mirrorState(s: ZoomState): ZoomState { + return { scale: s.scale, x: -s.x, y: s.y }; + } + + const cases: Array<[string, Geometry]> = [ + ['exact fill', EXACT], + ['landscape in landscape', WIDE], + ['portrait in landscape', TALL], + ['landscape in portrait', WIDE_IN_PORTRAIT], + ]; + + for (const [label, g] of cases) { + it(`zoomTo is mirror-symmetric (${label})`, () => { + const anchor = { x: g.stageW * 0.19, y: g.stageH * 0.62 }; + const start: ZoomState = clampPan({ scale: 2, x: 37.5, y: -12.25 }, g); + const out = zoomTo(start, 3.5, anchor, g); + const mirrored = zoomTo(mirrorState(start), 3.5, mirrorPoint(anchor, g), g); + expect(mirrored.scale).toBe(out.scale); + expectClose(mirrored.x, -out.x, 1e-9); + expectClose(mirrored.y, out.y, 1e-9); + }); + + it(`clampPan is mirror-symmetric (${label})`, () => { + const s = 3; + const state: ZoomState = { scale: s, x: 1e9, y: 1e9 }; + const a = clampPan(state, g); + const b = clampPan(mirrorState(state), g); + expectClose(b.x, -a.x, 0); + }); + } + + it('the symmetry assertion can fail — a direction-flipped anchor breaks it', () => { + // Control leg: proves the mirror test is load-bearing rather than + // tautological. Mirroring the anchor WITHOUT mirroring the pan is exactly + // the bug shape the test exists to catch. + const g = EXACT; + const anchor = { x: g.stageW * 0.19, y: g.stageH * 0.62 }; + const start: ZoomState = { scale: 2, x: 37.5, y: 0 }; + const out = zoomTo(start, 3.5, anchor, g); + const halfMirrored = zoomTo(start, 3.5, mirrorPoint(anchor, g), g); + expect(Math.abs(halfMirrored.x - -out.x)).toBeGreaterThan(1); + }); +}); + +// --------------------------------------------------------------------------- +// The hard bound: NO sequence of operations escapes [FIT, maxScale] or the pan +// bounds. Individually-correct operations can still compose into an escape — +// this is the property the phase's review round called binding. +// --------------------------------------------------------------------------- + +describe('bounds no sequence of operations can escape', () => { + function lcg(seed: number): () => number { + let s = seed >>> 0; + return () => { + s = (Math.imul(s, 1664525) + 1013904223) >>> 0; + return s / 0x1_0000_0000; + }; + } + + const fixtures: Array<[string, Geometry]> = [ + ['exact fill', EXACT], + ['landscape in landscape', WIDE], + ['portrait in landscape', TALL], + ['landscape in portrait', WIDE_IN_PORTRAIT], + ['smaller than the stage', SMALL], + ['fractional', geom(1257.6, 707.2, 3021, 2013)], + ]; + + for (const [label, g] of fixtures) { + it(`holds over 2000 pseudo-random operations (${label})`, () => { + const rand = lcg(0xc0ffee); + let state = reset(); + let sawCeiling = false; + let sawFloor = false; + let sawBoundedPan = false; + + for (let i = 0; i < 2000; i++) { + const anchor = { x: rand() * g.stageW * 1.4 - g.stageW * 0.2, y: rand() * g.stageH * 1.4 - g.stageH * 0.2 }; + const roll = rand(); + if (roll < 0.35) { + state = zoomTo(state, state.scale * ZOOM_STEP, anchor, g); + } else if (roll < 0.6) { + state = zoomTo(state, state.scale / ZOOM_STEP, anchor, g); + } else if (roll < 0.7) { + // A wild target, as a wheel burst or a hostile caller would give. + state = zoomTo(state, (rand() - 0.5) * 1e4, anchor, g); + } else if (roll < 0.85) { + // A drag: an unclamped offset handed straight to clampPan. + state = clampPan( + { ...state, x: state.x + (rand() - 0.5) * 4000, y: state.y + (rand() - 0.5) * 4000 }, + g, + ); + } else if (roll < 0.95) { + state = toggleFitOrActual(state, anchor, g); + } else { + state = clampState(state, g); + } + + expect(Number.isFinite(state.scale)).toBe(true); + expect(Number.isFinite(state.x)).toBe(true); + expect(Number.isFinite(state.y)).toBe(true); + expect(state.scale).toBeGreaterThanOrEqual(FIT); + expect(state.scale).toBeLessThanOrEqual(maxScale(g)); + expectWithinBounds(state, g); + + if (state.scale === maxScale(g)) sawCeiling = true; + if (state.scale === FIT) sawFloor = true; + if ( + state.scale > FIT && + Math.abs(state.x) === bound(g.stageW, g.fittedW, state.scale) && + state.x !== 0 + ) { + sawBoundedPan = true; + } + } + + // Coverage guards: without these the invariants above could pass on a + // walk that never approached a bound. + expect(sawCeiling).toBe(true); + expect(sawFloor).toBe(true); + expect(sawBoundedPan).toBe(true); + }); + } +}); + +describe('degenerate geometry', () => { + const zero: Geometry = { stageW: 0, stageH: 0, fittedW: 0, fittedH: 0, naturalW: 0, naturalH: 0 }; + + it('never emits NaN into the transform', () => { + for (const out of [ + zoomTo(reset(), 3, { x: 10, y: 10 }, zero), + clampPan({ scale: 3, x: 10, y: 10 }, zero), + toggleFitOrActual(reset(), { x: 10, y: 10 }, zero), + clampState({ scale: 99, x: 10, y: 10 }, zero), + ]) { + expect(Number.isFinite(out.scale)).toBe(true); + expect(Number.isFinite(out.x)).toBe(true); + expect(Number.isFinite(out.y)).toBe(true); + } + }); + + it('centres everything when there is no geometry to pan within', () => { + expect(clampPan({ scale: 3, x: 10, y: 10 }, zero)).toEqual({ scale: 3, x: 0, y: 0 }); + }); +}); + +/** + * Round 3 of the per-task Codex review took the numerical-robustness angle and + * found three P2s. Each is pinned here by a test that FAILS against the code as + * it stood before the fix — the point of the exercise, since a test that passes + * either way would have let all three back in. + */ +describe('extreme finite geometry (round 3)', () => { + it('keeps the pan bound finite when extent * scale would overflow', () => { + // Before the fix `fittedW * scale` was Infinity, so the bound was + // Infinity, so clampSymmetric left ANY offset untouched — the clamp + // silently stopped clamping and blank stage could show past the image. + const huge: Geometry = { + stageW: Number.MAX_VALUE, + stageH: 1000, + fittedW: Number.MAX_VALUE * 0.6, + fittedH: 800, + naturalW: Number.MAX_VALUE * 0.6, + naturalH: 800, + }; + const out = clampPan({ scale: 2, x: Number.MAX_VALUE, y: 0 }, huge); + expect(Number.isFinite(out.x)).toBe(true); + expect(out.x).toBeLessThan(Number.MAX_VALUE); + }); + + it('does not let a subnormal fitted extent turn actualScale infinite', () => { + const subnormal: Geometry = { + stageW: 1000, + stageH: 1000, + fittedW: Number.MIN_VALUE, + fittedH: Number.MIN_VALUE, + naturalW: 4000, + naturalH: 4000, + }; + expect(Number.isFinite(actualScale(subnormal))).toBe(true); + }); + + it('leaves the double-click toggle doing something visible on that geometry', () => { + // The regression this guards: an infinite actualScale reached zoomTo, + // clampScale rejected it as non-finite and fell back to fit, and the + // toggle became the silent no-op TOGGLE_SMALL_SCALE exists to prevent. + const subnormal: Geometry = { + stageW: 1000, + stageH: 1000, + fittedW: Number.MIN_VALUE, + fittedH: Number.MIN_VALUE, + naturalW: 4000, + naturalH: 4000, + }; + const out = toggleFitOrActual(reset(), { x: 500, y: 500 }, subnormal); + expect(out.scale).toBeGreaterThan(FIT + FIT_EPSILON); + }); +}); + +describe('the fit epsilon boundary is asymmetric, and cannot matter (round 3)', () => { + it('reads FIT + FIT_EPSILON as at-fit but FIT - FIT_EPSILON as not', () => { + // Neither value is exactly representable in binary; the subtraction lands + // a few ulps outside the window. Pinned rather than papered over. + expect(isAtFit({ scale: FIT + FIT_EPSILON, x: 0, y: 0 })).toBe(true); + expect(isAtFit({ scale: FIT - FIT_EPSILON, x: 0, y: 0 })).toBe(false); + }); + + it('cannot arise, because no emitted scale is ever below FIT', () => { + // This is what makes the asymmetry unobservable in practice. If the floor + // ever stops being FIT, this fails and the asymmetry becomes real. + const g: Geometry = { + stageW: 1000, + stageH: 800, + fittedW: 900, + fittedH: 700, + naturalW: 3600, + naturalH: 2800, + }; + for (const attempt of [-99, 0, FIT - FIT_EPSILON, 0.5, Number.MIN_VALUE]) { + expect(clampScale(attempt, g)).toBeGreaterThanOrEqual(FIT); + expect(clampState({ scale: attempt, x: 0, y: 0 }, g).scale).toBeGreaterThanOrEqual(FIT); + expect(zoomTo(reset(), attempt, { x: 0, y: 0 }, g).scale).toBeGreaterThanOrEqual(FIT); + } + }); +}); diff --git a/web/src/lib/attachments/zoom.ts b/web/src/lib/attachments/zoom.ts new file mode 100644 index 00000000..8085ff57 --- /dev/null +++ b/web/src/lib/attachments/zoom.ts @@ -0,0 +1,403 @@ +/** + * Zoom / pan math for the attachment viewer — DOM-free, framework-free, and + * the single owner of every number the viewer's transform is built from + * (PLAN-2392 phase 3b / TASK-2454). + * + * WHY IT IS A SEPARATE MODULE. jsdom reports all-zero rects, so nothing that + * reads layout can be unit-tested there. Keeping the arithmetic here means the + * hard properties — anchor invariance, the pan bounds, the scale floor and + * ceiling — are provable without a browser, and the component (TASK-2455) is + * left with measuring, wiring and clamping on resize. + * + * TWO WORDS, ONE MEANING EACH. + * + * - The **fitted box** is the rendered size `object-fit: contain` gives the + * image inside the stage. The browser computes it; this module never + * re-derives it from natural-image pixels. + * - **fit** is `scale === 1`, i.e. {@link FIT}. There is no third quantity + * called a "fit scale" — conflating the two is what put the CSS and the + * module in different coordinate systems in an early revision of the design. + * + * THE COORDINATE SYSTEM. The stage is a box sized as the old bare `` was + * (92vw x 92vh, centred), with the `` inside it at `max-width: 100%; + * max-height: 100%; object-fit: contain`. The transform is + * + * translate(px, px) scale() transform-origin: center + * + * so a point sitting `d` CSS px from the image's centre (measured in UNSCALED + * fitted-box px) paints at + * + * stageCentre + (x, y) + scale * d + * + * At `scale === 1, x === 0, y === 0` that is byte-identical to today's + * rendering — the parity property the phase is built around. + * + * All exported functions are pure: none mutates its arguments, and every one + * that returns a {@link ZoomState} returns a fresh object. + * + * TWO PROPERTIES CONSUMERS MUST NOT ASSUME, both consequences of clamping every + * result rather than tracking an unclamped shadow state: + * + * - **A gesture is not associative.** Ten small {@link zoomTo} steps do not + * equal one big one once a pan bound is reached — the intermediate results + * were clamped and the excess is gone. So a DRAG must be computed from the + * state captured at pointer-down plus the total pointer delta, never by + * feeding each `pointermove` delta into the previous clamped result: + * overshooting a bound and coming back would otherwise move the image + * immediately instead of taking up the slack (TASK-2458). + * - **Clamping is lossy.** Pan discarded because the stage shrank does not come + * back when it grows again. A consumer that wants that must keep its own + * unclamped intent; the viewer deliberately does not (TASK-2455). + * + * NOTHING HERE ROUNDS. Geometry arrives fractional on any non-integer + * `devicePixelRatio` (the browser lays elements out on device-pixel boundaries, + * which are not CSS-pixel boundaries at dpr 1.25 / 1.5 / 2.75), and rounding + * intermediate values there drifts the anchored point visibly across a sequence + * of zoom steps. + */ + +/** + * The measured geometry of one painted image, in CSS px. + * + * Every field is expected finite and positive — a caller with no decoded bitmap + * has nothing to zoom and does not call in at all (TASK-2460 keeps the + * transform inert until one exists). The functions below are nevertheless + * defensive about zeros and non-finite values, because geometry is read from + * live layout and a mid-teardown measurement can produce either; a degenerate + * geometry must still yield a finite, in-bounds transform — never `NaN` or + * `Infinity` in a CSS string. + * + * MEASURING `fittedW/H` (TASK-2455). They are the image's UNSCALED box, so they + * cannot come from `getBoundingClientRect()` on the transformed `` — that + * returns the box AFTER `scale()`, and feeding it back in would make the bounds + * grow with the zoom. Read the untransformed layout box (`offsetWidth` / + * `offsetHeight`, which transforms do not affect), or measure once at fit. + */ +export interface Geometry { + /** Stage box width. */ + stageW: number; + /** Stage box height. */ + stageH: number; + /** Rendered image width at `scale === 1`. Never exceeds `stageW`. */ + fittedW: number; + /** Rendered image height at `scale === 1`. Never exceeds `stageH`. */ + fittedH: number; + /** + * The CURRENTLY PAINTED bitmap's intrinsic width — not the original's. + * While only a `thumb-md` variant has decoded, "actual size" means 1:1 with + * the thumb, and {@link maxScale} grows when the original swaps in + * (TASK-2459). NOTE: this is not the mobile fetch trigger; that trigger is + * `scale > 1`, i.e. past fit (TASK-2460). + */ + naturalW: number; + /** The currently painted bitmap's intrinsic height. See {@link Geometry.naturalW}. */ + naturalH: number; +} + +/** A point in stage-local CSS px, measured from the stage's top-left corner. */ +export interface Point { + x: number; + y: number; +} + +/** + * The viewer's transform state. + * + * `scale` is relative to fit, which is what makes {@link FIT} a constant rather + * than a derived quantity. `x` / `y` are the image centre's offset from the + * stage centre in CSS px. + */ +export interface ZoomState { + scale: number; + x: number; + y: number; +} + +/** + * `scale` at fit — the floor {@link clampScale} and {@link clampState} enforce. + * + * Not enforced by {@link clampPan}, which passes any finite scale through so it + * stays orthogonal to the scale bounds; use {@link clampState} when both matter. + */ +export const FIT = 1; + +/** {@link maxScale} is this multiple of 1:1 (or of fit, whichever is larger). */ +export const MAX_SCALE_FACTOR = 4; + +/** + * Where {@link toggleFitOrActual} goes when the image is already at 1:1 at fit + * (an image smaller than the stage: `object-fit: contain` never upscales, so + * fit IS actual size). Without this, double-click would be a no-op on exactly + * the images where a user is most likely to try it. + */ +export const TOGGLE_SMALL_SCALE = 2; + +/** + * How close to {@link FIT} still counts as "at fit" for {@link isAtFit} and + * therefore for {@link toggleFitOrActual}'s direction. + * + * A tolerance rather than an equality test because a wheel or pinch gesture + * lands on 1.0000000002 routinely, and a double-click there must zoom IN, not + * perform a visually-invisible reset. + */ +export const FIT_EPSILON = 0.001; + +/** + * The multiplicative step for one discrete zoom command — the `+` / `-` keys + * (TASK-2455). Exported so no call site re-types the literal. + */ +export const ZOOM_STEP = 1.25; + +/** + * Coerce a possibly-NaN/Infinite measurement to something usable. + * + * Infinity takes the fallback rather than saturating at a bound: an infinite + * scale or offset is always a caller-side arithmetic bug (a pinch handler + * dividing by a zero pointer distance is the likely one), and quietly snapping + * it to maximum zoom hides the bug behind plausible-looking behaviour, where + * falling back to the neutral value shows it. + */ +function finiteOr(value: number, fallback: number): number { + return Number.isFinite(value) ? value : fallback; +} + +/** + * Clamp to `[-bound, bound]`, normalising negative zero away. + * + * `Math.max(-0, -5)` is `-0`, so a centred axis would otherwise emit + * `translate(-0px)` and, more importantly, make `Object.is(x, 0)` false for a + * value this module documents as exactly zero. + * + * It deliberately does NOT re-guard a negative bound: {@link panBound} is the + * single owner of the centring rule, and a second copy of it here would make + * that one untestable — breaking the real rule would look correct because this + * function silently repaired it. + */ +function clampSymmetric(value: number, bound: number): number { + const clamped = Math.min(bound, Math.max(-bound, value)); + return clamped === 0 ? 0 : clamped; +} + +/** True only for a measurement that can be divided by. */ +function usable(value: number): boolean { + return Number.isFinite(value) && value > 0; +} + +/** + * The largest extent any axis is allowed to claim, in CSS px. + * + * Ten million px is four orders of magnitude past any layout a browser will + * produce (the widest real stage is a few thousand), and its purpose is purely + * arithmetic: it makes `extent * scale` unable to overflow to `Infinity`. + * + * That overflow is not cosmetic. {@link panBound} subtracts the stage from the + * scaled extent, and `Infinity - stage` is `Infinity`, so the pan bound becomes + * infinite and the clamp silently stops clamping — the one failure mode that + * lets blank stage show past an image edge. Saturating the inputs instead keeps + * every bound finite and ordered. + */ +const MAX_EXTENT = 1e7; + +/** Saturate a measurement into `[0, MAX_EXTENT]` so products cannot overflow. */ +function saneExtent(value: number): number { + const v = finiteOr(value, 0); + if (v <= 0) return 0; + return v > MAX_EXTENT ? MAX_EXTENT : v; +} + +/** The stage's centre, in stage-local px. The anchor for centre-origin zooms. */ +export function stageCenter(g: Geometry): Point { + return { + x: finiteOr(g.stageW, 0) / 2, + y: finiteOr(g.stageH, 0) / 2, + }; +} + +/** + * The `scale` at which the painted bitmap is displayed 1:1 with its own pixels. + * + * Measured on the WIDTH axis, as the contract specifies. `object-fit: contain` + * preserves the aspect ratio, so for any geometry the browser actually produces + * the height ratio is the same number; a hand-built non-uniform `Geometry` is + * the only way to make them disagree, and the width pair wins there. + * + * `object-fit: contain` never upscales, so an image smaller than the stage has + * `fittedW === naturalW` and this returns exactly 1 — fit already IS actual + * size for it. + */ +export function actualScale(g: Geometry): number { + if (!usable(g.fittedW) || !usable(g.naturalW)) return FIT; + const ratio = g.naturalW / g.fittedW; + // A subnormal `fittedW` divides to `Infinity` even though both inputs passed + // `usable`. Returning it would not merely be untidy: `toggleFitOrActual` + // hands this value straight to `zoomTo`, whose `clampScale` treats a + // non-finite target as invalid and falls back to fit — turning the toggle + // into the silent no-op the small-image rule exists to prevent. Falling back + // to FIT here instead routes that geometry to TOGGLE_SMALL_SCALE, so the + // gesture still does something visible. + return Number.isFinite(ratio) ? ratio : FIT; +} + +/** + * The upper bound on `scale`, i.e. the contract's `MAX_SCALE(g)`. + * + * `max(actualScale, 1)` rather than `actualScale` so a small image — whose + * actual scale is 1 — still gets a real zoom range instead of being pinned at + * fit, and so a nonsensical sub-1 actual scale can never invert the bounds. + */ +export function maxScale(g: Geometry): number { + const ceiling = Math.max(actualScale(g), FIT) * MAX_SCALE_FACTOR; + // A finite ratio can still overflow when multiplied: a bitmap reported at + // Number.MAX_VALUE px gives a finite actual scale and an infinite ceiling, + // which would silently disable the scale clamp entirely. + return Number.isFinite(ceiling) ? ceiling : Number.MAX_VALUE; +} + +/** Clamp a scale into `[FIT, maxScale(g)]`. */ +export function clampScale(scale: number, g: Geometry): number { + const max = maxScale(g); + const s = finiteOr(scale, FIT); + if (s < FIT) return FIT; + if (s > max) return max; + return s; +} + +/** The largest `|offset|` an axis may take before a stage edge shows past the image. */ +function panBound(stageExtent: number, fittedExtent: number, scale: number): number { + // Saturated, not merely finiteness-checked: the product is what overflows, + // and a finite `fittedExtent` near Number.MAX_VALUE times a finite scale is + // still `Infinity`, which would make the bound infinite and disable the + // clamp entirely. + const stage = saneExtent(stageExtent); + const scaled = saneExtent(saneExtent(fittedExtent) * finiteOr(scale, FIT)); + // An axis that does not overflow the stage is CENTRED, not free to roam. + if (scaled <= stage) return 0; + return (scaled - stage) / 2; +} + +/** + * Clamp the pan offsets for the state's own scale. + * + * Per axis: if the scaled extent is at most the stage, the offset is forced to + * 0 — a smaller-than-stage axis is CENTRED, never draggable. Otherwise the + * offset is bounded so no stage edge can show past the image edge. + * + * `scale` is passed through untouched (only sanitised for finiteness) — clamping + * it is {@link clampScale}'s job, and {@link clampState} composes the two in the + * order a resize needs. + */ +export function clampPan(state: ZoomState, g: Geometry): ZoomState { + const scale = finiteOr(state.scale, FIT); + const bx = panBound(g.stageW, g.fittedW, scale); + const by = panBound(g.stageH, g.fittedH, scale); + const x = finiteOr(state.x, 0); + const y = finiteOr(state.y, 0); + return { + scale, + x: clampSymmetric(x, bx), + y: clampSymmetric(y, by), + }; +} + +/** + * Clamp scale FIRST, then pan — the order a geometry change requires. + * + * `maxScale` is geometry-dependent and geometry is viewport-dependent, so + * ENLARGING the window lowers `actualScale` and with it `maxScale`, stranding a + * previously-valid scale above the new ceiling. Clamping pan first would bound + * it against a scale that is about to change (TASK-2455). + */ +export function clampState(state: ZoomState, g: Geometry): ZoomState { + return clampPan({ ...state, scale: clampScale(state.scale, g) }, g); +} + +/** + * Zoom to `targetScale` keeping the image point currently under `anchor` under + * `anchor` afterwards. + * + * The invariant, with `u` the anchor relative to the stage centre and + * `k = newScale / oldScale`: + * + * t' = u - k * (u - t) + * + * Anchor invariance holds exactly UNLESS the result has to be pan-clamped — + * the bounds win, because letting the anchor survive would mean showing blank + * stage past an image edge. A non-finite anchor falls back to the stage centre. + * + * The incoming state is normalised through {@link clampState} first, so the + * invariant is stated against a VALID starting state. Clamping the incoming + * scale while trusting its pan would mix a pan belonging to one scale with + * another, and the invariant would then be false for the one caller shape most + * likely to hit it — a geometry change that lowered the ceiling under a state + * the consumer has not re-clamped yet. + */ +export function zoomTo( + state: ZoomState, + targetScale: number, + anchor: Point, + g: Geometry, +): ZoomState { + const current = clampState(state, g); + const from = current.scale; + const to = clampScale(targetScale, g); + const centre = stageCenter(g); + const ux = finiteOr(anchor?.x, centre.x) - centre.x; + const uy = finiteOr(anchor?.y, centre.y) - centre.y; + // `from` came out of clampScale, so it is at least FIT — never a zero divide. + const k = to / from; + const x = ux - k * (ux - current.x); + const y = uy - k * (uy - current.y); + return clampPan({ scale: to, x, y }, g); +} + +/** + * Whether `state` is at fit, within {@link FIT_EPSILON}. + * + * ASYMMETRIC AT THE BOUNDARY, AND DELIBERATELY LEFT SO. `FIT + FIT_EPSILON` + * reads as at-fit while `FIT - FIT_EPSILON` does not, because neither value is + * exactly representable in binary and the subtraction lands a few ulps outside + * the window. Widening the comparison to hide that would be papering over an + * unreachable case: every scale this module emits has been through + * {@link clampScale}, whose floor is FIT, so a sub-fit state cannot arise from + * any sequence of operations on it. The asymmetry is only observable on a + * hand-constructed state, and both legs are pinned by tests so a future change + * to the floor cannot make it matter silently. + */ +export function isAtFit(state: ZoomState): boolean { + return Math.abs(finiteOr(state.scale, FIT) - FIT) <= FIT_EPSILON; +} + +/** + * The double-click / double-tap toggle: fit <-> actual size. + * + * At fit (within {@link FIT_EPSILON}) it goes to {@link actualScale} anchored at + * `anchor`; anywhere else it goes back to fit, centred. When actual size IS fit + * — an image smaller than the stage — it goes to {@link TOGGLE_SMALL_SCALE} + * instead, so the gesture always does something visible. + * + * DELIBERATE GENERALISATION OF THE CONTRACT. The written rule says "when + * `actualScale === 1`, go to {@link TOGGLE_SMALL_SCALE}"; this uses + * `actualScale <= FIT + FIT_EPSILON` instead. Exact equality serves the rule's + * PURPOSE only on integer geometry — at a fractional `devicePixelRatio` a 1:1 + * image measures 1.0000000004 and at a rounded layout it can measure 1.0005, + * and a toggle to either is a zoom of at most 0.05%: invisible, i.e. exactly + * the no-op the rule exists to prevent. The epsilon is the same one + * {@link isAtFit} uses, so "already at fit" and "actual size is fit" cannot + * disagree about the same image. + */ +export function toggleFitOrActual(state: ZoomState, anchor: Point, g: Geometry): ZoomState { + if (!isAtFit(state)) return reset(); + const actual = actualScale(g); + const target = actual > FIT + FIT_EPSILON ? actual : TOGGLE_SMALL_SCALE; + return zoomTo(state, target, anchor, g); +} + +/** + * The identity transform: fit, centred. + * + * Takes no geometry, which is precisely what makes it safe to call before a + * bitmap exists — on open, on image change, and on close (TASK-2455). + */ +export function reset(): ZoomState { + return { scale: FIT, x: 0, y: 0 }; +} diff --git a/web/src/lib/collections/paneFocus.svelte.test.ts b/web/src/lib/collections/paneFocus.svelte.test.ts index 1fd1288b..3532e971 100644 --- a/web/src/lib/collections/paneFocus.svelte.test.ts +++ b/web/src/lib/collections/paneFocus.svelte.test.ts @@ -5,6 +5,7 @@ import { nextTrapTarget, resolvePaneReturnTarget, inExemptSurface, + handoffFocus, } from './paneFocus'; // jsdom has no layout engine, so `offsetParent` / `getClientRects` can't gate @@ -209,3 +210,118 @@ describe('resolvePaneReturnTarget', () => { expect(resolvePaneReturnTarget(root, null)).toBeNull(); }); }); + +describe('handoffFocus (TASK-2456)', () => { + // The modal-focus-retention helper: when a focused control leaves a surface + // (removed or disabled), focus must not fall to behind the inerted + // background. Two shapes — reactive (no `departing`, repair after the fact) + // and imperative (pass `departing`, hand off before removal/disable). + + it('CONTROL: without the handoff, removing a focused control strands focus on ', () => { + // The defect the helper exists to fix, proven real in this environment so + // the positive legs below are not vacuous: jsdom (like a real engine) drops + // focus to when the focused element is removed. + const el = mount(``); + const next = document.getElementById('next')!; + next.focus(); + expect(document.activeElement).toBe(next); + next.remove(); + expect(document.activeElement).toBe(document.body); + }); + + it('reactive shape: pulls focus off back to the first tabbable fallback', () => { + const el = mount(``); + const next = document.getElementById('next')!; + next.focus(); + next.remove(); + expect(document.activeElement).toBe(document.body); + + handoffFocus(el, null, allVisible); + expect(document.activeElement).toBe(document.getElementById('close')); + }); + + it('reactive shape: leaves a live focus INSIDE the surface untouched', () => { + // Only repairs a focus that has LEFT the surface — a focus resting on a + // real control must not be yanked to the first tabbable. + const el = mount(``); + const close = document.getElementById('close')!; + close.focus(); + handoffFocus(el, null, allVisible); + expect(document.activeElement).toBe(close); + }); + + it('imperative shape: hands focus off a control about to be DISABLED, never back onto it', () => { + // A real engine drops focus to the instant a focused control is + // disabled; jsdom keeps it there, so the imperative shape blurs explicitly + // while the control still holds focus. This is the call TASK-2459/2460 make + // before setting `disabled` on their retry / tap-to-load control. + // + // The departing control is ordered FIRST on purpose: it is `paneFocusables() + // [0]`, so a fallback that did not EXCLUDE it would re-select the very + // control about to be disabled — dropping focus to on a real engine. + const el = mount(``); + const retry = document.getElementById('retry') as HTMLButtonElement; + retry.focus(); + expect(document.activeElement).toBe(retry); + + handoffFocus(el, retry, allVisible); + // The caller now DISABLES the control it just handed focus off — the exact + // sequence TASK-2459/2460 run. On a real engine this is the step that would + // drop focus to had it still been on `retry`; because the handoff + // moved focus to Close first, the disable is now harmless. + retry.disabled = true; + expect(document.activeElement).toBe(document.getElementById('close')); + expect(document.activeElement).not.toBe(retry); + expect(document.activeElement).not.toBe(document.body); + }); + + it('imperative shape: falls back to the container when the departing control is the ONLY tabbable', () => { + // Mid-load a viewer's tap-to-load / retry control can be the only focusable + // thing. Handing off before disabling it must NOT re-select it (→ on + // disable) — it lands on the container (`tabindex="-1"`), still inside the + // surface. + const el = mount(``); + el.tabIndex = -1; + const tap = document.getElementById('tap') as HTMLButtonElement; + tap.focus(); + handoffFocus(el, tap, allVisible); + expect(document.activeElement).toBe(el); + expect(document.activeElement).not.toBe(tap); + }); + + it('imperative shape: no-op when the departing control is not the focused one', () => { + const el = mount(``); + const close = document.getElementById('close')!; + close.focus(); + const next = document.getElementById('next')!; + handoffFocus(el, next, allVisible); + expect(document.activeElement).toBe(close); + }); + + it('drops to the container when the preferred fallback REFUSES focus (inert / hidden)', () => { + // jsdom cannot reproduce inert (it always focuses), so stub the close + // button's focus() to no-op — the real-engine shape of a fallback that sits + // under an inert / hidden ancestor. Focus must still land INSIDE the surface + // (the tabindex="-1" container), never on . + const el = mount(``); + el.tabIndex = -1; + const next = document.getElementById('next')!; + next.focus(); + next.remove(); + const close = document.getElementById('close') as HTMLButtonElement; + close.focus = () => {}; // refuses focus, like an inert control + handoffFocus(el, null, allVisible); + expect(document.activeElement).toBe(el); + expect(document.activeElement).not.toBe(document.body); + }); + + it('falls back to the container itself when it has no tabbable control', () => { + // Mid-load a viewer can have no focusable control yet; the container is + // `tabindex="-1"` so focus still lands inside the surface, never on . + const el = mount(`loading…`); + el.tabIndex = -1; + expect(document.activeElement).toBe(document.body); + handoffFocus(el, null, allVisible); + expect(document.activeElement).toBe(el); + }); +}); diff --git a/web/src/lib/collections/paneFocus.ts b/web/src/lib/collections/paneFocus.ts index ca33a7b4..0452e999 100644 --- a/web/src/lib/collections/paneFocus.ts +++ b/web/src/lib/collections/paneFocus.ts @@ -4,7 +4,10 @@ * Split out of `[collection]/+page.svelte` so the DOM-selection and * focus-trap-cycle logic is unit testable without a full component mount * (mirrors `paneUrlParams` / `boardNav`). The `.svelte` side owns the effects - * that install/tear these down; this module is pure, side-effect-free DOM math. + * that install/tear these down; the selection + cycle helpers are pure DOM math. + * The one exception is {@link handoffFocus}, which deliberately MOVES focus (its + * whole job) — the `.svelte` side calls it from an effect, but it is exported + * here so it lives with the focus math it builds on. */ /** @@ -113,6 +116,72 @@ export function nextTrapTarget( return null; } +/** + * Keep focus INSIDE a modal surface when a focused control leaves it — is + * removed from the DOM, or becomes `disabled` (PLAN-2392 / TASK-2456). + * + * `aria-modal="true"` promises focus never escapes the surface while it is open, + * yet a conditionally-rendered control that had focus drops focus to `` + * when it unmounts, and a control that becomes `disabled` does the same on a real + * engine — landing focus BEHIND the surface's own inerted background. Neither the + * Tab trap (fires only on a later Tab) nor the teardown restore (only at close) + * repairs that gap; this does, moving focus to the surface's stable fallback. + * + * Two call shapes, one helper (the house pattern from + * `editor/attachment-image.ts` — blur the departing control, focus its + * replacement): + * + * - BEFORE an imperative removal/disable, pass the `departing` control: if it + * currently holds focus it is blurred and focus moves to the fallback. This + * is the shape TASK-2459's retry and TASK-2460's tap-to-load use (a real + * engine drops focus to `` the instant a focused control is disabled, + * so the handoff must run while the control is still focused). + * - AFTER a reactive removal (a Svelte `{#if}` dropped the control), pass no + * `departing`: if focus has ALREADY fallen out of `container`, it is pulled + * back within the same synchronous flush, so `` is never observably + * focused. A focus still resting on a live control inside `container` is left + * untouched. + * + * The fallback is the first tabbable control (for the viewer, its close button), + * else `container` itself — the same target entry focus uses, so it is always + * reachable even mid-load with no other control yet. + */ +export function handoffFocus( + container: HTMLElement, + departing: Element | null = null, + isVisible: (el: HTMLElement) => boolean = isFocusableVisible, +): void { + if (typeof document === 'undefined') return; + const active = document.activeElement; + // Imperative (departing given): act only while that control still owns focus, + // so a handoff for a control the user already left is a no-op. Reactive (no + // departing): act only once focus has left the surface — a live focus inside + // it is fine and must not be yanked to the fallback. + const leaving = + departing !== null + ? active === departing + : active === null || active === document.body || !container.contains(active); + if (!leaving) return; + if (departing instanceof HTMLElement) departing.blur(); + // EXCLUDE `departing` from the candidates: in the imperative shape it is still + // enabled and in the DOM at call time (the caller disables/removes it AFTER + // this), so a departing control that is the first — or only — tabbable would + // otherwise be re-selected here and then dropped to the instant the + // caller disables it. When it is the only one, fall through to the container. + const fallback = + paneFocusables(container, isVisible).find((el) => el !== departing) ?? container; + fallback.focus({ preventScroll: true }); + // Verify it took, then drop to the container — the same verified-restore + // pattern the viewer uses on close. `paneFocusables`' visibility filter is + // geometry-only, so it cannot see that a candidate sits under `inert` / + // `visibility: hidden` (a real engine refuses focus there); if the preferred + // target refuses, the container (a `tabindex="-1"` surface root) still keeps + // focus INSIDE the surface rather than letting it fall to . + if (fallback !== container && document.activeElement !== fallback) { + container.focus({ preventScroll: true }); + } +} + /** * Resolve the element to return focus to when the pane CLOSES (TASK-2122): the * row that opened / last drove the pane. diff --git a/web/src/lib/components/common/Lightbox.svelte b/web/src/lib/components/common/Lightbox.svelte index 4ffa4ebf..0ad043d6 100644 --- a/web/src/lib/components/common/Lightbox.svelte +++ b/web/src/lib/components/common/Lightbox.svelte @@ -2,9 +2,11 @@ /** * Full-screen image viewer for attachment thumbnails (IDEA-1660). * Opened by a host that captures a click on an `img[data-attachment-id]` - * and passes the attachment id(s) — the lightbox loads the ORIGINAL - * (un-variant) blob so the expanded view is full resolution regardless - * of the thumbnail variant shown inline. + * and passes the attachment id(s). Loading follows the DR-5b memory policy + * (PLAN-2392 phase 3b): a bounded `thumb-md` paints first and the viewer + * upgrades to the full-resolution original in the background on desktop, while + * a large image on mobile defers the original behind a tap-to-load affordance + * (TASK-2459 / TASK-2460) — see `$lib/attachments/viewerImageLoader`. * * MODAL CONTRACT (PLAN-2392 phase 3a / TASK-2429, DR-4b). This is a real * modal now: `role="dialog"` + `aria-modal`, portaled to ``, focus @@ -16,11 +18,15 @@ * * Keyboard: Esc closes (through `escapeStack`, NOT a local listener), * ←/→ navigate when multiple images were passed, Tab cycles within the - * viewer. Backdrop click closes; clicking the image itself does not. + * viewer, `+`/`-` zoom about the stage centre and `0` resets (PLAN-2392 + * phase 3b / TASK-2455). Backdrop click closes; clicking the image itself + * does not. */ import { untrack } from 'svelte'; - import { attachmentDownloadUrl } from '$lib/markdown/attachments'; - import { paneFocusables, nextTrapTarget } from '$lib/collections/paneFocus'; + import { paneFocusables, nextTrapTarget, handoffFocus } from '$lib/collections/paneFocus'; + import { createViewerImageLoader } from '$lib/attachments/viewerImageLoader.svelte'; + import type { Platform } from '$lib/attachments/viewerLoading'; + import { viewport } from '$lib/stores/breakpoint.svelte'; import { acquire, isBlockedByModal, @@ -30,6 +36,18 @@ } from '$lib/a11y/viewerBackdrop'; import { pushEscapeHandler, ESCAPE_PRIORITY } from '$lib/stores/escapeStack'; import { canOpenInViewer } from '$lib/attachments/display'; + import { + reset as resetZoom, + clampState, + clampPan, + zoomTo, + toggleFitOrActual, + stageCenter, + isAtFit, + ZOOM_STEP, + type Geometry, + type ZoomState, + } from '$lib/attachments/zoom'; /** * ONE definition of what an image in this viewer is (PLAN-2392 / TASK-2431). * @@ -43,7 +61,8 @@ * * Everything past `id` / `alt` is NULLABLE and this component treats it as * such: an inline image's metadata comes from a HEAD probe that may not have - * completed, and an upload event carries only four fields. + * completed, and an upload event carries only the `UploadedAttachment` fields + * (filename, MIME, size, and — since TASK-2459 — the pixel dimensions). * * Producers that mount this component directly import the type from the * CHANNEL too, not from here — a `.svelte` module cannot re-export a type, @@ -162,17 +181,398 @@ Math.min(Math.max(current, 0), Math.max(viewable.length - 1, 0)) ); let img = $derived(viewable[shownIndex]); - let src = $derived(img ? attachmentDownloadUrl(openWsSlug, img.id) : ''); // The accessible name: the image's own alt where there is one, else a // generic label. Never empty — an unnamed `role="dialog"` is announced as // nothing at all. let dialogLabel = $derived(img?.alt || 'Attachment viewer'); + // ── Image loading (PLAN-2392 phase 3b / TASK-2459) ──────────────────────── + // + // The DR-5b thumb-then-original policy. The loader owns which URL the + // shows (`displaySrc`, the canonical attachment URL) and the load phase; this + // component drives it from the shown image and reports each decode / error. + const loader = createViewerImageLoader(); + // The id AND the pixel dimensions, as ONE stable primitive string — never the + // `img` object. A prop re-emit with the same VALUES (a re-derived `viewable` + // array) must not re-fire the load effect, but a genuine dimension change (an + // async metadata fill that flips `unknown` → a sized class) MUST: the DR-5b + // policy is a function of the pixels, so a stale dimension is a stale policy. + // A shrink to no image collapses to `'::'`, still a change → the effect fires + // and releases the load. + let loadKey = $derived(`${img?.id ?? ''}:${img?.width ?? ''}:${img?.height ?? ''}`); + // Captured NON-reactively at load time (see the effect): a breakpoint flip + // alone must not reload — desktop→mobile must not abort an in-flight original, + // mobile→desktop must not retroactively auto-fetch (TASK-2459). + let platform = $derived(viewport.isMobile ? 'mobile' : 'desktop'); + // Whether a decoded bitmap exists to zoom / pan. False in the mobile `deferred` + // cell (a placeholder, nothing decoded), when there is no image, AND in the + // `error` state: `errored()` flips only the phase, leaving `displaySrc` set (the + // failed URL), so without the phase guard drag-arming and the zoom keys would + // act over the error UI with nothing decoded behind it. Zoom is then DISABLED, + // not merely a no-op (TASK-2460). A successful retry returns to loading/ready + // and re-enables it. + let bitmapPresent = $derived(!!img && !!loader.displaySrc && loader.phase !== 'error'); + // The tap-to-load placeholder's box. Where dimensions are known it takes the + // image's own aspect ratio (so the affordance previews the shape that will + // arrive); where they are not, a neutral box (TASK-2460). + let placeholderStyle = $derived( + img?.width && img?.height + ? `aspect-ratio: ${img.width} / ${img.height}; width: min(70vw, 520px); max-width: 90%; max-height: 80%;` + : `width: min(60vw, 360px); height: min(45vh, 270px); max-width: 90%; max-height: 80%;` + ); + // The portaled root. `$state` so the effect below re-runs once `bind:this` // lands; read-only inside every effect, so nothing here can self-invalidate // a flush (CONVE-1688). let rootEl = $state(null); + // ── Zoom / pan (PLAN-2392 phase 3b / TASK-2455) ────────────────────────── + // + // The transform is `translate(x,y) scale(scale)` on the , about the + // stage centre. Every number comes from `$lib/attachments/zoom` (TASK-2454), + // which owns the arithmetic and its bounds; this component only MEASURES the + // rendered geometry, wires the keys, and re-clamps on resize. + let zoom = $state(resetZoom()); + // The stage is the 92vw×92vh box the bare used to be; the sits + // inside it, `object-fit: contain`. Both are read live for geometry — never + // through `getBoundingClientRect()` on the transformed image, which returns + // the POST-scale box and would make the pan bounds grow with the zoom. + let stageEl = $state(null); + let imgEl = $state(null); + + /** + * The measured geometry, or null before there is anything to measure. + * + * `offsetWidth` / `offsetHeight` are the UNSCALED layout box — transforms do + * not touch them, which is the whole reason they, not `getBoundingClientRect`, + * are the source here. A not-yet-decoded bitmap reads back all zeros; the zoom + * module is defensive about that and still returns an in-bounds transform, so + * no guard is needed here. + */ + function readGeometry(): Geometry | null { + const stage = stageEl; + const image = imgEl; + if (!stage || !image) return null; + return { + stageW: stage.clientWidth, + stageH: stage.clientHeight, + fittedW: image.offsetWidth, + fittedH: image.offsetHeight, + naturalW: image.naturalWidth, + naturalH: image.naturalHeight, + }; + } + + // `+` / `-` zoom about the stage centre. Reading and writing `zoom` from an + // EVENT handler is fine — the CONVE-1688 rule is about `$effect`s that read + // the state they write, not about handlers. + function stepZoom(factor: number) { + const g = readGeometry(); + if (!g) return; + zoom = zoomTo(zoom, zoom.scale * factor, stageCenter(g), g); + rebaseDrag(); // keyboard zoom mid-drag must not desync the pan baseline + } + + // Wheel / ctrl-cmd-wheel zoom, anchored at the CURSOR (TASK-2457 / DR-4). Both + // plain AND ctrl/cmd wheel zoom, so there is no modifier gate — but the + // listener is registered NON-PASSIVELY (see the effect below) so + // `preventDefault` takes effect: the inert page behind must not scroll, and + // ctrl/cmd+wheel must not trigger the browser's own page zoom. `stopPropagation` + // as well, so the page's scroll-restoration listener does not count this as a + // user scroll (belt; `restore.svelte.ts` also ignores viewer input — braces). + function onWheel(e: WheelEvent) { + const el = rootEl; + // Same gates as `onKeydown`: only the frontmost, non-blocked viewer acts. + if (!el || !isViewerFrontmost(el) || isBlockedByModal(el)) return; + // We own the wheel while frontmost — consume it even before the bitmap is + // measurable, so a scroll can never leak past the modal into the inert app. + e.preventDefault(); + e.stopPropagation(); + // Consumed (the modal owns the wheel) but INERT with no decoded bitmap — the + // mobile deferred placeholder or the error UI, where the broken `` still + // satisfies `readGeometry` (TASK-2461). Same guard the keys use. + if (!bitmapPresent) return; + // A horizontal-only wheel (`deltaY === 0`, e.g. a trackpad side-swipe) is + // still consumed — the modal owns the wheel — but must NOT zoom, or it would + // read as a zoom-out. Direction comes from `deltaY` alone. + if (e.deltaY === 0) return; + const g = readGeometry(); + const rect = stageEl?.getBoundingClientRect(); + if (!g || !rect) return; + // Anchor in stage-local px (top-left origin) — the coordinate system the + // zoom module documents. The stage is untransformed, so its rect is stable. + const anchor = { x: e.clientX - rect.left, y: e.clientY - rect.top }; + // Wheel up / away (deltaY < 0) zooms in. + const factor = e.deltaY < 0 ? ZOOM_STEP : 1 / ZOOM_STEP; + zoom = zoomTo(zoom, zoom.scale * factor, anchor, g); + // A wheel DURING a captured drag just moved `zoom.{x,y}` from this pointer + // position — rebase so the next pointermove continues from here. + lastClientX = e.clientX; + lastClientY = e.clientY; + rebaseDrag(); + } + + // ── Double-click toggle + drag-to-pan (PLAN-2392 / TASK-2458) ──────────── + // + // Desktop single-pointer half of the drag-pan work (3d keeps two-pointer + // pinch, double-TAP and touch semantics). Modelled on the captured-drag house + // pattern at `graph/ItemGraph.svelte` — arm on pointerdown, capture only once a + // real drag engages (capturing on down would swallow the dblclick), re-check + // the arbitration gates on every move so a gesture that STRADDLES a + // frontmost/modal change aborts instead of panning an image the user no longer + // owns, and suppress the synthesized click a pan produces. + const DRAG_THRESHOLD = 4; // CSS px below which the gesture is a click, not a pan + let maybeDrag = false; + // `$state` so the template can drop the transform TRANSITION while dragging — + // otherwise the image eases toward each pan target and visibly trails the + // pointer. Read/written only in pointer handlers (never an $effect's tracked + // scope), so no CONVE-1688 self-write. + let dragging = $state(false); + let suppressClick = false; + let capturedPointerId: number | null = null; + // The pointer that OWNS the current gesture, captured at pointerdown. Every + // later move/up/cancel/lost must come from it: a touch or a second pointer + // (pen) is not captured, so its events still reach this root and would + // otherwise engage or terminate an in-flight mouse drag (touch stays native + // until 3d). + let gesturePointerId: number | null = null; + let dragStartClientX = 0; + let dragStartClientY = 0; + let dragOriginX = 0; // zoom.x at drag baseline + let dragOriginY = 0; // zoom.y at drag baseline + let lastClientX = 0; // last pointer position seen during the gesture + let lastClientY = 0; + + // Re-baseline the gesture to the CURRENT zoom + last pointer position. The drag + // computes `origin + total delta`, so anything that moves `zoom.{x,y}` from + // OUTSIDE the drag — wheel, `+`/`-`/`0` keys, a resize re-clamp — must rebase, + // or the next move snaps the pan back to the pre-change origin (a visible jump). + // Rebases an ARMED gesture too (not only a live drag): a zoom that lands + // between pointerdown and the drag threshold would otherwise leave the engage + // baseline stale. A no-op when no gesture is in flight. + function rebaseDrag(): void { + if (!maybeDrag && !dragging) return; + dragOriginX = zoom.x; + dragOriginY = zoom.y; + dragStartClientX = lastClientX; + dragStartClientY = lastClientY; + } + // A SINGLE owned timer for clearing `suppressClick`, so an EARLIER gesture's + // pending clear can never fire during a LATER one and unsuppress its pan's + // click. `cancelSuppressClear` runs whenever a new gesture is armed or a drag + // engages (the flag is held true for the whole drag); `armSuppressClear` runs + // once at the END of a pan to drop it on the next tick, after the synthesized + // click has been (and gone). + let suppressClickTimer: ReturnType | null = null; + function cancelSuppressClear(): void { + if (suppressClickTimer !== null) { + clearTimeout(suppressClickTimer); + suppressClickTimer = null; + } + } + function armSuppressClear(): void { + suppressClick = true; + cancelSuppressClear(); + suppressClickTimer = setTimeout(() => { + suppressClick = false; + suppressClickTimer = null; + }, 0); + } + + // The same gates `onKeydown` / `onWheel` carry: only the frontmost, non-blocked + // viewer owns the pointer. Applied at EVERY entry point (down / move / up / + // dblclick / backdrop click), not just the start. + function pointerGatesOpen(el: HTMLElement): boolean { + return isViewerFrontmost(el) && !isBlockedByModal(el); + } + + function releaseCapture(e: PointerEvent): void { + if (capturedPointerId !== null) { + try { + (e.currentTarget as Element).releasePointerCapture(capturedPointerId); + } catch { + // already released — ignore. + } + capturedPointerId = null; + } + } + + // Tear a gesture down mid-flight: release the capture (an early return alone + // would leave the pointer captured and `dragging` latched, still delivering + // moves), and — if a real pan was underway — still swallow the click it + // produces. The transform is LEFT WHERE IT WAS (the abort does not undo pan). + function abortGesture(e: PointerEvent): void { + const wasDragging = dragging; + maybeDrag = false; + dragging = false; + gesturePointerId = null; + releaseCapture(e); + if (wasDragging) armSuppressClear(); + } + + function onPointerDown(e: PointerEvent) { + if (e.button !== 0) return; // primary button only + if (e.pointerType === 'touch') return; // touch stays native until 3d + // A second pointer (a pen, say) pressing mid-drag must NOT seize the gesture: + // re-arming below would replace `gesturePointerId` and hand the pan to the + // interloper. An active drag is owned until its own pointer releases. + if (dragging) return; + // A new primary press supersedes any STALE armed gesture — one whose + // pointerup was missed off-root (no capture yet, so it was never delivered). + // Clear it before any early return below, or a later control / gated press + // leaves `maybeDrag` latched and the next move engages a phantom drag from a + // dead baseline. (We already returned above if a drag is live.) + maybeDrag = false; + // A press ON a control (close / nav / the DR-10 retry) is that control's + // click, never a pan: arming here would let a drag OFF a button still fire + // its click, since the buttons' own handlers don't consult `suppressClick` + // (the house pattern in ItemGraph excludes its interactive overlays the same + // way). Retry sits over the (broken) stage in the error state, so it needs + // the same exclusion as the always-present chrome. + if ((e.target as Element | null)?.closest?.('.lightbox-close, .lightbox-nav, .lightbox-retry, .lightbox-tap-load')) return; + // No decoded bitmap to pan (the mobile `deferred` placeholder shows no + // ``) — do NOT arm a drag, so the gesture stays fully inert instead of + // capturing the pointer and latching `dragging` for a pan that can never + // happen (TASK-2460). A plain press still reaches the backdrop to close. + if (!bitmapPresent) return; + const el = rootEl; + if (!el || !pointerGatesOpen(el)) return; // START gate + maybeDrag = true; + dragging = false; + gesturePointerId = e.pointerId; + suppressClick = false; + cancelSuppressClear(); // a fresh gesture — drop any prior pan's pending clear + dragStartClientX = e.clientX; + dragStartClientY = e.clientY; + lastClientX = e.clientX; + lastClientY = e.clientY; + // Capture the pan origin from the state at pointer-down, so every move + // computes origin + TOTAL delta — never delta-of-delta, which the zoom + // module warns loses the slack at a bound (its "gesture is not associative" + // note). No `setPointerCapture` yet: capturing here would swallow dblclick. + dragOriginX = zoom.x; + dragOriginY = zoom.y; + } + + function onPointerMove(e: PointerEvent) { + if (!maybeDrag) return; + if (e.pointerId !== gesturePointerId) return; // only the owning pointer drives + const el = rootEl; + // WHOLE-GESTURE arbitration: the press may have been captured before another + // viewer / a native modal became frontmost. Abort rather than early-return. + if (!el || !pointerGatesOpen(el)) { + abortGesture(e); + return; + } + // The bitmap vanished mid-drag — the background original failed and the phase + // went to `error` while a pan was live (TASK-2461). Abort: there is nothing + // left to pan, and continuing would move the (broken) error UI. + if (!bitmapPresent) { + abortGesture(e); + return; + } + // Primary button no longer held — a pointerup we never received (it ended + // off-target pre-capture, or was swallowed after a drag engaged). Tear the + // whole gesture down, not just `maybeDrag`: a full `abortGesture` also + // releases any capture and clears `dragging`, so nothing leaks into the next + // pointerdown (a bare `maybeDrag = false` left a live capture + `dragging`). + if ((e.buttons & 1) === 0) { + abortGesture(e); + return; + } + lastClientX = e.clientX; + lastClientY = e.clientY; + const dx = e.clientX - dragStartClientX; + const dy = e.clientY - dragStartClientY; + if (!dragging) { + if (Math.hypot(dx, dy) < DRAG_THRESHOLD) return; + dragging = true; + // This gesture is a pan, not a click: suppress until the drag ENDS. Held + // true for the whole drag; cancel any prior gesture's pending clear so it + // can't fire mid-pan and unsuppress us. + suppressClick = true; + cancelSuppressClear(); + capturedPointerId = e.pointerId; + try { + (e.currentTarget as Element).setPointerCapture(e.pointerId); + } catch { + // capture unsupported/failed — panning still works while over the root + } + } + const g = readGeometry(); + if (!g) return; + // Clamp PAN only — the drag never changes scale — from the captured origin + // plus the total delta. `clampPan` keeps a stage edge from showing past the + // image; at fit the bound is zero, so a drag is a no-op pan. + zoom = clampPan({ scale: zoom.scale, x: dragOriginX + dx, y: dragOriginY + dy }, g); + } + + function onPointerUp(e: PointerEvent) { + if (!maybeDrag && !dragging) return; // not a gesture we started + if (e.pointerId !== gesturePointerId) return; // a foreign pointer can't end ours + const el = rootEl; + // Re-check on release too (spec): a gesture that straddled a + // frontmost/modal change aborts rather than finalising a pan. + if (!el || !pointerGatesOpen(el)) { + abortGesture(e); + return; + } + maybeDrag = false; + gesturePointerId = null; + if (dragging) { + dragging = false; + releaseCapture(e); + // Keep the click this pan produces swallowed, then clear on the next tick + // so a later genuine click is unaffected. + armSuppressClear(); + } + } + + function onPointerCancel(e: PointerEvent) { + if (!maybeDrag && !dragging) return; // no gesture to cancel + if (e.pointerId !== gesturePointerId) return; + abortGesture(e); + } + + function onLostPointerCapture(e: PointerEvent) { + if (!maybeDrag && !dragging) return; // no gesture; ignore a stray capture-loss + if (e.pointerId !== gesturePointerId) return; + // Capture taken away (OS/browser, or our own release) — the gesture is over. + // Clear WITHOUT releasing (already gone); keep the click suppressed if a pan + // was underway. Idempotent with `onPointerUp`, which sets `dragging` false + // before releasing, so a release-driven event here is a no-op. + const wasDragging = dragging; + maybeDrag = false; + dragging = false; + capturedPointerId = null; + gesturePointerId = null; + if (wasDragging) armSuppressClear(); + } + + // Double-click toggles fit <-> actual size, anchored at the pointer + // (`toggleFitOrActual`). A pan suppresses its clicks, so a dblclick only fires + // on a genuine double-click, never after a drag. + function onDoubleClick(e: MouseEvent) { + // A pan just ended (its click is being swallowed) — don't also toggle: a + // gesture is either a drag or a double-click, never both. Same swallow the + // backdrop click consults. + if (suppressClick) return; + // A double-click ON a control (close / nav / retry) is that control's, not a + // zoom toggle — without this, double-clicking Next navigates twice AND + // toggles, or a double-click on Retry toggles zoom on the broken stage. + if ((e.target as Element | null)?.closest?.('.lightbox-close, .lightbox-nav, .lightbox-retry, .lightbox-tap-load')) return; + // Inert with no decoded bitmap (deferred placeholder / error UI) — the same + // guard the keys and wheel use (TASK-2461). + if (!bitmapPresent) return; + const el = rootEl; + if (!el || !pointerGatesOpen(el)) return; + const g = readGeometry(); + const rect = stageEl?.getBoundingClientRect(); + if (!g || !rect) return; + const anchor = { x: e.clientX - rect.left, y: e.clientY - rect.top }; + zoom = toggleFitOrActual(zoom, anchor, g); + } + // Stepped from `shownIndex`, not from `current`: after the set shrinks they // differ, and moving from the raw value would jump relative to a position // the user was never on. Both are no-ops on an empty set — reachable only @@ -289,6 +689,129 @@ }; }); + // Reset the transform whenever the SHOWN image changes — arrow nav, or the + // set shrinking under `current` so a different member is shown (TASK-2455). + // Close needs no handling: every producer keys the mount, so closing + // unmounts this instance and the next open starts from `resetZoom()`. + // + // `lastResetForId` is a PLAIN let, not `$state`: an effect that read and wrote + // the same `$state` would self-depend and abort its own flush (CONVE-1688), + // stranding unrelated reactivity nearby. This effect reads `img?.id` (tracked) + // and writes `zoom` (which it never reads) plus this sentinel (a plain let, so + // never tracked) — nothing it writes is anything it reads. Seeded to the + // current id so the mount does not fire a redundant reset. + let lastResetForId: string | undefined = untrack(() => img?.id); + $effect(() => { + const id = img?.id; + if (id === lastResetForId) return; + lastResetForId = id; + zoom = resetZoom(); + // A drag live ACROSS the image change (arrow-nav mid-drag) is left with a + // stale baseline, and deliberately so: `resetZoom` is fit, where `clampPan` + // pins the pan to 0 for ANY baseline, so the next move can't jump; and the + // next wheel/keyboard zoom rebases via `rebaseDrag`. Re-seeding here would be + // unobservable — and calling `rebaseDrag` (which reads `zoom`) inside this + // `zoom`-writing effect would self-invalidate it (CONVE-1688). + }); + + // (Re)load whenever the SHOWN image changes — nav, a dimension fill, or a set + // shrink. This is also the ABORT + release point: `loader.load` drops the URL + // the user left, and it re-runs on the set shrinking to empty (`loadKey` → + // `'::'`) so a closed / emptied viewer holds no in-flight request. Reads only + // `loadKey` (tracked, a stable string so a same-values prop re-emit doesn't + // re-fire); `img`, `openWsSlug` and `platform` are captured non-reactively so a + // breakpoint flip alone can't reload (TASK-2459). + $effect(() => { + void loadKey; + untrack(() => loader.load(img, openWsSlug, platform)); + }); + // Drop the load on unmount (close) — one teardown, no dependencies. + $effect(() => () => loader.dispose()); + + // Zoom-past-fit is the mobile THUMB cell's trigger to fetch the original + // (TASK-2460): once a thumbnail has PAINTED, zooming past FIT (`scale > 1`, not + // past the thumb's own 1:1) upgrades to the original. Threshold is + // `!isAtFit(zoom)` — exactly "past fit", robust to the FIT_EPSILON float noise. + // Depends on `zoom` AND `loader.painted` (both tracked): gating on `painted` + // means a pre-paint zoom cannot fetch the original early, AND — because the flip + // to painted RE-RUNS this effect — a zoom made WHILE the thumb was still loading + // still upgrades the instant it paints (not stranded on the thumbnail). + // `loadOriginal` writes neither `zoom` nor `painted`, so tracking them cannot + // self-invalidate the flush (CONVE-1688). Fires on every zoom step past fit, but + // `loadOriginal`'s own dedup makes all but the first a no-op — the original is + // requested exactly once — and it no-ops entirely wherever nothing is deferred + // (desktop, or an already-loaded cell). + $effect(() => { + if (isAtFit(zoom) || !loader.painted) return; + untrack(() => loader.loadOriginal()); + }); + + // Re-clamp on stage resize. `maxScale` is geometry-dependent and geometry is + // viewport-dependent, so ENLARGING the window lowers the ceiling and can + // strand a previously-valid scale above it. `clampState` re-clamps SCALE + // first, then pan — the order a geometry change needs; clamping pan first + // would bound it against a scale that is about to change. The callback runs + // outside any tracked scope, so its `zoom` read/write is not a self-write. + // Guarded for environments without `ResizeObserver` (SSR); the jsdom test + // project ships a global stand-in (`src/test/setup-jsdom.ts`). + $effect(() => { + const stage = stageEl; + if (!stage || typeof ResizeObserver === 'undefined') return; + const ro = new ResizeObserver(() => { + const g = readGeometry(); + if (g) { + zoom = clampState(zoom, g); + rebaseDrag(); // a resize re-clamp mid-drag must not desync the baseline + } + }); + ro.observe(stage); + return () => ro.disconnect(); + }); + + // Register the wheel listener imperatively with `{ passive: false }` so + // `preventDefault` is guaranteed to take effect (a declarative `onwheel` + // binding's passivity is not something to rely on), and on the viewer ROOT so + // a wheel over the backdrop letterbox is consumed too — scrolling "past" the + // modal into the inert app is exactly what the contract forbids (TASK-2457). + $effect(() => { + const el = rootEl; + if (!el) return; + el.addEventListener('wheel', onWheel, { passive: false }); + return () => el.removeEventListener('wheel', onWheel); + }); + + // aria-modal promises focus never leaves the surface, but a focused nav button + // is CONDITIONALLY rendered (`hasMultiple`) — when the set shrinks to one it + // unmounts, dropping focus to behind the inerted app until the next Tab + // (TASK-2456; a 3a defect every control 3b adds inherits). Keyed on the + // control-visibility signal, this hands focus back to the stable fallback + // (close button, else root) within the SAME flush the removal happens in, so + // is never observably focused. `handoffFocus` with no `departing` is + // the reactive-removal shape; TASK-2459/2460 will call the same helper with + // THEIR departing control before disabling it. + // + // Guarded to the frontmost, non-blocked viewer: a surface stacked OVER this + // one owns focus, and pulling it back here would be the wrong direction (the + // same reason the key handler stands down for those). Reads only derived / + // element state and mutates focus — a DOM side effect, not $state — so it + // cannot self-invalidate its own flush (CONVE-1688). + $effect(() => { + // Tracked so the effect re-runs whenever a focus-holding control mounts or + // unmounts: the nav buttons (`hasMultiple`), the phase-gated controls (the + // DR-10 retry button in `error`, the tap-to-load button in `deferred` — both + // replaced by the image they load, TASK-2459 / TASK-2460), and the image + // itself (`img`). A set shrinking to empty, an errored image navigated away, + // or a tap that swaps the affordance for the bitmap could otherwise strand + // focus on . Tracking the whole `phase` covers every such transition; + // `handoffFocus` no-ops while focus is still inside the surface. + void hasMultiple; + void loader.phase; + void img; + const el = rootEl; + if (!el || !isViewerFrontmost(el) || isBlockedByModal(el)) return; + handoffFocus(el); + }); + function onKeydown(e: KeyboardEvent) { // A control that already handled this key owns it. if (e.defaultPrevented) return; @@ -327,9 +850,49 @@ if (e.key === 'ArrowLeft' && hasMultiple) { e.preventDefault(); prev(); - } else if (e.key === 'ArrowRight' && hasMultiple) { + return; + } + if (e.key === 'ArrowRight' && hasMultiple) { e.preventDefault(); next(); + return; + } + + // Zoom: `+` / `-` about the stage centre, `0` resets. INSIDE the gates + // above by construction — placed after `defaultPrevented`, + // `isViewerFrontmost` and `isBlockedByModal` have each had their say, so a + // zoom key can never steal a press another owner or a layer above is due. + // + // The modifier rule is a CONTRACT, not a nicety: this listens on `window`, + // so acting on Ctrl/Cmd/Alt-`-`/`0` would swallow the browser's own + // page-zoom (and OS) shortcuts from every surface while a viewer is open. + // Leave those keys entirely — do not act, and do NOT `preventDefault`, or + // the native shortcut is cancelled even though we declined to handle it. + // `shiftKey` is fine (on most layouts `+` IS Shift+`=`), so it is absent + // from the guard. + if (e.ctrlKey || e.metaKey || e.altKey) return; + // Zoom keys are DISABLED, not merely no-ops, while no bitmap exists — the + // mobile `deferred` cell shows a placeholder with nothing to zoom + // (TASK-2460). Consume the key so it can't leak past the modal, but do not + // act. (`+`/`-` are already inert via `readGeometry`; `0` would otherwise + // still reset.) + const isZoomKey = e.key === '+' || e.key === '=' || e.key === '-' || e.key === '0'; + if (isZoomKey && !bitmapPresent) { + e.preventDefault(); + return; + } + if (e.key === '+' || e.key === '=') { + // `+` — including the numpad, whose `.key` is also `'+'` — and bare `=`. + e.preventDefault(); + stepZoom(ZOOM_STEP); + } else if (e.key === '-') { + // `-`, including the numpad, whose `.key` is also `'-'`. + e.preventDefault(); + stepZoom(1 / ZOOM_STEP); + } else if (e.key === '0') { + e.preventDefault(); + zoom = resetZoom(); + rebaseDrag(); // keyboard reset mid-drag must not desync the pan baseline } // NO Escape branch. See the registration above. } @@ -338,6 +901,16 @@ // controls have a different target, so they don't dismiss. This avoids // putting a click handler (and its a11y burden) on the . function onBackdropClick(e: MouseEvent) { + // A pan that released here produced this click — a drag is not a dismissal + // (TASK-2458). A below-threshold press (still a click) leaves this false, so + // it still closes; a plain click on the backdrop closes as before. + if (suppressClick) return; + const el = rootEl; + // Same gates as every other pointer entry point. Background inertness makes + // the normal path unreachable, so this is robustness, not a live fix — but a + // pointer owner that skips the gates the keyboard owner carries is the drift + // that breeds the next BUG-2441. + if (!el || !pointerGatesOpen(el)) return; if (e.target === e.currentTarget) onClose(); } @@ -359,6 +932,12 @@ aria-label={dialogLabel} tabindex="-1" onclick={onBackdropClick} + ondblclick={onDoubleClick} + onpointerdown={onPointerDown} + onpointermove={onPointerMove} + onpointerup={onPointerUp} + onpointercancel={onPointerCancel} + onlostpointercapture={onLostPointerCapture} > +
+ {#if img && loader.displaySrc} + + {#key loader.loadToken} + + + {img.alt { + const el = e.currentTarget as HTMLImageElement; + loader.decoded(el.naturalWidth, el.naturalHeight, el.getAttribute('src') ?? '', Number(el.dataset.gen)); + // RE-CLAMP on a fresh bitmap. A same-id reload can change the geometry + // (an async dimension fill swaps a large original for a smaller thumb, + // lowering actualScale and MAX_SCALE), stranding the current scale above + // the new ceiling with out-of-bounds pan. The reset effect fires only on + // image CHANGE (id) and the ResizeObserver only on stage resize, so + // neither catches this. `clampState`, NOT reset — the same image's valid + // zoom survives; only an over-ceiling scale/pan is pulled back. Geometry + // is measurable only post-decode. Gated to the LIVE element (`el === + // imgEl`, the same fence `decoded` applies) so a stale detached load can't + // drive the current zoom. Event handler, so the `zoom` read/write is not a + // CONVE-1688 self-write. + if (el === imgEl) { + const g = readGeometry(); + if (g) { + zoom = clampState(zoom, g); + rebaseDrag(); // a re-clamp mid-drag must not desync the pan baseline + } + } + }} + onerror={(e) => { + const el = e.currentTarget as HTMLImageElement; + loader.errored(el.getAttribute('src') ?? '', Number(el.dataset.gen)); + }} + style="transform: translate({zoom.x}px, {zoom.y}px) scale({zoom.scale});" + /> + {/key} + {/if} + + {#if img && loader.phase === 'loading'} + +
+ +
+ {/if} + + {#if img && loader.phase === 'deferred'} + +
+ +
+ {/if} + + {#if img && loader.phase === 'error'} + + {/if} +
{#if hasMultiple}