prism branch T-1099/bugfix-scroll-keys-behind-modals commits 2 files 9 touched (5 source, 1 test, 3 docs) lines +665 / -108

Pre-push review: T-1099/bugfix-scroll-keys-behind-modals

Third fix for T-1099 (scroll keys route behind modals). Consolidates every presentation owner into one gate, DocumentLayoutCoordinator.isDocumentModalPresented, applied by one shared DocumentPresentationGate modifier. Reviewed git diff origin/main...HEAD (2 commits). CI is billing-blocked and was ignored.

At a glance

  • isDocumentModalPresented unions coordinator-owned modals, the compact layout's TOC/notes/search sheets (pushed via layoutOwnsModalPresentation), the paywall (pushed from DocumentReaderView), the zoom sheets, and the iOS folder importer.
  • applyScrollSuspension() is the only production writer of KeyboardScrollController.suspended; both controllers are gated together.
  • DocumentPresentationGate replaces two per-layout .onChange copies and the compact layout's four-watcher focus-restore modifier, so focus restoration and suspension read the same predicate.
  • View-menu Page/Top/Bottom items now disable (canScroll && !suspended) instead of staying enabled and inert.
  • Verification: make lint 0 violations; targeted test run TEST SUCCEEDED on macOS. Full make test-quick not re-run here (author reports 4638/4685 with the failures in unrelated load-sensitive WebKit suites).

Verdict

Ready to push

The fix is structurally sound: one predicate, one writer of suspended, one modifier both layouts apply, and tests that assert the old half-gate stays false while the complete gate reads true. Lint is clean and the targeted suites (DocumentPresentationGateTests, ModalPresentationSuspendsScrollHostTests, DocumentLayoutCoordinatorModalPresentationTests, KeyboardScrollControllerTests) pass on macOS. No blocking defects. The findings below are minor and can be addressed in a follow-up: the macOS media zoom briefly flaps the gate (it opens a window, not a modal), a misleading comment in resetSessionState, unconditional Observable writes in applyScrollSuspension, and app-level sheets (Settings on iOS, URL input, file importer/exporter, alerts) that sit outside the gate by the same argument that put the paywall inside it. No source files were modified by this review.

Review findings

7 raised · 0 fixed · 7 skipped

Jump to findings →

Commits

Three-level explanation

What changed

When something opens on top of the document (a note editor, the table of contents on iPhone, a zoomed image, the unlock screen), keyboard scroll keys and the View menu's scroll commands should stop moving the document underneath. Twice before, the code that stopped them either vanished or only covered some of those things. Now there is a single question the app asks, isDocumentModalPresented, and everything that might cover the document reports into it.

Why it matters

Two separate lists of "what counts as open" had drifted apart. With one list, adding a new panel means adding it in one place.

Key concepts

  • Gate: a single true/false that says whether anything covers the document.
  • ViewModifier: a reusable piece of SwiftUI behaviour both layouts attach.
  • initial: true: run the check when the view first appears, not only when the value later changes.

Architecture

Presentation state lives in three places: the coordinator (notes, footnotes, zoom, folder picker), the layout/reader view (showTOCSheet, showNotesSheet, showSearchOverlay as @State/@Binding), and the per-scene PaywallPresenter reached via @Environment. The coordinator cannot read the last two, so the fix mirrors them onto the coordinator (layoutOwnsModalPresentation, paywallPresented) via .onChange(initial: true), and the computed isDocumentModalPresented unions all of it.

Patterns

  • DocumentPresentationGate does three things in order: push layout state, apply suspension (initial: true), fire onAllDismissed on the closing transition only.
  • RegularDocumentLayout passes false, which clears a stale compact-layout value on rotation or resize.
  • resetSessionState() calls applyScrollSuspension() itself because a session swap with nothing presented produces no transition.
  • Paywall is deliberately not cleared by resetSessionState: it is window-scoped, not session-scoped.

Trade-offs

Mirroring view state onto the coordinator is simple and testable but is duplicated state with a transition-only re-push. Including the zoom requests unconditionally is right on iOS (sheets) but flaps on macOS where MediaZoomPresenter nils the request immediately after opening a window.

Deep dive

The predicate reads ten observable properties inside the layout body via the gate modifier, so the layout body now invalidates on zoom/paywall/picker changes it previously did not depend on (bounded, user-driven). The two .onChange nodes on isDocumentModalPresented are ordered so the suspension closure ignores its arguments and re-reads through applyScrollSuspension(); it therefore converges whether the layout push lands before or after it in the same pass. Compact sheet toggles cost one extra body pass (write to layoutOwnsModalPresentation invalidates the layout, then suspension applies on the next pass).

Edge cases

  • macOS zoom: zoomMermaid/zoomImage go non-nil then nil within MediaZoomPresenter's onChange. The gate closes and reopens, applyScrollSuspension flaps, and onAllDismissed sets bodyHasFocus = true in the document window as openWindow raises the new one. Harmless in practice (bodyHasFocus only matters for raw source) but should get the same #if os(iOS) treatment as the folder picker.
  • Session switch with notes sheet open: resetSessionState clears layoutOwnsModalPresentation and the compact layout resets TOC and search but not showNotesSheet; the transition-only push would not re-fire. In practice a session change replaces the NavigationPath with a new UUID, giving a fresh DocumentReaderView (fresh @State coordinator), so this only bites the reload-in-place route. The comment claiming the layout "re-pushes on the next body pass" is wrong.
  • Compact-to-regular swap: the false push flips the gate and fires onAllDismissed, pulling focus to the body mid-rotation. Probably desired; behaviour change from the old per-source watchers.
  • Unconditional writes: applyScrollSuspension writes suspended even when unchanged; Observation notifies regardless, and DocumentReaderView.body now reads suspended, so every layout mount republishes focusedSceneValue. Same shape as the T-1289 canScroll guard would fix it.

Completeness assessment

Fully implemented: all eight owners named in the bugfix report, both layouts, menu enablement, tests per owner, docs. Partial: the report's claim "everything that hides the document behind a presentation belongs here" does not yet cover app-level presentations attached at the same MainContentView level as the paywall: iOS Settings sheet, URL input sheet, file importer/exporter, and the alerts. Missing: nothing within the ticket's stated scope.

Important changes — detailed

DocumentLayoutCoordinator: isDocumentModalPresented + applyScrollSuspension

prism/Views/DocumentLayoutCoordinator.swift

Why it matters. This is the gate. Every consumer (suspension, focus restore, menu enablement) reads this one predicate; the only production writer of suspended lives here.

What to look at. DocumentLayoutCoordinator.swift:166-242, resetSessionState 354-371

Takeaway. When two lists of the same fact drift, collapse them into one computed property with one writer, and make the platform exclusions (iOS-only picker) explicit in a private helper rather than inline #if.
Rationale. The 2026-08-10 re-fix bound to coordinatorOwnsModalPresentation, which by its own doc comment excluded layout-owned sheets. Per the bugfix report and commit message.

DocumentPresentationGate: the one modifier both layouts apply

prism/Views/DocumentPresentationGate.swift

Why it matters. Replaces two hand-copied .onChange blocks and the compact layout's four-watcher focus modifier; where the wiring can go missing again.

What to look at. DocumentPresentationGate.swift:22-61

Takeaway. Push view-owned state onto an observable with .onChange(initial: true), then have the reactive consumer re-read the source of truth rather than trust the closure argument; ordering between sibling onChanges then stops mattering.
Rationale. initial: true covers a modal already open on (re)mount and makes the regular layout's false push clear a stale compact value. Stated in the file header and comments.

DocumentReaderView: paywall push and menu enablement

prism/Views/DocumentReaderView.swift

Why it matters. The paywall is per-scene and only reachable from this view; and View-menu items now disable rather than sit enabled-but-inert.

What to look at. DocumentReaderView.swift:378-390 and 495-505

Takeaway. Read enablement from the same object the command drives (active.suspended) so UI state and behaviour cannot disagree about which surface is gated.
Rationale. PaywallHost attaches its sheet at the NavigationStack level over the pushed document, mechanically identical to the note sheets already gated. Second commit message.

CompactDocumentLayout: layoutOwnsModalPresentation replaces RestoreBodyFocusOnDismiss

prism/Views/CompactDocumentLayout.swift

Why it matters. This is where the omitted owners lived; a one-line computed property is now the only thing to keep in step, and a structural test pins its three members.

What to look at. CompactDocumentLayout.swift:297-157 (diff hunks)

Takeaway. A structural source test that names each required member is a reasonable substitute when the project has no SwiftUI hosting harness.
Rationale. Focus restoration driven from the gate cannot pull focus back while a second modal is still up. Stated in comments.

Tests: per-owner gate tests and retargeted host-level structural tests

prismTests/KeyboardScrollControllerTests.swift

Why it matters. Each test asserts the contrast (old half-gate false, complete gate true), so it documents the omission as well as the fix.

What to look at. KeyboardScrollControllerTests.swift:408-598 and 645-762

Takeaway. When re-fixing a reopened bug, write tests that would have failed on the previous fix, not only on the original bug.
Rationale. Behavioural tests per owner plus structural pins on the modifier, the compact sheet list, the paywall push, and the gate file. Stated in the report's 'Why this one should hold'.

Key decisions

Mirror view-owned state onto the coordinator rather than inject the presenter.

