mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-07-15 04:01:05 +00:00
fix(webui): stop mobile scroll jump-back — scope content-visibility + hold unpinned position (#5637)
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).
This commit is contained in:
+15
-1
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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)."
|
||||
)
|
||||
Reference in New Issue
Block a user