From 0668cfa77c759f5bc5ec0e0958f07783f940d229 Mon Sep 17 00:00:00 2001 From: Rod Boev Date: Wed, 24 Jun 2026 19:57:29 -0400 Subject: [PATCH] fix(#4856): suppress Chromium scroll-anchor re-selection during mobile DOM rebuild --- static/ui.js | 21 ++-- ...est_issue4856_android_scroll_regression.py | 98 +++++++++++++++++++ 2 files changed, 106 insertions(+), 13 deletions(-) create mode 100644 tests/test_issue4856_android_scroll_regression.py diff --git a/static/ui.js b/static/ui.js index 7f0a191a..31227c08 100644 --- a/static/ui.js +++ b/static/ui.js @@ -11029,22 +11029,19 @@ function _restoreMessageScrollSnapshot(snapshot){ } } /** - * Mobile scroll-jank guard: temporarily re-enable overflow-anchor so the - * browser preserves scroll position across the innerHTML='' + DOM rebuild gap. - * Safari/Chrome on iOS/Android can paint a frame with scrollTop=0 between - * innerHTML='' and snapshot restore, scroll-janking the user to the top. - * Called from renderMessages() before the DOM wipe — not from streaming ticks, - * since CSS already gives overflow-anchor:auto on mobile via media query. + * Mobile scroll-jank guard: temporarily disable overflow-anchor so + * Chromium cannot re-anchor to the topmost row during the innerHTML='' + * wipe-and-rebuild gap. The rAF callback restores CSS default afterward. */ 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 enable it. + // Mobile touch devices only: temporarily suppress anchor re-selection. if(window.matchMedia('(hover:hover) and (pointer:fine)').matches) return; - el.style.overflowAnchor='auto'; + el.style.overflowAnchor='none'; requestAnimationFrame(()=>{ - if(el.style.overflowAnchor==='auto') el.style.overflowAnchor=''; + if(el.style.overflowAnchor==='none') el.style.overflowAnchor=''; }); }; @@ -11282,10 +11279,8 @@ function renderMessages(options){ _recycleStash.set(Number(key), child); } } - // Mobile scroll-jank fix: temporarily re-enable overflow-anchor so browser - // preserves scroll position across the DOM wipe-and-rebuild gap. - // Safari/Chrome on iOS/Android can paint a frame with scrollTop=0 between - // innerHTML='' and snapshot restore, scroll-janking the user to the top. + // Mobile scroll-jank fix: temporarily disable overflow-anchor so Chromium + // cannot re-anchor to the topmost row during the DOM wipe-and-rebuild gap. if(window._fixMobileScrollJank) window._fixMobileScrollJank(); inner.innerHTML=''; const compressionNode=compressionState?_compressionCardsNode(compressionState):null; diff --git a/tests/test_issue4856_android_scroll_regression.py b/tests/test_issue4856_android_scroll_regression.py new file mode 100644 index 00000000..f765dbb2 --- /dev/null +++ b/tests/test_issue4856_android_scroll_regression.py @@ -0,0 +1,98 @@ +"""Regression tests for #4856: Android scroll-to-top on every interaction.""" + +from pathlib import Path + +REPO = Path(__file__).parent.parent +UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") +MESSAGES_JS = (REPO / "static" / "messages.js").read_text(encoding="utf-8") + +_FUNC_MARKER = "window._fixMobileScrollJank=function _fixMobileScrollJank(){" +_RAF_MARKER = "requestAnimationFrame(()=>{" + + +def _extract_fix_mobile_scroll_jank(src: str) -> str: + idx = src.find(_FUNC_MARKER) + assert idx != -1, "_fixMobileScrollJank not found in ui.js" + depth = 0 + for i, ch in enumerate(src[idx:], idx): + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return src[idx : i + 1] + raise AssertionError("Could not extract _fixMobileScrollJank") + + +def _extract_raf_body(fn_src: str) -> str: + idx = fn_src.find(_RAF_MARKER) + assert idx != -1, "requestAnimationFrame callback not found in function" + depth = 0 + for i, ch in enumerate(fn_src[idx:], idx): + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return fn_src[idx : i + 1] + raise AssertionError("Could not extract rAF body") + + +def test_fix_sets_none_not_auto(): + # Base-fails/head-passes: function must set 'none' to suppress Chromium + # scroll-anchor re-selection; 'auto' is already the CSS default so setting + # it is a no-op and leaves the anchor engine active during the DOM wipe. + fn = _extract_fix_mobile_scroll_jank(UI_JS) + assert "overflowAnchor='none'" in fn, ( + "_fixMobileScrollJank() must set overflowAnchor='none' to suppress " + "Chromium scroll-anchor re-selection during the DOM wipe (#4856)." + ) + assert "overflowAnchor='auto'" not in fn, ( + "_fixMobileScrollJank() must not set overflowAnchor='auto'; that is " + "already the CSS resting value on mobile and is a no-op." + ) + + +def test_raf_cleanup_checks_none(): + # The rAF guard must check for 'none' so it only clears the inline style + # it set; checking 'auto' was always false and left the inline style on. + fn = _extract_fix_mobile_scroll_jank(UI_JS) + raf = _extract_raf_body(fn) + assert "overflowAnchor==='none'" in raf, ( + "The rAF cleanup in _fixMobileScrollJank() must check for 'none' so it " + "clears the inline style after the synchronous scrollTop write lands." + ) + assert "overflowAnchor==='auto'" not in raf, ( + "The rAF cleanup must not check for 'auto'; that check was always false." + ) + + +def test_rebuild_path_calls_fix_before_wipe(): + # renderMessages() must call _fixMobileScrollJank() before innerHTML='' so + # anchor suppression is active during the full wipe-and-rebuild window. + fix_idx = UI_JS.find("window._fixMobileScrollJank()") + assert fix_idx != -1, ( + "renderMessages() must call window._fixMobileScrollJank() before innerHTML=''." + ) + wipe_idx = UI_JS.find("innerHTML=''", fix_idx) + assert wipe_idx != -1, ( + "innerHTML='' not found after _fixMobileScrollJank() call site." + ) + assert fix_idx < wipe_idx, ( + "_fixMobileScrollJank() must be called before innerHTML='' in renderMessages()." + ) + + +def test_streaming_tick_calls_fix_before_dom_writes(): + # The streaming render tick in messages.js must call _fixMobileScrollJank() + # before _lastRenderMs=performance.now() so anchor suppression covers every + # incremental DOM update during streaming. + guard_idx = MESSAGES_JS.find("window._fixMobileScrollJank") + assert guard_idx != -1, ( + "The streaming tick must call window._fixMobileScrollJank() before DOM writes." + ) + render_idx = MESSAGES_JS.find("_lastRenderMs=performance.now()") + assert render_idx != -1, "streaming render timestamp not found in messages.js" + assert guard_idx < render_idx, ( + "The mobile scroll-jank guard must run before streaming DOM work begins." + )