diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ba53a2b0..e67556087 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/static/ui.js b/static/ui.js index 72c84f2ff..6d54387f2 100644 --- a/static/ui.js +++ b/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); diff --git a/tests/test_anchor_fallback_ownership.py b/tests/test_anchor_fallback_ownership.py index 4bec8d1f1..2f388efda 100644 --- a/tests/test_anchor_fallback_ownership.py +++ b/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 ''; }} diff --git a/tests/test_csv_table_rendering.py b/tests/test_csv_table_rendering.py index b86c413f8..57cbbe9b4 100644 --- a/tests/test_csv_table_rendering.py +++ b/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" diff --git a/tests/test_excalidraw_inline_embed.py b/tests/test_excalidraw_inline_embed.py index 677aac075..73c83f87c 100644 --- a/tests/test_excalidraw_inline_embed.py +++ b/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, ( diff --git a/tests/test_issue347.py b/tests/test_issue347.py index 489e8d7e6..1b94ec4a5 100644 --- a/tests/test_issue347.py +++ b/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 diff --git a/tests/test_issue483_inline_diff_viewer.py b/tests/test_issue483_inline_diff_viewer.py index 215d20ce4..06e819fca 100644 --- a/tests/test_issue483_inline_diff_viewer.py +++ b/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 diff --git a/tests/test_issue484_json_tree_viewer.py b/tests/test_issue484_json_tree_viewer.py index 1b0978e37..7e46436a3 100644 --- a/tests/test_issue484_json_tree_viewer.py +++ b/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 diff --git a/tests/test_issue4856_android_scroll_regression.py b/tests/test_issue4856_android_scroll_regression.py index aa65e5b1b..0c9ba7c6d 100644 --- a/tests/test_issue4856_android_scroll_regression.py +++ b/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." + ) + diff --git a/tests/test_pdf_html_preview.py b/tests/test_pdf_html_preview.py index 51be6a2d8..c0a3c8caf 100644 --- a/tests/test_pdf_html_preview.py +++ b/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 ────────────────────────────────────────────────────────────