layoutOwnsModalPresentation and paywallPresented are copies pushed by .onChange(initial: true). Layout state genuinely cannot be injected (@State), but PaywallPresenter is an @Observable reference that could be assigned once to the coordinator, which would delete the push and the 'not cleared by resetSessionState' caveat.

(inferred — not stated by the author.)
Folder importer counts on iOS only.

imageAccessFolderPicker is a no-op on macOS while both setters are unguarded, so the flag would latch true and suspend scrolling for the session. Verified against MediaZoomPresenter.swift.

Zoom requests count on both platforms.

Not stated. On macOS the zoom opens a window and MediaZoomPresenter nils the request immediately, so the gate flaps rather than holds. The same iOS-only treatment as the picker would be consistent.

(inferred — not stated by the author.)
Paywall survives resetSessionState.

Window-scoped, not session-scoped; clearing it would reopen the gate with the unlock screen up. Stated in code and pinned by sessionResetKeepsPaywallGate.

Menu enablement folded into canScroll/hasContent.

Keeps DocumentActions unchanged, but the field doc comments in Models/DocumentActions.swift still describe the old meaning.

(inferred — not stated by the author.)

Review findings

SeverityAreaFindingResolution
minorDocumentLayoutCoordinator.swift:215-216 (macOS zoom)zoomMermaid/zoomImage are gated on both platforms, but on macOS MediaZoomPresenter opens a window and nils the request inside its onChange, so the gate closes and reopens in consecutive passes: suspension flaps, menu items blink, and onAllDismissed sets bodyHasFocus while openWindow raises the new window.Not fixed (review is read-only). Suggest #if os(iOS) for the two zoom terms, matching imageAccessPickerPresented.
minorDocumentLayoutCoordinator.swift:357-360 (resetSessionState comment)Comment says the layout 're-pushes its own sheet state on the next body pass', but the push is a transition-only .onChange and will not re-fire if the view-side value did not change. Only reachable via reload-in-place (a normal session switch remounts DocumentReaderView with fresh @State), and the compact layout resets TOC and search but not showNotesSheet.Not fixed. Correct the comment; optionally reset showNotesSheet alongside the other two.
minorDocumentLayoutCoordinator.swift:238-242 (applyScrollSuspension)Writes suspended unconditionally; Observation notifies on same-value writes and DocumentReaderView.body now reads suspended, so every layout mount and session switch republishes focusedSceneValue for nothing. Bounded (not per-frame), contrary to the T-1289 low-churn shape of canScroll.Not fixed. Guard each write with != present.
minorScope: app-level presentationsThe paywall was added because PaywallHost attaches its sheet at the MainContentView level over the pushed document. The iOS Settings sheet, URL input sheet, fileImporter/fileExporter, and the document alerts sit at the same level and remain outside the gate. The report's 'everything that hides the document belongs here' overstates coverage.Not fixed. Consider a follow-up ticket; the mechanism accepts new inputs cheaply.
minorDocumentReaderView.swift:504-505 / Models/DocumentActions.swift:77-82canScroll/hasContent now mean 'and not suspended' but the DocumentActions doc comments still describe 'has room to scroll' / 'a document is showing'.Not fixed. Update the comments or add an explicit isSuspended field.
minorDocumentLayoutCoordinator.swift:160 (coordinatorOwnsModalPresentation)Now has no production caller outside isDocumentModalPresented but stays internal; it is the exact surface the previous re-fix bound to and reopened T-1099 on.Not fixed. Make private; tests can assert through the composite.
minorDocumentPresentationGate.swift:40-56The 'exactly one layout mounted, the other clears the flag' invariant is documented in prose only. Applying the modifier once around the useCompact branch in DocumentReaderView would make it structural. Also, a compact-sheet toggle now costs one extra layout body pass because the gate writes an observable it observes.Not fixed. Optional restructure.

Per-file diffs

Click to expand.

