diff --git a/docs/channels.md b/docs/channels.md index f01700c3..f87cd3cc 100644 --- a/docs/channels.md +++ b/docs/channels.md @@ -274,3 +274,15 @@ after dwell; no automatic channel-prefix advance hides unseen siblings. Conversa options exposes local-only manual unread, explicit mark-through and sync recovery. Older synchronized hints may expire under bounded retention. Synced manual-unread and OS notifications are not enabled by this feature. + + +### Attachment layout and scrolling + +Image attachments reserve their preview geometry before loading and across virtualized +row remounts. Valid `imeta dim` metadata supplies the aspect ratio, bounded to 360px wide +and 320px tall without upscaling. Missing/invalid dimensions use a stable 360:320 frame +that shrinks with the available width; the image is contained without cropping or +upscaling. Unknown-size images may therefore have empty space in the frame. Loading, +failure, or retry does not resize it or force an above-bottom reader to the newest row. +`tests/browser/image-scroll.spec.mjs` covers delayed/failed loads, actual remounts, +bottom following, reading anchors and narrow layout in Chromium and WebKit. diff --git a/src/features/messages/MessageRow.test.tsx b/src/features/messages/MessageRow.test.tsx index 58ae2476..19808995 100644 --- a/src/features/messages/MessageRow.test.tsx +++ b/src/features/messages/MessageRow.test.tsx @@ -229,3 +229,34 @@ it.each([9, 40002])( } }, ); + +it.each([ + [ + { width: 700, height: 900 }, + "width:248.88888888888889px;aspect-ratio:700 / 900", + ], + [{ width: 1600, height: 900 }, "width:360px;aspect-ratio:1600 / 900"], + [{ width: 20, height: 10 }, "width:20px;aspect-ratio:20 / 10"], +])( + "reserves metadata-sized previews without waiting for load: %j", + (dimensions, style) => { + const html = renderToStaticMarkup( + url} + onOpenLink={() => false} + day={false} + retry={undefined} + />, + ); + expect(html).toContain(`style="${style}"`); + expect(html).toContain('aria-label="Open image attachment"'); + expect(html).toContain('loading="lazy"'); + }, +); diff --git a/src/features/messages/MessageRow.tsx b/src/features/messages/MessageRow.tsx index 7e5e2e53..5f7eb38d 100644 --- a/src/features/messages/MessageRow.tsx +++ b/src/features/messages/MessageRow.tsx @@ -126,6 +126,19 @@ export const MessageRow = memo(function MessageRow({ ) : ( ; -export type Attachment = Readonly<{ url: string; video: boolean }>; +export type Attachment = Readonly<{ + url: string; + video: boolean; + dimensions?: Readonly<{ width: number; height: number }>; +}>; /** Relay-authored membership activity, not a membership grant or user message. */ export type MembershipChange = Readonly<{ type: "member_joined" | "member_left" | "member_removed"; diff --git a/src/features/relay/fold.test.ts b/src/features/relay/fold.test.ts index c7a24324..d220c13c 100644 --- a/src/features/relay/fold.test.ts +++ b/src/features/relay/fold.test.ts @@ -1,4 +1,5 @@ import { assert, describe, expect, it } from "vitest"; +import { parseAttachments } from "./fold"; import { foldMessages } from "./fold"; import { foldProfiles } from "./profiles"; import { DiscoveryState } from "./discovery"; @@ -298,3 +299,38 @@ it("marks same-label identity replacement as edited without changing notificatio foldMessages(channel, relay.pubkey, [original])[0]?.edited, ).toBeUndefined(); }); + +it.each([ + ["700x900", { width: 700, height: 900 }], + ["1x999999", { width: 1, height: 999999 }], + [undefined, undefined], + ["0x900", undefined], + ["700x0", undefined], + ["-1x2", undefined], + ["1.5x2", undefined], + ["1x2px", undefined], + ["Infinityx2", undefined], + ["1000000x2", undefined], + ["1x2x3", undefined], +])("validates attachment layout dimensions %s", (dim, dimensions) => { + const event = message(keypair(), "channel", "", 1, [ + [ + "imeta", + "url https://x.test/image.png", + "m image/png", + ...(dim ? [`dim ${dim}`] : []), + ], + ]); + const attachments = parseAttachments(event, [ + "https://x.test/image.png", + "https://x.test/legacy.png", + ]); + expect(attachments).toEqual([ + { + url: "https://x.test/image.png", + video: false, + ...(dimensions ? { dimensions } : {}), + }, + { url: "https://x.test/legacy.png", video: false }, + ]); +}); diff --git a/src/features/relay/fold.ts b/src/features/relay/fold.ts index 9c3d1bb4..58fed26e 100644 --- a/src/features/relay/fold.ts +++ b/src/features/relay/fold.ts @@ -26,7 +26,16 @@ export function parseAttachments( const url = fields.url ? safeMessageUrl(fields.url) : undefined; if (!url || seen.has(url)) continue; seen.add(url); - result.push({ url, video: fields.m?.startsWith("video/") ?? false }); + // Treat signed metadata as untrusted layout input. Invalid/missing dimensions + // use the renderer's stable fallback rather than image-load-driven geometry. + const dim = /^(\d{1,6})x(\d{1,6})$/.exec(fields.dim ?? ""); + const width = Number(dim?.[1]), + height = Number(dim?.[2]); + result.push({ + url, + video: fields.m?.startsWith("video/") ?? false, + ...(width > 0 && height > 0 ? { dimensions: { width, height } } : {}), + }); } for (const url of markdownImages) { if (seen.has(url)) continue; diff --git a/tests/browser/image-scroll.spec.mjs b/tests/browser/image-scroll.spec.mjs new file mode 100644 index 00000000..b3e1c754 --- /dev/null +++ b/tests/browser/image-scroll.spec.mjs @@ -0,0 +1,210 @@ +import { test, expect } from "@playwright/test"; +import { createServer } from "vite"; +import react from "@vitejs/plugin-react"; +import { fileURLToPath } from "node:url"; +import { settle, anchor, expectAnchor } from "./timeline.mjs"; + +// Setup only: callers hold image responses until navigation has finished, then +// release them and assert stability without any corrective scrolling. +async function navigate(page, direction) { + const feed = page.getByRole("region", { name: "Channel message history" }); + const gap = () => + feed.evaluate((el) => el.scrollHeight - el.clientHeight - el.scrollTop); + const reached = (distance) => + direction < 0 ? distance > 5000 : distance < 4; + await feed.hover(); + for (let gesture = 0; gesture < 8; gesture++) { + const before = await gap(); + if (reached(before)) break; + const remaining = direction < 0 ? 6000 - before : before; + await page.mouse.wheel(0, direction * Math.min(2000, remaining)); + // Drain a timed-out DOM read before the caller tears down its page. + let pendingRead; + try { + await expect + .poll( + () => + (pendingRead = gap().then((after) => direction * (before - after))), + { message: "image navigation gesture makes progress" }, + ) + .toBeGreaterThan(0); + } finally { + await pendingRead; + } + await settle(page); + } + expect( + reached(await gap()), + "bounded image navigation reaches its setup", + ).toBe(true); +} + +test("delayed and failed images preserve bottom and reading anchors across remounts", async ({ + page, +}) => { + const server = await createServer({ + root: fileURLToPath(new URL("../../", import.meta.url)), + configFile: false, + envFile: false, + plugins: [react()], + logLevel: "error", + server: { host: "127.0.0.1", port: 0, strictPort: false }, + }); + const pending = new Set(); + const requests = new Map(); + let held = true; + async function release() { + held = false; + const waiting = [...pending]; + pending.clear(); + await Promise.all(waiting.map((resume) => resume())); + } + await page.route("https://image.test/**", async (route) => { + const url = route.request().url(); + requests.set(url, (requests.get(url) ?? 0) + 1); + if (held) await new Promise((resolve) => pending.add(resolve)); + // Routing deliberately disables HTTP cache: each remount can load late. + await new Promise((resolve) => setTimeout(resolve, 150)); + await route.fulfill( + url.endsWith("/96.svg") + ? { status: 404, body: "missing" } + : { + contentType: "image/svg+xml", + body: '', + }, + ); + }); + const feed = page.getByRole("region", { name: "Channel message history" }); + const gap = () => + feed.evaluate((el) => el.scrollHeight - el.clientHeight - el.scrollTop); + const loaded = () => + expect + .poll(() => + feed.locator('a[aria-label="Open image attachment"] img').evaluateAll( + (images) => + images.length > 0 && + images.every((img) => { + const rect = img.getBoundingClientRect(); + const feed = img + .closest("[data-channel-timeline]") + .getBoundingClientRect(); + return ( + rect.bottom <= feed.top || + rect.top >= feed.bottom || + img.complete + ); + }), + ), + ) + .toBe(true); + await server.listen(); + try { + await page.goto( + `http://127.0.0.1:${server.httpServer.address().port}/tests/fixtures/image-scroll.html`, + ); + await expect.poll(() => pending.size).toBeGreaterThan(0); + await settle(page); + expect(await gap()).toBeLessThan(4); + const before = await feed.evaluate((el) => el.scrollHeight); + await release(); + await loaded(); + await settle(page); + expect(await gap()).toBeLessThan(4); + expect(await feed.evaluate((el) => el.scrollHeight)).toBe(before); + // Reading above bottom survives decode; this must not be a force-bottom fix. + held = true; + pending.clear(); + await navigate(page, -1); + await expect.poll(() => pending.size).toBeGreaterThan(0); + await settle(page); + const reading = await anchor(page); + await release(); + await loaded(); + await settle(page); + await expectAnchor(page, reading); + for (let i = 0; i < 3; i++) { + held = true; + await navigate(page, 1); + await release(); + await loaded(); + await settle(page); + expect(await gap()).toBeLessThan(4); + held = true; + await navigate(page, -1); + await release(); + await loaded(); + await settle(page); + } + held = true; + await navigate(page, 1); + await release(); + await loaded(); + await settle(page); + expect(await gap()).toBeLessThan(4); + expect( + [...requests.values()].some((count) => count > 1), + "images actually remounted and reloaded", + ).toBe(true); + // Responsive reservation stays bounded, including missing-metadata fallback. + await page.setViewportSize({ width: 420, height: 950 }); + await settle(page); + expect(await gap()).toBeLessThan(4); + const bounds = await feed + .locator('a[aria-label="Open image attachment"]') + .evaluateAll((links) => + links.map((link) => ({ + width: link.getBoundingClientRect().width, + parent: link.parentElement.getBoundingClientRect().width, + height: link.getBoundingClientRect().height, + })), + ); + expect(bounds.length).toBeGreaterThan(0); + for (const box of bounds) { + expect(box.width).toBeLessThanOrEqual(box.parent); + expect(box.height).toBeLessThanOrEqual(320); + expect(box.height).toBeGreaterThan(0); + } + } finally { + await release(); + await page.unrouteAll({ behavior: "wait" }); + await server.close(); + } +}); + +// Isolate the setup helper from image loading: partial input must converge, but +// blocked input must fail instead of turning the preservation checks into retries. +test("image navigation handles partial gestures and rejects blocked input", async ({ + page, +}) => { + await page.setContent(` +
+ `); + const wheel = page.mouse.wheel.bind(page.mouse); + let gestures = 0; + page.mouse.wheel = (x, y) => { + gestures++; + return wheel(x, Math.sign(y) * Math.min(1800, Math.abs(y))); + }; + try { + await navigate(page, 1); + expect(gestures).toBeGreaterThan(1); + expect(gestures).toBeLessThanOrEqual(8); + gestures = 0; + await navigate(page, -1); + expect(gestures).toBeGreaterThan(1); + expect(gestures).toBeLessThanOrEqual(8); + await page.getByRole("region").evaluate((element) => { + element.addEventListener("wheel", (event) => event.preventDefault(), { + passive: false, + }); + }); + gestures = 0; + await expect(navigate(page, 1)).rejects.toThrow( + "image navigation gesture makes progress", + ); + expect(gestures).toBe(1); + } finally { + page.mouse.wheel = wheel; + } +}); diff --git a/tests/fixtures/image-scroll.html b/tests/fixtures/image-scroll.html new file mode 100644 index 00000000..40996461 --- /dev/null +++ b/tests/fixtures/image-scroll.html @@ -0,0 +1 @@ +Image scroll regression
\ No newline at end of file diff --git a/tests/fixtures/image-scroll.tsx b/tests/fixtures/image-scroll.tsx new file mode 100644 index 00000000..3d8e6c85 --- /dev/null +++ b/tests/fixtures/image-scroll.tsx @@ -0,0 +1,66 @@ +// Actual conversation UI/session and folded events; no live identity or relay. +import { StrictMode } from "react"; +import { createRoot } from "react-dom/client"; +import { ChannelTimeline } from "../../src/features/messages/ChannelTimeline"; +import { createRelaySession } from "../../src/features/relay/session"; +import { foldMessages } from "../../src/features/relay/fold"; +import { keypair, message } from "../../src/features/relay/testing"; +import "../../src/shared/styles/globals.css"; +const viewer = keypair(), + relay = keypair(); +const events = Array.from({ length: 100 }, (_, i) => + message( + viewer, + "images", + `Message ${i}. Reading should survive image loading.`, + 1700000000 + i, + i % 2 === 0 + ? [ + [ + "imeta", + `url https://image.test/${i}.svg`, + "m image/svg+xml", + ...(i % 4 === 0 ? ["dim 700x900"] : []), + ], + ] + : [], + ), +); +const rows = foldMessages("images", relay.pubkey, events); +const owner = createRelaySession({ + viewer: viewer.pubkey, + relayAuthor: relay.pubkey, + media: (url) => url, + query: async () => [], + subscribe: () => ({ update() {}, retry() {}, dispose() {} }), +}); +const root = document.getElementById("root"); +if (!root) throw new Error("Missing fixture root"); +createRoot(root).render( + +
+ false} + /> +
+ , +);