From 4dc87e9069982808541e4c6f1928335e9270b617 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sat, 4 Jul 2026 21:24:17 +0000 Subject: [PATCH] fix(#5544): re-pin auto-scroll only at true bottom after scroll-to-bottom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single scroll-up during streaming set _messageUserUnpinned=true and scrollIfPinned() then permanently stopped auto-follow (only scrollToBottom() cleared it) — a permanent auto-follow lockout even after the user returned to the bottom. scrollIfPinned() now re-pins, but ONLY when the reader has genuinely reached the true-bottom tail (<=80px) AND shows no active scroll intent (wheel/key/touch/non-message), reusing the listener's _nearBottomCount debounce. Proximity alone (the ~250px nearBottom band) must never re-pin — that is the #4295 invariant. Restores the listener's <=80px true-bottom gate too. Co-authored-by: luperrypf --- static/ui.js | 23 ++++++++++++++++++- ...t_issue3250_upward_scroll_intent_window.py | 15 ++++++++++-- tests/test_tars_scroll_reset_regressions.py | 13 ++++++++++- 3 files changed, 47 insertions(+), 4 deletions(-) diff --git a/static/ui.js b/static/ui.js index 3445b90e..a6bbf9fc 100644 --- a/static/ui.js +++ b/static/ui.js @@ -4858,6 +4858,10 @@ if(typeof window!=='undefined'){ }else if(movedDown&&nearBottom){ _nearBottomCount=_nearBottomCount+1; if(_nearBottomCount>=2){ + // Only re-pin when the reader has genuinely reached the true bottom + // tail (<=80px). nearBottom spans a ~250px band, so proximity alone + // must NOT clear the sticky unpin flag (#4295) — a reader scanning the + // last lines mid-stream would otherwise get yanked back to the bottom. if(!_messageUserUnpinned||bottomDistance<=80){ _messageUserUnpinned=false; _scrollPinned=true; @@ -5516,7 +5520,24 @@ function _settleFinalScroll(token){ } function scrollIfPinned(){ if(!_autoScrollFollow) return; - if(_messageUserUnpinned) return; + if(_messageUserUnpinned){ + // Only scrollToBottom() cleared this flag, so one scroll-up permanently + // killed auto-follow. Re-pin ONLY when the reader has genuinely returned to + // the true bottom tail (<=80px), NOT on mere near-bottom proximity — the + // #4295 invariant is that proximity alone (inside the ~250px band) must not + // re-pin, or a reader scanning the last few lines gets yanked to the bottom + // mid-stream. Also bail on ANY recent message-pane scroll intent (wheel, + // key, touch) and non-message intent, so an active scroll-up near the tail + // is never overridden. Uses the same _nearBottomCount debounce as the + // scroll listener (~4859-4866). + if(_recentNonMessageScrollIntent()||_recentMessageScrollIntent()||_recentMessageTouchScrollIntent()||_recentMessageWheelIntent()||_recentMessageKeyScrollIntent()){ _nearBottomCount=0; return; } + if(_messageBottomDistance()>80){ _nearBottomCount=0; return; } + _nearBottomCount=_nearBottomCount+1; + if(_nearBottomCount<2) return; + _nearBottomCount=0; + _messageUserUnpinned=false; + _scrollPinned=true; + } if(!_scrollPinned) return; if(_recentNonMessageScrollIntent()) return; if(_messageBottomDistance()>500) _setMessageScrollToBottom(); diff --git a/tests/test_issue3250_upward_scroll_intent_window.py b/tests/test_issue3250_upward_scroll_intent_window.py index 55363f02..2e671b48 100644 --- a/tests/test_issue3250_upward_scroll_intent_window.py +++ b/tests/test_issue3250_upward_scroll_intent_window.py @@ -32,8 +32,19 @@ def _scroll_listener_block() -> str: def test_scroll_if_pinned_respects_sticky_user_unpin(): fn = UI_JS[UI_JS.index("function scrollIfPinned"): UI_JS.index("function scrollToBottom")] compact = fn.replace(" ", "") - assert "if(_messageUserUnpinned)return" in compact, ( - "scrollIfPinned() must not fight a sticky manual unpin during streaming" + # scrollIfPinned() re-pins auto-follow when the reader returns to the tail + # (#5544), but it must still RESPECT a sticky manual unpin: the re-pin is + # gated behind the true-bottom threshold (<=80px, not mere proximity) and + # bails on any recent scroll intent, so it never fights a reader who has + # scrolled up. Assert the guards rather than the old unconditional bail. + assert "if(_messageUserUnpinned){" in compact, ( + "scrollIfPinned() must handle the sticky-unpin case explicitly" + ) + assert "_messageBottomDistance()>80" in compact, ( + "re-pin must require the true bottom tail (<=80px), not proximity — the #4295 invariant" + ) + assert "_recentMessageWheelIntent()" in compact and "_recentMessageKeyScrollIntent()" in compact, ( + "re-pin must bail on recent wheel/key scroll intent so it can't fight a manual scroll-up" ) diff --git a/tests/test_tars_scroll_reset_regressions.py b/tests/test_tars_scroll_reset_regressions.py index 2557f12c..f4c38872 100644 --- a/tests/test_tars_scroll_reset_regressions.py +++ b/tests/test_tars_scroll_reset_regressions.py @@ -100,7 +100,18 @@ def test_user_scroll_cancels_delayed_bottom_settling(): assert "_cancelBottomSettle();" in record assert "_lastNonMessageScrollIntentMs=performance.now();" in record assert "_scrollPinned=false" in record - assert "if(_messageUserUnpinned) return;" in pinned + # scrollIfPinned() may now re-pin auto-follow (#5544), but ONLY when the + # reader has genuinely returned to the true bottom tail (<=80px) AND no + # recent scroll intent is active — it must never fight a manual scroll-up. + # Assert those guards are present instead of the old "bail unconditionally + # when unpinned" lock. + _pinned_compact = pinned.replace(" ", "") + assert "_messageBottomDistance()>80" in _pinned_compact, ( + "scrollIfPinned() re-pin must require the true bottom tail (<=80px), not mere proximity" + ) + assert "_recentMessageWheelIntent()" in _pinned_compact and "_recentMessageKeyScrollIntent()" in _pinned_compact, ( + "scrollIfPinned() re-pin must bail on recent wheel/key scroll intent so it can't fight a reader" + ) assert "_messageUserUnpinned" in final and "return" in final assert "_recentMessageUpwardIntent()" not in pinned