Skip to content

fix(scroll): round 11 — MessageThread native scroll listener (WebKit E2E flake)#97

Merged
TortoiseWolfe merged 1 commit into
mainfrom
fix/messaging-scroll-webkit-native-listener
May 19, 2026
Merged

fix(scroll): round 11 — MessageThread native scroll listener (WebKit E2E flake)#97
TortoiseWolfe merged 1 commit into
mainfrom
fix/messaging-scroll-webkit-native-listener

Conversation

@TortoiseWolfe
Copy link
Copy Markdown
Owner

Summary

  • Monday 2026-05-18 scheduled cron E2E run on main (run 26020030467) failed webkit-msg 1/1 on messaging-scroll.spec.ts:266 (T007-T008: Jump button).
  • Round-10 fix (fix(ci): serialize E2E runs via repo-wide concurrency mutex #89, commit 996211e) added dispatchEvent(new Event('scroll', { bubbles: true })) after every programmatic scrollTop = N in the test, but the React onScroll JSX prop on MessageThread.tsx:353 does NOT reliably receive synthetic dispatched scroll events on WebKit.
  • Fix: bind handleScroll natively via addEventListener('scroll', ...) inside a useEffect, remove the React onScroll prop. Native listeners fire deterministically for both real user scrolls AND programmatic dispatchEvent across all three browser engines.

Root cause (why round 10 didn't fully close this)

React's synthetic onScroll is special — the native scroll event doesn't bubble, so React 17+ artificially makes it bubble through its synthetic event system by listening at the React root. That delegation pipeline assumes the event was generated through the browser's normal scroll mechanism. A programmatically-dispatched new Event('scroll', { bubbles: true }) produces a native event that bubbles through DOM listeners, but does NOT always reach React's synthetic onScroll handler on WebKit (chromium and firefox route it correctly through their respective scroll-delegation paths).

This is a real cross-engine difference. The test dispatch IS correct; the React JSX prop is the brittle layer.

What this change does

+  // Bind handleScroll as a native DOM event listener instead of via React's
+  // `onScroll` JSX prop. Reason: React's synthetic onScroll does not reliably
+  // fire on WebKit when test code dispatches a programmatic
+  // `dispatchEvent(new Event('scroll', { bubbles: true }))` after assigning
+  // `scrollTop`.
+  useEffect(() => {
+    const parent = parentRef.current;
+    if (!parent) return;
+    parent.addEventListener('scroll', handleScroll, { passive: true });
+    return () => parent.removeEventListener('scroll', handleScroll);
+  }, [handleScroll]);

   ...

   <div
     ref={parentRef}
-    onScroll={handleScroll}
     className="absolute inset-0 overflow-y-auto"
     data-testid="message-thread"
     style={{ overscrollBehavior: 'contain' }}
   >

Functionally identical for users — the handler is the same callback, only the binding mechanism changes.

Why this is the right fix (not a hack)

  • User scrolls still work — native addEventListener('scroll', ...) catches all scroll events, including real user scrolls.
  • Test scrolls now work — programmatic el.dispatchEvent(new Event('scroll')) fires native listeners deterministically.
  • No new abstraction — same callback, same effect deps, just bound via a different API surface.
  • Tracks an actual browser engine bug class — React's synthetic onScroll has known WebKit edge cases. Documented inline + in CLAUDE.md.

Test plan

Notes for reviewer

This unblocks PR #95. Merge order:

  1. This PR → main (green CI required)
  2. PR feat: #48 Three.js Game at /game/3d (047 — full SpecKit cascade) #95 → main (already green on its own branch CI; will be rebased or re-tested after this lands)

🤖 Generated with Claude Code

…tListener for scroll

ROOT CAUSE

Monday 2026-05-18 scheduled cron E2E run 26020030467 on main (sha
36cf2a6) failed webkit-msg 1/1. The failing test is `tests/e2e/
messaging/messaging-scroll.spec.ts:266` — "T007-T008: Jump button
appears when scrolled and does not overlap input".

This is the same test family the round-10 fix (commit 996211e)
targeted. That fix dispatches `new Event('scroll', { bubbles: true })`
after every programmatic `el.scrollTop = N` site in the test, so the
test should trigger React's `onScroll` handler.

But the assertion at line 299 (`expect(jumpButton).toBeVisible()`)
keeps failing on WebKit — and only on WebKit. CI log shows the
button locator returns "element(s) not found" after 5s — meaning the
React component never set `showScrollButton = true`, which means
`handleScroll` never ran, which means the synthetic React onScroll
event handler did not receive the dispatched native scroll event.

WHY THE ROUND-10 FIX WAS INCOMPLETE

React's synthetic `onScroll` event has special handling. The native
`scroll` event does NOT bubble by default. React 17+ artificially
makes scroll events bubble through its synthetic event system by
listening at the React root, but this routing depends on the event
having been generated through the browser's normal scroll pipeline.
A programmatically-dispatched `new Event('scroll', { bubbles: true })`
on the scrollable element produces a native event that bubbles
through DOM listeners but does NOT always reach React's synthetic
onScroll handler on WebKit (chromium and firefox happen to route it
correctly).

This is a known difference between browser engines' scroll-event
delegation paths. The test dispatch IS correct; the React JSX prop is
the brittle layer.

THE FIX

Replace the React `onScroll` JSX prop with a native
`addEventListener('scroll', handler, { passive: true })` inside a
useEffect that re-binds whenever `handleScroll` changes (i.e., when
its deps `hasMore`, `loading`, `onLoadMore` change). Native event
listeners fire deterministically for programmatic dispatchEvent
across all three browser engines.

Functionally identical for users (the handler is the same callback);
only the binding mechanism changes. No user-facing behavior change.

VERIFIED

- 31 unit tests pass (MessageThread.test.tsx + .accessibility.test.tsx)
- Type-check clean
- Lint clean
- Diff is minimal: +14 lines (the useEffect), -1 line (the onScroll prop)

DOES NOT INTRODUCE NEW BEHAVIOR

The two cases the round-10 fix addressed remain covered:
- Real user scrolls fire the native scroll event; native listener catches it
- Programmatic test scrolls + dispatched scroll event fire the same way
The change just makes the listener path identical for both.

PR FOR

This bug is blocking the merge of PR #95 (#48 Three.js Game). User's
branch-hygiene rule: "Never leave unmerged branches floating; never
merge a clean PR onto a red main." Round 11 fix lands first, then PR
#95 can merge onto a green main.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@TortoiseWolfe TortoiseWolfe merged commit 274ca3c into main May 19, 2026
29 checks passed
@TortoiseWolfe TortoiseWolfe deleted the fix/messaging-scroll-webkit-native-listener branch May 19, 2026 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants