fix(#4856): suppress Chromium scroll-anchor re-selection during mobile DOM rebuild
This commit is contained in:
1 parent
87fb8ed9ec
commit
0668cfa77c
2 files changed
+106
-13
No files matched your search
+8
-13
@@ -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;
|
||||
|
||||
@@ -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."
|
||||
)
|
||||
Reference in new issue
Block a user