fix: _loadOlderMessages scrolls to bottom instead of preserving position

When _loadOlderMessages prepends older messages, the viewport snaps
to the bottom instead of staying where the user was.

Two bugs compounding:
1. Wrong scrollable container. Code used `$("msgInner")` for scrollHeight
   and scrollTop, but #msgInner has no overflow-y — it is a flex column.
   The actual scrollable container is #messages (`.messages{overflow-y:auto}`).
   Setting msgInner.scrollTop was silently ignored.
2. renderMessages calls scrollToBottom at the end (ui.js:2552),
   which unconditionally scrolls #messages to the bottom and sets
   _scrollPinned=true. Since bug #1 made the scroll-restore a no-op,
   the page landed at the bottom every time.

Fix:
- Changed scroll restore target from `$("msgInner")` to `$("messages")`.
- Reset _scrollPinned = false after restoring the user position,
  so scrollToBottom does not re-fire on next tick.
This commit is contained in:
fxd-jason
2026-04-28 16:50:00 +08:00
committed by Hermes Agent
parent 24b1e6f3fc
commit f7f8fc6496
2 changed files with 63 additions and 6 deletions
+12 -6
View File
@@ -385,17 +385,23 @@ async function _loadOlderMessages() {
const olderMsgs = (data.session.messages || []).filter(m => m && m.role);
if (!olderMsgs.length) { _messagesTruncated = false; return; }
// Prepend older messages
const inner = $('msgInner');
const prevScrollH = inner ? inner.scrollHeight : 0;
// Use $('messages') — the scrollable container (#msgInner is not scrollable).
const container = $('messages');
const prevScrollH = container ? container.scrollHeight : 0;
S.messages = [...olderMsgs, ...S.messages];
_messagesTruncated = !!data.session._messages_truncated;
_oldestIdx = data.session._messages_offset || 0;
renderMessages();
// Restore scroll position so the user stays at the same message
if (inner) {
const newScrollH = inner.scrollHeight;
inner.scrollTop = newScrollH - prevScrollH;
// Restore scroll position so the user stays at the same message.
// renderMessages() calls scrollToBottom() at the end, so we must
// counter-scroll to where the user was before loading older messages.
if (container) {
const newScrollH = container.scrollHeight;
container.scrollTop = newScrollH - prevScrollH;
}
// renderMessages() called scrollToBottom() which set _scrollPinned=true.
// We just restored the user's scroll position, so mark as not pinned.
_scrollPinned = false;
} catch(e) {
console.warn('_loadOlderMessages failed:', e);
} finally {
+51
View File
@@ -557,3 +557,54 @@ class TestSessionSwitchCancellation:
assert active_check_idx >= 0 and mutation_idx >= 0 and active_check_idx < mutation_idx, (
"Active-session guard must run before S.messages mutation."
)
# ── 6. Scroll position preservation ──────────────────────────────────────────
class TestScrollPositionPreservation:
"""When _loadOlderMessages prepends messages, the user's scroll position
must be preserved — not snapped to the bottom.
The scrollable container is #messages (overflow-y:auto), not #msgInner
(which is a flex column with no overflow). Also, renderMessages() calls
scrollToBottom() at the end, so _scrollPinned must be reset."""
def test_uses_correct_scrollable_container(self):
"""_loadOlderMessages must use $('messages') not $('msgInner')."""
SESSIONS_JS = pathlib.Path(__file__).parent.parent / "static" / "sessions.js"
src = SESSIONS_JS.read_text(encoding="utf-8")
fn_start = src.find("async function _loadOlderMessages")
fn_end = src.find("\n}", fn_start) + 2
fn_body = src[fn_start:fn_end]
assert "$('messages')" in fn_body, (
"_loadOlderMessages should use $('messages') as the scrollable container "
"(#messages has overflow-y:auto). #msgInner has no overflow and is not scrollable."
)
assert "$('msgInner')" not in fn_body, (
"_loadOlderMessages must NOT use $('msgInner') for scroll position — "
"#msgInner is a flex column with no overflow-y."
)
def test_resets_scroll_pinned_after_restore(self):
"""_scrollPinned must be set to false after restoring scroll position."""
SESSIONS_JS = pathlib.Path(__file__).parent.parent / "static" / "sessions.js"
src = SESSIONS_JS.read_text(encoding="utf-8")
fn_start = src.find("async function _loadOlderMessages")
fn_end = src.find("\n}", fn_start) + 2
fn_body = src[fn_start:fn_end]
assert "_scrollPinned = false" in fn_body, (
"renderMessages() calls scrollToBottom() which sets _scrollPinned=true. "
"After restoring the user's scroll position we must set _scrollPinned=false "
"to prevent the next render from snapping back to the bottom."
)
# _scrollPinned must appear after the scrollTop restore
restore_idx = fn_body.find("container.scrollTop = newScrollH - prevScrollH")
pinned_idx = fn_body.find("_scrollPinned = false")
assert restore_idx >= 0 and pinned_idx >= 0 and restore_idx < pinned_idx, (
"_scrollPinned = false must appear AFTER the scrollTop restore."
)