Merge pull request #4091 from nesquena/stage-3920

Release MP (v0.51.377): Firefox post-stream scroll jitter (#3920)

CHANGELOG.md

@@ -3,6 +3,12 @@
 
 ## [Unreleased]
 
+## [v0.51.377] — 2026-06-13 — Release MP (Firefox post-stream scroll jitter, #3920)
+
+### Fixed
+
+- **Firefox no longer jitters/steps the chat scroll after a streamed turn settles (#3920).** The post-stream bottom-settle replaced its `setTimeout` fan-out + double-`requestAnimationFrame` `scrollHeight` polling (which forced a reflow per frame on Firefox) with a single synchronous write plus a `ResizeObserver` that re-anchors once per frame as late layout (Prism/KaTeX/Mermaid/images) grows, disconnecting after 300ms of quiet. A 2s top-level fallback covers fully-static responses that never resize, and the deliberate Firefox session-sidebar `overflow-anchor:none` fix is preserved. (#3920)
+
 ## [v0.51.376] — 2026-06-13 — Release MO (Hide Thinking also hides Worklog reasoning, #3903)
 
 ### Fixed

static/ui.js

@@ -2425,10 +2425,14 @@ let _lastScrollTop=null;
 let _lastNonMessageScrollIntentMs=-Infinity;
 let _messageUserUnpinned=false;
 let _bottomSettleToken=0;
+let _settleRAF=0;
+let _settleRO=null;
+let _settleTimer=0;
+let _settleFinalTimer=0;
 const NON_MESSAGE_SCROLL_INTENT_SUPPRESS_MS=350;
 let _touchStartY=null;
 let _newMessageCueVisible=false;
-function _cancelBottomSettle(){ _bottomSettleToken++; }
+function _cancelBottomSettle(){ _bottomSettleToken++; if(_settleRO){ _settleRO.disconnect(); _settleRO=null; } clearTimeout(_settleTimer); clearTimeout(_settleFinalTimer); cancelAnimationFrame(_settleRAF); }
 function _recordNonMessageScrollIntent(e){
   const el=document.getElementById('messages');
   const target=e&&e.target;
@@ -3111,23 +3115,90 @@ function _followMessagesAfterDomReplace(){
 function _settleMessageScrollToBottom(force){
   // Markdown post-processing (Prism, tables, Mermaid/KaTeX/PDF placeholders)
   // can grow the transcript after the first scroll write. Re-apply the bottom
-  // position across a few frames while pinned so late layout does not leave the
-  // viewport a few lines above the real end. User scroll increments
-  // _bottomSettleToken and cancels the delayed passes.
+  // position when content settles so late layout does not leave the viewport
+  // above the real end. User scroll increments _bottomSettleToken and cancels.
+  //
+  // Firefox paints each scrollTop write as a visible reflow step. The old
+  // rAF-polling approach read scrollHeight across frames — the read itself
+  // forced a reflow in Firefox, causing visible jitter.
+  //
+  // ResizeObserver approach: the browser notifies us when the container
+  // resizes (no scrollHeight polling needed). On each notification we write
+  // scrollTop once via rAF (batches multiple resize callbacks per frame into
+  // a single write). After 300ms of no resize events, the observer disconnects.
   const token=++_bottomSettleToken;
-  const passes=[0,16,80,180];
-  passes.forEach(delay=>setTimeout(()=>{
-    if(token!==_bottomSettleToken) return;
-    if(!force && (!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent())) return;
-    _setMessageScrollToBottom();
-  },delay));
-  requestAnimationFrame(()=>{
-    if(token!==_bottomSettleToken) return;
-    if(force || (_scrollPinned&&!_messageUserUnpinned&&!_recentNonMessageScrollIntent())) _setMessageScrollToBottom();
-    requestAnimationFrame(()=>{
+  cancelAnimationFrame(_settleRAF);
+  if(_settleRO){ _settleRO.disconnect(); _settleRO=null; }
+  clearTimeout(_settleTimer);
+  clearTimeout(_settleFinalTimer);
+
+  // Sync write anchors the viewport immediately.
+  _setMessageScrollToBottom();
+
+  if(force) return;
+
+  const el=document.getElementById('messages');
+  if(!el) return;
+  // Observe the GROWING content node, not the scroll container. #messages is the
+  // scroller but its box is fixed by the flex layout, so it never resizes — the
+  // transcript grows inside #msgInner (.messages-inner). Observing #messages
+  // would mean the callback never fires. (Codex review #2.)
+  const observed=document.getElementById('msgInner')||el;
+
+  // Instance-owned cleanup: close over THIS observer so a stale callback (from a
+  // superseded settle) only ever disconnects its own observer, never the newer
+  // active one that may now be in the global _settleRO. (Codex review #3.)
+  const ro=new ResizeObserver(()=>{
+    if(token!==_bottomSettleToken){ ro.disconnect(); if(_settleRO===ro) _settleRO=null; return; }
+    if(!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()){
+      ro.disconnect(); if(_settleRO===ro) _settleRO=null;
+      _programmaticScroll=false;
+      return;
+    }
+    // Write scrollTop once per frame — ResizeObserver batches multiple
+    // notifications per frame, so this is at most one write per frame.
+    cancelAnimationFrame(_settleRAF);
+    _settleRAF=requestAnimationFrame(()=>{
       if(token!==_bottomSettleToken) return;
-      if(force || (_scrollPinned&&!_messageUserUnpinned&&!_recentNonMessageScrollIntent())) _setMessageScrollToBottom();
+      _setMessageScrollToBottom();
     });
+    // After 300ms of quiet, disconnect — layout is stable.
+    clearTimeout(_settleTimer);
+    _settleTimer=setTimeout(()=>{
+      if(token!==_bottomSettleToken) return;
+      ro.disconnect(); if(_settleRO===ro) _settleRO=null;
+      _setMessageScrollToBottom();
+    },300);
+  });
+  _settleRO=ro;
+  ro.observe(observed);
+
+  // Static-content safety net: a fully-static response (no Prism/KaTeX/Mermaid/
+  // late images) never resizes after the initial sync write, so the
+  // ResizeObserver callback above never fires and its 300ms quiet-timer is never
+  // armed. Arm a single 2s top-level fallback so a late settle still runs for
+  // that case. The token check inside _settleFinalScroll makes this a no-op if a
+  // newer settle started, and it self-skips if the user unpinned. (Review #2/#3.)
+  clearTimeout(_settleFinalTimer);
+  _settleFinalTimer=setTimeout(()=>{
+    if(token!==_bottomSettleToken) return;
+    ro.disconnect(); if(_settleRO===ro) _settleRO=null;
+    if(!_scrollPinned||_messageUserUnpinned||_recentNonMessageScrollIntent()){ _programmaticScroll=false; return; }
+    _settleFinalScroll(token);
+  },2000);
+}
+
+function _settleFinalScroll(token){
+  if(token!==_bottomSettleToken) return;
+  const el=document.getElementById('messages');
+  if(!el){ _programmaticScroll=false; return; }
+  _programmaticScroll=true;
+  el.scrollTop=el.scrollHeight;
+  _lastScrollTop=el.scrollTop;
+  _nearBottomCount=2;
+  _scrollPinned=true;
+  requestAnimationFrame(()=>{
+    setTimeout(()=>{ _programmaticScroll=false; },0);
   });
 }
 function scrollIfPinned(){
@@ -3141,12 +3212,13 @@ function scrollToBottom(){
   _clearNewMessageScrollCue();
   _scrollPinned=true;
   _messageUserUnpinned=false;
-  // Write the first bottom position synchronously. A final renderMessages()
-  // rebuild can queue a native scroll event from the temporary scrollTop=0
-  // layout state; if we only schedule delayed settles, that event can cancel
-  // them before the viewport ever reaches the bottom.
+  // Write scrollTop once synchronously to anchor the viewport, then let
+  // ResizeObserver settle handle any late layout growth (Prism, KaTeX,
+  // Mermaid, images).  Using force=false so the observer runs — force=true
+  // was skipping the observer and causing Firefox paint jumps when
+  // renderMessages({preserveScroll:true}) + scrollToBottom() fired back-to-back.
   _setMessageScrollToBottom();
-  _settleMessageScrollToBottom(true);
+  _settleMessageScrollToBottom(false);
   _syncScrollToBottomCue(false,{newMessage:false});
   if(typeof _updateSessionStartJumpButton==='function') _updateSessionStartJumpButton();
 }

tests/test_tars_scroll_reset_regressions.py

@@ -43,10 +43,21 @@ def test_scroll_to_bottom_settles_across_late_markdown_layout_growth():
     scroll = _function_body(UI_JS, "function scrollToBottom")
     pinned = _function_body(UI_JS, "function scrollIfPinned")
 
+    # The settle survives late markdown layout growth (Prism/KaTeX/Mermaid/images)
+    # via a ResizeObserver on the growing content node (#msgInner) plus a 2s
+    # static-content fallback — this replaced the old [0,16,80,180]ms setTimeout
+    # fan-out + double-rAF scrollHeight polling (#3920, Firefox reflow jitter).
     assert "requestAnimationFrame" in settle
     assert "setTimeout" in settle
-    assert "const passes=[0,16,80,180]" in settle
-    assert "_settleMessageScrollToBottom(true)" in scroll
+    assert "new ResizeObserver" in settle
+    assert "getElementById('msgInner')" in settle, (
+        "the ResizeObserver must observe the growing content node (#msgInner), "
+        "not the fixed #messages scroll container"
+    )
+    assert "2000" in settle, "a 2s fallback must cover fully-static content that never resizes"
+    # scrollToBottom uses force=false so the observer actually runs (the old
+    # force=true skipped it and caused Firefox paint jumps).
+    assert "_settleMessageScrollToBottom(false)" in scroll
     assert "_settleMessageScrollToBottom(false)" in pinned
     assert "!_scrollPinned" in settle
     assert "const token=++_bottomSettleToken" in settle
@@ -57,11 +68,12 @@ def test_scroll_to_bottom_writes_scroll_position_immediately_before_delayed_sett
     scroll = _function_body(UI_JS, "function scrollToBottom")
 
     immediate_idx = scroll.index("_setMessageScrollToBottom();")
-    settle_idx = scroll.index("_settleMessageScrollToBottom(true)")
+    settle_idx = scroll.index("_settleMessageScrollToBottom(false)")
 
     assert immediate_idx < settle_idx, (
-        "scrollToBottom() must write scrollTop synchronously before scheduling delayed settles; "
-        "otherwise a DOM-rebuild scroll event can cancel the delayed passes and strand the viewport at the top"
+        "scrollToBottom() must write scrollTop synchronously before scheduling the "
+        "ResizeObserver settle; otherwise a DOM-rebuild scroll event can cancel the "
+        "delayed settle and strand the viewport at the top"
     )