test(mobile): pin the keyboard scroll-loop guard; honest cross-platform notes

Follow-up to 90b73b5b from an adversarial review.

Defects:
- The loop-prevention WIRING (ViewPlugin subscribe → gate → dispatch) was
  untested — only the pure shouldReassertCaret helper was. Reverting the gate
  to "always dispatch" (the looping behavior) passed every test. Add a jsdom
  test that builds a focused EditorView with the real extension and asserts a
  keyboard-open resize re-asserts once while the scroll it triggers does NOT
  (the loop is broken). Mutation-checked: forcing always-dispatch turns it red.
- Header comment overstated "every mobile browser native-scrolls the editable."
  Name the layout-anchored Android case (Edge/Samsung) as the unverified one,
  with the regression risk and the correct (non-sentinel) fix path if it ever
  matters. Document the accepted settle-frame trade-off (a trailing pure-scroll
  nudge is ignored by design — don't re-add scroll-reassert) and the innerHeight
  fragility (shielded by the toolbar arm).

Cleanup:
- Extract getVisualViewportHeight() for the duplicated
  Math.round(window.visualViewport?.height ?? 0), co-located with the other
  viewport readers.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: Stvad

src/utils/keyboardAwareScroll.ts

@@ -4,6 +4,7 @@ import { completionStatus } from '@codemirror/autocomplete'
 import {
   getEditingToolbarHeight,
   getKeyboardOverlap,
+  getVisualViewportHeight,
   subscribeKeyboardViewport,
 } from './keyboardViewport.js'
 
@@ -29,7 +30,16 @@ const MIN_KEYBOARD_OVERLAP = 60
  *  2. There must actually be something to clear — a real keyboard
  *     (overlap ≥ MIN) or a mounted toolbar. The toolbar height is only nonzero
  *     while the toolbar is rendered, so it's a reliable signal even on Chrome
- *     Android's resizes-content path where the keyboard overlap stays 0. */
+ *     Android's resizes-content path where the keyboard overlap stays 0.
+ *     (`keyboardOverlap` is derived from `window.innerHeight`, which iOS can
+ *     under-report in Stage Manager + scroll — but the toolbar arm carries the
+ *     gate whenever editing on mobile, so a corrupt overlap can't disable it.)
+ *
+ *  Accepted edge: the keyboard opening always fires a resize (geometry change),
+ *  so the caret is lifted; but if the browser's FINAL settle nudge arrives as a
+ *  trailing pure `scroll` (height already stable), it's ignored — the preceding
+ *  resize already cleared the caret, so this is a deliberate trade, not a miss.
+ *  Do NOT "fix" it by re-asserting on scroll: that resurrects the 60fps loop. */
 export const shouldReassertCaret = (
   prev: {vvHeight: number; toolbarHeight: number},
   cur: {vvHeight: number; toolbarHeight: number; keyboardOverlap: number},
@@ -44,16 +54,24 @@ export const shouldReassertCaret = (
  *  know about while editing on a touch device.
  *
  *  Division of labor with the browser:
- *  - The on-screen KEYBOARD is the browser's job. Every mobile browser
- *    natively scrolls a focused editable above the keyboard (iOS pans the
- *    visual viewport; Chrome/Android shrinks the layout viewport). We do NOT
- *    re-do that. Earlier this extension fed the keyboard *overlap* into
+ *  - The on-screen KEYBOARD is the browser's job. Mobile browsers natively
+ *    scroll a focused editable above the keyboard (iOS pans the visual
+ *    viewport; Chrome/Android resizes-content shrinks the layout viewport). We
+ *    do NOT re-do that. Earlier this extension fed the keyboard *overlap* into
  *    CodeMirror's scrollMargins, but on iOS that overlap is measured against
  *    a full-height layout viewport while the visible region is the panned
  *    visual viewport — and scrolling the document to satisfy it itself moves
  *    the pan, so CM chased a moving target and the block jittered / scrolled
  *    out of view on every keystroke. Letting the browser own the keyboard
  *    avoids that coordinate fight entirely.
+ *    CAVEAT: verified on iOS (device) and desktop (inert). On *layout-anchored*
+ *    Android browsers (Edge / Samsung Internet) the layout viewport stays full
+ *    and the keyboard overlays — if such a browser also does NOT native-scroll
+ *    the focused editable, the caret could sit behind the keyboard (the bug
+ *    this code originally fixed for those browsers by reserving the overlap).
+ *    Unverified there, and not our fleet; if it ever regresses, reintroduce the
+ *    overlap margin gated to a RELIABLE "does the visual viewport pan?" probe
+ *    (NOT the MobileKeyboardToolbar sentinel, which misdetects on iOS).
  *  - The editing TOOLBAR is OUR job. The mobile keyboard toolbar floats
  *    (`position: fixed`) just above the keyboard, so the browser's native
  *    scroll — which only clears the keyboard — leaves the caret behind it.
@@ -133,12 +151,12 @@ export const keyboardAwareScroll = (): Extension => [
         // Seed the geometry baseline with the *current* values so a stream of
         // pure-scroll notifications (no keyboard/toolbar change) can't trigger
         // a spurious first re-assert.
-        this.lastVvHeight = Math.round(window.visualViewport?.height ?? 0)
+        this.lastVvHeight = getVisualViewportHeight()
         this.lastToolbarHeight = getEditingToolbarHeight()
         this.unsubscribe = subscribeKeyboardViewport(() => {
           if (!view.hasFocus) return
           const cur = {
-            vvHeight: Math.round(window.visualViewport?.height ?? 0),
+            vvHeight: getVisualViewportHeight(),
             toolbarHeight: getEditingToolbarHeight(),
             keyboardOverlap: getKeyboardOverlap(),
           }

src/utils/keyboardViewport.ts

@@ -36,6 +36,12 @@ const computeOverlap = (): number => {
  *  NOT as a scroll amount: the keyboard itself is the browser's job. */
 export const getKeyboardOverlap = (): number => computeOverlap()
 
+/** The visual viewport's current height in CSS px (0 when unavailable). The
+ *  geometry signal keyboardAwareScroll compares to tell a keyboard open/close
+ *  (height changed) apart from a pure scroll (offset moved, height same). */
+export const getVisualViewportHeight = (): number =>
+  typeof window === 'undefined' ? 0 : Math.round(window.visualViewport?.height ?? 0)
+
 const listeners = new CallbackSet('keyboard-viewport')
 let attached = false
 

src/utils/test/keyboardAwareScroll.test.ts

@@ -1,5 +1,9 @@
-import { describe, expect, it } from 'vitest'
-import { shouldReassertCaret } from '@/utils/keyboardAwareScroll'
+// @vitest-environment jsdom
+import { afterEach, describe, expect, it, vi } from 'vitest'
+import { EditorState } from '@codemirror/state'
+import { EditorView } from '@codemirror/view'
+import { keyboardAwareScroll, shouldReassertCaret } from '@/utils/keyboardAwareScroll'
+import { setEditingToolbarHeight } from '@/utils/keyboardViewport'
 
 // The on-device bug this guards: on iOS, re-asserting the caret on a pure
 // scroll fed a 60fps loop, because scrolling the caret into view itself moves
@@ -47,3 +51,76 @@ describe('shouldReassertCaret', () => {
     ).toBe(false)
   })
 })
+
+/** Minimal visualViewport stand-in whose listeners we can fire by hand. */
+const installViewport = (height: number) => {
+  const listeners = new Map<string, Set<() => void>>()
+  const vv = {
+    height,
+    offsetTop: 0,
+    addEventListener: (type: string, cb: () => void) => {
+      if (!listeners.has(type)) listeners.set(type, new Set())
+      listeners.get(type)!.add(cb)
+    },
+    removeEventListener: (type: string, cb: () => void) => listeners.get(type)?.delete(cb),
+    emit: (type: string) => listeners.get(type)?.forEach(cb => cb()),
+  }
+  vi.stubGlobal('visualViewport', vv)
+  return vv
+}
+
+// Guards the actual ViewPlugin WIRING (subscribe → gate → dispatch), not just
+// the pure helper above. The original bug lived here: re-asserting on every
+// viewport notification looped at 60fps. The decisive property is that a pure
+// scroll after a re-assert produces NO further dispatch — reverting the gate to
+// "always dispatch" must turn this red.
+describe('keyboardAwareScroll ViewPlugin', () => {
+  let view: EditorView | null = null
+
+  afterEach(() => {
+    view?.destroy()
+    view = null
+    vi.unstubAllGlobals()
+    setEditingToolbarHeight(0)
+  })
+
+  const mountFocused = (vvHeight: number) => {
+    vi.stubGlobal('innerHeight', 800)
+    const vv = installViewport(vvHeight)
+    const parent = document.createElement('div')
+    document.body.appendChild(parent)
+    view = new EditorView({
+      state: EditorState.create({doc: 'hello', extensions: [keyboardAwareScroll()]}),
+      parent,
+    })
+    view.focus()
+    // CM flags focusChanged from the DOM focus event but only delivers it to
+    // plugins on its next update cycle; flush one so the ViewPlugin actually
+    // subscribes (it gates the subscription on focus).
+    view.dispatch({})
+    return vv
+  }
+
+  it('re-asserts once on keyboard-open, then ignores the scroll it triggers (no loop)', () => {
+    const vv = mountFocused(800) // focused, keyboard not yet up; plugin seeds vvHeight=800
+    expect(view!.hasFocus).toBe(true) // precondition: the plugin only subscribes while focused
+
+    const dispatch = vi.spyOn(view!, 'dispatch')
+
+    // Keyboard opens: visualViewport height shrinks (a geometry change).
+    vv.height = 500 // 800 - 500 = 300px overlap, well above MIN
+    vv.emit('resize')
+    expect(dispatch).toHaveBeenCalledTimes(1) // lifted the caret once
+
+    // The scrollIntoView above moves the visual viewport offset on iOS, which
+    // fires a `scroll`. That MUST NOT re-assert — otherwise it loops forever.
+    vv.offsetTop = 40
+    vv.emit('scroll')
+    expect(dispatch).toHaveBeenCalledTimes(1) // still 1: the loop is broken
+
+    // A second self-induced scroll: still nothing.
+    vv.offsetTop = 80
+    vv.emit('scroll')
+    expect(dispatch).toHaveBeenCalledTimes(1)
+  })
+})