#5514 (deterministic): with the chat pinned to the bottom, growing the composer past one row (multi-line typing, Shift+Enter, a multi-line / WisprFlow paste) shrinks the flex:1 .messages viewport by the same delta. scrollTop is unchanged but max-scrollTop rose by delta, so the reader is stranded delta-px above the bottom — the transcript 'scrolls up one row per composer row.' autoResize() resized the textarea and called updateSendBtn() but never re-pinned the transcript. Reproduced live: composer 176px -> viewport shrank 745->613 -> transcript stranded 132px above bottom while _scrollPinned stayed true. #5515 (intermittent 'random scroll up'): the same viewport-shrink from composer growth via paths that don't route through input->autoResize (paste, draft restore, programmatic value set, reflow) reads to the user as a random upward jump. Covered by a ResizeObserver seam (below). Fix: new _repinMessagesAfterComposerResize() (static/ui.js) re-pins to the bottom ONLY when the reader is genuinely still pinned (honors _messageUserUnpinned / _scrollPinned — never yanks a reader who scrolled away; verified live). Called from: (a) autoResize() after the height write, and (b) a ResizeObserver on the #msg textarea that re-pins on GROW only, catching every composer height-change path (not just the typed-input seam). Verified live on a 40-message seeded session: pinned reader stays at bottomDist=0 after a 4-line composer grow; an unpinned (scrolled-up) reader is NOT moved. Tests: tests/test_issue5514_composer_grow_scroll_pin.py (3 static wiring + 3 node-vm behavioral). 524 scroll/composer regression tests across 39 files still green. NOTE on #5515: this resolves the composer-growth cause (which the reporter's WisprFlow-paste symptom points at). If a purely non-composer intermittent jump persists, the remaining suspect is touch-device overflow-anchor:auto on content- above height change — that path has no repro yet and is tracked as needinfo on #5515.
This commit is contained in:
1 parent
43f989eaa5
commit
dde008748e
4 files changed
+182
No files matched your search
@@ -1916,6 +1916,30 @@ $('msg').addEventListener('input',()=>{
|
||||
hideCmdDropdown();
|
||||
}
|
||||
});
|
||||
// #5514/#5515: re-pin the transcript on ANY composer height change, not only the
|
||||
// ones that route through the input->autoResize path. A multi-line paste
|
||||
// (WisprFlow), a draft restore, a programmatic value set, or a font/reflow can
|
||||
// all grow the composer and shrink the flex:1 transcript viewport, stranding a
|
||||
// pinned reader above the bottom (reads as a "random" upward jump — #5515). A
|
||||
// ResizeObserver on the textarea catches every case at one seam. The re-pin
|
||||
// itself is guarded (only fires when genuinely pinned), so this never fights a
|
||||
// reader who scrolled away. First callback fires on observe (initial size) — the
|
||||
// guard makes that a cheap no-op.
|
||||
(()=>{
|
||||
const _c=$('msg');
|
||||
if(!_c || typeof ResizeObserver!=='function' || typeof _repinMessagesAfterComposerResize!=='function') return;
|
||||
let _lastComposerH=_c.offsetHeight;
|
||||
const _ro=new ResizeObserver(()=>{
|
||||
const h=_c.offsetHeight;
|
||||
if(h===_lastComposerH) return; // width-only / no-op resize: ignore
|
||||
const grew=h>_lastComposerH;
|
||||
_lastComposerH=h;
|
||||
// Only a GROW shrinks the transcript viewport; a shrink enlarges it and can't
|
||||
// strand a pinned reader. Re-pin on grow.
|
||||
if(grew) _repinMessagesAfterComposerResize();
|
||||
});
|
||||
try{ _ro.observe(_c); }catch(_){ }
|
||||
})();
|
||||
// Track IME composition for East Asian input. Safari fires the committing
|
||||
// keydown AFTER compositionend with isComposing=false, so we also keep a
|
||||
// manual flag and reset it on the next tick to swallow that trailing Enter.
|
||||
|
||||
@@ -6184,6 +6184,10 @@ function autoResize(){
|
||||
el.style.height='auto';
|
||||
el.style.height=Math.min(el.scrollHeight,200)+'px';
|
||||
updateSendBtn();
|
||||
// #5514/#5515: growing the composer shrinks the flex:1 transcript viewport, so
|
||||
// a reader pinned to the bottom would be stranded above it ("scrolls up 1 row
|
||||
// per composer row"). Re-pin the transcript when it's genuinely still pinned.
|
||||
if(typeof _repinMessagesAfterComposerResize==='function') _repinMessagesAfterComposerResize();
|
||||
}
|
||||
function scheduleComposerAutoResize(){
|
||||
if(typeof requestAnimationFrame!=='function'){autoResize();return;}
|
||||
|
||||
@@ -5380,6 +5380,26 @@ function _messageBottomDistance(){
|
||||
if(!el) return 0;
|
||||
return el.scrollHeight-el.scrollTop-el.clientHeight;
|
||||
}
|
||||
// #5514/#5515: when the composer grows (typing multiple rows, Shift+Enter, a
|
||||
// multi-line paste / WisprFlow), the flex:1 `.messages` viewport shrinks by the
|
||||
// same delta. A reader pinned to the bottom is then stranded Δpx above it — the
|
||||
// transcript appears to "scroll up" one row per composer row, and (the #5515
|
||||
// half) it reads as a random upward jump during normal use. autoResize() only
|
||||
// resized the textarea; nothing re-pinned the transcript. Re-pin the bottom, but
|
||||
// ONLY when the reader is genuinely still pinned (sticky-unpin model: honor
|
||||
// _messageUserUnpinned so we never yank a reader who scrolled away, and never
|
||||
// fight a stream that already unpinned). Cheap no-op when not pinned.
|
||||
function _repinMessagesAfterComposerResize(){
|
||||
if(_messageUserUnpinned || !_scrollPinned) return;
|
||||
const el=$('messages');
|
||||
if(!el) return;
|
||||
// Already at/very near the bottom? nothing to do (avoids needless writes while
|
||||
// idle-reading a short conversation that isn't scrollable).
|
||||
if(el.scrollHeight-el.scrollTop-el.clientHeight<=1) return;
|
||||
if(typeof _setMessageScrollToBottom==='function') _setMessageScrollToBottom();
|
||||
else { el.scrollTop=el.scrollHeight; }
|
||||
}
|
||||
if(typeof window!=='undefined') window._repinMessagesAfterComposerResize=_repinMessagesAfterComposerResize;
|
||||
function _shouldFollowMessagesOnDomReplace(){
|
||||
// Final stream settlement replaces the live DOM with persisted messages. Keep
|
||||
// following only for users who are still pinned or effectively at the tail.
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
"""Regression coverage for #5514 + #5515 — transcript scrolls up when the composer
|
||||
grows while the reader is pinned to the bottom.
|
||||
|
||||
#5514 (deterministic): with the chat pinned to the bottom, growing the composer
|
||||
(multi-line typing, Shift+Enter, a multi-line / WisprFlow paste) shrinks the
|
||||
`flex:1` `.messages` viewport by the same delta, stranding the reader Δpx above
|
||||
the bottom — the transcript "scrolls up one row per composer row." autoResize()
|
||||
resized the textarea but never re-pinned the transcript.
|
||||
|
||||
#5515 (intermittent "random scroll up"): the same class of viewport-shrink from
|
||||
composer growth via paths that don't route through the input->autoResize seam
|
||||
(paste, draft restore, programmatic value set, reflow) reads to the user as a
|
||||
random upward jump.
|
||||
|
||||
Fix: `_repinMessagesAfterComposerResize()` (static/ui.js) re-pins the transcript
|
||||
to the bottom ONLY when the reader is genuinely still pinned (honors
|
||||
`_messageUserUnpinned` / `_scrollPinned` so a reader who scrolled away is never
|
||||
yanked back). It is called (a) from autoResize() and (b) from a ResizeObserver on
|
||||
the composer textarea that catches every height-change path.
|
||||
|
||||
This module verifies BOTH the static wiring and the helper's guard logic via a
|
||||
node `vm` sandbox that reproduces the pinned-vs-unpinned viewport-shrink scenario.
|
||||
"""
|
||||
import json
|
||||
import shutil
|
||||
import subprocess
|
||||
import textwrap
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
ROOT = Path(__file__).parents[1]
|
||||
UI_JS = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
|
||||
MESSAGES_JS = (ROOT / "static" / "messages.js").read_text(encoding="utf-8")
|
||||
BOOT_JS = (ROOT / "static" / "boot.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Static wiring
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _helper_body() -> str:
|
||||
start = UI_JS.find("function _repinMessagesAfterComposerResize(")
|
||||
assert start != -1, "the _repinMessagesAfterComposerResize helper must exist in ui.js"
|
||||
end = UI_JS.find("\nif(typeof window!=='undefined') window._repinMessagesAfterComposerResize", start)
|
||||
assert end != -1, "helper must be immediately followed by its window export"
|
||||
return UI_JS[start:end]
|
||||
|
||||
|
||||
def test_helper_exists_and_is_pin_guarded():
|
||||
body = _helper_body()
|
||||
# Never re-pin a reader who scrolled away (sticky-unpin model).
|
||||
assert "if(_messageUserUnpinned || !_scrollPinned) return;" in body
|
||||
# Uses the canonical bottom-pin primitive.
|
||||
assert "_setMessageScrollToBottom" in body
|
||||
# Cheap no-op when already at bottom.
|
||||
assert "<=1) return;" in body
|
||||
|
||||
|
||||
def test_autoresize_calls_the_repin():
|
||||
start = MESSAGES_JS.find("function autoResize(")
|
||||
assert start != -1
|
||||
end = MESSAGES_JS.find("function scheduleComposerAutoResize(", start)
|
||||
assert end > start
|
||||
body = MESSAGES_JS[start:end]
|
||||
# The height write must still be there...
|
||||
assert "el.style.height=Math.min(el.scrollHeight,200)+'px'" in body
|
||||
# ...and the re-pin must be called after it.
|
||||
assert "_repinMessagesAfterComposerResize()" in body
|
||||
|
||||
|
||||
def test_resize_observer_installed_on_composer():
|
||||
# A ResizeObserver on #msg re-pins on grow (catches non-autoResize paths).
|
||||
assert "new ResizeObserver(" in BOOT_JS
|
||||
assert "_repinMessagesAfterComposerResize" in BOOT_JS
|
||||
# Only re-pin on GROW (a shrink enlarges the viewport, can't strand a reader).
|
||||
assert "if(grew) _repinMessagesAfterComposerResize();" in BOOT_JS
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Behavioral (node vm) — the pin guard under a viewport shrink
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def _run(scenario):
|
||||
node = shutil.which("node")
|
||||
if not node: # pragma: no cover
|
||||
pytest.skip("node not available")
|
||||
body = _helper_body()
|
||||
harness = textwrap.dedent(
|
||||
"""
|
||||
// Minimal DOM/scroll-state stub reproducing the composer-grow viewport shrink.
|
||||
let _messageUserUnpinned = %(unpinned)s;
|
||||
let _scrollPinned = %(pinned)s;
|
||||
// messages pane: scrollHeight fixed; clientHeight shrinks when composer grows.
|
||||
const el = { scrollHeight: 8768, clientHeight: 745, scrollTop: %(scrolltop)s };
|
||||
const $ = (id) => (id === 'messages' ? el : null);
|
||||
function _setMessageScrollToBottom(){ el.scrollTop = el.scrollHeight - el.clientHeight; el._pinnedCalled = true; }
|
||||
|
||||
%(helper)s
|
||||
|
||||
// Simulate the composer growing by 132px: the viewport shrinks.
|
||||
el.clientHeight -= 132;
|
||||
_repinMessagesAfterComposerResize();
|
||||
const bottomDist = el.scrollHeight - el.scrollTop - el.clientHeight;
|
||||
console.log(JSON.stringify({ scrollTop: el.scrollTop, bottomDist, pinnedCalled: !!el._pinnedCalled }));
|
||||
"""
|
||||
) % {**scenario, "helper": body}
|
||||
proc = subprocess.run([node, "-e", harness], capture_output=True, text=True, timeout=30)
|
||||
assert proc.returncode == 0, f"node harness failed: {proc.stderr}"
|
||||
return json.loads(proc.stdout.strip())
|
||||
|
||||
|
||||
def test_pinned_reader_is_repinned_after_composer_grow():
|
||||
# Reader pinned to bottom (scrollTop at old max = 8768-745 = 8023), composer
|
||||
# grows -> viewport shrinks -> must re-pin so bottomDist returns to 0.
|
||||
out = _run({"unpinned": "false", "pinned": "true", "scrolltop": 8023})
|
||||
assert out["pinnedCalled"] is True
|
||||
assert out["bottomDist"] == 0, f"pinned reader must be re-pinned to the bottom; got {out}"
|
||||
|
||||
|
||||
def test_unpinned_reader_is_not_yanked_down():
|
||||
# Reader scrolled up (unpinned). Composer grow must NOT re-pin — no yank.
|
||||
out = _run({"unpinned": "true", "pinned": "false", "scrolltop": 3000})
|
||||
assert out["pinnedCalled"] is False
|
||||
assert out["scrollTop"] == 3000, f"unpinned reader must not be moved; got {out}"
|
||||
|
||||
|
||||
def test_pinned_but_already_at_bottom_is_a_noop():
|
||||
# scrollTop already exactly at the (post-shrink) bottom -> nothing to do.
|
||||
# scrollHeight 8768, clientHeight after shrink 613 -> bottom scrollTop = 8155.
|
||||
out = _run({"unpinned": "false", "pinned": "true", "scrolltop": 8155})
|
||||
# bottomDist is already <=1 at call time (8768-8155-613 = 0), so no re-pin call.
|
||||
assert out["pinnedCalled"] is False
|
||||
assert out["bottomDist"] == 0
|
||||
Reference in new issue
Block a user