diff --git a/static/ui.js b/static/ui.js index 63d592ee..b34953bc 100644 --- a/static/ui.js +++ b/static/ui.js @@ -983,6 +983,28 @@ function _browserOverflowAnchorActive(el){ if(!el) return false; try{ return getComputedStyle(el).overflowAnchor==='auto'; }catch(_){ return false; } } +// Stable touch-viewport predicate for the issue #5637 stale-anchor hold gate. +// The two stale-anchor refusals below assume the browser's native overflow-anchor +// layer will hold the viewport once the JS restore is refused. That layer is only +// active where `.messages` computes to `overflow-anchor:auto` — touch viewports. +// On hover+fine-pointer desktops the CSS keeps it `none`, so refusing the restore +// leaves NOTHING to hold the reader (the desktop regression). We must NOT decide +// "is this a touch viewport" with `_browserOverflowAnchorActive(#messages)` alone, +// because `_restoreMessageViewportAnchor` temporarily writes an inline +// `overflowAnchor:'none'` on #messages for its own scroll write and only restores +// it on the next frame; when the realign fires every live tick that inline 'none' +// persists across ticks, so a computed-value probe would read 'none' mid-realign +// and wrongly classify a touch device as "desktop", letting the stale realign +// through. A matchMedia('(pointer:coarse)') test reflects the input device and +// cannot be mutated by that inline override, so it stays steady mid-realign; +// desktop (fine pointer) stays false. Fall back to the computed-anchor probe when +// matchMedia is unavailable. +function _isTouchLikeMessageViewport(el){ + try{ + if(typeof matchMedia==='function' && matchMedia('(pointer:coarse)').matches) return true; + }catch(_){} + return _browserOverflowAnchorActive(el); +} function _suppressBrowserOverflowAnchor(container){ if(!container||!container.style) return null; // Only engage when the browser layer is actually active (auto). On desktop @@ -1046,13 +1068,22 @@ function _restoreMessageViewportAnchor(anchor, rawIdxDelta){ // scrollTop non-trivially, refuse it and let the browser overflow-anchor hold. An // actively scrolling reader (recent intent) keeps the legitimate realign; legacy // snapshots without the captured geometry keep prior behavior. + // + // Desktop guard (issue #5637 gate cert): the refusal is only safe where the + // browser's native overflow-anchor layer can actually hold the viewport, i.e. + // touch viewports where `.messages` computes to `overflow-anchor:auto`. On + // hover+fine-pointer desktops `.messages` is `overflow-anchor:none`, so refusing + // the realign would leave NOTHING to hold the reader after above-viewport growth + // — the very yank this fixes on mobile, reintroduced on desktop. Gate the refusal + // on `_isTouchLikeMessageViewport` so desktop keeps its semantic scrollTop realign. const _realignDelta=(rect.top-containerRect.top)-targetTop; const _shAtCap=Number(anchor.scrollHeightAtCapture); if(Number.isFinite(_shAtCap)){ const _grewSinceCapture=(container.scrollHeight-_shAtCap)>4; const _activeIntent=(typeof _recentMessageScrollIntent==='function' && _recentMessageScrollIntent()) || (typeof _recentMessageTouchScrollIntent==='function' && _recentMessageTouchScrollIntent()); - if(_grewSinceCapture&&!_activeIntent&&Math.abs(_realignDelta)>8){ + const _touchHold=(typeof _isTouchLikeMessageViewport==='function' && _isTouchLikeMessageViewport(container)); + if(_touchHold&&_grewSinceCapture&&!_activeIntent&&Math.abs(_realignDelta)>8){ return false; } } @@ -13296,11 +13327,19 @@ function _restoreMessageScrollSnapshotSameFrame(snapshot){ // scrollTop non-trivially, refuse it and let the browser overflow-anchor hold. // Pinned tail-followers (target is bottom-relative, not snapshot.top) are // unaffected; an actively scrolling reader has intent and keeps the restore. + // + // Desktop guard (issue #5637 gate cert): like the realign guard, this refusal + // only holds where the browser's native overflow-anchor layer is active (touch + // viewports, `.messages` computes to `overflow-anchor:auto`). Desktop `.messages` + // is `overflow-anchor:none`, so refusing the absolute fallback write there would + // leave the reader unheld AND latch `_messageUserUnpinned=true`. Gate on + // `_isTouchLikeMessageViewport` so desktop keeps its absolute snapshot.top restore. const _snapSH=Number(snapshot.scrollHeight); const _grewSinceSnap=Number.isFinite(_snapSH)&&_snapSH>0&&(el.scrollHeight-_snapSH)>4; const _fbActiveIntent=(typeof _recentMessageScrollIntent==='function' && _recentMessageScrollIntent()) || (typeof _recentMessageTouchScrollIntent==='function' && _recentMessageTouchScrollIntent()); - if(snapshot.pinned!==true && _grewSinceSnap && !_fbActiveIntent + const _fbTouchHold=(typeof _isTouchLikeMessageViewport==='function' && _isTouchLikeMessageViewport(el)); + if(_fbTouchHold && snapshot.pinned!==true && _grewSinceSnap && !_fbActiveIntent && Math.abs((Math.max(0,Math.min(target,maxTop)))-el.scrollTop)>8){ _lastScrollTop=el.scrollTop;_lastMessageClientHeight=el.clientHeight; _messageUserUnpinned=true; diff --git a/tests/test_issue5637_stale_anchor_guard.py b/tests/test_issue5637_stale_anchor_guard.py index 93cada57..81cbd775 100644 --- a/tests/test_issue5637_stale_anchor_guard.py +++ b/tests/test_issue5637_stale_anchor_guard.py @@ -84,9 +84,11 @@ function extractFunc(name) { # ---- realign guard (_restoreMessageViewportAnchor) ------------------------------- def _realign_harness(*, anchor_extra: str, cur_scroll_height: int, rect_top: int, - top_offset: int, active_intent: bool = False) -> str: + top_offset: int, active_intent: bool = False, + touch_like: bool = True) -> str: js = UI_JS_PATH.read_text(encoding="utf-8") intent_js = "true" if active_intent else "false" + touch_js = "true" if touch_like else "false" return _extract_func_script(js) + f""" let writes = []; let stTop = 90030; @@ -101,6 +103,7 @@ const container = {{ function $(id){{ return id === 'messages' ? container : null; }} function _recentMessageScrollIntent(){{ return {intent_js}; }} function _recentMessageTouchScrollIntent(){{ return {intent_js}; }} +function _isTouchLikeMessageViewport(){{ return {touch_js}; }} let _programmaticScroll = false; let _programmaticScrollSetAt = 0; const performance = {{ now(){{ return 1; }} }}; function _suppressBrowserOverflowAnchor(){{ return null; }} @@ -151,12 +154,29 @@ def test_realign_backward_compatible_without_capture_geometry(): assert m["returned"] is True and m["wrote"] == 1 +def test_realign_allows_on_desktop_no_native_anchor(): + """Desktop regression (#5637 gate cert): the exact stale-anchor case + (content grew, no intent, delta -453) but on a hover+fine-pointer viewport where + `.messages` is `overflow-anchor:none`. There is no native anchoring layer to hold + the reader, so the guard MUST NOT refuse — the semantic scrollTop realign has to + run or the desktop reader is left unheld after above-viewport growth. + Mutation: drop the `_touchHold&&` term from the realign guard and this FAILS + (the write is wrongly refused on desktop).""" + m = json.loads(_run_node(_realign_harness( + anchor_extra="{scrollHeightAtCapture: 90000}", + cur_scroll_height=90453, rect_top=-453, top_offset=0, active_intent=False, + touch_like=False, + ))) + assert m["returned"] is True and m["wrote"] == 1 + + # ---- fallback guard (_restoreMessageScrollSnapshotSameFrame) --------------------- def _fallback_harness(*, snapshot_scroll_height, cur_scroll_height, snapshot_top, - active_intent: bool = False) -> str: + active_intent: bool = False, touch_like: bool = True) -> str: js = UI_JS_PATH.read_text(encoding="utf-8") intent_js = "true" if active_intent else "false" + touch_js = "true" if touch_like else "false" sh = "null" if snapshot_scroll_height is None else str(snapshot_scroll_height) return _extract_func_script(js) + f""" let writes = []; @@ -168,6 +188,7 @@ const el = {{ function $(id){{ return id === 'messages' ? el : null; }} function _recentMessageScrollIntent(){{ return {intent_js}; }} function _recentMessageTouchScrollIntent(){{ return {intent_js}; }} +function _isTouchLikeMessageViewport(){{ return {touch_js}; }} // realign path fails (no anchor) so execution reaches the absolute fallback function _restorePinnedMessageScrollSnapshot(){{ return false; }} function _restoreMessageViewportAnchor(){{ return false; }} @@ -231,3 +252,20 @@ def test_fallback_allows_snapshot_top_with_active_intent(): ))) assert m["writes"] == [89577] + +def test_fallback_allows_snapshot_top_on_desktop_no_native_anchor(): + """Desktop regression (#5637 gate cert): the exact stale-snapshot case (content grew, + reader not pinned, no intent, absolute write would move >8px) but on a + hover+fine-pointer viewport where `.messages` is `overflow-anchor:none`. With no + native anchoring layer to hold the reader, the fallback MUST keep its absolute + snapshot.top restore rather than refuse and latch userUnpinned — otherwise the + desktop reader is left at the wrong absolute position and force-unpinned. + Mutation: drop the `_fbTouchHold&&` term from the fallback guard and this FAILS + (the write is wrongly refused and userUnpinned is latched on desktop).""" + m = json.loads(_run_node(_fallback_harness( + snapshot_scroll_height=90000, cur_scroll_height=90453, snapshot_top=89577, + active_intent=False, touch_like=False, + ))) + assert m["writes"] == [89577] + assert m["messageUserUnpinned"] is False and m["scrollPinned"] is True +