fix(messages): reserve a stable scrollbar gutter on the channel feed

.github/workflows/ci.yml

@@ -122,12 +122,23 @@ jobs:
         env:
           PLAYWRIGHT_JSON_OUTPUT_FILE: test-results/browser/ci-report.json
         run: node scripts/ci-test-report.mjs kind=playwright report=test-results/browser/ci-report.json evidence=test-results/browser/ci-timing.json title="Browser measurements" -- pnpm test:browser:ci --project '*-measurements' --workers=1 --reporter=list,json
+      # Cases tagged @classic-scrollbars need Chromium's platform scrollbars,
+      # which Linux always draws as classic ones. One file cannot join the
+      # six-way engine matrix (empty shards are errors), so it runs here, and
+      # CI required already blocks on this job. The project writes to its own
+      # outputDir so this run does not clear the measurement evidence above.
+      - name: Chromium classic-scrollbar layout
+        env:
+          PLAYWRIGHT_JSON_OUTPUT_FILE: test-results/browser-classic-scrollbars/ci-report.json
+        run: node scripts/ci-test-report.mjs kind=playwright report=test-results/browser-classic-scrollbars/ci-report.json evidence=test-results/browser-classic-scrollbars/ci-timing.json title="Browser classic scrollbars" -- pnpm test:browser:ci --project chromium-classic-scrollbars --no-deps --reporter=list,json
       - name: Measurement evidence
         if: always()
         uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
         with:
           name: browser-measurements
-          path: test-results/browser
+          path: |
+            test-results/browser
+            test-results/browser-classic-scrollbars
           retention-days: 7
 
   browser:

docs/browser-testing.md

