From aa73295b8d62e12b87146fe8401a2eef6a9de84d Mon Sep 17 00:00:00 2001
From: Michael Ramos <mdramos8@gmail.com>
Date: Sun, 12 Apr 2026 16:55:11 -0700
Subject: [PATCH] fix(review): work around Safari scroll-reset bug with shadow
 DOM diffs (#540) (#547)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Safari's compositor resets scrollTop to 0 when momentum-scrolling ends
inside a container whose child is a web-component shadow DOM (the
@pierre/diffs `<diffs-container>`). The reset bypasses all JavaScript
APIs — neither scrollTo nor the scrollTop setter fires.

Add a WebKit-only scroll event listener that detects the bogus reset
(scrollTop jumping from >200 to 0 in a single event, which cannot
happen during normal scrolling) and synchronously restores the last
known good position before the browser paints the wrong frame.

Closes #540

For provenance purposes, this commit was AI assisted.
---
 .../review-editor/components/DiffViewer.tsx   | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/packages/review-editor/components/DiffViewer.tsx b/packages/review-editor/components/DiffViewer.tsx
index 1aa83f73..5a4aaf65 100644
--- a/packages/review-editor/components/DiffViewer.tsx
+++ b/packages/review-editor/components/DiffViewer.tsx
@@ -323,6 +323,46 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
     }
   }, [filePath, onLineSelection]);
 
+  // Safari scroll-position guardian. Safari has a compositor bug where
+  // scrollTop resets to 0 (sometimes multiple times in quick succession)
+  // when momentum-scrolling ends inside a container whose child is a
+  // web-component shadow DOM (@pierre/diffs `<diffs-container>`). The reset
+  // bypasses JavaScript entirely — no scrollTo / scrollTop setter fires.
+  // Detect the bogus resets and restore the last known good position.
+  // Only active in WebKit — Chrome / Firefox / Edge are unaffected.
+  //
+  // filePath is in the dep array so the guardian resets when the user
+  // switches files (the file-switch useLayoutEffect legitimately scrolls
+  // to 0 — without resetting here the guardian would fight it).
+  useEffect(() => {
+    if (!viewport) return;
+    const ua = navigator.userAgent;
+    const isWebKit = ua.includes('Safari') && !ua.includes('Chrome');
+    if (!isWebKit) return;
+
+    let lastGoodST = 0;
+
+    const onScroll = () => {
+      const st = viewport.scrollTop;
+      if (st > 0) {
+        lastGoodST = st;
+      } else if (lastGoodST > 200) {
+        // scrollTop jumped from a distant position to 0 — Safari compositor bug.
+        // A legitimate scroll-to-top always has intermediate events that bring
+        // lastGoodST down to a small value before reaching 0. A jump from >200
+        // to 0 in a single event can only be the bug. Restore synchronously so
+        // the browser never paints the wrong frame.
+        viewport.scrollTop = lastGoodST;
+      } else {
+        // Near the top already (lastGoodST ≤ 200) — legitimate scroll to top
+        lastGoodST = 0;
+      }
+    };
+
+    viewport.addEventListener('scroll', onScroll, { passive: true });
+    return () => viewport.removeEventListener('scroll', onScroll);
+  }, [viewport, filePath]);
+
   // Scroll to selected annotation when it changes
   useEffect(() => {
     if (!selectedAnnotationId || !containerRef.current) return;