prism/Views/DocumentLayoutCoordinator.swift Modified +90 / -3
diff --git a/prism/Views/DocumentLayoutCoordinator.swift b/prism/Views/DocumentLayoutCoordinator.swiftindex 13a49d7b..8abaf9c5 100644--- a/prism/Views/DocumentLayoutCoordinator.swift+++ b/prism/Views/DocumentLayoutCoordinator.swift@@ -152,9 +152,11 @@ final class DocumentLayoutCoordinator {     /// Keyboard scroll controller for the raw-source view.     let rawSourceScroll = KeyboardScrollController() -    /// True when the coordinator owns a presented modal / popover.-    /// Layout-owned presentations (search overlay, URL input, TOC sheet,-    /// notes sheet) are tracked separately in each layout.+    /// True when the coordinator owns a presented modal / popover: the note+    /// popover, the add-note / reply / document-note sheets, and the footnote+    /// popover. This is only one half of the presentation gate — the media+    /// zoom sheets and the layout's own sheets are the other half, and+    /// ``isDocumentModalPresented`` is what callers should ask.     var coordinatorOwnsModalPresentation: Bool {         notePopoverBlock != nil             || addNoteBlock != nil@@ -163,6 +165,82 @@ final class DocumentLayoutCoordinator {             || activeFootnoteId != nil     } +    /// Modal presentations owned by the mounted layout rather than by the+    /// coordinator, folded into ``isDocumentModalPresented`` (T-1099).+    ///+    /// The compact layout's TOC, notes, and search sheets are `@State` /+    /// `@Binding` at view level (the View menu toggles two of them through+    /// `DocumentActions`), so the coordinator cannot read them directly. The+    /// mounted layout pushes them here with an `initial: true` `.onChange`,+    /// which is also what makes a layout swap self-correcting: only+    /// `CompactDocumentLayout` presents these, and `RegularDocumentLayout`+    /// clears the flag when it takes over, so a sheet that unmounts with the+    /// compact layout on rotation cannot strand the gate closed.+    var layoutOwnsModalPresentation = false++    /// The paywall sheet's contribution to the gate (T-1099).+    ///+    /// `PaywallHost` presents `PaywallSheet` with a plain+    /// `.sheet(isPresented:)` attached at the `MainContentView`+    /// `NavigationStack` level — the same level that hosts the pushed+    /// `DocumentReaderView` — so a blocked export raises it over the open+    /// document exactly as `showDocumentNoteSheet` does. The presenter is+    /// per-scene (T-1779) and the coordinator holds no reference to it, so+    /// `DocumentReaderView`, which does, pushes `paywall.isPresented` here+    /// with an `initial: true` `.onChange`, the same shape as+    /// ``layoutOwnsModalPresentation``.+    ///+    /// Deliberately NOT cleared by ``resetSessionState``: the paywall belongs+    /// to the window, not the session, and it survives a document switch+    /// untouched. Clearing it would reopen the gate with the sheet still on+    /// screen — this bug, one presentation over.+    var paywallPresented = false++    /// The single presentation gate (T-1099): true while ANY document-owned+    /// modal, popover, or fullscreen zoom covers the document body.+    ///+    /// Everything that hides the document behind a presentation belongs here,+    /// because everything that suspends against a presentation reads this one+    /// property: ``applyScrollSuspension`` (which gates every entry point on+    /// both `KeyboardScrollController`s, including the View menu's Page /+    /// Top / Bottom commands routed through `DocumentActions`) and the+    /// layouts' focus restoration. Gating on the coordinator-owned half alone+    /// is what reopened T-1099 a third time: the compact TOC / notes / search+    /// sheets and the media zoom sheets were left out, so View > Page Down+    /// still scrolled the document underneath them.+    var isDocumentModalPresented: Bool {+        coordinatorOwnsModalPresentation+            || layoutOwnsModalPresentation+            || paywallPresented+            || zoomMermaid != nil+            || zoomImage != nil+            || imageAccessPickerPresented+    }++    /// The folder importer's contribution to the gate. iOS-only on purpose:+    /// `imageAccessFolderPicker` is a no-op on macOS, so a flag set there+    /// (nothing clears it, since no picker is ever presented to dismiss)+    /// would suspend scrolling for the rest of the session.+    private var imageAccessPickerPresented: Bool {+        #if os(iOS)+        presentImageAccessPicker+        #else+        false+        #endif+    }++    /// Applies the presentation gate to both keyboard scroll controllers.+    ///+    /// Both are suspended together regardless of which one is currently+    /// active (`showRawSource`): either can be the `DocumentActions` target,+    /// a modal such as the document-note Cmd+Shift+N shortcut can open over+    /// either surface, and the active surface can change while one is up.+    func applyScrollSuspension() {+        let present = isDocumentModalPresented+        renderedScroll.suspended = present+        rawSourceScroll.suspended = present+    }+     // MARK: - Footnote Popover State      /// Identifier of the footnote currently displayed in a popover/sheet.@@ -276,12 +354,21 @@ final class DocumentLayoutCoordinator {         zoomImage = nil         imageAccessNeededDirectory = nil         presentImageAccessPicker = false+        // The mounted layout re-pushes its own sheet state on the next body+        // pass; clearing it here keeps the gate from carrying a previous+        // session's presentation across a document switch (T-1099).+        layoutOwnsModalPresentation = false         // Clear the screen banner so a document switch inside the toast's         // 3-second window can't show the previous session's message, and         // reset the export flow so a username prompt left open can't share         // the previous session's notes (notes-action-placement Decision 15).         bannerMessage = nil         exportNotesFlow.reset()+        // Every gate input was just cleared, so re-apply rather than waiting+        // for the layout's `.onChange`: that fires on a TRANSITION, and a+        // session swapped while nothing was presented produces none — which+        // would leave a stale `suspended` from before standing (T-1099).+        applyScrollSuspension()     }      // MARK: - Note Helpers
prism/Views/DocumentPresentationGate.swift Added +74 / -0
diff --git a/prism/Views/DocumentPresentationGate.swift b/prism/Views/DocumentPresentationGate.swiftnew file mode 100644index 00000000..409a4a66--- /dev/null+++ b/prism/Views/DocumentPresentationGate.swift@@ -0,0 +1,74 @@+//+//  DocumentPresentationGate.swift+//  prism+//+//  The single wiring point between a document layout's presentation state and+//  `DocumentLayoutCoordinator.isDocumentModalPresented` (T-1099).+//+//  Both layouts apply this one modifier, so there is one gate rather than a+//  per-layout copy that can drift — which is how T-1099 was reopened twice:+//  once because the WebKit cutover removed the only binding, and again+//  because the re-fix bound only the coordinator-owned half of the+//  presentation state, leaving the compact TOC / notes / search sheets, the+//  media zoom sheets, and the folder importer outside the gate.+//++import SwiftUI++/// Feeds a layout's own modal presentations into the coordinator's gate, then+/// drives everything that depends on the gate from it: keyboard/menu scroll+/// suspension, and document-body focus restoration once nothing is presented.+struct DocumentPresentationGate: ViewModifier {+    /// True while THIS layout presents a modal the coordinator cannot see —+    /// the compact TOC / notes / search sheets. The regular layout owns none+    /// (its TOC and notes are sidebars, its search an inline bar) and passes+    /// `false`, which is also what clears a stale value left by the compact+    /// layout when the two swap on rotation or a window resize.+    let layoutOwnsModalPresentation: Bool++    let coordinator: DocumentLayoutCoordinator++    /// Called when the last presentation over the document goes away, so the+    /// layout can re-acquire keyboard focus on the document body (T-1103).+    let onAllDismissed: () -> Void++    func body(content: Content) -> some View {+        content+            // Pushed first: the gate below has to see this layout's sheets.+            // `initial: true` publishes the mounted layout's state before any+            // transition, which is what makes a layout swap self-correcting.+            .onChange(of: layoutOwnsModalPresentation, initial: true) { _, present in+                coordinator.layoutOwnsModalPresentation = present+            }+            // `suspended` gates every entry point on `KeyboardScrollController`:+            // the View menu's Page Up/Down and Scroll to Top/Bottom (routed+            // through `DocumentActions`) and raw source's own `.onKeyPress`.+            // `initial: true` covers a modal already presented when this+            // layout (re)mounts — e.g. after a raw-source toggle.+            .onChange(of: coordinator.isDocumentModalPresented, initial: true) { _, _ in+                coordinator.applyScrollSuspension()+            }+            // Focus restoration is a transition, not a state: it must not fire+            // on the initial pass, or mounting a document would pull focus to+            // the body regardless of what else is going on.+            .onChange(of: coordinator.isDocumentModalPresented) { _, present in+                if !present { onAllDismissed() }+            }+    }+}++extension View {+    /// Applies the document presentation gate (T-1099). Every document layout+    /// must apply this — see `DocumentPresentationGate`.+    func documentPresentationGate(+        layoutOwnsModalPresentation: Bool,+        coordinator: DocumentLayoutCoordinator,+        onAllDismissed: @escaping () -> Void+    ) -> some View {+        modifier(DocumentPresentationGate(+            layoutOwnsModalPresentation: layoutOwnsModalPresentation,+            coordinator: coordinator,+            onAllDismissed: onAllDismissed+        ))+    }+}
prism/Views/DocumentReaderView.swift Modified +19 / -2
diff --git a/prism/Views/DocumentReaderView.swift b/prism/Views/DocumentReaderView.swiftindex 28eeef29..54d5144e 100644--- a/prism/Views/DocumentReaderView.swift+++ b/prism/Views/DocumentReaderView.swift@@ -378,6 +378,17 @@ struct DocumentReaderView: View {             .onChange(of: settings.showHTMLComments) { _, _ in                 session.search.recomputeAfterVisibilityChange()             }+            // The paywall's contribution to the presentation gate (T-1099).+            // `PaywallHost` attaches its `.sheet` at the `MainContentView`+            // `NavigationStack` level, so a blocked export covers the pushed+            // document with a plain sheet — the same shape as the note sheets+            // that are already gated. The presenter is per-scene (T-1779) and+            // reaches this view through `@Environment`, not the coordinator,+            // so this view is the one that can push it. `initial: true` covers+            // a document opened while an unlock screen is already up.+            .onChange(of: paywall.isPresented, initial: true) { _, present in+                coordinator.paywallPresented = present+            }             #if os(macOS)             // Auto-collapse sidebars when macOS window gets too narrow (T-495)             .onChange(of: geo.size.width) { _, newWidth in@@ -484,8 +495,14 @@ struct DocumentReaderView: View {             scrollToBottom: { active.scrollToBottom(reduceMotion: reduceMotion) },             pageUp: { active.pageUp(reduceMotion: reduceMotion) },             pageDown: { active.pageDown(reduceMotion: reduceMotion) },-            canScroll: active.canScroll,-            hasContent: active.hasContent+            // Menu enablement runs off the same presentation gate as the+            // commands themselves (T-1099): while a modal covers the document+            // the View menu's Page Up/Down and Scroll to Top/Bottom items are+            // disabled rather than enabled-but-inert. `suspended` is read from+            // the controller the menu actually drives, so the two can never+            // disagree about which surface is gated.+            canScroll: active.canScroll && !active.suspended,+            hasContent: active.hasContent && !active.suspended         )     } 
prism/Views/CompactDocumentLayout.swift Modified +23 / -48
diff --git a/prism/Views/CompactDocumentLayout.swift b/prism/Views/CompactDocumentLayout.swiftindex 9113a9ab..671240f3 100644--- a/prism/Views/CompactDocumentLayout.swift+++ b/prism/Views/CompactDocumentLayout.swift@@ -296,29 +296,15 @@ struct CompactDocumentLayout: View {                 Text(error)             }         }-        .modifier(RestoreBodyFocusOnDismiss(-            showTOCSheet: showTOCSheet,-            showNotesSheet: showNotesSheet,-            showSearchOverlay: showSearchOverlay,-            coordinatorOwnsModalPresentation: coordinator.coordinatorOwnsModalPresentation,-            action: restoreBodyFocusIfIdle-        ))-        // Gate keyboard/menu scroll commands while a coordinator-owned modal-        // (note popover, add-note / reply / document-note sheet, footnote-        // popover) is presented over the document (T-1099). The WebKit-        // cutover dropped this binding entirely — `suspended` gates every-        // entry point on `KeyboardScrollController` (View-menu Page/Top/-        // Bottom commands via `DocumentActions`, and raw source's own-        // `.onKeyPress`), but nothing in production ever set it. `initial:-        // true` covers a modal already up when this view (re)mounts — e.g.-        // after a raw-source toggle. Both controllers are suspended together-        // since either can be the active `DocumentActions` target and a-        // modal (like the document-note shortcut) can open regardless of-        // which one is currently driving.-        .onChange(of: coordinator.coordinatorOwnsModalPresentation, initial: true) { _, present in-            coordinator.renderedScroll.suspended = present-            coordinator.rawSourceScroll.suspended = present-        }+        // The single presentation gate (T-1099): feeds this layout's own TOC /+        // notes / search sheets into the coordinator, suspends both keyboard+        // scroll controllers while anything is presented over the document,+        // and restores body focus once nothing is.+        .documentPresentationGate(+            layoutOwnsModalPresentation: layoutOwnsModalPresentation,+            coordinator: coordinator,+            onAllDismissed: restoreBodyFocusIfIdle+        )         #if os(iOS)         .onChange(of: scenePhase) { _, phase in             if phase == .active { restoreBodyFocusIfIdle() }@@ -326,34 +312,23 @@ struct CompactDocumentLayout: View {         #endif     } -    // Bundles per-source dismiss watchers into one outer modifier so the-    // body chain stays under the Swift type-checker complexity limit. The-    // `coordinator.coordinatorOwnsModalPresentation` watcher (T-1103) is-    // required at the layout level because raw source replaces-    // `DocumentScrollContent` and the equivalent watcher inside it would be-    // unmounted while raw source is shown.-    private struct RestoreBodyFocusOnDismiss: ViewModifier {-        let showTOCSheet: Bool-        let showNotesSheet: Bool-        let showSearchOverlay: Bool-        let coordinatorOwnsModalPresentation: Bool-        let action: () -> Void--        func body(content: Content) -> some View {-            content-                .onChange(of: showTOCSheet) { _, presented in if !presented { action() } }-                .onChange(of: showNotesSheet) { _, presented in if !presented { action() } }-                .onChange(of: showSearchOverlay) { _, presented in if !presented { action() } }-                .onChange(of: coordinatorOwnsModalPresentation) { _, present in if !present { action() } }-        }+    /// This layout's own modal presentations, pushed onto the coordinator so+    /// the single presentation gate can see them (T-1099). They live here as+    /// `@State`/`@Binding` — the View menu toggles TOC and Notes through+    /// `DocumentActions` — so the coordinator cannot read them directly, and+    /// omitting them from the gate is what reopened T-1099 a third time.+    private var layoutOwnsModalPresentation: Bool {+        showTOCSheet || showNotesSheet || showSearchOverlay     } +    /// Focus restoration (T-1103) is driven from the gate rather than from a+    /// watcher per presentation source, so it cannot pull focus back to the+    /// body while a second modal is still up. It lives at layout level+    /// because raw source replaces `DocumentScrollContent`, and a watcher+    /// inside that view would be unmounted while raw source is shown.     private func restoreBodyFocusIfIdle() {-        guard !showTOCSheet,-              !showNotesSheet,-              !showSearchOverlay,-              !session.search.isSearchActive,-              !coordinator.coordinatorOwnsModalPresentation else { return }+        guard !session.search.isSearchActive,+              !coordinator.isDocumentModalPresented else { return }         bodyHasFocus = true     } 
prism/Views/RegularDocumentLayout.swift Modified +18 / -23
diff --git a/prism/Views/RegularDocumentLayout.swift b/prism/Views/RegularDocumentLayout.swiftindex 49f9f17b..cf8ac02a 100644--- a/prism/Views/RegularDocumentLayout.swift+++ b/prism/Views/RegularDocumentLayout.swift@@ -181,28 +181,23 @@ struct RegularDocumentLayout: View {             .onChange(of: session.search.isSearchActive) { _, isActive in                 if !isActive { restoreBodyFocusIfIdle() }             }-            // Re-acquire focus when a coordinator-owned modal (footnote popover,-            // note popover, add-note / reply / document-note sheet) closes so the-            // active scroll surface — rendered body or raw source — regains-            // scroll-key focus without requiring a click. T-1103.-            .onChange(of: coordinator.coordinatorOwnsModalPresentation) { _, present in-                if !present { restoreBodyFocusIfIdle() }-            }-            // Gate keyboard/menu scroll commands while a coordinator-owned modal-            // is presented over the document (T-1099). The WebKit cutover-            // dropped this binding entirely — `suspended` gates every entry-            // point on `KeyboardScrollController` (View-menu Page/Top/Bottom-            // commands via `DocumentActions`, and raw source's own-            // `.onKeyPress`), but nothing in production ever set it. `initial:-            // true` covers a modal already up when this view (re)mounts. Both-            // controllers are suspended together since either can be the-            // active `DocumentActions` target and a modal (like the-            // document-note shortcut) can open regardless of which one is-            // currently driving.-            .onChange(of: coordinator.coordinatorOwnsModalPresentation, initial: true) { _, present in-                coordinator.renderedScroll.suspended = present-                coordinator.rawSourceScroll.suspended = present-            }+            // The single presentation gate (T-1099): suspends both keyboard+            // scroll controllers while any modal — note / footnote / add-note /+            // reply / document-note sheet, or the iPad media zoom sheets — is+            // presented over the document, and re-acquires body focus once+            // nothing is, so the active scroll surface (rendered body or raw+            // source) regains scroll-key focus without requiring a click+            // (T-1103).+            //+            // This layout owns no modal presentations of its own — its TOC and+            // notes surfaces are sidebars and its search is an inline bar — so+            // it passes `false`, which also clears the value the compact+            // layout left behind when the two swap on rotation or a resize.+            .documentPresentationGate(+                layoutOwnsModalPresentation: false,+                coordinator: coordinator,+                onAllDismissed: restoreBodyFocusIfIdle+            )             #if os(macOS)             .onChange(of: controlActiveState) { _, state in                 if state == .key { restoreBodyFocusIfIdle() }@@ -216,7 +211,7 @@ struct RegularDocumentLayout: View {      private func restoreBodyFocusIfIdle() {         guard !session.search.isSearchActive,-              !coordinator.coordinatorOwnsModalPresentation else { return }+              !coordinator.isDocumentModalPresented else { return }         bodyHasFocus = true     } 
prismTests/KeyboardScrollControllerTests.swift Modified +296 / -17
diff --git a/prismTests/KeyboardScrollControllerTests.swift b/prismTests/KeyboardScrollControllerTests.swiftindex afe334ac..9aa3fac5 100644--- a/prismTests/KeyboardScrollControllerTests.swift+++ b/prismTests/KeyboardScrollControllerTests.swift@@ -405,6 +405,197 @@ struct DocumentLayoutCoordinatorModalPresentationTests {     } } +// MARK: - The complete presentation gate (T-1099, third reopen)++/// The third T-1099 reopen. PR #353 restored a suspension binding, but drove it+/// from `coordinatorOwnsModalPresentation` — which covers only the note /+/// add-note / reply / document-note / footnote states. Everything else that+/// covers the document was outside it: the compact layout's TOC, notes, and+/// search sheets (layout-owned `@State`/`@Binding`, invisible to the+/// coordinator), the media zoom sheets, and the folder importer. A focused+/// `DocumentActions` calls Page/Top/Bottom straight into the controllers, so+/// the document still scrolled underneath all of those.+///+/// Each test below pins one previously-omitted owner, and asserts the omission+/// explicitly: the old half-gate stays `false` while the complete gate is+/// `true`. Without the fix these read the same value and every one of them+/// fails.+@Suite("Document presentation gate covers every owner (T-1099)")+@MainActor+struct DocumentPresentationGateTests {++    @Test("A coordinator with nothing presented has an open gate")+    func defaultGateOpen() {+        let coordinator = DocumentLayoutCoordinator()+        #expect(!coordinator.isDocumentModalPresented)+    }++    @Test("A coordinator-owned modal closes the gate")+    func coordinatorOwnedModalClosesGate() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.showFootnote(identifier: "1", blockId: "block-1")+        #expect(coordinator.isDocumentModalPresented)++        coordinator.dismissFootnote()+        #expect(!coordinator.isDocumentModalPresented)+    }++    @Test("A layout-owned sheet (compact TOC / notes / search) closes the gate")+    func layoutOwnedSheetClosesGate() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.layoutOwnsModalPresentation = true++        // The omission this ticket was reopened for: the coordinator-owned+        // half sees nothing, so binding suspension to it left Page/Top/Bottom+        // live under the compact TOC, notes, and search sheets.+        #expect(!coordinator.coordinatorOwnsModalPresentation)+        #expect(coordinator.isDocumentModalPresented)++        coordinator.layoutOwnsModalPresentation = false+        #expect(!coordinator.isDocumentModalPresented)+    }++    @Test("The paywall sheet closes the gate")+    func paywallClosesGate() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.paywallPresented = true++        // `PaywallHost` presents a plain `.sheet` at the `MainContentView`+        // `NavigationStack` level, over the pushed document — mechanically the+        // same as the note sheets that were already gated. The coordinator+        // holds no `PaywallPresenter` (it is per-scene, T-1779), so the+        // coordinator-owned half cannot see it and a blocked export left+        // View > Page Down scrolling the document under the unlock screen.+        #expect(!coordinator.coordinatorOwnsModalPresentation)+        #expect(coordinator.isDocumentModalPresented)++        coordinator.paywallPresented = false+        #expect(!coordinator.isDocumentModalPresented)+    }++    /// The paywall belongs to the window, not the document: a document switch+    /// leaves `PaywallPresenter.isPresented` untouched, so `resetSessionState`+    /// must NOT clear the mirrored flag the way it clears the session-scoped+    /// ones. Doing so would reopen the gate with the unlock screen still up.+    @Test("A session change leaves the paywall's contribution to the gate standing")+    func sessionResetKeepsPaywallGate() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.paywallPresented = true+        coordinator.layoutOwnsModalPresentation = true+        coordinator.applyScrollSuspension()+        #expect(coordinator.renderedScroll.suspended)++        coordinator.resetSessionState()+        #expect(coordinator.isDocumentModalPresented)+        #expect(coordinator.renderedScroll.suspended)+        #expect(coordinator.rawSourceScroll.suspended)++        coordinator.paywallPresented = false+        coordinator.applyScrollSuspension()+        #expect(!coordinator.renderedScroll.suspended)+    }++    @Test("The mermaid zoom sheet closes the gate")+    func mermaidZoomClosesGate() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.zoomMermaid = MermaidZoomRequest(+            id: "b-abc-0", source: "graph TD;", diagramType: "flowchart", byteOffset: 0+        )+        #expect(!coordinator.coordinatorOwnsModalPresentation)+        #expect(coordinator.isDocumentModalPresented)++        coordinator.zoomMermaid = nil+        #expect(!coordinator.isDocumentModalPresented)+    }++    @Test("The image zoom sheet closes the gate")+    func imageZoomClosesGate() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.zoomImage = ImageZoomRequest(+            id: "b-abc-0", source: "diagram.png", alt: "", title: nil+        )+        #expect(!coordinator.coordinatorOwnsModalPresentation)+        #expect(coordinator.isDocumentModalPresented)++        coordinator.zoomImage = nil+        #expect(!coordinator.isDocumentModalPresented)+    }++    @Test("The folder importer closes the gate on iOS and is ignored on macOS")+    func folderImporterGate() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.presentImageAccessPicker = true+        #if os(iOS)+        #expect(coordinator.isDocumentModalPresented)+        coordinator.presentImageAccessPicker = false+        #expect(!coordinator.isDocumentModalPresented)+        #else+        // `imageAccessFolderPicker` is a no-op on macOS: nothing presents the+        // importer, so nothing would ever clear the flag, and counting it+        // would suspend scrolling for the rest of the session.+        #expect(!coordinator.isDocumentModalPresented)+        #endif+    }++    @Test("applyScrollSuspension gates both scroll controllers from the gate")+    func applyScrollSuspensionDrivesBothControllers() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.applyScrollSuspension()+        #expect(!coordinator.renderedScroll.suspended)+        #expect(!coordinator.rawSourceScroll.suspended)++        // A layout-owned sheet must suspend the raw-source controller too: it+        // can be the active `DocumentActions` target, and the compact search+        // and TOC sheets open over raw source as readily as over the document.+        coordinator.layoutOwnsModalPresentation = true+        coordinator.applyScrollSuspension()+        #expect(coordinator.renderedScroll.suspended)+        #expect(coordinator.rawSourceScroll.suspended)++        coordinator.layoutOwnsModalPresentation = false+        coordinator.applyScrollSuspension()+        #expect(!coordinator.renderedScroll.suspended)+        #expect(!coordinator.rawSourceScroll.suspended)+    }++    @Test("Two presentations at once only lift the gate when the last one closes")+    func gateNeedsEveryOwnerDismissed() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.layoutOwnsModalPresentation = true+        coordinator.showDocumentNoteSheet = true+        coordinator.applyScrollSuspension()+        #expect(coordinator.renderedScroll.suspended)++        coordinator.showDocumentNoteSheet = false+        coordinator.applyScrollSuspension()+        #expect(coordinator.isDocumentModalPresented)+        #expect(coordinator.renderedScroll.suspended)++        coordinator.layoutOwnsModalPresentation = false+        coordinator.applyScrollSuspension()+        #expect(!coordinator.renderedScroll.suspended)+    }++    @Test("A session change clears the gate and resumes both controllers")+    func sessionResetResumesScrolling() {+        let coordinator = DocumentLayoutCoordinator()+        coordinator.layoutOwnsModalPresentation = true+        coordinator.zoomImage = ImageZoomRequest(+            id: "b-abc-0", source: "diagram.png", alt: "", title: nil+        )+        coordinator.applyScrollSuspension()+        #expect(coordinator.renderedScroll.suspended)++        // `resetSessionState` re-applies the gate itself rather than waiting+        // for a layout `.onChange`: that fires on a transition, and swapping+        // documents while nothing is presented produces none.+        coordinator.resetSessionState()+        #expect(!coordinator.isDocumentModalPresented)+        #expect(!coordinator.renderedScroll.suspended)+        #expect(!coordinator.rawSourceScroll.suspended)+    }+}+ // MARK: - Modal presentation is actually wired to `suspended` (T-1099, host-level)  /// The T-1099 regression, twice over. Fixed once in PR #231 by mirroring@@ -452,34 +643,122 @@ struct ModalPresentationSuspendsScrollHostTests {     }      @Test(-        "Both document layouts bind coordinatorOwnsModalPresentation into an initial + live suspended gate",+        "Both document layouts apply the shared document presentation gate",         arguments: ["CompactDocumentLayout.swift", "RegularDocumentLayout.swift"]     )-    func bothLayoutsSuspendScrollControllersOnModalPresentation(fileName: String) throws {+    func bothLayoutsApplyThePresentationGate(fileName: String) throws {         let source = try Self.layoutSource(fileName)         #expect(-            source.contains(-                ".onChange(of: coordinator.coordinatorOwnsModalPresentation, initial: true)"-            ),+            source.contains(".documentPresentationGate("),             """-            \(fileName) must mirror `coordinator.coordinatorOwnsModalPresentation` into \-            `KeyboardScrollController.suspended` via an `.onChange` with `initial: true`. \-            Without `initial: true`, a modal already presented when this layout (re)mounts \-            (e.g. after a raw-source toggle) leaves scroll commands live until presentation \-            state next changes. Without the binding at all, View > Page Down/Top/Bottom \-            keep scrolling the document underneath a note/footnote/add-note/reply/document-note \-            modal (T-1099 — reopened after the WebKit rendering cutover dropped this binding).+            \(fileName) must apply `.documentPresentationGate(...)`. It is the only thing \+            that sets `KeyboardScrollController.suspended` in production, and without it \+            View > Page Down/Top/Bottom (and raw source's own key handling) keep scrolling \+            the document underneath a presented modal — T-1099, reopened once after the \+            WebKit cutover dropped the binding entirely.             """         )         #expect(-            source.contains("coordinator.renderedScroll.suspended = present"),-            "\(fileName) must suspend the rendered-document scroll controller."+            source.contains("layoutOwnsModalPresentation:"),+            """+            \(fileName) must state its own layout-owned presentations to the gate — `false` \+            if it presents none. That argument is what carries the compact TOC / notes / \+            search sheets into the gate, and what clears a stale value when the layouts swap.+            """+        )+    }++    /// The compact layout is the one that owns sheets of its own, so its+    /// contribution is pinned by name: dropping any of the three from the+    /// computed property is exactly the omission that reopened T-1099 a third+    /// time, and the gate cannot tell the difference.+    @Test(+        "The compact layout feeds its TOC, notes, and search sheets into the gate",+        arguments: ["showTOCSheet", "showNotesSheet", "showSearchOverlay"]+    )+    func compactLayoutFeedsAllItsSheets(sheetState: String) throws {+        let source = try Self.layoutSource("CompactDocumentLayout.swift")+        let marker = "private var layoutOwnsModalPresentation: Bool {"+        let start = try #require(+            source.range(of: marker),+            "CompactDocumentLayout must declare its layout-owned presentation state."+        )+        let end = try #require(+            source.range(of: "}", range: start.upperBound..<source.endIndex),+            "Unterminated layoutOwnsModalPresentation body."         )         #expect(-            source.contains("coordinator.rawSourceScroll.suspended = present"),+            source[start.upperBound..<end.lowerBound].contains(sheetState),+            """+            CompactDocumentLayout.layoutOwnsModalPresentation must include \(sheetState). \+            A sheet left out of it is invisible to the coordinator's gate, so the document \+            keeps scrolling underneath it (T-1099, third reopen).+            """+        )+    }++    /// View-menu enablement is built inside a private method on a `View`, so+    /// it is out of reach of a unit test; pin it structurally instead. Without+    /// this the Page/Top/Bottom items stay enabled behind a modal and silently+    /// do nothing when chosen, which is the symptom minus the scrolling.+    @Test("View-menu scroll enablement folds in the suspension")+    func menuEnablementFollowsTheGate() throws {+        let source = try Self.layoutSource("DocumentReaderView.swift")+        #expect(+            source.contains("canScroll: active.canScroll && !active.suspended"),+            "DocumentActions.canScroll must disable while the active controller is suspended."+        )+        #expect(+            source.contains("hasContent: active.hasContent && !active.suspended"),+            "DocumentActions.hasContent must disable while the active controller is suspended."+        )+    }++    /// `DocumentReaderView` is the only place that holds both the coordinator+    /// and this scene's `PaywallPresenter`, so it is the only place the+    /// paywall can reach the gate from. A unit test can set+    /// `coordinator.paywallPresented` itself and pass with nothing in+    /// production writing it — which is exactly how the presentations this+    /// ticket was reopened for stayed invisible — so pin the push.+    @Test("DocumentReaderView pushes the paywall sheet into the gate")+    func paywallIsPushedIntoTheGate() throws {+        let source = try Self.layoutSource("DocumentReaderView.swift")+        #expect(+            source.contains(".onChange(of: paywall.isPresented, initial: true)"),+            """+            DocumentReaderView must push `paywall.isPresented` onto the coordinator with \+            `initial: true`. `PaywallHost` attaches its sheet at the NavigationStack level, \+            above the pushed document, so without this a blocked export leaves View > Page \+            Down scrolling the document under the unlock screen (T-1099).+            """+        )+        #expect(+            source.contains("coordinator.paywallPresented = present"),+            "The pushed value must land on the coordinator property the gate reads."+        )+    }++    /// The gate itself: the layouts only declare their state, so the file that+    /// applies it to the controllers is where the suspension can go missing.+    @Test("The presentation gate suspends the controllers and feeds the coordinator")+    func gateModifierWiresSuspension() throws {+        let source = try Self.layoutSource("DocumentPresentationGate.swift")+        #expect(+            source.contains("coordinator.layoutOwnsModalPresentation = present"),+            "The gate must publish the layout's own presentations onto the coordinator."+        )+        #expect(+            source.contains("coordinator.applyScrollSuspension()"),+            "The gate must apply the suspension — nothing else sets `suspended` in production."+        )+        #expect(+            source.contains(+                ".onChange(of: coordinator.isDocumentModalPresented, initial: true)"+            ),             """-            \(fileName) must suspend the raw-source scroll controller too — a modal (e.g. \-            the document-note Cmd+Shift+N shortcut) can open while raw source is displayed.+            The suspension must be applied with `initial: true`: without it, a modal \+            already presented when a layout (re)mounts — e.g. after a raw-source toggle — \+            leaves scroll commands live until presentation state next changes.             """         )     }
docs/agent-notes/keyboard-scrolling.md Modified +19 / -10
diff --git a/docs/agent-notes/keyboard-scrolling.md b/docs/agent-notes/keyboard-scrolling.mdindex f79e3b59..99fa23c8 100644--- a/docs/agent-notes/keyboard-scrolling.md+++ b/docs/agent-notes/keyboard-scrolling.md@@ -78,28 +78,37 @@ Mutation-checked: disabling the `ResizeObserver` branch fails only `resizeObserv  `.task(id: session.id)` covers cold start. `.onChange(of: scenePhase)` (iOS) and `.onChange(of: controlActiveState)` (macOS) plus a `restoreBodyFocusIfIdle()` helper handle Slide Over / Stage Manager / minimize / Cmd+\` cases. The helper asserts focus only when no text input or layout-owned modal currently holds it, so it does not steal from search fields on resume. -## Suspension behind coordinator-owned modals (T-1099)+## Suspension behind presented modals (T-1099)  `KeyboardScrollController.suspended` gates every scroll entry point on both `renderedScroll` and `rawSourceScroll`: `handleKeyPress` returns `.ignored` (letting SwiftUI route the key to the actually focused responder — the popover / sheet — instead of consuming it on the body), and every public scroll method (`arrowUp/Down`, `pageUp/Down`, `scrollToTop/Bottom`) early-returns so explicit callers (heading-row Space forwarding, `DocumentActions`' View-menu commands) no-op without each call site having to check coordinator state.  **Where the binding lives has moved twice.** The original (pre-WebKit-cutover) fix bound `keyboardScroll.suspended = coordinator.coordinatorOwnsModalPresentation` inside `DocumentScrollContent`, which at the time owned both a `KeyboardScrollController` and a `bodyHasFocus` `@FocusState`. The T-1542 WebKit cutover restructured `DocumentScrollContent` into a bridge-driven view with neither — the rendered document has no SwiftUI scroll geometry or `.onKeyPress` at all; `DocumentActions`' Page/Top/Bottom commands call straight into `coordinator.renderedScroll` / `coordinator.rawSourceScroll` (selected by `coordinator.showRawSource`). The suspension binding was not re-created anywhere, and `suspended` sat permanently `false` in production for three months (T-1099 reopened, 2026-08-10) even though every controller-level guard above stayed fully unit-tested — the tests exercised a gate nothing in production ever engaged. -The binding now lives at the **layout level**, one per host, each independent:-- `CompactDocumentLayout` and `RegularDocumentLayout` each carry their own `.onChange(of: coordinator.coordinatorOwnsModalPresentation, initial: true)` that sets `coordinator.renderedScroll.suspended = present` and `coordinator.rawSourceScroll.suspended = present` together.-- Both controllers are suspended regardless of which one `DocumentActions` currently targets: a modal (e.g. the document-note Cmd+Shift+N shortcut) can open while either surface is displayed, and `showRawSource` can flip while a modal is up.-- `initial: true` is load-bearing, not decorative — it covers a modal already open when a layout (re)mounts, e.g. immediately after a raw-source toggle.-- There is no SwiftUI view-hosting harness in this project (no ViewInspector, no `NSHostingController` test pump) to drive a real `.onChange` from a unit test. `ModalPresentationSuspendsScrollHostTests` (`prismTests/KeyboardScrollControllerTests.swift`) instead reads both layout sources and fails if either the `initial: true` binding or either `.suspended = present` assignment goes missing — the same source-structural shape `FootnotePresentationHostTests` uses for the T-1893 sibling of this failure mode.+**Then it was restored incomplete** (T-1099 reopened again, 2026-08-29). The 2026-08-10 re-fix bound suspension to `coordinator.coordinatorOwnsModalPresentation`, whose own doc comment said it covered only the coordinator's own five states and that layout-owned presentations were "tracked separately in each layout". Everything else that covers the document was therefore outside the gate: the compact TOC / notes / search sheets (`showTOCSheet` and `showNotesSheet` are `DocumentReaderView` `@State`, `showSearchOverlay` is `CompactDocumentLayout` `@State`), the mermaid/image zoom sheets, the iOS folder importer, and the paywall sheet. Meanwhile the compact layout's focus-restoration code kept its **own** list of the same three sheets, in a different file — two lists of "what counts as presented", which is what let them drift. -If this binding is ever refactored again (e.g. centralised back onto the coordinator), keep the host-level test in step, or the fix can regress silently a third time.+There is now **one gate**:+- `DocumentLayoutCoordinator.isDocumentModalPresented` unions `coordinatorOwnsModalPresentation`, `layoutOwnsModalPresentation` (pushed by whichever layout is mounted), `paywallPresented` (pushed by `DocumentReaderView`), `zoomMermaid`, `zoomImage`, and — on iOS only — `presentImageAccessPicker`. The iOS restriction is not tidiness: `imageAccessFolderPicker` is a no-op on macOS, so a flag set there would never be cleared and would suspend scrolling for the rest of the session.+- The paywall is pushed from `DocumentReaderView` rather than from a layout, because `PaywallPresenter` is per-scene (T-1779) and reaches that view through `@Environment` — `PaywallHost` attaches its plain `.sheet` at the `MainContentView` `NavigationStack` level, i.e. over the pushed document, exactly like the note sheets that were already gated. It is the one gate input `resetSessionState()` must **not** clear: the paywall belongs to the window, not the session, and survives a document switch untouched, so clearing the mirror would reopen the gate with the unlock screen still on screen.+- `DocumentLayoutCoordinator.applyScrollSuspension()` is the only production writer of `suspended`, and it sets both controllers. Both are suspended regardless of which one `DocumentActions` currently targets: a modal (e.g. the document-note Cmd+Shift+N shortcut) can open while either surface is displayed, and `showRawSource` can flip while a modal is up.+- `DocumentPresentationGate` (`prism/Views/DocumentPresentationGate.swift`) is the single `ViewModifier` both layouts apply via `.documentPresentationGate(...)`. It publishes the layout's own presentations, applies the suspension, and restores body focus when the gate opens — so focus restoration and suspension read the same predicate by construction.+- `initial: true` is load-bearing on both `.onChange`es. On the suspension it covers a modal already open when a layout (re)mounts (e.g. after a raw-source toggle). On the layout-state push it makes a **layout swap self-correcting**: `RegularDocumentLayout` passes `layoutOwnsModalPresentation: false`, which clears a value the compact layout left behind when a rotation or resize unmounts its sheets — otherwise the gate could be stranded closed with nothing on screen.+- `resetSessionState()` calls `applyScrollSuspension()` itself, because the layout `.onChange` fires on a transition and a session swapped while nothing was presented produces none.+- View-menu **enablement** runs off the same gate: `DocumentReaderView.makeDocumentActions` reports `canScroll`/`hasContent` as `… && !active.suspended`, read from the controller the menu actually drives.+- There is no SwiftUI view-hosting harness in this project (no ViewInspector, no `NSHostingController` test pump) to drive a real `.onChange` from a unit test. `ModalPresentationSuspendsScrollHostTests` (`prismTests/KeyboardScrollControllerTests.swift`) instead reads the sources and fails if either layout stops applying the modifier, if the compact layout's `layoutOwnsModalPresentation` stops naming all three of its sheets, or if the gate file loses its push / suspension / `initial: true` — the same source-structural shape `FootnotePresentationHostTests` uses for the T-1893 sibling of this failure mode. `DocumentPresentationGateTests` covers each owner behaviourally, asserting the contrast explicitly (old half-gate `false`, complete gate `true`).++If this is ever refactored again, keep those tests in step, and treat any **new** presentation over the document as a gate input — the two reopens were "the wiring vanished" and "the wiring was partial", in that order.  ## Layout-level focus restore (T-1103)  `bodyHasFocus` is owned at the layout level (`CompactDocumentLayout` / `RegularDocumentLayout`) as an `@FocusState` and passed to `RawSourceView` as `@FocusState.Binding`. The rendered path (`DocumentScrollContent`) does not take `bodyHasFocus` at all post-cutover — it has no `.onKeyPress` or focusable surface of its own — so this state and its restore triggers matter only while raw source is mounted.  Layout-level `.onChange` triggers that re-assert `bodyHasFocus` via `restoreBodyFocusIfIdle()`:-- Compact: `showTOCSheet`, `showNotesSheet`, `showSearchOverlay`, `coordinator.coordinatorOwnsModalPresentation`, and `scenePhase` (iOS).-- Regular: `session.isSearchActive`, `coordinator.coordinatorOwnsModalPresentation`, `controlActiveState` (macOS), and `scenePhase` (iOS).+- Both layouts: `coordinator.isDocumentModalPresented` falling to `false`, delivered by `DocumentPresentationGate`'s `onAllDismissed` (T-1099, 2026-08-29). It replaced the compact layout's four separate per-sheet watchers and the regular layout's `coordinatorOwnsModalPresentation` watcher, so focus can no longer be pulled back to the body while a second modal is still up.+- Compact: plus `scenePhase` (iOS).+- Regular: plus `session.isSearchActive`, `controlActiveState` (macOS), `scenePhase` (iOS).++Note the deliberate asymmetry inside the gate modifier: the suspension `.onChange` uses `initial: true`, the focus-restore one does not. Focus restoration is a transition, not a state — firing it on mount would pull focus to the body whenever a document appears. -The `coordinatorOwnsModalPresentation` watcher (T-1103) is required at the layout level because raw source replaces `DocumentScrollContent` rather than augmenting it, so a watcher scoped inside `DocumentScrollContent` would be unmounted while raw source is shown. This focus-restore `.onChange` is a separate modifier from the T-1099 suspension `.onChange` above — both are attached to each layout's body and fire independently off the same `coordinatorOwnsModalPresentation` value.+The watcher (T-1103) is required at the layout level because raw source replaces `DocumentScrollContent` rather than augmenting it, so a watcher scoped inside `DocumentScrollContent` would be unmounted while raw source is shown. `restoreBodyFocusIfIdle()` still guards on `session.search.isSearchActive` separately: the regular layout's inline search bar is not a modal and is not in the gate, but it does hold the text focus.  Known follow-ups:
specs/bugfixes/document-scroll-keys-route-behind-modals/report.md Modified +125 / -4
diff --git a/specs/bugfixes/document-scroll-keys-route-behind-modals/report.md b/specs/bugfixes/document-scroll-keys-route-behind-modals/report.mdindex c2001c8f..e95159bc 100644--- a/specs/bugfixes/document-scroll-keys-route-behind-modals/report.md+++ b/specs/bugfixes/document-scroll-keys-route-behind-modals/report.md@@ -1,9 +1,108 @@ # Bugfix Report: Document scroll keys still route behind modal presentations -**Date:** 2026-05-04 (original fix); reopened and re-fixed 2026-08-10+**Date:** 2026-05-04 (original fix); reopened and re-fixed 2026-08-10;+reopened and re-fixed again 2026-08-29 **Status:** Fixed **Ticket:** T-1099 +## Second reopening (2026-08-29) — the gate was incomplete++### Symptom++Unchanged in shape, different in scope. On the compact (iPhone) layout, open+the table of contents, notes, or search sheet — or a diagram/image zoom sheet+on either layout — and the View menu's **Page Down** / **Page Up** /+**Scroll to Top** / **Scroll to Bottom** still scroll the document+underneath. Raw source's own key handling does the same, since it shares the+gate.++### Root cause++The 2026-08-10 re-fix (PR #353) restored the binding, but bound it to+`coordinator.coordinatorOwnsModalPresentation` — a predicate whose own+doc comment said it covered only *coordinator-owned* presentations and that+"layout-owned presentations (search overlay, URL input, TOC sheet, notes+sheet) are tracked separately in each layout". So the fix was correct for the+five states that predicate names and silently absent for everything else that+covers the document:++| Owner | State | Where it lives | In the old gate? |+|---|---|---|---|+| Note popover, add-note, reply, document-note, footnote | `notePopoverBlock`, `addNoteBlock`, `replyToNote`, `showDocumentNoteSheet`, `activeFootnoteId` | coordinator | yes |+| Compact TOC sheet | `showTOCSheet` | `DocumentReaderView` `@State` → layout `@Binding` | **no** |+| Compact notes sheet | `showNotesSheet` | `DocumentReaderView` `@State` → layout `@Binding` | **no** |+| Compact search sheet | `showSearchOverlay` | `CompactDocumentLayout` `@State` | **no** |+| Mermaid zoom sheet | `zoomMermaid` | coordinator | **no** |+| Image zoom sheet | `zoomImage` | coordinator | **no** |+| Folder importer (iOS) | `presentImageAccessPicker` | coordinator | **no** |+| Paywall sheet | `PaywallPresenter.isPresented` | `MainContentView` `@State`, per scene | **no** |++Two of those eight are coordinator state that was simply never added to the+predicate; four are view-level state the coordinator structurally cannot+read. The compact layout's *focus-restoration* code already treated the three+sheets as modal — with its own private list, in a different file, kept in+step by hand — which is precisely the duplication that let the two lists+drift.++**Defect type:** incomplete predicate, kept incomplete by having two+independent lists of "what counts as presented".++### Fix++One gate, on the coordinator, that everything reads:++- `DocumentLayoutCoordinator.isDocumentModalPresented` unions the+  coordinator-owned half with the media zoom sheets, the iOS folder importer,+  and a new `layoutOwnsModalPresentation` flag carrying whatever the mounted+  layout presents that the coordinator cannot see.+- The paywall gets its own flag, `paywallPresented`, pushed by+  `DocumentReaderView` rather than by a layout: `PaywallPresenter` is+  per-scene (T-1779) and reaches that view through `@Environment`, and+  `PaywallHost` attaches its plain `.sheet` at the `MainContentView`+  `NavigationStack` level — i.e. over the pushed document, exactly like the+  note sheets that were already gated. It is the one gate input+  `resetSessionState()` deliberately does **not** clear: the paywall belongs+  to the window, not the session, and survives a document switch untouched, so+  clearing the mirror would reopen the gate with the unlock screen still up.+- `DocumentLayoutCoordinator.applyScrollSuspension()` is the only place that+  writes `KeyboardScrollController.suspended`, and it writes both controllers+  from that one gate.+- `DocumentPresentationGate` (new, `prism/Views/DocumentPresentationGate.swift`)+  is a single `ViewModifier` applied by **both** layouts. It publishes the+  layout's own presentations onto the coordinator, applies the suspension with+  `initial: true`, and restores document-body focus when the gate opens. The+  per-layout copies of that wiring — and the compact layout's separate+  four-watcher focus-restoration modifier — are gone, so there is nothing left+  to keep in step by hand.+- `RegularDocumentLayout` passes `layoutOwnsModalPresentation: false` (its TOC+  and notes surfaces are sidebars, its search an inline bar). With+  `initial: true`, that also *clears* a value the compact layout left behind+  when the two swap on rotation or a window resize — otherwise a sheet that+  unmounts with the compact layout could strand the gate closed.+- `DocumentReaderView.makeDocumentActions` now reports+  `canScroll`/`hasContent` as `… && !active.suspended`, so the View menu's+  scroll items are **disabled** while a modal is up rather than enabled and+  inert. Read from the controller the menu actually drives, so enablement and+  behaviour cannot disagree about which surface is gated.+- The folder importer counts on iOS only: `imageAccessFolderPicker` is a no-op+  on macOS, so nothing there would ever present — or therefore clear — that+  flag, and counting it would suspend scrolling for the rest of the session.+- `resetSessionState()` re-applies the suspension itself. The layout's+  `.onChange` fires on a *transition*, and swapping documents while nothing is+  presented produces none, which would leave a stale `suspended` standing.++### Why this one should hold++The previous two failures were both "the wiring went missing" (cutover) or+"the wiring was partial" (one of two lists). The gate is now a single+property with a single writer and a single modifier, and three kinds of test+sit on it: behavioural tests per omitted owner, a source-structural test that+both layouts apply the modifier, and a source-structural test that the compact+layout's contribution still names all three of its sheets. The behavioural+tests assert the omission explicitly — the old half-gate stays `false` while+the complete gate is `true` — so they document what was missed rather than+just what works.+ ## Reopening (2026-08-10)  The original fix (PR #231, described below) mirrored@@ -143,13 +242,35 @@ The keyboard scroll controller has no concept of being suspended while a modal o |------|--------| | `prism/Services/KeyboardScrollController.swift` | Added `suspended` flag and gating in `handleKeyPress` and all scroll command methods (original fix; unchanged by the reopening). | | `prism/Views/DocumentScrollContent.swift` | Original fix bound `suspended` here; removed by the T-1542 WebKit cutover along with `DocumentScrollContent`'s own `KeyboardScrollController`/`bodyHasFocus` ownership. No longer where the binding lives. |-| `prism/Views/CompactDocumentLayout.swift` | **(2026-08-10)** Added `.onChange(of: coordinator.coordinatorOwnsModalPresentation, initial: true)` mirroring presentation state into both `renderedScroll.suspended` and `rawSourceScroll.suspended`. |-| `prism/Views/RegularDocumentLayout.swift` | **(2026-08-10)** Same binding as the compact layout. |-| `prismTests/KeyboardScrollControllerTests.swift` | Regression coverage for suspension behaviour and coordinator presentation flag (original fix); **(2026-08-10)** added `ModalPresentationSuspendsScrollHostTests`, a host-level structural test pinning that both layouts apply the binding. |+| `prism/Views/DocumentLayoutCoordinator.swift` | **(2026-08-29)** Added `layoutOwnsModalPresentation`, the complete `isDocumentModalPresented` gate, and `applyScrollSuspension()` (the only production writer of `suspended`); `resetSessionState()` re-applies it. |+| `prism/Views/DocumentPresentationGate.swift` | **(2026-08-29, new)** The one modifier both layouts apply: publishes layout-owned presentations, applies the suspension with `initial: true`, restores body focus when the gate opens. |+| `prism/Views/CompactDocumentLayout.swift` | **(2026-08-10)** Added `.onChange(of: coordinator.coordinatorOwnsModalPresentation, initial: true)` mirroring presentation state into both `renderedScroll.suspended` and `rawSourceScroll.suspended`. **(2026-08-29)** Replaced by `.documentPresentationGate(...)`, feeding `showTOCSheet || showNotesSheet || showSearchOverlay`; the four-watcher `RestoreBodyFocusOnDismiss` modifier is gone, focus restoration now runs off the gate. |+| `prism/Views/RegularDocumentLayout.swift` | **(2026-08-10)** Same binding as the compact layout. **(2026-08-29)** Replaced by `.documentPresentationGate(layoutOwnsModalPresentation: false, …)`, which also clears a stale compact value on a layout swap. |+| `prism/Views/DocumentReaderView.swift` | **(2026-08-29)** View-menu enablement (`canScroll` / `hasContent` on `DocumentActions`) now folds in `!active.suspended`, so the scroll items disable rather than no-op behind a modal. |+| `prismTests/KeyboardScrollControllerTests.swift` | Regression coverage for suspension behaviour and coordinator presentation flag (original fix); **(2026-08-10)** added `ModalPresentationSuspendsScrollHostTests`, a host-level structural test pinning that both layouts apply the binding; **(2026-08-29)** added `DocumentPresentationGateTests` (one test per previously-omitted owner) and retargeted the host-level suite at the shared modifier, the compact layout's sheet list, and the gate file's own wiring. | | `docs/agent-notes/keyboard-scrolling.md` | Documented the suspension gate and removed the T-1099 follow-up entry (original fix). |  ## Verification +**Automated (2026-08-29 re-fix):**+- [x] Targeted run of `DocumentPresentationGateTests`,+  `ModalPresentationSuspendsScrollHostTests`,+  `DocumentLayoutCoordinatorModalPresentationTests`,+  `KeyboardScrollControllerTests`, `FootnotePresentationHostTests` — 49/49+  passed+- [x] `make lint` — 0 violations+- [x] `make build-ios`, `make build-macos`+- [~] `make test-quick` — 4638/4685 passed. The 7 failures are load-sensitive+  live-WebKit and notes-timing tests (`WebSearchScrollOwnershipTests`+  `.loadTimedOut` at 39s, `WebDetailsNavigationOrderingTests`,+  `WebScrollabilityReportingTests`, `WebScrollNavigationTests`,+  `NotesManagerLoadRaceTests`), none of which touch the presentation gate;+  the machine was running several parallel fix agents. A first attempt was+  discarded outright: the test host segfaulted inside+  `WebKit::DisplayLink::notifyObserversDisplayDidRefresh` on a CVDisplayLink+  thread, and the ~290 "failures" that followed were queued tests that never+  ran+ **Automated (2026-08-10 re-fix):** - [x] Targeted regression tests pass: `KeyboardScrollControllerTests`, `DocumentLayoutCoordinatorModalPresentationTests`, `ModalPresentationSuspendsScrollHostTests`, `FootnotePresentationHostTests` (`** TEST SUCCEEDED **`) - [x] Linters/validators pass (`make lint`)
CHANGELOG.md Modified +1 / -1
diff --git a/CHANGELOG.md b/CHANGELOG.mdindex bf353052..76a84bdf 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -57,7 +57,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - The unlock screen now belongs to the window that asked for it (T-1779). With more than one document window open the paywall was a single app-wide thing: reaching the free-export limit in one window put it in front of every window, closing it anywhere closed it everywhere, and a second blocked export elsewhere quietly took over the first one's place — so a completed purchase could run the wrong export, or none at all. An unlock started from Settings on the Mac appeared over a document window rather than over Settings. Each window, and the Settings window, now keeps its own unlock screen and its own memory of which export was waiting, so a purchase always finishes the export that prompted it and leaves other windows untouched. What you have bought and how many free exports remain are still shared across the whole app, as before. - The unlock screen now belongs to the window that asked for it (T-1779). With more than one document window open the paywall was a single app-wide thing: reaching the free-export limit in one window put it in front of every window, closing it anywhere closed it everywhere, and a second blocked export elsewhere quietly took over the first one's place — so a completed purchase could run the wrong export, or none at all. An unlock started from Settings on the Mac appeared over a document window rather than over Settings. Each window, and the Settings window, now keeps its own unlock screen and its own memory of which export was waiting, so buying from the unlock screen a blocked export raised finishes that export and leaves other windows untouched. What you have bought and how many free exports remain are still shared across the whole app, as before. - The unlock screen now belongs to the window that asked for it (T-1779). With more than one document window open the paywall was a single app-wide thing: reaching the free-export limit in one window put it in front of every window, closing it anywhere closed it everywhere, and a second blocked export elsewhere quietly took over the first one's place — so a completed purchase could run the wrong export, or none at all. An unlock started from Settings on the Mac appeared over a document window rather than over Settings. Each window, and the Settings window, now keeps its own unlock screen and its own memory of which export was waiting, so closing one leaves the others alone. Unlocking finishes the waiting export wherever you bought it: buy or restore from the unlock screen the blocked export raised, or from Settings while that screen sits open in another window, and closing that screen completes the export you were stopped on. Closing an unlock screen without buying still abandons the export it was raised for, as before. What you have bought and how many free exports remain are still shared across the whole app.-- Arrow keys, Page Up/Down, Space, and the View menu's **Page Down**/**Page Up**/**Scroll to Top**/**Scroll to Bottom** no longer scroll the document behind an open note, footnote, add-note, reply, or document-note modal (T-1099, reopened). This was fixed once before; the WebKit rendering cutover restructured the views that carried the fix and the binding was never rebuilt, so scroll commands kept reaching the document underneath a modal that should have blocked them. The gate is restored on both the iPhone and iPad/Mac layouts, taking effect immediately when a modal is already open and staying live across every presentation and dismissal.+- Arrow keys, Page Up/Down, Space, and the View menu's **Page Down**/**Page Up**/**Scroll to Top**/**Scroll to Bottom** no longer scroll the document behind a panel opened over it (T-1099, reopened twice). This was fixed once before; the WebKit rendering cutover restructured the views that carried the fix and the binding was never rebuilt, so scroll commands kept reaching the document underneath a panel that should have blocked them. Rebuilding it then covered only half of what can open over a document — a note, footnote, add-note, reply, or document-note panel — and left out the table of contents, notes, and search sheets on iPhone, the full-screen diagram and image views, the folder-access prompt, and the unlock screen, so the document still scrolled behind those. All of them now block scrolling through a single shared rule, which is also what the app asks before returning keyboard focus to the document, so the two can no longer disagree about what counts as open. While something is open, the View menu's scroll commands are shown as unavailable rather than staying available and doing nothing. The block takes effect immediately when something is already open as the document appears, lifts only once the last thing over the document closes, and follows the document as the layout changes with rotation or a window resize. - Replying to a document-level note is no longer silently discarded on a document that has only imported notes (T-1865). NotesPanel and SidebarNotesView create replies through a convenience method that used the document's saved user notes as its source of context; on a document where no user note had ever been created — only imported ones — that context was `nil`, so the guard returned early before the reply was ever built, leaving the tap with no visible effect and nothing written to disk. The method now falls back to the document's cached identifier, the same fallback its sibling document-note-creation method already used, so a reply always creates the note container it needs. - The safeguard that stops a broken document from reloading forever now holds when the crashes keep landing mid-load (T-2107). When a document's rendering process stops, the app reloads it, and if the reloads repeatedly fail to bring the document back it gives up after a few attempts and shows a banner offering a manual reload rather than retrying endlessly (T-1943 below). But a reload was counted as having succeeded the moment the page reported in — before it had finished laying out — so a renderer that reliably crashed in that window looked like a fresh failure each time instead of the same one continuing: the count started over on every attempt, and the document reloaded forever, which is exactly the loop the safeguard exists to prevent. A recovery now only counts as successful once the reloaded document has actually settled on screen, so crashes landing in that window accumulate toward the limit and reach the banner. Recoveries that do bring the document back still reset the count, and the banner's reload still restores everything as before. - Completing the unlock purchase no longer sometimes leaves the app still locked (T-1868). To know whether you have bought unlimited exports, the app asks the App Store — when it starts, and each time you bring it back to the front — and that question takes a moment to come back. If your purchase completed while an answer to an earlier question was still on its way, the app recorded the purchase and was then told, by that older answer, that you had bought nothing: the paywall came back and the export the purchase was meant to unblock was lost, even though the purchase had gone through and you had been charged. Restoring purchases on a new device could be undone the same way, by a question asked before the restore and answered after it. The reverse ordering hid a refund, leaving the unlock in place until the next check. Answers are now ranked by when the information behind them was gathered rather than by when they happen to arrive, so the most recent word on your purchase is the one that stands and an older answer can no longer overwrite it. A check cut short before it finishes is also no longer mistaken for "nothing purchased".

Things to double-check

iPhone: open TOC / notes / search sheet, press Page Down on a hardware keyboard.

Document must not scroll; View menu items must appear disabled. Then dismiss and confirm scrolling and focus return.

iPad rotation with the compact TOC sheet open.

After the compact-to-regular swap, confirm scrolling works (gate cleared by the regular layout's false push) and that the focus jump to the body is acceptable.

macOS: zoom a diagram, then use Page Down in the document window.

Confirm there is no lingering suspension after the gate flap, and that raw-source focus behaviour is unaffected.

Blocked export raises the paywall; switch documents via reload-in-place.

Paywall flag must survive resetSessionState and scrolling must stay suspended until the sheet closes.