@@ -95,6 +95,9 @@ Results go to ignored `test-results/browser/`: each built-app test writes `evide
 with runtime versions, HEAD/dirty status, request ledger, runtime errors and
 measurements. Failure screenshots and traces are retained too. The next invocation
 replaces that output; copy artifacts before a rerun if you need to compare them.
+The one exception is the `chromium-classic-scrollbars` project, whose `evidence.json`,
+failure screenshots and traces land in `test-results/browser-classic-scrollbars/`
+so that its separate invocation cannot clear the measurement evidence.
 A dirty-status listing is not a content hash; tie release claims to a separately
 verified clean commit or source manifest.
 
@@ -120,6 +123,20 @@ check's shell against failed, skipped, cancelled and missing lane results.
 These safeguards must change with the matrix; do not maintain feature allowlists
 or move existing required cases out of CI to reduce its duration.
 
+Cases tagged `@classic-scrollbars` need a scrollbar that takes space. They run
+only in the `chromium-classic-scrollbars` project, which keeps Chromium's
+platform scrollbars visible, and the engine projects exclude them. CI runs that
+project as a second step of the measurements job, so `CI required` blocks on it;
+the integration gate checks that the step exists and that the tagged cases are
+selected there and nowhere else. Linux always draws classic scrollbars, so the
+cases fail there if the scrollbar takes no space. On macOS Chromium follows the
+system "Show scroll bars" setting, so they skip unless `BUZZ_CLASSIC_SCROLLBARS=1`
+is set on a Mac whose scrollbars take space:
+
+```sh
+BUZZ_CLASSIC_SCROLLBARS=1 bin/pnpm test:browser --project chromium-classic-scrollbars --no-deps
+```
+
 The following **three WebKit cases are local-only**, not passing CI coverage:
 
 | Case | Reason and coverage gap |

patches/README.md

@@ -54,6 +54,14 @@ gesture must continue to work; sustained real-history/media acceptance must asse
 whether repeated braking is acceptable. No wheel ownership, permanent scrolling
 CSS, forced layout, alternate store sizing, or new scroll scheduler is introduced.
 
+The `overflow-y: hidden` frame also removes a classic, space-taking scrollbar.
+Every scroller driven by the patched Virtualizer on Mac WebKit must therefore
+keep a constant inline size across the toggle (`scrollbar-gutter: stable`, as
+`.feed` in `src/features/messages/Messages.module.css` does). Otherwise the wider
+content box re-wraps rows above the viewport, their new heights become the next
+nonzero correction, and the interrupt feeds itself until the scroller oscillates
+between two wrap widths.
+
 The momentum-interruption predicate requires MacIntel and Apple vendor, excluding
 Virtua's iOS detector (including desktop-mode iPad). Chrome/Firefox, non-Mac WebKit
 and iOS keep existing momentum policy. Store/layout/observer timing and imperative

src/features/messages/MessageRow.tsx

@@ -13,6 +13,7 @@ import {
   useEffect,
   useCallback,
   useSyncExternalStore,
+  type FocusEvent,
   type ReactNode,
 } from "react";
 import type { RelaySession } from "../relay/session";
@@ -88,6 +89,48 @@ export type MessageRowProps = {
   ) => void;
 };
 
+// Engines skip horizontal focus scrolling for a target that is already partly
+// visible: Blink sets the partial-visibility behaviour to no-scroll in
+// Element::UpdateSelectionOnFocus, and WebKit keeps its 32px legacy horizontal
+// visibility threshold for focus reveals. Only scrollIntoView() opts out of
+// both, so a thumbnail whose far edge is clipped keeps focus without ever
+// coming fully into view. Reveal it within the strip's scroll-padding.
+//
+// Keyboard focus only. React's onFocus is the bubbling focusin, so it also
+// fires for pointer focus, and Chromium focuses a link on mousedown (WebKit
+// does not): revealing there would slide the strip under a held pointer before
+// mouseup, so the press lands on a neighbour or on padding, and the strip
+// visibly jumps on every click of a clipped tile. DESIGN.md wants pointer
+// focus quiet, so gate on html[data-keyboard-navigation] like the composer and
+// the floating action bar. useKeyboardFocusVisibility sets that attribute in
+// a capturing keydown listener, which runs before Tab's default action moves
+// focus, and clears it on the capturing pointerdown that precedes mousedown,
+// so a Tab reveal still runs and a press never does.
+function revealFocusedThumbnail(event: FocusEvent<HTMLDivElement>) {
+  const strip = event.currentTarget;
+  const target = event.target;
+  if (
+    !(target instanceof HTMLElement) ||
+    target === strip ||
+    !document.documentElement.hasAttribute("data-keyboard-navigation")
+  )
+    return;
+  const style = getComputedStyle(strip);
+  const bounds = strip.getBoundingClientRect();
+  const rect = target.getBoundingClientRect();
+  const start =
+    bounds.left +
+    strip.clientLeft +
+    (Number.parseFloat(style.scrollPaddingInlineStart) || 0);
+  const end =
+    bounds.left +
+    strip.clientLeft +
+    strip.clientWidth -
+    (Number.parseFloat(style.scrollPaddingInlineEnd) || 0);
+  if (rect.right > end) strip.scrollLeft += rect.right - end;
+  else if (rect.left < start) strip.scrollLeft -= start - rect.left;
+}
+
 export const MessageRow = memo(function MessageRow({
   row,
   session,
@@ -550,6 +593,7 @@ export const MessageRow = memo(function MessageRow({
                   className={styles.imageStrip}
                   role="group"
                   aria-label={`${group.length} ${group.length === 1 ? "image" : "images"}`}
+                  onFocus={revealFocusedThumbnail}
                 >
                   {items}
                 </div>

src/features/messages/Messages.module.css

@@ -22,18 +22,26 @@
   padding: 0 var(--space-panel-inset);
   /* Keep fractional last rows reachable despite integer scroll extents. */
   padding-bottom: var(--space-half);
+  /* The vendored Virtua patch (interruptMomentum in patches/virtua@0.51.0.patch)
+     flips this scroller to overflow-y: hidden !important on every nonzero size
+     correction on Mac WebKit. With classic, space-taking scrollbars that frame
+     removes the scrollbar, widens the content box, re-wraps rows above the
+     viewport, and their new heights feed the next correction. A stable gutter
+     keeps the inline size constant across the toggle. It reserves the thin
+     width because Chromium 121+ and WebKit from Safari 18.2 honour the standard
+     scrollbar-width/scrollbar-color (set here and by the global * rule in
+     shared/design-system/styles/scrollbars.css) and then ignore
+     ::-webkit-scrollbar, which is why .feed carries no such rules. */
+  scrollbar-gutter: stable;
   scrollbar-width: thin;
   scrollbar-color: color-mix(in srgb, var(--text) 20%, transparent) transparent;
 }
-.feed::-webkit-scrollbar,
 .threadHistory::-webkit-scrollbar {
   width: 8px;
 }
-.feed::-webkit-scrollbar-track,
 .threadHistory::-webkit-scrollbar-track {
   background: transparent;
 }
-.feed::-webkit-scrollbar-thumb,
 .threadHistory::-webkit-scrollbar-thumb {
   border: 2px solid transparent;
   border-radius: var(--radius-pill);

tests/browser/image-strip.spec.mjs

@@ -2,6 +2,32 @@ import { test, expect } from "@playwright/test";
 import react from "@vitejs/plugin-react";
 import { createServer } from "./vite-server.mjs";
 
+// Neither engine scrolls horizontally on focus for a tile that is already
+// partly visible, so a focused tile must sit inside the strip's scroll-padding
+// box, not merely its border box: a reveal that stops short of the strip's
+// edge lands the tile's far edge in the padding where the focus ring is
+// clipped. A strip too narrow to hold a tile plus both paddings (390px once a
+// scrollbar gutter is reserved) can only honour the edge focus travelled
+// towards, which then has to sit exactly on that scroll-padding edge.
+function insideScrollPadding(el, edge) {
+  const strip = el.parentElement;
+  const style = getComputedStyle(strip);
+  const a = el.getBoundingClientRect(),
+    b = strip.getBoundingClientRect();
+  const start =
+    b.left +
+    strip.clientLeft +
+    (Number.parseFloat(style.scrollPaddingInlineStart) || 0);
+  const end =
+    b.left +
+    strip.clientLeft +
+    strip.clientWidth -
+    (Number.parseFloat(style.scrollPaddingInlineEnd) || 0);
+  return edge === "end"
+    ? a.right <= end && a.left >= Math.min(start, end - a.width)
+    : a.left >= start && a.right <= Math.max(end, start + a.width);
+}
+
 // Browser-only: posted layout, overflow, focus scrolling and real viewer wiring.
 test("posted image strips keep counts visible and every image reachable beside documents", async ({
   page,
@@ -59,6 +85,15 @@ test("posted image strips keep counts visible and every image reachable beside d
     const links = strip.getByRole("link");
     await expect(links).toHaveCount(8);
     await expect(history.getByText("8 images", { exact: true })).toBeVisible();
+    const viewer = page.getByRole("dialog", {
+      name: "Image viewer",
+      exact: true,
+    });
+    // macOS WebKit only tabs to links with Option held.
+    const optionTab = browserName === "webkit" && process.platform === "darwin";
+    const forward = optionTab ? "Alt+Tab" : "Tab";
+    const backward = optionTab ? "Shift+Alt+Tab" : "Shift+Tab";
+    let pressedClippedTile = false;
     for (const width of [390, 768, 1280]) {
       await page.setViewportSize({ width, height: 950 });
       await expect(strip).toBeVisible();
@@ -95,21 +130,10 @@ test("posted image strips keep counts visible and every image reachable beside d
         true,
       );
       await links.first().focus();
-      for (let i = 1; i < 8; i++)
-        await page.keyboard.press(
-          browserName === "webkit" && process.platform === "darwin"
-            ? "Alt+Tab"
-            : "Tab",
-        );
+      for (let i = 1; i < 8; i++) await page.keyboard.press(forward);
       await expect(links.last()).toBeFocused();
       await expect
-        .poll(() =>
-          links.last().evaluate((el) => {
-            const a = el.getBoundingClientRect(),
-              b = el.parentElement.getBoundingClientRect();
-            return a.left >= b.left && a.right <= b.right;
-          }),
-        )
+        .poll(() => links.last().evaluate(insideScrollPadding, "end"))
         .toBe(true);
       await expect(links.last()).toHaveCSS("outline-style", "solid");
       // A declared outline can still be clipped away. Compare actual pixels
@@ -130,6 +154,54 @@ test("posted image strips keep counts visible and every image reachable beside d
       await expect(
         history.getByText("8 images", { exact: true }),
       ).toBeVisible();
+      // Shift+Tab back from the right end exercises the reveal's other branch:
+      // the tile whose leading edge the start clips is partly visible too, so
+      // the same native no-scroll rule would leave it focused and cut off.
+      for (let i = 6; i >= 0; i--) {
+        await page.keyboard.press(backward);
+        await expect(links.nth(i)).toBeFocused();
+        await expect
+          .poll(() => links.nth(i).evaluate(insideScrollPadding, "start"))
+          .toBe(true);
+      }
+      // Pointer focus stays quiet. Chromium focuses a link on mousedown, and a
+      // reveal there would slide the strip under the held pointer before
+      // mouseup; WebKit never focuses a link from a press. Press a tile the end
+      // clips and check the strip has not moved while the button is held.
+      const clipped = await links.evaluateAll((items) => {
+        const strip = items[0].parentElement;
+        const end =
+          strip.getBoundingClientRect().left +
+          strip.clientLeft +
+          strip.clientWidth;
+        for (const el of items) {
+          const r = el.getBoundingClientRect();
+          if (r.left < end && r.right > end)
+            return { x: (r.left + end) / 2, y: r.top + r.height / 2 };
+        }
+        return null;
+      });
+      if (clipped) {
+        pressedClippedTile = true;
+        const before = await strip.evaluate((el) => el.scrollLeft);
+        await page.mouse.move(clipped.x, clipped.y);
+        await page.mouse.down();
+        await page.evaluate(
+          () =>
+            new Promise((resolve) =>
+              requestAnimationFrame(() => requestAnimationFrame(resolve)),
+            ),
+        );
+        expect(await strip.evaluate((el) => el.scrollLeft)).toBe(before);
+        await page.mouse.up();
+        // The press completed a click on the tile, which opens the viewer.
+        await expect(viewer).toBeVisible();
+        await expect(viewer).not.toHaveAttribute("data-review-opening");
+        await viewer
+          .getByRole("button", { name: "Close fullscreen viewer" })
+          .click();
+        await expect(viewer).toHaveCount(0);
+      }
       await links.first().focus();
       await expect
         .poll(() =>
@@ -167,13 +239,10 @@ test("posted image strips keep counts visible and every image reachable beside d
         ),
       )
       .toBe(true);
+    expect(pressedClippedTile).toBe(true);
     await links.last().focus();
     const lastSource = await links.last().locator("img").getAttribute("src");
     await page.keyboard.press("Enter");
-    const viewer = page.getByRole("dialog", {
-      name: "Image viewer",
-      exact: true,
-    });
     await expect(
       viewer.getByRole("img", { name: "Attachment preview" }),
     ).toHaveAttribute("src", lastSource);

tests/browser/playwright.config.mjs

@@ -1,6 +1,10 @@
 import { defineConfig } from "@playwright/test";
 
 const measurementFiles = ["channel-opening.spec.mjs", "scroll.spec.mjs"];
+// Cases that need a scrollbar that takes space. Headless Chromium passes
+// --hide-scrollbars by default and only Linux guarantees classic scrollbars,
+// so they run in their own Chromium project and never in the engine projects.
+const classicScrollbars = /@classic-scrollbars/;
 
 export default defineConfig({
   testDir: ".",
@@ -42,7 +46,23 @@ export default defineConfig({
       name: browserName,
       use: { browserName },
       testIgnore: measurementFiles,
+      grepInvert: classicScrollbars,
       dependencies: ["webkit-measurements"],
     })),
+    {
+      name: "chromium-classic-scrollbars",
+      use: {
+        browserName: "chromium",
+        launchOptions: { ignoreDefaultArgs: ["--hide-scrollbars"] },
+      },
+      testIgnore: measurementFiles,
+      grep: classicScrollbars,
+      // CI runs this project as a second Playwright invocation in the
+      // measurements job, and every run clears the outputDir of the projects
+      // it selects. A separate directory keeps the measurement evidence intact.
+      outputDir: "../../test-results/browser-classic-scrollbars",
+      // Locally the full gate still runs measurements first and alone.
+      dependencies: ["webkit-measurements"],
+    },
   ],
 });