diff --git a/static/ui.js b/static/ui.js index a1d67d39..1405dd78 100644 --- a/static/ui.js +++ b/static/ui.js @@ -839,11 +839,17 @@ function _captureMessageViewportAnchor(){ const rect=row.getBoundingClientRect(); if(rect.bottom>containerRect.top+1){ const sessionIdx=Number(row&&row.dataset&&row.dataset.sessionMsgIdx); + // Record the current top-spacer (virtual topPad) height so the compensation + // path can fall back to a topPad-delta shift when the anchor row itself is + // recycled out of the render window after a measurement-driven re-render. + const spacer=container.querySelector('[data-virtual-spacer="before"]'); + const topPadBefore=spacer?parseFloat(spacer.style.height||'0')||0:0; return { rawIdx, sessionIdx:Number.isFinite(sessionIdx)?sessionIdx:_messageSessionIndexForRawIdx(rawIdx), key:row&&row.dataset?String(row.dataset.messageAnchorKey||''):'', topOffset:rect.top-containerRect.top, + topPadBefore, }; } } @@ -994,8 +1000,43 @@ function _compensateScrollForMeasurementDelta(renderFn){ const spacer=container.querySelector('[data-virtual-spacer="before"]'); if(!spacer||parseFloat(spacer.style.height||'0')<=0) return; } - const row=container.querySelector(`[data-msg-idx="${anchorBefore.rawIdx}"]`); - if(!row) return; + // Re-find the anchor row after the measurement-driven re-render. The primary + // lookup is by rawIdx (the DOM index), but on a big virtualized session a large + // scroll delta can RECYCLE the old anchor row out of the render window entirely + // (verified via real-device telemetry: DOM collapsed to 1 row, scrollHeight + // lurched by tens of thousands of px). The old code did `if(!row) return` here, + // abandoning compensation → the full estimated↔measured height lurch hit + // scrollTop uncompensated and threw the viewport to the top (the recurring + // mobile scroll jump-back). Fall back to the stable sessionIdx anchor (captured in + // _captureMessageViewportAnchor) before giving up, mirroring the "recover via + // sessionIdx when the primary anchor key is gone" approach used elsewhere but for + // the virtualization-measurement compensation path. + let row=container.querySelector(`[data-msg-idx="${anchorBefore.rawIdx}"]`); + if(!row&&Number.isFinite(Number(anchorBefore.sessionIdx))){ + row=container.querySelector(`[data-session-msg-idx="${anchorBefore.sessionIdx}"]`); + } + if(!row){ + // Anchor row is no longer rendered (recycled out of the virtual window). We + // cannot measure its live offset, but we CAN keep the viewport visually + // stable by compensating for the top-spacer (topPad) height change: the + // whole reason scrollHeight lurched is that the estimated topPad was replaced + // by a measured one. Shift scrollTop by that same delta so content under the + // viewport does not appear to jump. Without this the browser lands at an + // uncompensated absolute scrollTop against a wildly different scrollHeight. + const spacerAfter=container.querySelector('[data-virtual-spacer="before"]'); + const topPadAfter=spacerAfter?parseFloat(spacerAfter.style.height||'0')||0:0; + const topPadBefore=Number(anchorBefore.topPadBefore); + if(Number.isFinite(topPadBefore)){ + const padDelta=topPadAfter-topPadBefore; + if(Math.abs(padDelta)>=2){ + _programmaticScroll=true;_programmaticScrollSetAt=performance.now(); + container.scrollTop=Math.max(0,scrollTopBefore+padDelta); + _lastScrollTop=container.scrollTop; + _deferClearProgrammaticScroll(); + } + } + return; + } const containerRect=container.getBoundingClientRect(); const rowRect=row.getBoundingClientRect(); const actualOffset=rowRect.top-containerRect.top; diff --git a/tests/test_issue4346_vscroll_recycled_anchor_jumpback.py b/tests/test_issue4346_vscroll_recycled_anchor_jumpback.py new file mode 100644 index 00000000..92ea57a0 --- /dev/null +++ b/tests/test_issue4346_vscroll_recycled_anchor_jumpback.py @@ -0,0 +1,211 @@ +"""Regression tests for the virtualization scroll-compensation jump-back. + +Root cause: `_compensateScrollForMeasurementDelta` re-found its viewport anchor +ONLY by rawIdx and bailed with a bare `if(!row) return` when that row was +recycled out of the render window. On big sessions containing multi-thousand-px +turns, a large scroll delta recycles the anchor row, so the estimated->measured +topPad swap (a scrollHeight change of tens of thousands of px) hit scrollTop +uncompensated and threw the viewport to the top -- the recurring mobile scroll +jump-back. + +Fix: when the rawIdx row is gone, fall back to (a) the stable sessionIdx anchor, +and (b) if that is also unrendered, compensate by the top-spacer (topPad) height +delta captured before the re-render. + +Every behavioral test below is designed to FAIL on the known-buggy version +(bare `if(!row) return`) and PASS only on the fixed version. +""" +import json +import pathlib +import shutil +import subprocess +import tempfile + +import pytest + +ROOT = pathlib.Path(__file__).parent.parent +UI_JS_PATH = ROOT / "static" / "ui.js" +NODE = shutil.which("node") + +pytestmark = pytest.mark.skipif(NODE is None, reason="node not on PATH") + + +def _run_node(source: str) -> str: + with tempfile.NamedTemporaryFile( + "w", suffix=".cjs", encoding="utf-8", dir=ROOT, delete=False + ) as script: + script.write(source) + script_path = pathlib.Path(script.name) + try: + result = subprocess.run( + [NODE, str(script_path)], + cwd=str(ROOT), + capture_output=True, + text=True, + timeout=30, + ) + finally: + script_path.unlink(missing_ok=True) + if result.returncode != 0: + raise RuntimeError(result.stderr) + return result.stdout.strip() + + +def _extract_func_script(js: str) -> str: + return f""" +const src = {js!r}; +function extractFunc(name) {{ + const re = new RegExp('function\\\\s+' + name + '\\\\s*\\\\('); + const start = src.search(re); + if (start < 0) throw new Error(name + ' not found'); + let i = src.indexOf('{{', start); + let depth = 1; i++; + while (depth > 0 && i < src.length) {{ + if (src[i] === '{{') depth++; + else if (src[i] === '}}') depth--; + i++; + }} + return src.slice(start, i); +}}""" + + +def test_compensate_recovers_via_session_idx_when_rawidx_row_recycled(): + """When the rawIdx anchor row is recycled out of the render window but the + SAME row is still locatable by its stable data-session-msg-idx, the + compensation must recover via the sessionIdx lookup and shift scrollTop by + the measured delta -- NOT bail out (the buggy `if(!row) return`).""" + js = UI_JS_PATH.read_text(encoding="utf-8") + source = _extract_func_script(js) + """ +let scrollTopValue = 20000; +let scrollHistory = []; +const container = { + get scrollTop(){ return scrollTopValue; }, + set scrollTop(v){ scrollHistory.push(v); scrollTopValue = v; }, + getBoundingClientRect(){ return {top: 0, bottom: 600}; }, + classList: { add(){}, remove(){} }, + querySelector(selector){ + // rawIdx row is RECYCLED OUT (gone); the sessionIdx row IS still rendered. + if(selector.indexOf('[data-msg-idx="42"]') !== -1) return null; + if(selector.indexOf('[data-session-msg-idx="42"]') !== -1){ + return { getBoundingClientRect(){ return {top: 150}; } }; + } + if(selector.indexOf('virtual-spacer') !== -1) return {style:{height:'1000px'}}; + return null; + }, +}; +function $(id){ return id === 'messages' ? container : null; } +function _captureMessageViewportAnchor(){ + return {rawIdx: 42, sessionIdx: 42, topOffset: 100, topPadBefore: 1000}; +} +let _programmaticScroll = false; +let _programmaticScrollSetAt = 0; +let _programmaticScrollResetTimer = 0; +let _lastScrollTop = 0; +const performance = { now(){ return 1000; } }; +function clearTimeout(){} +function setTimeout(cb){ cb(); return 1; } +function _deferClearProgrammaticScroll(){} +function requestAnimationFrame(cb){ cb(); } +eval(extractFunc('_compensateScrollForMeasurementDelta')); +_compensateScrollForMeasurementDelta(()=>{}); +console.log(JSON.stringify({scrollHistory})); +""" + metrics = json.loads(_run_node(source)) + # sessionIdx row found at top=150; anchor.topOffset=100 -> delta = 150-100 = 50 + # scrollTop shifts 20000 + 50 = 20050 (recovered, NOT abandoned). + assert metrics["scrollHistory"] == [20050], ( + "compensation must recover via data-session-msg-idx when the rawIdx row " + "is recycled out (buggy code bailed with `if(!row) return`, leaving " + "scrollHistory empty)" + ) + + +def test_compensate_uses_toppad_delta_when_anchor_row_fully_recycled(): + """When BOTH the rawIdx and sessionIdx rows are recycled out (large scroll + delta on a big virtualized session), the compensation must fall back to the + top-spacer (topPad) height delta so the huge estimated->measured scrollHeight + lurch does not throw the viewport to the top. Buggy code abandoned entirely + and left scrollTop uncompensated.""" + js = UI_JS_PATH.read_text(encoding="utf-8") + source = _extract_func_script(js) + """ +let scrollTopValue = 20500; +let scrollHistory = []; +// topPad shrank from 20000 (estimated) to 1000 (measured): a -19000px lurch. +const container = { + get scrollTop(){ return scrollTopValue; }, + set scrollTop(v){ scrollHistory.push(v); scrollTopValue = v; }, + getBoundingClientRect(){ return {top: 0, bottom: 600}; }, + classList: { add(){}, remove(){} }, + querySelector(selector){ + // both anchor lookups miss (row recycled out entirely) + if(selector.indexOf('data-msg-idx') !== -1) return null; + if(selector.indexOf('data-session-msg-idx') !== -1) return null; + // the top spacer now measures 1000px (was 20000 at capture time) + if(selector.indexOf('virtual-spacer') !== -1) return {style:{height:'1000px'}}; + return null; + }, +}; +function $(id){ return id === 'messages' ? container : null; } +function _captureMessageViewportAnchor(){ + return {rawIdx: 42, sessionIdx: 42, topOffset: 100, topPadBefore: 20000}; +} +let _programmaticScroll = false; +let _programmaticScrollSetAt = 0; +let _lastScrollTop = 0; +const performance = { now(){ return 1000; } }; +function clearTimeout(){} +function setTimeout(cb){ cb(); return 1; } +function _deferClearProgrammaticScroll(){} +function requestAnimationFrame(cb){ cb(); } +eval(extractFunc('_compensateScrollForMeasurementDelta')); +_compensateScrollForMeasurementDelta(()=>{}); +console.log(JSON.stringify({scrollHistory})); +""" + metrics = json.loads(_run_node(source)) + # padDelta = topPadAfter(1000) - topPadBefore(20000) = -19000 + # scrollTop = max(0, 20500 + (-19000)) = 1500 -- compensated for the lurch. + assert metrics["scrollHistory"] == [1500], ( + "compensation must shift scrollTop by the topPad delta (-19000) when the " + "anchor row is fully recycled; buggy code left scrollTop uncompensated " + "(empty scrollHistory) and the viewport was thrown to the top" + ) + + +def test_compensate_still_bails_cleanly_when_no_toppad_before_captured(): + """Defensive: if the captured anchor has no topPadBefore (older shape) AND + the row is gone, the fallback must NOT throw and must NOT mutate scrollTop.""" + js = UI_JS_PATH.read_text(encoding="utf-8") + source = _extract_func_script(js) + """ +let scrollTopValue = 500; +let scrollTopMutated = false; +const container = { + get scrollTop(){ return scrollTopValue; }, + set scrollTop(v){ scrollTopMutated = true; scrollTopValue = v; }, + getBoundingClientRect(){ return {top: 0, bottom: 600}; }, + classList: { add(){}, remove(){} }, + querySelector(selector){ + if(selector.indexOf('virtual-spacer') !== -1) return {style:{height:'1000px'}}; + return null; // no anchor row by any lookup + }, +}; +function $(id){ return id === 'messages' ? container : null; } +function _captureMessageViewportAnchor(){ + // no topPadBefore field (older capture shape) -> fallback must no-op safely + return {rawIdx: 42, sessionIdx: 42, topOffset: 100}; +} +let _programmaticScroll = false; +let _lastScrollTop = 0; +const performance = { now(){ return 1000; } }; +function clearTimeout(){} +function setTimeout(cb){ cb(); return 1; } +function _deferClearProgrammaticScroll(){} +function requestAnimationFrame(cb){ cb(); } +eval(extractFunc('_compensateScrollForMeasurementDelta')); +_compensateScrollForMeasurementDelta(()=>{}); +console.log(JSON.stringify({scrollTopMutated})); +""" + metrics = json.loads(_run_node(source)) + assert metrics["scrollTopMutated"] is False, ( + "with no topPadBefore captured and no anchor row, the fallback must " + "leave scrollTop untouched (NaN-guarded), not throw or write garbage" + )