From f657014aa3d9a57d8ce724f6dabc22fdbac4754c Mon Sep 17 00:00:00 2001 From: allenliang2022 Date: Sun, 5 Jul 2026 22:52:59 +0800 Subject: [PATCH] =?UTF-8?q?fix(webui):=20stop=20mobile=20scroll=20jump-bac?= =?UTF-8?q?k=20=E2=80=94=20scope=20content-visibility=20+=20hold=20unpinne?= =?UTF-8?q?d=20position=20(#5637)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two linked causes of the mobile scroll jump-back, both confirmed by the maintainer against origin/master in #5637. Primary (CSS): the @media (pointer: coarse) block applied a flat `content-visibility: auto; contain-intrinsic-size: auto 1px` to every .msg-row. Off-screen assistant rows can be multi-thousand px (tool-result / long answers), so a 1px reservation collapses their off-screen height; scrollHeight lurches down by tens of thousands of px on a long transcript, the browser force-clamps scrollTop, and the viewport is thrown to the top. A flat intrinsic-size cannot both reserve tall rows and keep scrollHeight stable for iOS flick momentum (why 1px was chosen), so content-visibility is scoped to the short, size-predictable user rows; assistant rows keep real rendered height. Mirrors the existing #liveAssistantTurn opt-out pattern. Secondary (JS): _restoreMessageScrollSnapshotSameFrame fell back to an absolute snapshot.top when the semantic anchor restore failed. During streaming the live activity-scene refresh fires this every tick; for a reader scrolled up into history (unpinned), snapping to a stale absolute top nudges the viewport backward by an amount that grows with scrollHeight. Now: hold position (no scrollTop write) for the unpinned/anchor-failed case and let the browser's own scroll anchoring keep the reader put. Pinned/near-bottom readers keep the tail-relative restore. Adds tests/test_issue5637_mobile_scroll_clamp_jumpback.py (mutation-checked: reverting either fix fails the matching assertion). --- static/style.css | 16 ++- static/ui.js | 16 +++ ..._issue5637_mobile_scroll_clamp_jumpback.py | 121 ++++++++++++++++++ 3 files changed, 152 insertions(+), 1 deletion(-) create mode 100644 tests/test_issue5637_mobile_scroll_clamp_jumpback.py diff --git a/static/style.css b/static/style.css index c0dccfe98..b120a580d 100644 --- a/static/style.css +++ b/static/style.css @@ -5729,7 +5729,21 @@ main.main > #mainPlugin{display:none;} enough to avoid scrollHeight recalculation jumps that kill flick-scroll momentum on iOS. */ @media (pointer: coarse) { - .msg-row { content-visibility: auto; contain-intrinsic-size: auto 1px; contain: layout style; } + /* #5637: content-visibility:auto is kept ONLY for user rows. Assistant rows can + contain very tall content (multi-thousand-px tool-result / long-answer rows), + and a flat `contain-intrinsic-size` cannot reserve their real off-screen + height: `1px` (or any single small value) collapses the reserved height of + every off-screen assistant row, so scrollHeight lurches down by tens of + thousands of px on a long transcript, the browser force-clamps scrollTop, and + the viewport jumps to the top (the mobile scroll jump-back). A value large + enough to reserve tall rows would reintroduce the scrollHeight churn the 1px + was chosen to avoid — so instead we gate content-visibility off for the tall + row class (assistant) and keep it for the short, size-predictable user rows, + which is where most of the off-screen-skip win comes from anyway. This mirrors + the existing "some rows must stay fully rendered" opt-out pattern for + #liveAssistantTurn below. */ + .msg-row[data-role="user"] { content-visibility: auto; contain-intrinsic-size: auto 96px; contain: layout style; } + .msg-row { contain: layout style; } /* Keep the LIVE streaming turn fully rendered while a response streams. The live turn is always id="liveAssistantTurn" (assigned on every render path: Compact Worklog, Transparent Stream, restored-live, etc.), but ONLY the diff --git a/static/ui.js b/static/ui.js index a1d67d397..277a46d8b 100644 --- a/static/ui.js +++ b/static/ui.js @@ -12808,6 +12808,22 @@ function _restoreMessageScrollSnapshotSameFrame(snapshot){ if(!restoredViaAnchor){ const maxTop=Math.max(0,el.scrollHeight-el.clientHeight); const bottom=Number(snapshot.bottom); + // #5637: when the reader has scrolled UP into history (userUnpinned) and the + // semantic anchor restore failed, do NOT snap scrollTop to the captured + // ABSOLUTE snapshot.top. During streaming, the live activity-scene refresh + // fires this every tick; above-viewport height keeps changing, so the old + // absolute top no longer maps to the same content and the viewport is nudged + // backward by an amount that grows with scrollHeight. Leaving scrollTop + // untouched lets the browser's own scroll anchoring hold the reader's + // position. Pinned / near-bottom readers still get the tail-relative restore + // below (that path is correct and must run). + if(snapshot.userUnpinned===true&&snapshot.pinned!==true){ + _lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight; + _messageUserUnpinned=true; + _scrollPinned=false; + _nearBottomCount=0; + return; + } const target=(snapshot.pinned===true&&Number.isFinite(bottom)) ? maxTop-Math.max(0,bottom) : Number(snapshot.top)||0; diff --git a/tests/test_issue5637_mobile_scroll_clamp_jumpback.py b/tests/test_issue5637_mobile_scroll_clamp_jumpback.py new file mode 100644 index 000000000..c5ac3e3d0 --- /dev/null +++ b/tests/test_issue5637_mobile_scroll_clamp_jumpback.py @@ -0,0 +1,121 @@ +"""Regression tests for #5637: mobile scroll jump-back (two linked causes). + +Real-device instrumentation (Android Chrome) traced the mobile scroll +jump-back to two causes: + +1. Primary (CSS): the ``@media (pointer: coarse)`` block set a single flat + ``content-visibility: auto; contain-intrinsic-size: auto 1px`` on every + ``.msg-row``. Off-screen assistant rows (which can be multi-thousand px: + tool-result / long-answer) then reserved only ~1px each, so ``scrollHeight`` + collapsed by tens of thousands of px on a long transcript, the browser + force-clamped ``scrollTop``, and the viewport jumped to the top. A flat + value cannot square "reserve tall rows" with "keep scrollHeight stable for + iOS flick momentum" (the reason 1px was chosen), so content-visibility is + now gated to the short, size-predictable user rows only; assistant rows keep + their real rendered height. + +2. Secondary (JS): ``_restoreMessageScrollSnapshotSameFrame`` fell back to an + ABSOLUTE ``snapshot.top`` when the semantic anchor restore failed. During + streaming, the live activity-scene refresh fires this every tick; for a + reader scrolled up into history (unpinned), snapping to a stale absolute top + nudges the viewport backward by an amount that grows with ``scrollHeight``. + The fix holds position (no scrollTop write) for the unpinned/anchor-failed + case and lets the browser's own scroll anchoring keep the reader put. + +Both tests fail on the pre-fix tree and pass only on the fixed tree. +""" + +from pathlib import Path + +REPO = Path(__file__).resolve().parents[1] +UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") +STYLE_CSS = (REPO / "static" / "style.css").read_text(encoding="utf-8") + + +def _coarse_pointer_block() -> str: + """Return the @media (pointer: coarse) block body from style.css.""" + anchor = "@media (pointer: coarse)" + start = STYLE_CSS.index(anchor) + brace = STYLE_CSS.index("{", start) + depth = 0 + for i in range(brace, len(STYLE_CSS)): + ch = STYLE_CSS[i] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return STYLE_CSS[brace : i + 1] + raise AssertionError("@media (pointer: coarse) block not found") + + +def _restore_same_frame_fn() -> str: + """Return the _restoreMessageScrollSnapshotSameFrame function body.""" + anchor = "function _restoreMessageScrollSnapshotSameFrame" + start = UI_JS.index(anchor) + brace = UI_JS.index("{", start) + depth = 0 + for i in range(brace, len(UI_JS)): + ch = UI_JS[i] + if ch == "{": + depth += 1 + elif ch == "}": + depth -= 1 + if depth == 0: + return UI_JS[start : i + 1] + raise AssertionError("_restoreMessageScrollSnapshotSameFrame not found") + + +def test_content_visibility_not_flat_on_all_msg_rows(): + """content-visibility:auto must NOT be applied to a bare .msg-row selector. + + A flat `.msg-row { content-visibility: auto; contain-intrinsic-size: auto 1px }` + collapses tall assistant rows' off-screen height and drives the scrollHeight + clamp jump-back (#5637). The rule must be scoped so tall assistant rows are + not off-screen-skipped with a tiny flat estimate. + """ + block = _coarse_pointer_block() + # The exact pre-fix line: content-visibility on an unqualified .msg-row. + assert ".msg-row { content-visibility: auto; contain-intrinsic-size: auto 1px" not in block, ( + "Flat content-visibility on every .msg-row is the #5637 primary cause; " + "it must be scoped away from tall (assistant) rows." + ) + + +def test_content_visibility_scoped_to_user_rows(): + """content-visibility should be retained for the short user rows only.""" + block = _coarse_pointer_block() + assert 'content-visibility: auto' in block, ( + "The iOS off-screen-skip optimization should still apply to short rows." + ) + assert '.msg-row[data-role="user"]' in block, ( + "content-visibility:auto must be scoped to user rows (size-predictable), " + "not applied flat to assistant rows that can be multi-thousand px (#5637)." + ) + + +def test_restore_same_frame_holds_position_for_unpinned_reader(): + """The absolute-top fallback must be skipped for an unpinned reader. + + When the reader is scrolled up (userUnpinned) and the anchor restore failed, + _restoreMessageScrollSnapshotSameFrame must NOT write an absolute snapshot.top + (it goes stale and nudges the viewport backward as scrollHeight grows, #5637). + It should hold position and return before the el.scrollTop write. + """ + fn = _restore_same_frame_fn() + assert "snapshot.userUnpinned===true&&snapshot.pinned!==true" in fn, ( + "The fallback must guard the unpinned/anchor-failed case (#5637)." + ) + # The guard must sit BEFORE the absolute-top scrollTop write and short-circuit it. + guard_idx = fn.index("snapshot.userUnpinned===true&&snapshot.pinned!==true") + # the absolute-top write in the fallback + write_idx = fn.index("el.scrollTop=Math.max(0,Math.min(target,maxTop))") + assert guard_idx < write_idx, ( + "The unpinned guard must precede (and return before) the absolute-top " + "scrollTop write so it is never reached for an unpinned reader (#5637)." + ) + # Between the guard and the write there must be an early return. + between = fn[guard_idx:write_idx] + assert "return;" in between, ( + "The unpinned guard must `return` before the absolute-top write (#5637)." + )