Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions docs/channels.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add the required DCO sign-off

The reviewed commit message has no Signed-off-by trailer, so this commit violates the repository's per-commit DCO requirement and will not satisfy the hosted DCO gate. Recreate the commit with git commit --signoff using the verified effective author identity before integration.

AGENTS.md reference: AGENTS.md:L51-L54

Useful? React with 👍 / 👎.

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.
31 changes: 31 additions & 0 deletions src/features/messages/MessageRow.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<MessageRow
row={{
...row,
attachments: [
{ url: "https://image.test/shot.png", video: false, dimensions },
],
}}
profile={undefined}
media={(url) => 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"');
},
);
13 changes: 13 additions & 0 deletions src/features/messages/MessageRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,19 @@ export const MessageRow = memo(function MessageRow({
) : (
<a
className={styles.attachmentImage}
style={
attachment.dimensions
? {
width: Math.min(
360,
attachment.dimensions.width,
(320 * attachment.dimensions.width) /
attachment.dimensions.height,
),
aspectRatio: `${attachment.dimensions.width} / ${attachment.dimensions.height}`,
}
: undefined
}
key={url}
href={url}
target="_blank"
Expand Down
17 changes: 11 additions & 6 deletions src/features/messages/Messages.module.css
Original file line number Diff line number Diff line change
Expand Up @@ -176,20 +176,25 @@
font-size: calc(12px * var(--buzz-text-scale, 1));
}
.attachmentImage {
/* Reserve geometry even across virtualizer remounts and failed/lazy loads.
imeta dimensions override this bounded fallback without waiting for decode. */
position: relative;
display: block;
width: fit-content;
max-width: min(360px, 100%);
width: 360px;
max-width: 100%;
aspect-ratio: 360 / 320;
margin-top: 8px;
overflow: hidden;
border-radius: 10px;
background: var(--surface-hover);
}
.attachmentImage img {
position: absolute;
inset: 0;
display: block;
width: auto;
max-width: 100%;
max-height: 320px;
object-fit: contain;
width: 100%;
height: 100%;
object-fit: scale-down;
}
.composer {
border: 1px solid var(--border-input);
Expand Down
6 changes: 5 additions & 1 deletion src/features/relay/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ export type Profile = Readonly<{
picture?: string;
about?: string;
}>;
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";
Expand Down
36 changes: 36 additions & 0 deletions src/features/relay/fold.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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 },
]);
});
11 changes: 10 additions & 1 deletion src/features/relay/fold.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
210 changes: 210 additions & 0 deletions tests/browser/image-scroll.spec.mjs
Original file line number Diff line number Diff line change
@@ -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: '<svg xmlns="http://www.w3.org/2000/svg" width="700" height="900"><rect width="700" height="900" fill="orange"/></svg>',
},
);
});
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(`
<section role="region" aria-label="Channel message history"
style="height:700px;overflow:auto"><div style="height:14000px"></div></section>
`);
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;
}
});
1 change: 1 addition & 0 deletions tests/fixtures/image-scroll.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<!doctype html><html lang="en"><head><meta charset="UTF-8"><title>Image scroll regression</title></head><body><div id="root"></div><script type="module" src="./image-scroll.tsx"></script></body></html>
Loading
Loading