Merge pull request #4838 from nesquena/fix/4702-portrait-scroll-bottom

Release v0.51.623 — iOS portrait: opening a conversation lands at the bottom (#4702)
This commit is contained in:
nesquena-hermes 2026-06-24 03:28:54 -07:00 committed by GitHub
commit a287980799
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 95 additions and 6 deletions

View file

@ -3,6 +3,12 @@
## [Unreleased]
## [v0.51.623] — 2026-06-24 — Release WD (iOS portrait: opening a conversation lands at the bottom)
### Fixed
- **On iOS Safari in portrait, opening or switching a conversation no longer jumps to the top of the transcript.** iOS resolves its dynamic toolbar height after first paint; when the toolbar collapsed, the message scroller grew and fired a scroll event with a decreased `scrollTop` that the WebUI misread as the user scrolling up — so it unpinned the freshly-opened session and the late re-anchor settle then cancelled itself, stranding the reader at the oldest message (landscape was unaffected because it doesn't resize late). The scroll tracker now distinguishes a container-height increase (toolbar/keyboard reflow) from a real upward scroll, and an open-time settle also re-anchors the bottom after the late portrait viewport change. Desktop and landscape behavior is unchanged. Also addresses the sibling symptom in #4701. (#4702)
## [v0.51.622] — 2026-06-24 — Release WC (Tasks tab no longer 500s when the cron package is absent)
### Fixed

View file

@ -3710,6 +3710,7 @@ let _programmaticScrollResetTimer=0;
function _deferClearProgrammaticScroll(ms){clearTimeout(_programmaticScrollResetTimer);_programmaticScrollResetTimer=setTimeout(()=>{_programmaticScroll=false;},ms||80);}
let _nearBottomCount=0;
let _lastScrollTop=null;
let _lastMessageClientHeight=null; // #4702: track scroller height to ignore iOS portrait toolbar-settle reflows (a clientHeight increase fires a scroll event with decreased scrollTop that is NOT a user scroll)
// Sticky-unpin model (#3343 supersedes #3330's proximity re-pin): once the user
// scrolls up, streaming stops auto-following until they return to the bottom or
// click ↓. The upward-intent TIMEOUT mechanism (_lastMessageUpwardIntentMs /
@ -3846,6 +3847,7 @@ if(typeof document!=='undefined'){
function _resetScrollDirectionTracker(){
_clearNewMessageScrollCue();
_lastScrollTop=null;
_lastMessageClientHeight=null;
_messageUserUnpinned=false;
_scrollPinned=true;
_nearBottomCount=0;
@ -3967,7 +3969,17 @@ if(typeof window!=='undefined'){
const top=el.scrollTop;
const bottomDistance=el.scrollHeight-top-el.clientHeight;
const nearBottom=bottomDistance<250;
const movedUp=_lastScrollTop!==null&&top<_lastScrollTop-2;
// #4702: iOS Safari (esp. portrait) resolves its dynamic toolbar height
// AFTER first paint. When the toolbar collapses the scroller GROWS
// (clientHeight increases), which fires a scroll event with a DECREASED
// scrollTop even though the user never scrolled. Without this guard that
// reflow is misread as an upward scroll and falsely unpins a freshly-opened
// session, stranding portrait readers at the top (sibling: #4701). On
// desktop/landscape the scroller height is stable, so `grew` is always
// false and behavior is byte-identical.
const grew=_lastMessageClientHeight!==null&&el.clientHeight>_lastMessageClientHeight+1;
_lastMessageClientHeight=el.clientHeight;
const movedUp=!grew&&_lastScrollTop!==null&&top<_lastScrollTop-2;
const movedDown=_lastScrollTop!==null&&top>_lastScrollTop+2;
_lastScrollTop=top;
if(movedUp){
@ -4460,7 +4472,7 @@ function _setMessageScrollToBottom(){
if(!el) return;
_programmaticScroll=true;_programmaticScrollSetAt=performance.now();
el.scrollTop=el.scrollHeight;
_lastScrollTop=el.scrollTop;
_lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight;
_nearBottomCount=2;
_scrollPinned=true;
requestAnimationFrame(()=>{
@ -4475,7 +4487,7 @@ function _setMessageScrollToBottom(){
return;
}
el.scrollTop=el.scrollHeight;
_lastScrollTop=el.scrollTop;
_lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight;
_nearBottomCount=2;
_scrollPinned=true;
_deferClearProgrammaticScroll();
@ -4568,6 +4580,12 @@ function _settleMessageScrollToBottom(force, explicit){
});
_settleRO=ro;
ro.observe(observed);
// #4702: for an explicit (user/open) settle, also observe the SCROLLER itself.
// On iOS the transcript content (#msgInner) may not resize, but the scroller
// grows when the portrait toolbar collapses after first paint — observing both
// re-anchors the bottom after that late viewport settle. Desktop never resizes
// here, so this is a no-op off-mobile.
if(explicit&&observed!==el){ try{ ro.observe(el); }catch(_){ } }
// Static-content safety net: a fully-static response (no Prism/KaTeX/Mermaid/
// late images) never resizes after the initial sync write, so the
@ -4594,7 +4612,7 @@ function _settleFinalScroll(token){
}
_programmaticScroll=true;_programmaticScrollSetAt=performance.now();
el.scrollTop=el.scrollHeight;
_lastScrollTop=el.scrollTop;
_lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight;
_nearBottomCount=2;
_scrollPinned=true;
_deferClearProgrammaticScroll();
@ -10984,7 +11002,7 @@ function _restoreMessageScrollSnapshot(snapshot){
el.scrollTop=Math.max(0,Math.min(Number(snapshot.top)||0,maxTop));
}
// Sync _lastScrollTop after programmatic restore so sticky-unpin does not false-trigger (#1731).
_lastScrollTop=el.scrollTop;
_lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight;
if(snapshot.userUnpinned===true){
_messageUserUnpinned=true;
_scrollPinned=false;
@ -11050,7 +11068,7 @@ function _restoreMessageScrollSnapshotSameFrame(snapshot){
_programmaticScroll=true;_programmaticScrollSetAt=performance.now();
el.scrollTop=Math.max(0,Math.min(target,maxTop));
}
_lastScrollTop=el.scrollTop;
_lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight;
if(snapshot.pinned===true){
_messageUserUnpinned=false;
_scrollPinned=true;

View file

@ -58,6 +58,7 @@ def _run_scroll_listener(samples: list[dict[str, int]]) -> dict[str, int | bool
const step = new Function(
'el',
'_lastScrollTop',
'_lastMessageClientHeight',
'_nearBottomCount',
'_scrollPinned',
'_messageUserUnpinned',
@ -73,6 +74,7 @@ const step = new Function(
payload.body + `
return {
_lastScrollTop,
_lastMessageClientHeight,
_nearBottomCount,
_scrollPinned,
_messageUserUnpinned,
@ -82,6 +84,7 @@ return {
let state = {
_lastScrollTop: 800,
_lastMessageClientHeight: null,
_nearBottomCount: 0,
_scrollPinned: true,
_messageUserUnpinned: false,
@ -92,6 +95,7 @@ for (const sample of payload.samples) {
state = step(
sample,
state._lastScrollTop,
state._lastMessageClientHeight,
state._nearBottomCount,
state._scrollPinned,
state._messageUserUnpinned,

View file

@ -0,0 +1,61 @@
"""Regression test for #4702 (sibling #4701).
iOS Safari in PORTRAIT resolves its dynamic toolbar height *after* first paint.
When the toolbar collapses, the #messages scroller grows (clientHeight increases),
which fires a native scroll event with a DECREASED scrollTop even though the user
never scrolled. Before the fix that reflow was misread as an upward scroll
(`movedUp`), which falsely set `_messageUserUnpinned=true; _scrollPinned=false` on
a freshly-opened session — stranding portrait readers at the top, and the late
ResizeObserver settle then self-cancelled because of the false unpin.
These are source-level guards (the runtime behavior is iOS-Safari-specific and not
reproducible in CI), mirroring the static-assertion style of
test_issue1360_streaming_scroll_hardening.py.
"""
import pathlib
REPO = pathlib.Path(__file__).resolve().parent.parent
UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8")
def test_client_height_seeded_with_scrolltop_on_programmatic_writes():
"""Every programmatic seed of `_lastScrollTop` must also seed
`_lastMessageClientHeight`, else the FIRST native toolbar-collapse scroll event
sees a null/stale prior height, `grew` is false, and the false-unpin still
fires (Codex gate finding). The two must always be written together."""
# No bare `_lastScrollTop=el.scrollTop;` without the height co-seed remains.
assert "_lastScrollTop=el.scrollTop;\n" not in UI_JS, (
"A programmatic _lastScrollTop=el.scrollTop write is missing the paired "
"_lastMessageClientHeight=el.clientHeight seed (#4702)."
)
# The paired form is present at the programmatic-write sites.
assert UI_JS.count("_lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight;") >= 4
def test_client_height_growth_guard_declared():
"""The scroller-height tracker must be declared so a toolbar-settle reflow can
be distinguished from a real user scroll."""
assert "let _lastMessageClientHeight=null;" in UI_JS
def test_moved_up_ignores_container_growth():
"""`movedUp` must be gated on `!grew` so a clientHeight increase (toolbar
collapse) can't be misread as an upward user scroll (#4702)."""
assert "const grew=_lastMessageClientHeight!==null&&el.clientHeight>_lastMessageClientHeight+1;" in UI_JS
assert "const movedUp=!grew&&_lastScrollTop!==null&&top<_lastScrollTop-2;" in UI_JS
# The height must be sampled every scroll event (so the next delta compares fresh).
assert "_lastMessageClientHeight=el.clientHeight;" in UI_JS
def test_client_height_tracker_reset_on_session_switch():
"""A genuine session switch must reset the tracker so a stale cross-session
height comparison can't suppress a real first scroll."""
reset_idx = UI_JS.index("function _resetScrollDirectionTracker(){")
body = UI_JS[reset_idx: reset_idx + 600]
assert "_lastMessageClientHeight=null;" in body
def test_explicit_settle_observes_scroller_for_portrait_toolbar():
"""An explicit (open/user) settle must also observe the scroller itself, so a
late portrait toolbar collapse re-anchors the bottom (#4702 defense-in-depth)."""
assert "if(explicit&&observed!==el){ try{ ro.observe(el); }catch(_){ } }" in UI_JS