Merge pull request #5352 from nesquena/stage-5338

Release — suppress mobile scroll jump-back on message realign (#5338)

CHANGELOG.md

@@ -9,6 +9,8 @@ _No unreleased changes. Entries are moved into their version block when a releas
 
 ### Fixed
 
+- **No more scroll jump-back on mobile while messages load.** When the app realigns the viewport after loading older messages, mobile browsers no longer double-compensate the scroll position (their native overflow-anchor fighting the app's own scroll write), which caused a visible jump. Desktop behavior is unchanged. (#5338)
+
 - **No more white flicker/flash while the assistant streams on light themes.** Live token-by-token markdown updates no longer inherit the global dark/light `color`/`background` transition, so the streaming turn paints instantly instead of briefly fading on each token. Theme-switch transitions elsewhere are unchanged. (#5328)
 
 - **No more false "Clarify endpoint unavailable. Please restart server." toast.** The clarify-pending poll used to fire the restart-server warning on any error whose message merely contained "404"/"not found" — so a harmless stale `Session not found` from a just-switched profile mis-triggered it even though the clarify endpoint was fine. The poll now branches on the structured HTTP status: a session-scoped 404 is treated as a stale poll (silently stops), and the restart warning fires only on a genuine missing-route 404. Interrupt provenance is also now labeled (explicit Stop vs stream-lifecycle) for clearer settlement. (#5345)

static/ui.js

@@ -824,6 +824,46 @@ function _captureMessageViewportAnchor(){
   }
   return null;
 }
+// Temporarily suppress the browser's native overflow-anchor on a scroll
+// container so a JS scrollTop write is not double-compensated by the browser's
+// own scroll-anchoring in the same frame. Returns a release fn that restores the
+// prior inline value on the NEXT frame (after layout settles). No-op on desktop,
+// where the resting computed value is already `none` (CSS hover/fine-pointer
+// media query) — suppressing `none` changes nothing and the release restores the
+// same empty inline value. Only mobile (resting `auto`) is actually affected,
+// which is exactly where the double-compensation jump-back happens.
+//
+// Both this helper and _fixMobileScrollJank() gate on the SAME question — "is
+// the browser's native scroll-anchor layer currently active on this element?" —
+// routed through this one predicate so the two guards can't drift apart if the
+// CSS media query ever changes (maintainer review on #5338). The computed-value
+// test is more robust than a matchMedia('(hover:hover) and (pointer:fine)')
+// check because it reflects the real resting value, including any inline
+// override, not just the viewport media state.
+function _browserOverflowAnchorActive(el){
+  if(!el) return false;
+  try{ return getComputedStyle(el).overflowAnchor==='auto'; }catch(_){ return false; }
+}
+function _suppressBrowserOverflowAnchor(container){
+  if(!container||!container.style) return null;
+  // Only engage when the browser layer is actually active (auto). On desktop
+  // (none) there is nothing to suppress.
+  if(!_browserOverflowAnchorActive(container)) return null;
+  const prevInline=container.style.overflowAnchor||'';
+  container.style.overflowAnchor='none';
+  let released=false;
+  return function _release(){
+    if(released) return;
+    released=true;
+    const restore=()=>{
+      // Only restore if we still own the suppression (another render may have
+      // re-set it); compare against the value we wrote.
+      if(container.style.overflowAnchor==='none') container.style.overflowAnchor=prevInline;
+    };
+    if(typeof requestAnimationFrame==='function') requestAnimationFrame(restore);
+    else restore();
+  };
+}
 function _restoreMessageViewportAnchor(anchor, rawIdxDelta){
   const container=$('messages');
   if(!container||!anchor) return false;
@@ -842,7 +882,19 @@ function _restoreMessageViewportAnchor(anchor, rawIdxDelta){
   const rect=row.getBoundingClientRect();
   const targetTop=Number(anchor.topOffset)||0;
   _programmaticScroll=true;_programmaticScrollSetAt=performance.now();
+  // Mobile-only jump fix: the resting overflow-anchor on .messages is `auto` on
+  // touch devices (CSS media query keeps it `none` only for hover+fine-pointer
+  // desktops). When we write scrollTop here to realign the anchor row, a mobile
+  // browser's OWN overflow-anchor machinery ALSO shifts scrollTop in the same
+  // frame if content height above the viewport changed — the two compensations
+  // stack and yank the reader to an unrelated turn (the mobile jump-back). This is why the
+  // bug is mobile-only and never reproduces on a desktop (none) browser. Suppress
+  // the browser layer for this write; _releaseAnchorSuppression restores it next
+  // frame. Desktop is already `none`, so this is a no-op there.
+  const _releaseAnchorSuppression=(typeof _suppressBrowserOverflowAnchor==='function')
+    ? _suppressBrowserOverflowAnchor(container) : null;
   container.scrollTop+=(rect.top-containerRect.top)-targetTop;
+  if(_releaseAnchorSuppression) _releaseAnchorSuppression();
   if(typeof _deferClearProgrammaticScroll==='function') _deferClearProgrammaticScroll();
   else requestAnimationFrame(()=>{ setTimeout(()=>{ _programmaticScroll=false; },0); });
   return true;
@@ -7855,7 +7907,7 @@ function restoreLiveTurnHtmlForSession(sid){
   const liveGroup=restored.querySelector('.tool-call-group[data-live-tool-call-group="1"]');
   if(liveGroup&&typeof _startActivityElapsedTimer==='function') _startActivityElapsedTimer(liveGroup);
   if(typeof placeLiveToolCardsHost==='function') placeLiveToolCardsHost();
-  requestAnimationFrame(()=>postProcessRenderedMessages(restored));
+  requestAnimationFrame(()=>_postProcessWithAnchorSuppression(restored));
   return true;
 }
 
@@ -12184,9 +12236,12 @@ function _restoreMessageScrollSnapshot(snapshot){
 window._fixMobileScrollJank=function _fixMobileScrollJank(){
   const el=document.getElementById('messages');
   if(!el) return;
-  // Desktop with a mouse: keep overflow-anchor:none (explicitly set by CSS).
-  // Mobile touch devices only: temporarily suppress anchor re-selection.
-  if(window.matchMedia('(hover:hover) and (pointer:fine)').matches) return;
+  // Route through the same "is the browser scroll-anchor layer active?" predicate
+  // as _suppressBrowserOverflowAnchor so the two guards can't drift (#5338 review).
+  // Desktop rests at overflow-anchor:none (predicate false) → nothing to suppress.
+  // Mobile touch devices rest at auto (predicate true) → temporarily suppress
+  // anchor re-selection during the wipe-and-rebuild gap.
+  if(!_browserOverflowAnchorActive(el)) return;
   el.style.overflowAnchor='none';
   requestAnimationFrame(()=>{
     if(el.style.overflowAnchor==='none') el.style.overflowAnchor='';
@@ -12475,7 +12530,7 @@ function renderMessages(options){
       _scrollAfterMessageRender(preserveScroll, scrollSnapshot);
       if(_maybeRecoverVirtualizedBlankViewport(options, preserveScroll, virtualWindow)) return;
       _updateMessageVirtualMeasurements(renderVisWithIdx, renderVisibleIdxs, virtualWindow);
-      requestAnimationFrame(()=>postProcessRenderedMessages(inner));
+      requestAnimationFrame(()=>_postProcessWithAnchorSuppression(inner));
       if(typeof _initMediaPlaybackObserver==='function') _initMediaPlaybackObserver();
       if(typeof loadTodos==='function'&&document.getElementById('panelTodos')&&document.getElementById('panelTodos').classList.contains('active')){loadTodos();}
       return;
@@ -13829,7 +13884,7 @@ function renderMessages(options){
   _scrollAfterMessageRender(preserveScroll, scrollSnapshot);
   if(_maybeRecoverVirtualizedBlankViewport(options, preserveScroll, virtualWindow)) return;
   // Apply syntax highlighting after DOM is built
-  requestAnimationFrame(()=>postProcessRenderedMessages(inner));
+  requestAnimationFrame(()=>_postProcessWithAnchorSuppression(inner));
   // Refresh todo panel if it's currently open
   if(typeof loadTodos==='function' && document.getElementById('panelTodos') && document.getElementById('panelTodos').classList.contains('active')){
     loadTodos();
@@ -14955,6 +15010,36 @@ async function regenerateResponse(btn) {
   } catch(e) { setStatus(t('regen_failed') + e.message); }
 }
 
+// postProcessRenderedMessages() runs one frame AFTER the render + JS scroll
+// restore (it is scheduled via requestAnimationFrame). It performs syntax
+// highlighting, inline diff/csv/pdf/html/excalidraw hydration, mermaid/katex
+// rendering — all of which can CHANGE the height of rows above the viewport.
+//
+// On mobile the scroller rests at overflow-anchor:auto, so any above-viewport
+// height change in this post-render frame makes the browser's native anchor
+// engine compensate scrollTop a SECOND time — after the JS restore already
+// settled the reader's position — yanking them to an unrelated turn ("往回大跳").
+// The synchronous _fixMobileScrollJank / _suppressBrowserOverflowAnchor guards
+// only cover the render frame itself; they have already released by the time
+// this rAF fires. Wrap the post-process (and the media-reflow frame right after
+// it) in the same suppression so the browser layer cannot re-anchor during the
+// async settle window. Desktop rests at `none`, so this is a no-op there.
+function _postProcessWithAnchorSuppression(container){
+  const scroller=$('messages');
+  const release=(scroller&&typeof _suppressBrowserOverflowAnchor==='function')
+    ? _suppressBrowserOverflowAnchor(scroller) : null;
+  try{
+    postProcessRenderedMessages(container);
+  }finally{
+    // Hold suppression across ONE more frame so late media/layout reflow
+    // (image decode, katex/mermaid measure) cannot re-anchor either, then let
+    // _suppressBrowserOverflowAnchor's own rAF-deferred restore run.
+    if(release){
+      if(typeof requestAnimationFrame==='function') requestAnimationFrame(release);
+      else release();
+    }
+  }
+}
 function postProcessRenderedMessages(container) {
   highlightCode(container);
   addCopyButtons(container);

tests/test_anchor_fallback_ownership.py

@@ -669,6 +669,7 @@ def test_render_messages_keeps_anchor_owned_turn_out_of_legacy_activity_rebuilds
         function _maybeRecoverVirtualizedBlankViewport() {{ return false; }}
         function _updateMessageVirtualMeasurements() {{}}
         function postProcessRenderedMessages() {{}}
+        function _postProcessWithAnchorSuppression() {{}}
         function _formatGatewayModelLabel() {{ return ''; }}
         function _gatewayRoutingFailoverText() {{ return ''; }}
         function _gatewayModelWarningText() {{ return ''; }}

tests/test_csv_table_rendering.py

@@ -112,7 +112,16 @@ def test_csv_loadCsvInline_called_after_render():
     """Verify loadCsvInline is called by the consolidated post-render pass."""
     with open('static/ui.js') as f:
         src = f.read()
-    assert 'requestAnimationFrame(()=>postProcessRenderedMessages(inner))' in src
+    # Behavior assertion (not a brittle rAF-literal match): the post-render pass
+    # is scheduled a frame later, now routed through _postProcessWithAnchorSuppression
+    # (which holds overflow-anchor suppression across the media/layout reflow, then
+    # calls postProcessRenderedMessages). Assert the behavior chain, so a future
+    # wrapper rename doesn't re-orphan this test. (#5338)
+    assert 'requestAnimationFrame(()=>_postProcessWithAnchorSuppression(' in src
+    wrap_idx = src.find('function _postProcessWithAnchorSuppression')
+    assert wrap_idx != -1, "post-render should be wrapped by _postProcessWithAnchorSuppression"
+    assert 'postProcessRenderedMessages(container)' in src[wrap_idx:wrap_idx + 500], \
+        "the wrapper must still invoke postProcessRenderedMessages"
     idx = src.find('function postProcessRenderedMessages')
     body = src[idx:idx + 500]
     assert 'loadCsvInline(container)' in body, "post-process should call loadCsvInline once per render"

tests/test_excalidraw_inline_embed.py

@@ -123,7 +123,13 @@ def test_excalidraw_called_after_render():
     """Verify loadExcalidrawInline is called by the consolidated post-render pass."""
     with open('static/ui.js') as f:
         src = f.read()
-    assert 'requestAnimationFrame(()=>postProcessRenderedMessages(inner))' in src
+    # Behavior assertion (#5338): post-render is now scheduled through
+    # _postProcessWithAnchorSuppression (which still calls postProcessRenderedMessages).
+    assert 'requestAnimationFrame(()=>_postProcessWithAnchorSuppression(' in src
+    _wrap = src.find('function _postProcessWithAnchorSuppression')
+    assert _wrap != -1, "post-render should be wrapped by _postProcessWithAnchorSuppression"
+    assert 'postProcessRenderedMessages(container)' in src[_wrap:_wrap + 500], \
+        "the wrapper must still invoke postProcessRenderedMessages"
     idx = src.find('function postProcessRenderedMessages')
     body = src[idx:idx + 500]
     assert 'loadExcalidrawInline(container)' in body, (

tests/test_issue347.py

@@ -204,8 +204,14 @@ def test_katex_throw_on_error_false():
 
 def test_render_katex_blocks_wired_into_raf():
     """renderKatexBlocks() must run from the post-render requestAnimationFrame pass."""
-    raf_call = 'requestAnimationFrame(()=>postProcessRenderedMessages(inner))'
+    # Behavior assertion (#5338): post-render is scheduled through
+    # _postProcessWithAnchorSuppression, which still calls postProcessRenderedMessages.
+    raf_call = 'requestAnimationFrame(()=>_postProcessWithAnchorSuppression('
     assert raf_call in UI_JS, 'post-render requestAnimationFrame pass not found'
+    _wrap = UI_JS.find('function _postProcessWithAnchorSuppression')
+    assert _wrap != -1, "post-render should be wrapped by _postProcessWithAnchorSuppression"
+    assert 'postProcessRenderedMessages(container)' in UI_JS[_wrap:_wrap + 500], \
+        "the wrapper must still invoke postProcessRenderedMessages"
     idx = UI_JS.find('function postProcessRenderedMessages')
     body = UI_JS[idx:idx + 500]
     assert 'renderMermaidBlocks(container)' in body

tests/test_issue483_inline_diff_viewer.py

@@ -62,7 +62,13 @@ class TestMediaDiffInline:
         """loadDiffInline() should be called by the consolidated post-render pass."""
         with open("static/ui.js", "r", encoding="utf-8") as f:
             content = f.read()
-        assert "requestAnimationFrame(()=>postProcessRenderedMessages(inner))" in content
+        # Behavior assertion (#5338): post-render scheduled via
+        # _postProcessWithAnchorSuppression, which still calls postProcessRenderedMessages.
+        assert "requestAnimationFrame(()=>_postProcessWithAnchorSuppression(" in content
+        _wrap = content.find('function _postProcessWithAnchorSuppression')
+        assert _wrap != -1, "post-render should be wrapped by _postProcessWithAnchorSuppression"
+        assert 'postProcessRenderedMessages(container)' in content[_wrap:_wrap + 500], \
+            "the wrapper must still invoke postProcessRenderedMessages"
         start = content.find("function postProcessRenderedMessages")
         body = content[start:start + 500]
         assert "addCopyButtons(container)" in body

tests/test_issue484_json_tree_viewer.py

@@ -55,7 +55,13 @@ class TestTreeRenderer:
     def test_initTreeViews_called_in_post_render(self):
         with open("static/ui.js", "r", encoding="utf-8") as f:
             content = f.read()
-        assert "requestAnimationFrame(()=>postProcessRenderedMessages(inner))" in content
+        # Behavior assertion (#5338): post-render scheduled via
+        # _postProcessWithAnchorSuppression, which still calls postProcessRenderedMessages.
+        assert "requestAnimationFrame(()=>_postProcessWithAnchorSuppression(" in content
+        _wrap = content.find('function _postProcessWithAnchorSuppression')
+        assert _wrap != -1, "post-render should be wrapped by _postProcessWithAnchorSuppression"
+        assert 'postProcessRenderedMessages(container)' in content[_wrap:_wrap + 500], \
+            "the wrapper must still invoke postProcessRenderedMessages"
         start = content.find("function postProcessRenderedMessages")
         body = content[start:start + 500]
         assert "initTreeViews(container)" in body

tests/test_issue4856_android_scroll_regression.py

@@ -514,3 +514,59 @@ def test_streaming_tick_calls_fix_before_dom_writes():
     assert guard_idx < render_idx, (
         "The mobile scroll-jank guard must run before streaming DOM work begins."
     )
+
+
+def test_post_process_runs_under_overflow_anchor_suppression():
+    """#5338 follow-up: the async post-render settle window must stay suppressed.
+
+    Root cause of the residual mobile "往回大跳": postProcessRenderedMessages()
+    is scheduled a FRAME LATER via requestAnimationFrame(), after the synchronous
+    _fixMobileScrollJank()/_suppressBrowserOverflowAnchor() guards have already
+    released. It runs highlightCode()/load*Inline()/katex/mermaid, all of which
+    can change the height of rows ABOVE the viewport. On mobile (overflow-anchor:
+    auto) the browser's native anchor engine then compensates scrollTop a SECOND
+    time in that unguarded frame, yanking an unpinned reader to another turn.
+
+    The fix wraps every deferred post-process in _postProcessWithAnchorSuppression()
+    so the browser layer stays suppressed across the post-process + one media-reflow
+    frame. Desktop rests at overflow-anchor:none so the wrapper is a no-op there.
+    """
+    # The wrapper exists and engages the shared suppression helper.
+    wrapper_idx = UI_JS.find("function _postProcessWithAnchorSuppression(")
+    assert wrapper_idx != -1, (
+        "_postProcessWithAnchorSuppression() wrapper must exist to keep the "
+        "browser overflow-anchor layer suppressed across the deferred post-render "
+        "settle window (#5338 mobile 往回大跳 follow-up)."
+    )
+    wrapper = UI_JS[wrapper_idx: wrapper_idx + 900]
+    assert "_suppressBrowserOverflowAnchor(scroller)" in wrapper, (
+        "_postProcessWithAnchorSuppression() must route through the shared "
+        "_suppressBrowserOverflowAnchor() helper so desktop stays a verified no-op."
+    )
+    assert "postProcessRenderedMessages(container)" in wrapper, (
+        "_postProcessWithAnchorSuppression() must still call the real "
+        "postProcessRenderedMessages() inside the suppression window."
+    )
+    # Suppression is held across ONE extra frame so late media/layout reflow
+    # cannot re-anchor either.
+    assert "requestAnimationFrame(release)" in wrapper, (
+        "_postProcessWithAnchorSuppression() must defer the suppression release "
+        "by one frame so image-decode / katex / mermaid reflow is also covered."
+    )
+
+    # EVERY deferred post-process dispatch must go through the wrapper — a raw
+    # requestAnimationFrame(()=>postProcessRenderedMessages(...)) would leave that
+    # path unguarded and re-open the jump.
+    raw_dispatch = "requestAnimationFrame(()=>postProcessRenderedMessages("
+    assert raw_dispatch not in UI_JS, (
+        "All deferred postProcessRenderedMessages() dispatches must go through "
+        "_postProcessWithAnchorSuppression(); a raw rAF dispatch re-opens the "
+        "unguarded async settle window on mobile (#5338)."
+    )
+    wrapped_dispatch = "requestAnimationFrame(()=>_postProcessWithAnchorSuppression("
+    assert UI_JS.count(wrapped_dispatch) >= 3, (
+        "All three post-render paths (fast-path cache branch, main render tail, "
+        "live-tool remount) must dispatch post-process through the suppression "
+        "wrapper; found fewer than 3."
+    )
+

tests/test_pdf_html_preview.py

@@ -242,7 +242,15 @@ class TestRAFIntegration:
 
     def test_message_render_uses_single_post_process_raf(self):
         ui = _read_js('ui.js')
-        assert ui.count('requestAnimationFrame(()=>postProcessRenderedMessages(inner))') == 2
+        # Behavior assertion (#5338): the two `(inner)` post-render dispatches are
+        # now routed through _postProcessWithAnchorSuppression (which still calls
+        # postProcessRenderedMessages). Assert on the wrapper's `(inner)` dispatch
+        # count rather than the old literal so a future rename doesn't re-orphan this.
+        assert ui.count('requestAnimationFrame(()=>_postProcessWithAnchorSuppression(inner))') == 2
+        _wrap = ui.find('function _postProcessWithAnchorSuppression')
+        assert _wrap != -1, "post-render should be wrapped by _postProcessWithAnchorSuppression"
+        assert 'postProcessRenderedMessages(container)' in ui[_wrap:_wrap + 500], \
+            "the wrapper must still invoke postProcessRenderedMessages"
 
 
 # ── CSS classes ────────────────────────────────────────────────────────────