PR #315 by @ArjenSchwarz · T-1719/bugfix-webkit-state-integration → main · view on GitHub
WebDocumentStateSynchronizer becomes the single, view-independent owner of state pushes and navigation routing.Ready to merge
All three tickets are now genuinely fixed and the issues review raised have been addressed in-branch. Review independently cleared the two highest-risk areas: the withObservationTracking re-arm in WebDocumentStateSynchronizer has no retain cycle and no infinite loop, and the BlockDOMID string-id parsing is unambiguous by construction.
The most valuable outcome of the review was disproving the PR's own claim that T-1662 was already fixed. It was not — TOC and fragment navigation was still broken for every heading following a <details> block, and section expansion was failing on the same wrong id. That is now fixed properly, with a regression test proven red-before-green.
Remaining items are follow-ups rather than blockers: a dual-owner search-scroll interaction (tracked as T-1918), some dead production API left behind by the cutover, and spec/decision-log updates for the new synchronizer.
Shown verbatim — the markdown the author wrote, unmodified.
Fixes the P0.1 rendering-cutover regression cluster: three tickets that all land in the same navigation/presentation code, so they ship together rather than conflicting with each other. ## T-1719 — WebKit navigation and state integration (rebased) The original work rebuilt the state/navigation wiring the ScrollViewProxy cutover severed: `WebDocumentStateSynchronizer` is the single production owner (observation-driven, view-independent, mounted via `makeAssembly`); details and table state replay exactly; `scrollDirectionChanged` restores compact hide-on-scroll; keyboard/menu scrolling routes through the bridge. This branch sat as a conflicting draft while T-1680 (#314) and T-1681 (#316) landed. Rebased onto `main`, with two conflicts resolved: - `DocumentScrollContent.swift` — `main` added T-1680's search-highlight push inside the block this change deletes. Resolved by keeping the synchronizer as the owner of note/section/details/table pushes while search highlights stay a deliberate view-fed seam (their payload depends on debounced `SearchCoordinator` state this view already observes). This is exactly the seam the original T-1719 work left for T-1680 — the two designs agree. - `CHANGELOG.md` — additive both sides; both entries kept. `WebSearchWiringTests` (added on `main`) assembles a controller to mirror `DocumentScrollContent` "call for call" and called the factory's `pushNoteState`, which this change removes in favour of the synchronizer. Its helper now builds the assembly through `makeAssembly` + `start()`, which its own docstring requires. ## T-1662 — TOC / fragment anchor DOM ids **Verified resolved by T-1719; no separate code change.** Every navigation-target producer — ToC entry taps (`RegularDocumentLayout:215,:324`, `CompactDocumentLayout:188,:369`), in-document `#fragment` links (`WebDocumentMessageRouter:296` → `session.scrollToAnchor`), and restore-from-recent — sets `session.pendingAnchorScroll`, which `WebDocumentStateSynchronizer.scrollToTarget` translates through `BlockDOMID.navigationDOMID`. Covered by `WebStateSynchronizerAssemblyTests.tocNavigationTargetsWebDOMID` plus the existing `AnchorNavigationDuplicateHeadingsTests`. ## T-1893 — Footnote taps do nothing in the regular layout Footnote badge taps did nothing on macOS and regular-width iPad. The presentation observing `coordinator.activeFootnoteId` was written inline in `CompactDocumentLayout` only, and `DocumentReaderView` never mounts that layout on macOS or wide iPad — so the tap updated state, showed no UI, and left `coordinatorOwnsModalPresentation` true with nothing on screen. Extracted into a shared `FootnotePresenter` modifier applied by **both** layouts, following the `MediaZoomPresenter` pattern already used for media-zoom and image-access presentations. `FootnotePopoverView` already sizes itself per platform, so compact behaviour is unchanged. New regression tests pin both layouts as presentation hosts, so they cannot drift apart again. Report: `specs/bugfixes/footnote-popover-regular-layout/report.md` ## Verification - `FootnotePresentationHostTests` — committed red first (failed for both layouts before the fix), green after - `WebStateSynchronizerAssemblyTests`, `WebScrollIntegrationContractTests`, `WebSearchWiringTests`, `OffMainEmitTests` — all pass together post-rebase - `make build-macos` / `make build-ios` — zero warnings, zero errors - `make lint` — 0 violations across 491 files - Full macOS unit suite compared against the `origin/main` baseline: no new failures (both runs hit the same pre-existing crash-cascade and network/timing-dependent failures — T-1541, T-1457)
be1ff97 T-1719: Add investigation report and failing regression tests 4fb67fe T-1719: Rebuild the WebKit navigation/state integration the cutover severed 0fa73e7 T-1719: Finalize bugfix report, solution comparison, changelog 9386833 T-1719: Rebase onto main and reconcile with T-1680/T-1681 ec1c055 Fix T-1893: Present footnotes in the regular layout 49d0f53 T-1719/T-1662: Document rebase reconciliation and TOC id verification 2f39deb T-1893: Record full-suite baseline comparison in the bugfix report bb4ba5c T-1719: Walk blocks once per sync pass, and cache the walk per parse a2fddce T-1662: Correct the verification claim — the ticket stays open 573cd18 Fix T-1662: Give TOC entries a resolvable scroll target Prism recently swapped the engine that draws your markdown document. It used to draw everything with Apple's own UI toolkit; now it renders the document as a web page inside the app. That swap worked visually, but it quietly cut several wires — features that used to work stopped working, without any error appearing.
This pull request reconnects those wires and fixes a second, separate bug:
These are all invisible failures. Nothing crashes and no error appears; the feature just does nothing. That is the worst kind of bug to leave in, because a user assumes they tapped the wrong thing.
Two names for the same thing. The app identifies each paragraph or heading by a short fingerprint of its content. The web page identifies the same paragraph with a slightly different label. Any time the app says “scroll to this one”, something has to translate between the two. This PR puts that translation in exactly one place, so there is one piece of code to get right.
Don't attach important work to a screen. The original bug happened because the wiring lived inside a piece of the screen. When that piece was not on display, the wiring did not exist. The fix moves it into an object that lives as long as the document does, whether anything is on screen or not.
Write the same thing once. The footnote bug existed because the pop-up was written into one layout and simply forgotten in the other. The fix writes it once and has both layouts use that one copy, so they cannot drift apart again.
The PR claimed a third bug (the table-of-contents one) was already fixed. Review found it was not: in any document with a collapsible section, every heading after it could not be reached from the table of contents, because the entry was numbered by counting the collapsible block's hidden contents while the document counts that block only once. It has now been properly fixed, and headings inside a collapsible section — which previously went nowhere at all — now scroll to the section containing them.
The T-1542 cutover replaced the SwiftUI/Textual in-flow renderer with WebKit. Native stayed the
source of truth — parsing, search counting, notes and persistence all run natively; the web view
only renders HTML and reports interactions back over a bridge. What the cutover missed is that a
lot of native→web state pushing was implemented as SwiftUI .onChange modifiers
hanging off the document body view. Several of those closures were not even attached any more, so
the behaviour silently died.
The fix introduces WebDocumentStateSynchronizer: an @MainActor final class
that owns every native→web push and every navigation route. It runs on
withObservationTracking — one tracked read computes every domain (typography, comment
visibility, note payloads, section collapse, details open-state, table modes, navigation targets),
then re-arms; a dirty-flag diff pushes only what changed. Because tracking disarms on the first
fire, a burst of same-turn mutations coalesces into a single scheduled pass.
Critically it is mounted through makeAssembly, a factory that builds controller +
message router + synchronizer exactly as production does — and the new tests mount that same
factory. The original bug class was “tests exercise an assembly production doesn't use”,
so the factory is the regression guard.
FootnotePresenter follows the existing
MediaZoomPresenter shape: a ViewModifier plus a View extension,
applied by both layouts. The bug being fixed was the absence of this pattern.BlockDOMID handles occurrence-qualified DOM ids,
legacy composites, sub-block row/item anchors and bare content hashes, and verifies rather than
trusts — a composite whose hash no longer sits at its recorded index is treated as stale and dropped.prism-scroll.js posts direction flips and
threshold crossings, not per-frame scroll offsets.Coalescing vs. granularity. One pass computing all domains is simple and correct,
but the dirty diff sits after computation, so it saved the bridge hop and none of the CPU.
Review found the pass rebuilding the full note payload and walking every block 3–4× on every
mutation — including search keystrokes and details toggles, which have nothing to do with notes.
Fixed by walking once per pass and caching that walk per parseRevision. The deeper
version (computing domains lazily, only when dirty) is left as follow-up.
Sheet vs. popover. The footnotes spec asks for an anchored popover on iPad/macOS. A web-rendered badge has no SwiftUI anchor view, and the regular layout already presents its note editors as sheets, so a sheet was chosen — a deliberate supersession of Req 3.1/3.6/3.7 that still needs a decision-log entry.
Structural tests. The T-1893 guard asserts on source text because evaluating layout view bodies in tests is a known suite-crasher here. It is honest about the constraint but brittle: it would pass on a comment. Composing the shared presentations into one modifier would make the drift impossible and retire the test.
The re-arm loop is the highest-risk construct here and it holds up. synchronize() wraps
computePass() in withObservationTracking with an onChange that hops
back to the MainActor and calls synchronize() again. Three properties make it safe:
tracking disarms on the first fire (so a same-turn mutation burst yields one pass, not N);
computePass never reads the WebDocumentController, so pushes cannot feed back into
the tracked set; and dispatch's only tracked writes are the guarded nil-clears of
pendingAnchorScroll / noteNavigationTarget, so the documented settle pass reads
nil targets, dispatches nothing and terminates. [weak self] means a discarded synchronizer's
armed registration fires at most once more as a no-op — one stale closure per remount on the
long-lived AppSettings/NotesManager registrars, bounded rather than growing.
MarkdownBlock.id looks memoized but only the SHA-256 is cached: every access rebuilds
contentForHashing (recursively concatenated for .details/.list) and takes
the process-global BlockIDCache mutex to hash that string. BlockDOMID.map touches
block.id once per block, so each unshared walk is O(total content), not O(N) pointer chasing.
The pass was performing 3–4 of them plus the note payload's own walks and two JSONEncoder
encodes with .sortedKeys, on every tracked mutation.
The fix threads a BlockDOMID.Mapping through navigationDOMID/restoreDOMID/
firstOccurrenceDOMID/NoteStateFeeder.payloads, and caches the mapping on
parseRevision. The cache key is sound because parsedBlocks is assigned exactly once
per parse with parseRevision &+= 1 in the same synchronous stretch — no await between them —
which is the same invariant the T-1681 document-HTML cache relies on. The cache lives on a
non-@Observable class, so mutating it inside the tracked read registers no dependency and
cannot retrigger a pass. visibleSourceIndices moved into scrollToTarget: it was an
O(N) map + Set build per pass feeding a rarely-taken branch, and its dependency is already covered
transitively because visibleBlocks only recomputes when collapsedSectionIds changes.
Two index spaces are conflated. TOCCoordinator.buildTOCEntries threads an
inout runningIndex that increments for a .details block and recurses into its
children, incrementing per child. But .details is a single top-level block holding its
children nested (DetailsBlockParser parses the body into a child array), so those children
occupy no top-level index. MarkdownSectionBuilder and BlockHTMLEmitter both index by
blocks.enumerated(), and BlockDOMID.restoreDOMID validates
mapped[sourceIndex].block.id == hash against that top-level mapping.
For [h1, details(2 children), h2, p]: h2 is at top-level index 2, but the TOC assigns
it runningIndex 4 (h1→0, details→1, children→2,3). restoreDOMID then fails the
bounds check outright (4 < 4 is false) and returns nil; where the index happens to be in range
the hash comparison fails instead. Either way navigationDOMID returns nil and
scrollToTarget silently skips — the exact “stale target” path the design intends,
firing on ids that are not actually stale. Headings inside <details> are worse:
emitDetails gives non-details children no section id, so no anchor exists at all.
Fixed by splitting the field. TOCEntry.scrollTargetId now carries the top-level composite and scrollId returns it, while id keeps the flattened counter for Identifiable. Renumbering id was not an option: headings nested inside <details> share their ancestor's top-level slot, so it would stop being unique. TOCCoordinator threads a topLevelAnchor through its recursion, and nested headings — which previously resolved to nothing at all — now target the enclosing details block. The same correction repairs section expansion, since DocumentSession.scrollToAnchor feeds the identical id to expandSectionAndAncestors, which matches MarkdownSection.id built from the same top-level index.
The verification gap is instructive: tocNavigationTargetsWebDOMID constructs its composite as
"\(heading.block.id)-\(heading.index)", i.e. it feeds the resolver a correct id and
asserts the resolver works. It never reads toc.tocEntries[].scrollId, so the producer is
untested. AnchorNavigationDuplicateHeadingsTests asserts pendingAnchorScroll == entry.scrollId
and stops there. Both halves pass; the composition is what breaks. Any fix must assert through the
real producers.
Search-match scrolling now has two owners — the synchronizer's block-level
scrollIntoView({block:"start"}) and prism-search.js's range-level
viewportHeight/3 positioning — dispatched as independently queued bridge commands. They
converge (the JS side no-ops when the match is already on screen) but the final anchor is
ordering-dependent. They are plausibly layered on purpose, since prism-search.js only
registers highlights for in-window blocks and therefore needs the native scroll to bring a distant
block into range first. That interaction deserves an explicit decision, especially with T-1839
(matches outside the highlight window) still open.
Also outstanding: pushInitialState is production-dead with a live test caller — the same
“tests exercise what production doesn't” shape that caused T-1719 — along with
expandedBlockIds, willExpand and LayoutContext.useBlockFootnotePopovers.
The nested-details id format "\(domID)-d\(index)" is constructed independently in the emitter
and the expansion coordinator despite BlockDOMID existing to prevent exactly that drift.
prism/ViewModels/WebDocumentStateSynchronizer.swift
Why it matters. This is the architectural heart of the PR. State pushes and navigation used to hang off SwiftUI .onChange handlers in the document view, so they silently stopped running whenever that view was not mounted — which is exactly how the cutover broke TOC taps, hide-on-scroll and details/table replay. The synchronizer is observation-driven and view-independent, so the wiring survives view lifecycle.
What to look at. WebDocumentStateSynchronizer.swift:144-250 (synchronize / computePass / dispatch)
prism/ViewModels/WebDocumentStateSynchronizer.swift
Why it matters. The pass fires on any tracked mutation, and it rebuilt the entire note payload and walked every block 3-4x each time. block.id rebuilds the block's content string and takes a global mutex per access, so this was O(content) over the document on interactions unrelated to notes — on the main actor, the same cost class T-1681 had just removed.
What to look at. computePass + mapping(for:) — added during review
prism/Views/FootnotePresenter.swift
Why it matters. The T-1893 bug was that footnote presentation existed only in CompactDocumentLayout while DocumentReaderView never mounts that layout on macOS or wide iPad. Copying the sheet into the regular layout would have fixed the symptom and preserved the duplication that caused it.
What to look at. prism/Views/FootnotePresenter.swift:1-61, applied in both layouts
prism/Services/WebRendering/BlockDOMID.swift
Why it matters. Native carries several id formats (occurrence-qualified DOM ids, legacy composites, sub-block row/item ids, bare content hashes) and the DOM understands only one. Funnelling every translation through one verified seam is what makes TOC/notes/search navigation resolvable at all.
What to look at. BlockDOMID.swift:68-160 (navigationDOMID and the mapped: overloads)
prism/Resources/WebRenderer/prism-scroll.js
Why it matters. The iPhone toolbar hide-on-scroll behaviour depended on SwiftUI scroll geometry that no longer exists. This adds a bridge message posting direction flips and the hide-threshold crossing, rather than per-frame scroll positions.
What to look at. prism/Resources/WebRenderer/prism-scroll.js:99-150 + BridgeMessageRouter allowlist
prism/Models/TOCEntry.swift
Why it matters. One field was serving two incompatible purposes — uniqueness for Identifiable (which needs a counter that never repeats) and a navigation target (which must match the top-level block index). Those requirements conflict the moment a <details> block appears, and the conflict silently broke both scrolling and section expansion.
What to look at. TOCEntry.scrollTargetId + TOCCoordinator.buildTOCEntries topLevelAnchor threading
setSearchState push into the very block this PR deletes. It was resolved by keeping the synchronizer as owner of note/section/details/table pushes while search highlights remain pushed from DocumentScrollContent on mount and on searchStateKey change. Review pushed back on this: pushSearchState needs only session and settings, both of which the synchronizer holds, so the seam arguably reproduces the exact failure class T-1719 fixes — a push that stops working if the view is not mounted. Worth an explicit decision either way.specs/footnotes/requirements.md 3.1/3.6/3.7 specify an anchored popover, max 350pt wide, on iPad and macOS. The web-rendered badge has no SwiftUI anchor view to attach a popover to, and the regular layout already presents all its note editors as sheets. This supersedes those requirements and should be recorded in the footnotes decision log.open flag. The synchronizer now pushes setDetailsState and consumes detailsToggled echoes, making native authoritative. This inverts what design.md documented and warrants a decision-log entry. (inferred — not stated by the author.)id to the top-level index would have made it non-unique for headings nested inside <details> (they share their ancestor's slot), breaking Identifiable. Adding scrollTargetId keeps identity intact and confines the change to the navigation path. Nested headings target their enclosing details block — the nearest ancestor with a DOM anchor, since the emitter gives non-details children no section id.| Severity | Area | Finding | Resolution |
|---|---|---|---|
| major | T-1662 — TOC/fragment navigation with <details> | The PR claimed T-1662 was verified fixed with no code change. It was not. TOCEntry.id is numbered by a FLATTENED counter that also advances through a <details> block's nested children, but .details is a single top-level block, and both MarkdownSection.id and BlockDOMID verify a composite's trailing index against the top-level parsedBlocks enumeration. For [h1, details(2 children), h2, p] the TOC gave h2 the id {hash}-4 while h2 sits at index 2; restoreDOMID rejected it (4 < count 4 is false). Every heading after a <details> block was unreachable from the TOC and from #fragment links, and headings inside <details> had no destination at all. It broke TWO consumers on the same id: BlockDOMID scrolling, and expandSectionAndAncestors section expansion. The original verification traced that producers REACH the seam but never that the ids they emit SATISFY it. | Fixed. TOCEntry gains a stored scrollTargetId built from the top-level index (scrollId returns it) while id keeps the flattened counter for Identifiable, so existing consumers are untouched; TOCCoordinator threads a topLevelAnchor through its recursion so nested headings target the enclosing details block. New regression test goes through the real producer (TOCCoordinator.tocEntries), and red/green was proven by reverting the fix — the four <details> tests fail without it while the control and identity tests pass in both states. |
| major | WebDocumentStateSynchronizer performance | Every observation pass rebuilt the full note payload and walked BlockDOMID.map 3-4x (note payloads, table-mode translation, search-match lookup, each navigation resolve). A pass fires for ANY tracked mutation — a search keystroke, a <details> toggle, a section collapse — not just note changes, where the deleted code used four targeted .onChange handlers. block.id is not O(1): it rebuilds the block's content string and takes the global BlockIDCache mutex on every access, so each extra walk is O(content) across the whole document. Same class of main-thread cost T-1681 just removed for HTML emit. | Added BlockDOMID.Mapping plus mapped: overloads of navigationDOMID/restoreDOMID/firstOccurrenceDOMID (blocks: forms kept as delegating wrappers); NoteStateFeeder.payloads and noteStatePayloads accept a pre-walked mapping; the synchronizer now walks once per pass AND caches it keyed on parseRevision, so passes between re-parses reuse it entirely. |
| minor | DocumentLayoutCoordinator.applyScrollDirection | isCompactToolbarVisible was assigned on every scroll-direction flip regardless of its current value. @Observable notifies on assignment, not on change, so every 'up' flip invalidated the compact layout body and re-armed its animation even when the toolbar was already visible. The pre-cutover updateToolbarVisibility guarded both branches. | Guarded both branches so only genuine visibility changes notify. |
| minor | DetailsExpansionCoordinator.applyDetailsToggle | setDetailsState writes .open on every <details> element, and each element that flips echoes a detailsToggled message back. applyDetailsToggle inserted/removed unconditionally, and even a no-op Set mutation triggers the observable write — so a single user toggle cost two full synchronizer passes. | Made it change-detecting, so the echo is a genuine no-op. |
| minor | CHANGELOG accuracy | The T-1719 entry claimed table display modes and <details> state 'survive a document reload or a WebKit process recovery exactly as left'. DocumentSession.reloadContent explicitly calls expansionCoordinator.reset() and tableDisplayModes.removeAll(), so the file-change/URL-refresh reload path re-seeds from the document by design. The user-facing release note over-claimed. | Reworded to name the paths actually covered (WebContent recovery, raw/rendered remount, image-access re-fetch) and to state that a content reload re-seeds. |
| minor | CLAUDE.md footnote architecture | CLAUDE.md still described the footnote popover as 'a popover (iPad/macOS) or sheet (iPhone)' — the exact line T-1893 invalidated by making both layouts present a sheet through one shared modifier. The PR's single-line CLAUDE.md edit missed it. | Rewritten to describe the shared FootnotePresenter, note that both layouts must apply it, and record why the anchored popover no longer applies (the web-rendered badge has no SwiftUI anchor view). |
| minor | Search-match scrolling has two owners | The synchronizer scrolls to the current match's block (deduped per block, scrollIntoView block:'start') while prism-search.js scrollCurrentIntoView also scrolls the match (every setSearchState push, off-screen only, targets viewportHeight/3). Both fire on a cross-block navigation as independently queued bridge commands, so the final anchor is ordering-dependent and a double-jump is possible. | Filed as T-1918 rather than changed here. They are plausibly layered on purpose — prism-search.js only registers highlights for in-window blocks, so the native scroll is what brings a distant block into range before JS fine-tunes within it. Collapsing them blind risks regressing T-1680 and interacts with the open T-1839. |
| minor | Dead production API left behind | WebDocumentControllerFactory.pushInitialState is now production-dead — its theme/typography/comment-visibility duties moved into the synchronizer, and only HTMLCommentVisibilityLiveTests still calls it. This is the same 'tests exercise an API production does not use' pattern the PR's own report names as the root cause of T-1719, and its Prevention section says to delete retired integration API in the same change. DetailsExpansionCoordinator.expandedBlockIds / isExpanded / willExpand and LayoutContext.useBlockFootnotePopovers are dead for the same reason. | Not changed — deleting production API and migrating its tests is a cleanup beyond this PR's three tickets, and pushInitialState still has a live test caller that would need converting to makeAssembly. Recommended as an immediate follow-up chore. |
| minor | Spec + decision log not updated | specs/webview-rendering/design.md enumerates the complete bridge message set and the production component list; this PR adds an inbound message (scrollDirectionChanged) and a new production component (WebDocumentStateSynchronizer) without touching either. No decision-log entry records moving native→web sync out of SwiftUI .onChange into an Observation-driven non-view owner, nor details open-state becoming native-authoritative in both directions — which inverts what design.md previously stated. The project mandates the Enhanced Nygard ADR format and the log already carries narrower decisions. | Not changed in this pass — recorded here so the author can add Decision 16 (+ a footnotes entry for the sheet-over-popover change) with the rationale, which already exists in the bugfix reports. |
| minor | FootnotePresentationHostTests is source-scanning | The T-1893 wiring guard reads the two layout .swift files from disk and asserts they contain the string '.footnotePresentation('. It cannot tell a live modifier from one in a dead branch, it passes on a comment — and both layouts have a comment naming the modifier directly above the call — and presentationReadsCoordinatorState asserts that FootnotePresenter.swift contains the identifiers its own implementation uses, which is tautological. | Kept for now: view-body evaluation in tests is a known suite-crasher in this project (T-1541), and the two coordinator tests are genuinely useful. The durable fix is structural — compose the shared presentations into one .documentPresentations(coordinator:session:) modifier so a future presentation reaches both layouts by construction and the source scan becomes unnecessary. Recorded as follow-up. |
| minor | Duplicated ids and constants across boundaries | The nested-<details> DOM id format is built independently in BlockHTMLEmitter ("\(domID)-d\(index)") and DetailsExpansionCoordinator, despite BlockDOMID existing precisely so the emitter and feeder cannot drift; a suffix change silently breaks setDetailsState replay for nested details with no compile error. Separately, prism-scroll.js hardcodes HIDE_THRESHOLD = 50 mirroring DocumentLayoutCoordinator.compactToolbarHideThreshold, bound only by a comment, and the sub-block-suffix regex in BlockDOMID is byte-identical to one in InlineNotesExporter. | Not changed — each is a small consolidation but they touch the emitter and exporter, outside this PR's scope. Recommended: BlockDOMID.nestedDetailsID(parent:childIndex:), feed the threshold through the page's existing config payload, and share one sub-block-suffix helper. |
Click to expand.
diff --git a/CHANGELOG.md b/CHANGELOG.mdindex faa8fdb1..c52d7e64 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -20,6 +20,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Opening or reloading a large or HTML-heavy document no longer freezes the UI (T-1681). Since the WebKit rendering cutover, the full document HTML — including a SwiftSoup sanitisation pass per raw-HTML block and per inline HTML run — was built synchronously on the main thread on every serve, so a big or markup-dense document blocked the app while it rendered, and re-built the HTML on every reload. The build now runs off the main thread and its result is cached per parse: the UI stays responsive, and reloads (external file change, URL refresh, WebContent-process recovery, the iOS folder-access retry) reuse the cached HTML instead of re-emitting. Very HTML-dense documents can still take a noticeable moment to appear; making that incremental is tracked separately. - Search matches are highlighted in the rendered document again (T-1680). Since the WebKit rendering cutover, searching counted matches and navigated natively but the page never showed a highlight — the native→web search-state feed was never connected. Matches now light up as you type, the current match gets its distinct emphasis and scrolls into view when navigating (including when a result is picked from the iPhone search overlay), footnote badges whose content matches are marked, and dismissing search clears the highlights.+- Navigation and display state reach the rendered document again (T-1719). The rendering-engine cutover left core behaviours attached to a retired scroll surface, so they silently stopped running: tapping a table-of-contents entry, a note, or a search result now scrolls the document again; the iPhone bottom toolbar hides when scrolling down and returns when scrolling up; table display-mode choices and expanded/collapsed `<details>` sections now survive a WebKit process recovery, the raw/rendered toggle, and the image-access re-fetch exactly as left (including collapsing a section that was open by default; reloading changed file content still re-seeds them from the document, as designed); and the macOS View-menu scroll commands (Page Up/Down, Top, Bottom) work on the rendered document. A new `WebDocumentStateSynchronizer` owns keeping the page in sync with native state independent of any view being mounted, backed by production-assembly regression suites (`specs/bugfixes/webkit-state-integration/report.md`).+- Table-of-contents entries and `#fragment` links work again in documents containing collapsible sections (T-1662). Once a `<details>` block appeared, every heading after it became unreachable: tapping its table-of-contents entry did nothing, and a link to it did nothing, because the entry was numbered by counting the collapsible block's hidden contents while the document itself counts the block only once. Headings *inside* a collapsible section previously had no destination at all and now scroll to the section containing them. Collapsed sections also expand correctly again when navigating to a heading after a collapsible block.+- Tapping a footnote badge opens the footnote again on Mac and on iPad in wide windows (T-1893). The footnote popover was only ever attached to the iPhone layout, so on every Mac window and every regular-width iPad window a badge tap registered but showed nothing. Both layouts now present the footnote through one shared presentation, so they cannot drift apart again. - Task list items written `- [ ] 1. Task name` render their number again (T-1640). Since the WebKit rendering cutover, inline text beginning with a list marker (`1.`, `3)`, `-`) was re-parsed as a list and the marker silently dropped — this also affected headings like `## 1. Introduction` and table cells starting with a number. The literal marker the author wrote is now rendered, and text selection and search work on it. - Text beginning with an `@`-prefixed word (for example a list item `- @Observable macro is used`) renders again instead of showing as empty (T-1641). Since the WebKit rendering cutover, the inline re-parse enabled block directives, so `@Observable`, `@MainActor`, `@State` and similar leading words were consumed as a directive and the whole run was dropped. This affected paragraphs, headings, and table cells as well as list items; all now render the text verbatim with working selection and search. - Reading position is retained again on the new rendering engine (T-1639): toggling between rendered and raw source returns to the same place in both directions, reopening a document restores the last position (including positions saved before the engine cutover), and closing or switching documents now saves the position immediately instead of only when the app goes to the background. Programmatic jumps (restore, table-of-contents navigation) also update the saved position once the scroll settles.
diff --git a/CLAUDE.md b/CLAUDE.mdindex 47d0463d..f9e5f00d 100644--- a/CLAUDE.md+++ b/CLAUDE.md@@ -50,7 +50,7 @@ The document is rendered by WebKit-for-SwiftUI (`WebView`/`WebPage`). The SwiftU 1. **Parse**: `swift-markdown` → AST → `MarkdownBlock` enum variants (`MarkdownBlockParser`), unchanged from before. T-1558 made the model lossless for nested blockquotes, ordered-list `start`, and rich blocks inside list items (see `specs/web-markdown-fidelity/`). 2. **Emit**: `BlockHTMLEmitter` (`prism/Services/WebRendering/`) is a pure, deterministic function of `[MarkdownBlock]` + `FootnoteData` + `RenderSettings`. It emits one `<section>` per block carrying the content-hash block identity and an occurrence-qualified DOM id (`b-{hash}-{sourceIndex}`, allocated via the shared `BlockDOMID`), escapes by default, and is total (a block that fails to emit falls back to escaped-source `<pre>`, never dropped). `InlineHTMLRenderer` wraps mappable text runs in `<span data-prism-run>` and records a `DocumentSourceMap` (UTF-16 offsets, shipped as an inert `<div hidden>` data island) for selection-anchored notes. `emit` (and the model/service value types it reads) is `nonisolated`, so it runs off the MainActor: `WebDocumentControllerFactory.precomputeDocumentHTML` emits once per `parseRevision` on a `Task.detached` and caches the HTML on `DocumentSession`; the scheme handler serves that cache (synchronous on-main emit only on a miss). Its per-block/inline `HTMLSanitizer` (SwiftSoup) passes are serialized behind a shared `Mutex` because SwiftSoup keeps unsynchronized static pools (T-1681, `specs/offmain-html-emit/`). 3. **Serve**: `PrismDocSchemeHandler` (`prism-doc://` `URLSchemeHandler`) is the single audited I/O path — it serves the document HTML, `document.css` (the only asset fetched through the scheme), and mediates every image subresource through `/img/?src=` (rewritten absolute/relative URLs routed via `ImagePathResolver`/`ImageLoader`/`SVGSourceLoader`). It serves the verbatim CSP (`script-src 'none'`, `connect-src 'none'`, …) as a response header. The document is loaded via the scheme, never `loadHTMLString`.-4. **Host + bridge**: `WebDocumentController`/`WebDocumentView` (`prism/ViewModels/`, `prism/Views/`) own one `WebPage` per session (non-persistent store, `allowsContentJavaScript = false`, all JS injected as user scripts via `WebDocumentControllerFactory.userScripts()`). The native↔JS bridge runs in a dedicated isolated `WKContentWorld`; every message in both directions carries a generation tag (sessionID, parseRevision, processGeneration). `BridgeMessageRouter` validates inbound messages against an enumerated allowlist + exact-generation match and drops forged/stale/malformed ones; `WebDocumentMessageRouter` routes accepted messages onto the existing native session/coordinator state. Outbound commands queue until `ready`; scroll restore waits for `layoutSettled`; on WebContent termination the controller bumps the process generation, reloads, and replays one coalesced state snapshot.+4. **Host + bridge**: `WebDocumentController`/`WebDocumentView` (`prism/ViewModels/`, `prism/Views/`) own one `WebPage` per session (non-persistent store, `allowsContentJavaScript = false`, all JS injected as user scripts via `WebDocumentControllerFactory.userScripts()`). The native↔JS bridge runs in a dedicated isolated `WKContentWorld`; every message in both directions carries a generation tag (sessionID, parseRevision, processGeneration). `BridgeMessageRouter` validates inbound messages against an enumerated allowlist + exact-generation match and drops forged/stale/malformed ones; `WebDocumentMessageRouter` routes accepted messages onto the existing native session/coordinator state. Outbound commands queue until `ready`; scroll restore waits for `layoutSettled`; on WebContent termination the controller bumps the process generation, reloads, and replays one coalesced state snapshot. `WebDocumentStateSynchronizer` (T-1719) is the single production owner that pushes native truth (sections, details open-state, table modes, notes, typography, comment visibility) to the controller and routes navigation targets (TOC/fragment via `session.pendingAnchorScroll`, notes via `coordinator.noteNavigationTarget`, search current match) through `controller.scrollTo` with `BlockDOMID.navigationDOMID` id translation — Observation-framework driven, so it works with no view mounted; `DocumentScrollContent` mounts the whole assembly via `WebDocumentStateSynchronizer.makeAssembly` (theme alone is view-fed, via `applyTheme(themeKey:)`, because it depends on colorScheme). 5. **Notes**: `NoteStateFeeder` (`prism/Services/WebRendering/`) maps `NotesManager` state onto `setNoteIndicators`/`setInlineNotes` payloads; `NoteHTMLBuilder` renders the (escaped) bubble/banner HTML natively; `prism-notes.js` (isolated world) injects it as `data-prism-chrome` and posts interaction messages back. 6. **Search**: counts and navigation order stay in `SearchService`/`SearchCoordinator`. `SearchStateFeeder` translates that into a per-block `setSearchState` payload; `prism-search.js` re-finds the query in each block's rendered text and registers ranges on two named **CSS Custom Highlights** (`prism-search`, `prism-search-current`), windowed to the viewport. The web view's built-in find navigator stays disabled so Cmd+F routes to Prism's search. 7. **Security**: `HTMLSanitizer` (over SwiftSoup) reduces raw HTML embedded in markdown to an allowlist subset on load (Req 1.8/8.1); its `plainText` feeds searchable text. Combined with `allowsContentJavaScript = false` and the served CSP, active-content vectors are blocked by construction.@@ -65,7 +65,7 @@ Implementation history and gotchas (the `<script>` data-island CSP trap, the nat 4. `BlockHTMLEmitter` replaces each resolvable `[^id]` reference in the emitted HTML with a styled pill-badge anchor (`prism://footnote/{id}`, the display number from `FootnoteData`) — footnote chrome, not document text 5. The badge anchor carries `data-prism-chrome` so it is not selectable; the badge's display number and id come from the `FootnoteData` sidecar threaded into the emitter 6. Badge taps produce `prism://footnote/{id}` links → `linkActivated` over the bridge → `WebDocumentMessageRouter` → `DocumentLayoutCoordinator` popover state (`PrismLinkRoute.footnotePrefix`)-7. `FootnotePopoverView` displays rendered footnote content (paragraphs, lists, blockquotes only) over a shared, non-persistent `FootnotePopoverWebPage` (emitter-rendered fragment) in a popover (iPad/macOS) or sheet (iPhone)+7. `FootnotePopoverView` displays rendered footnote content (paragraphs, lists, blockquotes only) over a shared, non-persistent `FootnotePopoverWebPage` (emitter-rendered fragment). Since the WebKit cutover it is presented as a sheet on every platform via the shared `FootnotePresenter` modifier (`View.footnotePresentation(coordinator:session:)`), which **both** `CompactDocumentLayout` and `RegularDocumentLayout` must apply — the regular layout lacking it is T-1893. The web-rendered badge has no SwiftUI anchor view, so the anchored popover the original footnotes spec called for (Req 3.1/3.6/3.7) no longer applies; `FootnotePopoverView` self-sizes per platform 8. `FootnoteStripping` removes `[^id]` references from heading text used in ToC entries, anchor IDs, and window titles 9. Search integration: `searchableText(with:)` appends footnote content to host blocks; badge highlighting indicates matches
diff --git a/prism/Models/MarkdownBlock.swift b/prism/Models/MarkdownBlock.swiftindex 636d190a..16c7c115 100644--- a/prism/Models/MarkdownBlock.swift+++ b/prism/Models/MarkdownBlock.swift@@ -383,6 +383,19 @@ nonisolated enum TableDisplayMode: Sendable, Hashable, CaseIterable { } } + /// The `data-prism-table-mode` string the rendered document uses for this+ /// mode ("scroll" ↔ `.wide`). Shared by `BlockHTMLEmitter` (initial+ /// attribute) and `WebDocumentStateSynchronizer` (`setTableModes` payload)+ /// so the emitter and the state push cannot drift; the inverse mapping is+ /// `WebDocumentMessageRouter.tableDisplayMode(from:)`.+ var webModeAttribute: String {+ switch self {+ case .fitted: "fitted"+ case .readable: "readable"+ case .wide: "scroll"+ }+ }+ /// Table-level accessibility label with mode-specific description. /// /// Uses `usesHorizontalScroll` to gate the "Scrollable" prefix so the
diff --git a/prism/Models/TOCEntry.swift b/prism/Models/TOCEntry.swiftindex c0692de6..401b7a86 100644--- a/prism/Models/TOCEntry.swift+++ b/prism/Models/TOCEntry.swift@@ -49,23 +49,42 @@ struct TOCEntry: Identifiable, Equatable { !detailsAncestorIds.isEmpty } - /// Unique scroll ID for ScrollViewReader targeting.+ /// The navigation target for this entry: a composite `{blockId}-{sourceIndex}`+ /// whose index is the heading's position in the TOP-LEVEL `parsedBlocks`+ /// array, which is what `MarkdownSection.id` and `BlockDOMID` verify against. ///- /// This is the same as `id` and must match the ID applied to the- /// rendered block in layout views via `.id(scrollId)`.- var scrollId: String { id }+ /// This is deliberately NOT `id`. `id` numbers entries with a flattened+ /// counter that also advances through a `<details>` block's nested children,+ /// which keeps it unique for `Identifiable` but makes it unresolvable: a+ /// `<details>` block occupies ONE top-level slot however many children it+ /// holds, so after one appears the two counters diverge and every later+ /// heading's composite pointed at the wrong block (or past the end). Scroll+ /// and section-expansion both silently did nothing (T-1662).+ ///+ /// For a heading nested inside `<details>` this is the enclosing top-level+ /// details block's composite — the nearest ancestor that actually has a DOM+ /// anchor, since the emitter gives non-details children no section id.+ let scrollTargetId: String++ /// Unique scroll ID for scroll targeting. Must match the id applied to the+ /// rendered block; see `scrollTargetId` for why this is not `id`.+ var scrollId: String { scrollTargetId } // MARK: - Initializers /// Creates a TOC entry for a top-level heading (not inside details). /// /// Backward-compatible initializer for existing code.- init(id: String, level: Int, text: String, blockIndex: Int) {+ /// `scrollTargetId` defaults to `id`, which is correct for any document+ /// with no `<details>` block ahead of the heading (the two index schemes+ /// agree there). `TOCCoordinator` passes it explicitly.+ init(id: String, level: Int, text: String, blockIndex: Int, scrollTargetId: String? = nil) { self.id = id self.level = level self.text = text self.blockIndex = blockIndex self.detailsAncestorIds = []+ self.scrollTargetId = scrollTargetId ?? id } /// Creates a TOC entry with optional ancestor tracking.@@ -78,11 +97,19 @@ struct TOCEntry: Identifiable, Equatable { /// - text: Heading text content /// - blockIndex: Position in parsed blocks array /// - detailsAncestorIds: IDs of containing details blocks (root first)- init(id: String, level: Int, text: String, blockIndex: Int, detailsAncestorIds: [String]) {+ init(+ id: String,+ level: Int,+ text: String,+ blockIndex: Int,+ detailsAncestorIds: [String],+ scrollTargetId: String? = nil+ ) { self.id = id self.level = level self.text = text self.blockIndex = blockIndex self.detailsAncestorIds = detailsAncestorIds+ self.scrollTargetId = scrollTargetId ?? id } }
diff --git a/prism/Resources/WebRenderer/prism-scroll.js b/prism/Resources/WebRenderer/prism-scroll.jsindex 4da7f3ba..76f04832 100644--- a/prism/Resources/WebRenderer/prism-scroll.js+++ b/prism/Resources/WebRenderer/prism-scroll.js@@ -13,6 +13,10 @@ * - visibleBlock: reports the topmost block's DOM id + in-document scroll fraction * (debounced), SUPPRESSED while a programmatic scroll is in flight (mirrors the * DocumentLayoutCoordinator clobber guard).+ * - scrollDirectionChanged: reports user-scroll direction flips (and the one+ * hide-threshold crossing) with ~10px hysteresis — never per scroll frame —+ * for the compact layout's hide-on-scroll toolbar (T-1719). Suppressed while+ * a programmatic scroll is in flight. * - scrollToBlock/scrollToEdge/scrollByPage: native commands drive the scroll. * - link routing: clicks on document anchors post linkActivated; native enforces * the scheme allowlist (Req 2.4). file:// and other schemes are never dispatched.@@ -94,6 +98,44 @@ window.addEventListener("scroll", reportVisibleBlock, { passive: true }); + // ---- scrollDirectionChanged (hide-on-scroll, T-1719) ------------------+ // Native applies the threshold rules (DocumentLayoutCoordinator+ // .applyScrollDirection); this side only detects direction with ~10px+ // hysteresis and posts on a flip — plus one extra post when a downward+ // scroll first crosses the native hide threshold, because a descent that+ // STARTED above the fold flips at an offset the native rule ignores and+ // would otherwise never re-post. Never per scroll frame.++ var DIRECTION_HYSTERESIS = 10;+ // Mirrors DocumentLayoutCoordinator.compactToolbarHideThreshold.+ var HIDE_THRESHOLD = 50;+ var directionAnchorY = window.scrollY;+ var lastDirection = null;+ var lastReportedY = window.scrollY;++ function reportScrollDirection() {+ var y = window.scrollY;+ if (programmaticScrollInFlight) {+ // Re-anchor so the first user scroll after a programmatic move is+ // measured from the settled position, not across the jump.+ directionAnchorY = y;+ lastReportedY = y;+ return;+ }+ var delta = y - directionAnchorY;+ if (Math.abs(delta) < DIRECTION_HYSTERESIS) { return; }+ var direction = delta > 0 ? "down" : "up";+ directionAnchorY = y;+ var crossedHideThreshold = direction === "down"+ && lastReportedY <= HIDE_THRESHOLD && y > HIDE_THRESHOLD;+ if (direction === lastDirection && !crossedHideThreshold) { return; }+ lastDirection = direction;+ lastReportedY = y;+ bridge.post("scrollDirectionChanged", { direction: direction, offsetY: y });+ }++ window.addEventListener("scroll", reportScrollDirection, { passive: true });+ // ---- Native → JS scroll commands ------------------------------------- bridge.registerCommand("scrollToBlock", function (payload) {
diff --git a/prism/Services/DetailsExpansionCoordinator.swift b/prism/Services/DetailsExpansionCoordinator.swiftindex 8fcbc573..902f34b4 100644--- a/prism/Services/DetailsExpansionCoordinator.swift+++ b/prism/Services/DetailsExpansionCoordinator.swift@@ -32,6 +32,26 @@ final class DetailsExpansionCoordinator { /// IDs remain in this set even if the user manually collapses the section. private(set) var expandedBlockIds: Set<String> = [] + /// The authoritative open-state of every `<details>` in the rendered+ /// document, keyed by occurrence-qualified DOM section id (T-1719).+ ///+ /// Unlike `expandedBlockIds` (additive request log, mixed id formats),+ /// this set IS the current open state: seeded from each details block's+ /// `isOpenByDefault` at parse, grown by expansion requests (`expand`),+ /// and updated in BOTH directions by user toggles from the page+ /// (`applyDetailsToggle`) — last write wins. `WebDocumentStateSynchronizer`+ /// pushes it as `setDetailsState`, whose payload membership forces each+ /// `<details>` open or closed, so a reload/WebContent recovery replays the+ /// exact open state.+ private(set) var openDetailsDOMIDs: Set<String> = []++ /// Maps a details block's composite path (`{hash}-{sourceIndex}`, nested+ /// `{path}/{childIndex}`) to the DOM section id the emitter assigns+ /// (`b-{hash}-{occurrence}`, nested `{domID}-d{childIndex}`). Built with+ /// `ancestorMap` at parse; lets `expand` translate native expansion+ /// requests onto `openDetailsDOMIDs`.+ @ObservationIgnored private var domIDByDetailsPath: [String: String] = [:]+ /// Pending scroll target after expansion completes. /// Layout views observe this to trigger scrolling. var pendingScrollTarget: String?@@ -58,10 +78,14 @@ final class DetailsExpansionCoordinator { /// - scrollTo: Optional ID to scroll to after expansion. func expand(blockId: String, scrollTo: String? = nil) { expandedBlockIds.insert(blockId)+ openDetails(matching: blockId) // Expand all ancestors (Requirement 4.7) if let ancestors = ancestorMap[blockId] { expandedBlockIds.formUnion(ancestors)+ for ancestorId in ancestors {+ openDetails(matching: ancestorId)+ } } if let scrollTarget = scrollTo {@@ -70,6 +94,48 @@ final class DetailsExpansionCoordinator { } } + /// Applies a user `<details>` toggle reported by the rendered page+ /// (`detailsToggled`, T-1719). Authoritative in BOTH directions: a later+ /// user collapse removes an id an earlier expansion request inserted.+ /// An expand also records the DOM id in `expandedBlockIds`, preserving the+ /// has-been-requested contract (`isExpanded`); a collapse leaves that+ /// request log untouched, matching its documented additive semantics.+ /// Change-detecting: `setDetailsState` writes `open` on every `<details>`,+ /// and each element that flips echoes a `detailsToggled` message back. An+ /// unconditional insert/remove would mutate observable state for those+ /// echoes and cost a second full synchronizer pass per user toggle.+ func applyDetailsToggle(domID: String, expanded: Bool) {+ if expanded {+ guard !openDetailsDOMIDs.contains(domID) else { return }+ openDetailsDOMIDs.insert(domID)+ expandedBlockIds.insert(domID)+ } else {+ guard openDetailsDOMIDs.contains(domID) else { return }+ openDetailsDOMIDs.remove(domID)+ }+ }++ /// Folds an expansion-request id into `openDetailsDOMIDs`: a DOM id+ /// (`b-` prefix) applies directly; a composite / nested path resolves+ /// through the parse-built map, stripping trailing `/{childIndex}`+ /// segments until a mapped details ancestor is found (a request naming a+ /// nested non-details child opens its enclosing details).+ private func openDetails(matching blockId: String) {+ if blockId.hasPrefix("b-") {+ openDetailsDOMIDs.insert(blockId)+ return+ }+ var path = Substring(blockId)+ while true {+ if let domID = domIDByDetailsPath[String(path)] {+ openDetailsDOMIDs.insert(domID)+ return+ }+ guard let slash = path.lastIndex(of: "/") else { return }+ path = path[..<slash]+ }+ }+ /// Signals that a block is about to expand. /// /// Called by DetailsBlockView before starting expansion animation.@@ -93,9 +159,16 @@ final class DetailsExpansionCoordinator { /// This ensures identical blocks at different positions get distinct entries /// instead of overwriting each other (T-300). ///+ /// Also rebuilds the details DOM-id map and seeds `openDetailsDOMIDs`+ /// from each details block's `isOpenByDefault` (T-1719), so the pushed+ /// `setDetailsState` replay preserves default-open details.+ /// /// - Parameter blocks: The parsed document blocks. func buildAncestorMap(from blocks: [MarkdownBlock]) { ancestorMap.removeAll()+ domIDByDetailsPath.removeAll()+ var seededOpen: Set<String> = []+ let mapped = BlockDOMID.map(blocks: blocks) for (index, block) in blocks.enumerated() { let compositeId = "\(block.id)-\(index)" traverseForAncestors(@@ -103,6 +176,33 @@ final class DetailsExpansionCoordinator { compositeId: compositeId, currentPath: [] )+ registerDetails(+ block: block, path: compositeId,+ domID: mapped[index].domID, openIDs: &seededOpen+ )+ }+ openDetailsDOMIDs = seededOpen+ }++ /// Registers a details block (and its nested details children) in the+ /// composite-path → DOM-id map, seeding the open set from+ /// `isOpenByDefault`. Nested ids mirror `BlockHTMLEmitter.emitDetails`'+ /// `-d{childIndex}` suffix scheme.+ private func registerDetails(+ block: MarkdownBlock,+ path: String,+ domID: String,+ openIDs: inout Set<String>+ ) {+ guard case .details(_, let children, let isOpen, _) = block else { return }+ domIDByDetailsPath[path] = domID+ if isOpen { openIDs.insert(domID) }+ for (childIndex, child) in children.enumerated() {+ guard case .details = child else { continue }+ registerDetails(+ block: child, path: "\(path)/\(childIndex)",+ domID: "\(domID)-d\(childIndex)", openIDs: &openIDs+ ) } } @@ -148,9 +248,11 @@ final class DetailsExpansionCoordinator { /// Clears all expansion requests and pending scroll targets. /// Views will reinitialize to their `isOpenByDefault` state. ///- /// Note: ancestorMap is rebuilt separately after parsing.+ /// Note: ancestorMap is rebuilt separately after parsing, which also+ /// re-seeds `openDetailsDOMIDs` from `isOpenByDefault`. func reset() { expandedBlockIds.removeAll()+ openDetailsDOMIDs.removeAll() pendingScrollTarget = nil expandingBlockId = nil }
diff --git a/prism/Services/KeyboardScrollController.swift b/prism/Services/KeyboardScrollController.swiftindex d42f2f60..9da08320 100644--- a/prism/Services/KeyboardScrollController.swift+++ b/prism/Services/KeyboardScrollController.swift@@ -85,6 +85,44 @@ final class KeyboardScrollController { private var lastCommandAt: Date? private static let animationDuration: TimeInterval = 0.2 + // MARK: - Web bridge backend (T-1719)++ /// The scroll commands the rendered web document supports. When attached,+ /// page/edge commands (and the macOS View-menu items that route through+ /// `DocumentActions`) drive the web bridge instead of the SwiftUI+ /// `ScrollPosition`, which is bound to nothing on the rendered path.+ struct WebCommands {+ var pageUp: () -> Void+ var pageDown: () -> Void+ var scrollToTop: () -> Void+ var scrollToBottom: () -> Void+ }++ /// The attached web backend, nil on the SwiftUI (raw-source) path. Not+ /// observed by any view — the observable enablement signal stays+ /// `hasContent`/`canScroll`.+ @ObservationIgnored private var webCommands: WebCommands?++ /// Routes this controller's commands to the rendered web document and+ /// marks the controller scrollable (`hasContent`/`canScroll`) so menu+ /// items and key handling enable without SwiftUI scroll geometry+ /// (T-1719; page/edge scrolling is a silent no-op on short content, the+ /// same contract the menu items rely on).+ func attachWebBridge(_ commands: WebCommands) {+ webCommands = commands+ hasContent = true+ canScroll = true+ }++ /// Detaches the web backend (session change / controller teardown),+ /// restoring the geometry-driven enablement (disabled until a SwiftUI+ /// scroll surface reports content).+ func detachWebBridge() {+ webCommands = nil+ hasContent = false+ canScroll = false+ }+ func arrowUp(reduceMotion: Bool) { guard !suspended else { return } applyDelta(-3 * stepHeight, reduceMotion: reduceMotion)@@ -97,33 +135,40 @@ final class KeyboardScrollController { func pageUp(reduceMotion: Bool) { guard !suspended else { return }+ // The web page scrolls in-page; reduceMotion is irrelevant there+ // (the bridge command scrolls without animation).+ if let webCommands { return webCommands.pageUp() } applyDelta(-floor(viewportHeight * 0.9), reduceMotion: reduceMotion) } func pageDown(reduceMotion: Bool) { guard !suspended else { return }+ if let webCommands { return webCommands.pageDown() } applyDelta(floor(viewportHeight * 0.9), reduceMotion: reduceMotion) } func scrollToTop(reduceMotion: Bool) { guard !suspended else { return }+ if let webCommands { return webCommands.scrollToTop() } applyEdge(.top, reduceMotion: reduceMotion) } func scrollToBottom(reduceMotion: Bool) { guard !suspended else { return }+ if let webCommands { return webCommands.scrollToBottom() } applyEdge(.bottom, reduceMotion: reduceMotion) } - /// Zeros the geometry inputs and `hasContent`. Called when the document- /// session changes so a brand-new (and possibly short) document does not- /// briefly inherit the previous document's `canScroll == true` while- /// waiting for the new view's `onScrollGeometryChange` to fire.+ /// Zeros the geometry inputs and `hasContent`, and detaches any web+ /// backend. Called when the document session changes so a brand-new (and+ /// possibly short) document does not briefly inherit the previous+ /// document's `canScroll == true` while waiting for the new view's+ /// `onScrollGeometryChange` to fire (or the new web assembly to attach). func resetForNewSession() {+ detachWebBridge() contentOffset = 0 contentHeight = 0 viewportHeight = 0- hasContent = false } private func applyDelta(_ delta: CGFloat, reduceMotion: Bool) {
diff --git a/prism/Services/TOCCoordinator.swift b/prism/Services/TOCCoordinator.swiftindex 0c59fe50..01efef14 100644--- a/prism/Services/TOCCoordinator.swift+++ b/prism/Services/TOCCoordinator.swift@@ -82,12 +82,30 @@ final class TOCCoordinator { /// - runningIndex: A running counter tracking block position for unique TOC entry IDs. /// - ancestorDetailIds: Composite IDs of ancestor details blocks (root first). /// - parentCompositeId: The composite ID of the parent details block, or nil for top-level.+ /// Two index spaces are in play and they must not be confused (T-1662):+ ///+ /// - `runningIndex` is a FLATTENED counter that also advances through a+ /// `<details>` block's nested children. It makes `TOCEntry.id` unique for+ /// `Identifiable`, and that is all it is good for.+ /// - The TOP-LEVEL index (`blocks.enumerated()` at depth 0) is what+ /// `MarkdownSection.id` and `BlockDOMID` verify a composite against. A+ /// `<details>` block occupies ONE top-level slot no matter how many+ /// children it holds, so once one appears the two counters diverge.+ ///+ /// `scrollTargetId` therefore always uses the top-level index.+ ///+ /// - Parameter topLevelAnchor: the resolvable composite of the enclosing+ /// top-level block when recursing into `<details>` children; nil at depth+ /// 0. Nested headings have no section id of their own (the emitter gives+ /// non-details children none), so they navigate to the details block that+ /// contains them — the nearest ancestor with a DOM anchor. private func buildTOCEntries( from blocks: [MarkdownBlock], into entries: inout [TOCEntry], runningIndex: inout Int, ancestorDetailIds: [String],- parentCompositeId: String?+ parentCompositeId: String?,+ topLevelAnchor: String? = nil ) { for (blockIndex, block) in blocks.enumerated() { // Build the composite ID matching the view hierarchy scheme@@ -97,6 +115,9 @@ final class TOCCoordinator { } else { compositeId = "\(block.id)-\(runningIndex)" }+ // Resolvable target: this block's own top-level composite at depth 0,+ // otherwise the enclosing top-level block's.+ let resolvableTarget = topLevelAnchor ?? "\(block.id)-\(blockIndex)" switch block { case .heading(let level, let text):@@ -108,7 +129,8 @@ final class TOCCoordinator { // like footnote references (Decision 11, Decision 19). text: FootnoteStripping.strip(HTMLCommentStripping.strip(text)), blockIndex: runningIndex,- detailsAncestorIds: ancestorDetailIds+ detailsAncestorIds: ancestorDetailIds,+ scrollTargetId: resolvableTarget ) entries.append(entry) runningIndex += 1@@ -116,13 +138,16 @@ final class TOCCoordinator { case .details(_, let children, _, _): let newAncestorPath = ancestorDetailIds + [compositeId] runningIndex += 1- // Recursively process children with updated ancestor path+ // Recursively process children with updated ancestor path. Every+ // heading beneath here navigates to this details block, which is+ // the deepest ancestor that has a DOM anchor. buildTOCEntries( from: children, into: &entries, runningIndex: &runningIndex, ancestorDetailIds: newAncestorPath,- parentCompositeId: compositeId+ parentCompositeId: compositeId,+ topLevelAnchor: resolvableTarget ) default:
diff --git a/prism/Services/WebRendering/BlockDOMID.swift b/prism/Services/WebRendering/BlockDOMID.swiftindex 77f545e8..edc0540c 100644--- a/prism/Services/WebRendering/BlockDOMID.swift+++ b/prism/Services/WebRendering/BlockDOMID.swift@@ -25,10 +25,20 @@ nonisolated enum BlockDOMID { "b-\(contentHash)-\(occurrence)" } + /// A blocks→DOM id pairing in document order, as produced by `map(blocks:)`.+ ///+ /// Every resolver here has a `mapped:` overload taking one of these. Prefer it+ /// whenever more than one lookup runs against the same blocks: `block.id`+ /// rebuilds the block's content string and takes the global `BlockIDCache`+ /// mutex on each access, so a walk is O(content), not O(1) per block. Callers+ /// that resolve several ids per turn (the state synchronizer) walk once and+ /// share the result rather than paying that per lookup.+ typealias Mapping = [(block: MarkdownBlock, domID: String)]+ /// Walks `blocks` in order and pairs each with its occurrence-qualified DOM id, /// assigning a per-content-hash occurrence index exactly as `BlockHTMLEmitter.emit` /// does. Duplicate blocks (same content hash) get distinct ids that share the hash.- static func map(blocks: [MarkdownBlock]) -> [(block: MarkdownBlock, domID: String)] {+ static func map(blocks: [MarkdownBlock]) -> Mapping { var occurrence: [String: Int] = [:] var result: [(block: MarkdownBlock, domID: String)] = [] result.reserveCapacity(blocks.count)@@ -52,6 +62,95 @@ nonisolated enum BlockDOMID { .map(\.domID) } + /// Resolves a native navigation target to a DOM id the rendered page can+ /// scroll to, or nil when the target no longer resolves (T-1719).+ ///+ /// Accepts every target format the native navigation surfaces produce:+ /// - an occurrence-qualified DOM id (`b-{hash}-{occurrence}`), as-is;+ /// - a composite id (`{hash}-{sourceIndex}` — TOC entries, section ids,+ /// search scroll ids), verified against `blocks` like `restoreDOMID`;+ /// - a sub-block id (`{hash}-row-N`, `{hash}-row-header`, `{hash}-item-N`),+ /// resolved to its parent block;+ /// - a bare content hash (notes panel/sidebar targets), resolved to the+ /// first occurrence — preferring one whose source index is in+ /// `visibleSourceIndices` (a collapsed section's blocks are hidden in+ /// the DOM, so scrolling to a visible occurrence is preferred).+ static func navigationDOMID(+ forTarget target: String,+ blocks: [MarkdownBlock],+ visibleSourceIndices: Set<Int>? = nil+ ) -> String? {+ navigationDOMID(+ forTarget: target,+ mapped: map(blocks: blocks),+ visibleSourceIndices: visibleSourceIndices+ )+ }++ /// `navigationDOMID` against a pre-walked `Mapping`. Identical semantics; use+ /// this when the caller already holds the mapping (see `Mapping`).+ static func navigationDOMID(+ forTarget target: String,+ mapped: Mapping,+ visibleSourceIndices: Set<Int>? = nil+ ) -> String? {+ guard !target.isEmpty else { return nil }+ // Occurrence-qualified DOM ids pass through unchanged.+ if target.hasPrefix("b-") { return target }+ // Sub-block ids (table rows / list items are not scrollable sections)+ // resolve to their parent block's content hash. Checked before the+ // composite parse: `{hash}-item-3` has an integer tail but is a+ // sub-block id, never a composite (hashes are dash-free hex).+ let parent = strippingSubBlockSuffix(target)+ if parent != target {+ return firstOccurrenceDOMID(+ forContentHash: parent, mapped: mapped,+ visibleSourceIndices: visibleSourceIndices+ )+ }+ // Composite id: verified hash-at-index, exactly like scroll restore.+ // A composite whose hash no longer sits at that index is stale.+ if let dashIndex = target.lastIndex(of: "-"),+ Int(target[target.index(after: dashIndex)...]) != nil {+ return restoreDOMID(forStored: target, mapped: mapped)+ }+ // Bare content hash: first occurrence, preferring a visible one.+ return firstOccurrenceDOMID(+ forContentHash: target, mapped: mapped,+ visibleSourceIndices: visibleSourceIndices+ )+ }++ /// Strips a trailing sub-block suffix (`-row-N`, `-row-header`, `-item-N`,+ /// nested `-item-N-item-M`) from a note-anchor id, yielding the parent+ /// block's content hash. Returns the input unchanged when no suffix matches+ /// (same grammar as the retired `SharedBlockViews.parentBlockId`).+ private static func strippingSubBlockSuffix(_ subBlockId: String) -> String {+ let pattern = /-(row-(?:header|\d+)|item-\d+(?:-item-\d+)*)$/+ guard let match = subBlockId.firstMatch(of: pattern) else { return subBlockId }+ return String(subBlockId[subBlockId.startIndex..<match.range.lowerBound])+ }++ /// The DOM id of the first block whose content hash is `contentHash`,+ /// preferring the first occurrence whose source index is in+ /// `visibleSourceIndices` (so navigation lands on a block that is actually+ /// present in the DOM when duplicates exist under collapsed sections).+ private static func firstOccurrenceDOMID(+ forContentHash contentHash: String,+ mapped: Mapping,+ visibleSourceIndices: Set<Int>?+ ) -> String? {+ var firstMatch: String?+ for (sourceIndex, entry) in mapped.enumerated()+ where entry.block.id == contentHash {+ if firstMatch == nil { firstMatch = entry.domID }+ if let visible = visibleSourceIndices, visible.contains(sourceIndex) {+ return entry.domID+ }+ }+ return firstMatch+ }+ /// Resolves a stored scroll-position id to a DOM id the rendered page can /// scroll to, or nil when there is nothing to restore (T-1639). ///@@ -63,14 +162,19 @@ nonisolated enum BlockDOMID { /// and the document opens at the top, matching the pre-cutover behaviour /// for externally-modified files. static func restoreDOMID(forStored stored: String, blocks: [MarkdownBlock]) -> String? {+ restoreDOMID(forStored: stored, mapped: map(blocks: blocks))+ }++ /// `restoreDOMID` against a pre-walked `Mapping`. Identical semantics; use+ /// this when the caller already holds the mapping (see `Mapping`).+ static func restoreDOMID(forStored stored: String, mapped: Mapping) -> String? { guard !stored.isEmpty else { return nil } if stored.hasPrefix("b-") { return stored }- // Legacy composite: the hash may itself contain dashes; the trailing- // integer is the source index.+ // Legacy composite: the trailing integer is the source index. The hash+ // itself is dash-free hex, so the last dash always separates the two. guard let dashIndex = stored.lastIndex(of: "-"), let sourceIndex = Int(stored[stored.index(after: dashIndex)...]), sourceIndex >= 0 else { return nil }- let mapped = map(blocks: blocks) guard sourceIndex < mapped.count, mapped[sourceIndex].block.id == String(stored[..<dashIndex]) else { return nil } return mapped[sourceIndex].domID
diff --git a/prism/Services/WebRendering/BlockHTMLEmitter.swift b/prism/Services/WebRendering/BlockHTMLEmitter.swiftindex 6688c755..72aa56a4 100644--- a/prism/Services/WebRendering/BlockHTMLEmitter.swift+++ b/prism/Services/WebRendering/BlockHTMLEmitter.swift@@ -536,23 +536,13 @@ nonisolated enum BlockHTMLEmitter { // Wrap in .prism-table-wrap with the initial display mode so the stylesheet's // horizontal containment applies (a bare table whose min-content exceeds the // viewport otherwise scrolls the whole page). prism-theme.js's setTableModes- // toggles the same attribute on this wrapper (Req 1.5).- let mode = tableModeAttribute(TableDisplayMode.initialMode(headers: headers, rows: rows))+ // toggles the same attribute on this wrapper (Req 1.5). The mode string is the+ // shared `TableDisplayMode.webModeAttribute` mapping (T-1719).+ let mode = TableDisplayMode.initialMode(headers: headers, rows: rows).webModeAttribute return "<div class=\"prism-table-wrap\" data-prism-table-mode=\"\(mode)\">" + html + "</div>" } - /// Maps a `TableDisplayMode` to the `data-prism-table-mode` string the stylesheet and- /// `prism-theme.js` use. Inverse of `WebDocumentMessageRouter.tableDisplayMode(from:)`- /// ("scroll" ↔ `.wide`).- private static func tableModeAttribute(_ mode: TableDisplayMode) -> String {- switch mode {- case .fitted: return "fitted"- case .readable: return "readable"- case .wide: return "scroll"- }- }- // MARK: Image (task 14) — every src rewritten through prism-doc://img/ /// The image block's associated values, bundled so the emit helper stays within the
diff --git a/prism/Services/WebRendering/NoteStateFeeder.swift b/prism/Services/WebRendering/NoteStateFeeder.swiftindex b5e820ab..70a5a606 100644--- a/prism/Services/WebRendering/NoteStateFeeder.swift+++ b/prism/Services/WebRendering/NoteStateFeeder.swift@@ -79,8 +79,28 @@ enum NoteStateFeeder { // Walk the blocks ONCE for the (block, domID) pairing and share it with both // payload builders — both used to call BlockDOMID.map independently, two full // walks per push on the note-change hot path.- let mapped = BlockDOMID.map(blocks: blocks)- return Payloads(+ payloads(+ mapped: BlockDOMID.map(blocks: blocks),+ notesManager: notesManager,+ showInlineNotes: showInlineNotes,+ bannerPlacement: bannerPlacement,+ exportUsername: exportUsername,+ strings: strings+ )+ }++ /// `payloads` against a pre-walked mapping, so a caller that already holds one+ /// (the state synchronizer, which resolves several ids per pass) does not pay+ /// another full walk here.+ static func payloads(+ mapped: BlockDOMID.Mapping,+ notesManager: NotesManager,+ showInlineNotes: Bool,+ bannerPlacement: DocumentNotesBannerPlacement,+ exportUsername: String,+ strings: Strings = .fallback+ ) -> Payloads {+ Payloads( indicatorsJSON: indicatorsJSON(mapped: mapped, notesManager: notesManager), inlineNotesJSON: inlineNotesJSON( mapped: mapped,
diff --git a/prism/ViewModels/BridgeMessageRouter.swift b/prism/ViewModels/BridgeMessageRouter.swiftindex de0a23e4..ee6bbd60 100644--- a/prism/ViewModels/BridgeMessageRouter.swift+++ b/prism/ViewModels/BridgeMessageRouter.swift@@ -201,6 +201,12 @@ struct BridgeMessageRouter { guard let blockID = dict["blockID"] as? String, let mode = dict["mode"] as? String else { return nil } return .tableModeToggled(blockID: blockID, mode: mode)+ case .scrollDirectionChanged:+ // An unknown direction string is dropped, never defaulted (T-1719).+ guard let directionString = dict["direction"] as? String,+ let direction = ScrollDirection(rawValue: directionString),+ let offsetY = double(dict["offsetY"]) else { return nil }+ return .scrollDirectionChanged(direction: direction, offsetY: offsetY) case .perfSample: guard let gap = double(dict["maxFrameGapMS"]) else { return nil } return .perfSample(maxFrameGapMS: gap, longAnimationFrames: int(dict["longAnimationFrames"]) ?? 0)
diff --git a/prism/ViewModels/WebBridgeContract.swift b/prism/ViewModels/WebBridgeContract.swiftindex 441b8a3e..90be817b 100644--- a/prism/ViewModels/WebBridgeContract.swift+++ b/prism/ViewModels/WebBridgeContract.swift@@ -85,6 +85,11 @@ enum InboundBridgeMessage: Equatable, Sendable { case imageActivated(blockID: String, src: String?) /// `tableModeToggled` — block id, mode (Req 1.5). case tableModeToggled(blockID: String, mode: String)+ /// `scrollDirectionChanged` — user-scroll direction flip + current y offset+ /// (T-1719). Posted by prism-scroll.js only on direction changes and+ /// threshold crossings (never per scroll frame), suppressed during+ /// programmatic scrolls; drives the compact layout's hide-on-scroll toolbar.+ case scrollDirectionChanged(direction: ScrollDirection, offsetY: Double) /// `perfSample` — max frame gap (ms) + LoAF entry count (Req 9.2). case perfSample(maxFrameGapMS: Double, longAnimationFrames: Int) /// `diagFailure` — category (Req 11.5).@@ -146,6 +151,7 @@ enum InboundMessageType: String, CaseIterable, Sendable { case diagramActivated case imageActivated case tableModeToggled+ case scrollDirectionChanged case perfSample case diagFailure }
diff --git a/prism/ViewModels/WebDocumentControllerFactory.swift b/prism/ViewModels/WebDocumentControllerFactory.swiftindex 7ffb232e..8549c337 100644--- a/prism/ViewModels/WebDocumentControllerFactory.swift+++ b/prism/ViewModels/WebDocumentControllerFactory.swift@@ -268,28 +268,30 @@ enum WebDocumentControllerFactory { } /// Computes the note payloads from the session's blocks + NotesManager + settings- /// and pushes them onto the controller (Req 5.2/5.6). Like the other state pushes,- /// these queue until `ready` and replay after a WebContent recovery (they are part of- /// the coalesced snapshot). Called once after the controller is created and again- /// whenever the document's notes or the relevant note settings change.- static func pushNoteState(- to controller: WebDocumentController,+ /// (Req 5.2/5.6). `WebDocumentStateSynchronizer` pushes them at start and re-pushes+ /// whenever the notes or the relevant note settings change; like the other state+ /// pushes they queue until `ready` and replay after a WebContent recovery (they are+ /// part of the coalesced snapshot).+ /// `mapped` lets a caller that already walked the blocks share that walk (the+ /// state synchronizer resolves several ids per pass); omit it and the blocks+ /// are walked here.+ static func noteStatePayloads( session: DocumentSession, notesManager: NotesManager,- settings: AppSettings- ) {+ settings: AppSettings,+ mapped: BlockDOMID.Mapping? = nil+ ) -> (indicatorsJSON: String, inlineNotesJSON: String) { let payloads = NoteStateFeeder.payloads(- blocks: session.parsedBlocks,+ mapped: mapped ?? BlockDOMID.map(blocks: session.parsedBlocks), notesManager: notesManager, showInlineNotes: settings.showInlineNotes, bannerPlacement: settings.documentNotesBannerPlacement, exportUsername: settings.exportUsername, strings: noteStrings() )- controller.setNoteIndicators(json: payloads.indicatorsJSON)- // When there is nothing inline to show, push an empty payload so a prior push is+ // When there is nothing inline to show, use an empty payload so a prior push is // cleared on re-push (e.g. a note was deleted or inline notes were turned off).- controller.setInlineNotes(json: payloads.inlineNotesJSON ?? "{}")+ return (payloads.indicatorsJSON, payloads.inlineNotesJSON ?? "{}") } /// Computes the per-block search payload from the session's SearchCoordinator
diff --git a/prism/ViewModels/WebDocumentMessageRouter.swift b/prism/ViewModels/WebDocumentMessageRouter.swiftindex 90edc271..201376ce 100644--- a/prism/ViewModels/WebDocumentMessageRouter.swift+++ b/prism/ViewModels/WebDocumentMessageRouter.swift@@ -83,6 +83,11 @@ struct WebDocumentMessageRouter { session.tableDisplayModes[blockID] = displayMode } + case .scrollDirectionChanged(let direction, let offsetY):+ // Hide-on-scroll (T-1719): the bridge posts direction flips only;+ // the coordinator applies the pre-cutover threshold rules.+ coordinator.applyScrollDirection(direction, offsetY: offsetY)+ case .copyContent(let blockID, let kind): copyContent(blockID: blockID, kind: kind) @@ -307,14 +312,13 @@ struct WebDocumentMessageRouter { // MARK: - Details (Req 1.6) - /// The `<details>` open state lives in the DOM; native tracks expanded IDs only- /// so a reload/recovery can re-expand programmatically (`setDetailsState`). There- /// is no native collapse — the DOM is the source of truth for the open flag — so- /// a collapse toggle is a no-op on native state.+ /// Native is authoritative for the `<details>` open state (T-1719): a user+ /// toggle from the page updates `DetailsExpansionCoordinator.openDetailsDOMIDs`+ /// in BOTH directions, so a reload/WebContent recovery replays the exact+ /// open state via `setDetailsState` — a later user collapse beats an+ /// earlier expansion request. private func applyDetailsToggle(id: String, expanded: Bool) {- if expanded {- session.expansionCoordinator.expand(blockId: id)- }+ session.expansionCoordinator.applyDetailsToggle(domID: id, expanded: expanded) } /// Maps the in-page table-mode string onto the native `TableDisplayMode`. The
diff --git a/prism/ViewModels/WebDocumentStateSynchronizer.swift b/prism/ViewModels/WebDocumentStateSynchronizer.swiftnew file mode 100644index 00000000..adacc5e4--- /dev/null+++ b/prism/ViewModels/WebDocumentStateSynchronizer.swift@@ -0,0 +1,366 @@+//+// WebDocumentStateSynchronizer.swift+// prism+//+// T-1719: the single production owner that keeps the rendered web document in+// sync with native truth and routes navigation into it. The ScrollViewProxy+// cutover (T-1542) removed the SwiftUI scroll surface but left navigation and+// state observers in a closure the web path never attaches; this type is the+// web-path owner those behaviours move into.+//+// Responsibilities (see specs/bugfixes/webkit-state-integration/report.md):+// - Navigation: consume `session.pendingAnchorScroll` (TOC / fragment+// targets) and `coordinator.noteNavigationTarget` (notes panel/sidebar+// targets), translate native ids to occurrence-qualified DOM ids via+// `BlockDOMID`, and drive `controller.scrollTo`.+// - Search navigation: observe the session's current match and scroll the+// rendered document to it. (Search HIGHLIGHT state — setSearchState — is+// owned by T-1680 and intentionally not pushed here.)+// - State pushes: section collapse, details open-state, table display+// modes, note indicators/inline notes, typography, comment visibility —+// pushed on change so the controller's coalesced snapshot can replay+// native truth exactly after reload/WebContent recovery.+// - Theme: pushed through `applyTheme(themeKey:)`, fed by the hosting view+// (the effective theme depends on the view-world colorScheme).+//+// Change reaction runs through the Observation framework, NOT SwiftUI+// `.onChange`, so the wiring exists independent of any view being mounted —+// the failure mode this ticket fixes. Mechanism: a SINGLE coalescing+// observation pass with dirty-flag dispatch, not per-domain re-arm loops.+// Each `synchronize()` computes every domain's desired value inside one+// `withObservationTracking` read (registering exactly the observable state+// each computation touches — no hand-maintained dependency list to drift),+// re-arms on the first subsequent mutation, then diff-compares each domain+// against its last-pushed value and dispatches only the dirty ones. A burst+// of same-turn mutations coalesces into one scheduled pass because tracking+// disarms after its first fire.+//++import Foundation+import Observation++@MainActor+final class WebDocumentStateSynchronizer {++ private let controller: WebDocumentController+ private let session: DocumentSession+ private let coordinator: DocumentLayoutCoordinator+ private let settings: AppSettings+ private let notesManager: NotesManager++ /// Guards `start()` against arming a second observation pass.+ private var isStarted = false++ // MARK: - Last-pushed values (the dirty flags)+ //+ // Each domain re-pushes only when its freshly computed value differs from+ // the value last handed to the controller. All start nil, so the first+ // pass (from `start()`) pushes every domain — the initial state the+ // recovery replay needs (Req 9.6).++ private var lastTypography: [String: String]?+ private var lastCommentVisibility: Bool?+ private var lastNoteIndicatorsJSON: String?+ private var lastInlineNotesJSON: String?+ private var lastSectionCollapsedIDs: Set<String>?+ private var lastDetailsOpenDOMIDs: Set<String>?+ private var lastTableModes: [String: String]?++ /// The last DOM id search navigation scrolled to, so navigating between+ /// matches inside the same block does not re-jump to the block top+ /// (matching the pre-cutover `onChange(of: currentMatch?.blockId)`+ /// semantics). Cleared when the current match clears.+ private var lastSearchScrollDOMID: String?++ /// The blocks→DOM id mapping and the `parseRevision` it was built for. Plain+ /// stored state on a non-`@Observable` class, so writing it inside the tracked+ /// read registers nothing and cannot re-trigger a pass.+ private var cachedMapping: (revision: UInt64, mapped: BlockDOMID.Mapping)?++ init(+ controller: WebDocumentController,+ session: DocumentSession,+ coordinator: DocumentLayoutCoordinator,+ settings: AppSettings,+ notesManager: NotesManager+ ) {+ self.controller = controller+ self.session = session+ self.coordinator = coordinator+ self.settings = settings+ self.notesManager = notesManager+ }++ /// Runs the first synchronization pass (which pushes every domain — the+ /// initial state the recovery replay needs) and leaves the observation+ /// armed for changes. Idempotent; safe to call once after the assembly is+ /// built.+ ///+ /// Search HIGHLIGHT state (`setSearchState` via `SearchStateFeeder`) is+ /// intentionally NOT part of the pass: T-1680 owns the highlight pipeline+ /// and adds its domain at this seam. Search NAVIGATION (scroll-to-match)+ /// is wired below.+ func start() {+ guard !isStarted else { return }+ isStarted = true+ synchronize()+ }++ /// Pushes the effective theme. Called by the hosting view, which owns the+ /// colorScheme-resolved theme key (initial push and on change) — theme is+ /// the one view-fed domain because the view world owns colorScheme.+ func applyTheme(themeKey: String) {+ controller.applyTheme(+ theme: themeKey,+ variables: [:],+ mermaidConfig: WebDocumentControllerFactory.mermaidThemeConfigJSON(for: themeKey)+ )+ }++ // MARK: - The coalescing observation pass++ /// Everything one tracked read computed: the desired value of each state+ /// domain (diffed by `dispatch`), the one-shot navigation targets (consumed+ /// by `dispatch`), and the block context navigation translation needs.+ private struct SyncPass {+ var typography: [String: String]+ var commentVisibility: Bool+ var noteIndicatorsJSON: String+ var inlineNotesJSON: String+ var sectionCollapsedIDs: Set<String>+ var detailsOpenDOMIDs: Set<String>+ var tableModes: [String: String]+ var anchorTarget: String?+ var noteNavigationTarget: String?+ var searchScrollDOMID: String?+ /// Navigation context, not desired state: the pass's single blocks→DOM id+ /// walk, reused by `scrollToTarget`.+ var mapped: BlockDOMID.Mapping+ }++ /// One pass: compute every domain under a single observation tracking,+ /// re-arm for the next mutation, then dirty-diff and dispatch.+ ///+ /// The consume-and-clear writes in `dispatch` land AFTER the tracking is+ /// armed, so a consumed navigation target fires the observation once more;+ /// that settle pass reads nil targets and equal state, dispatches nothing,+ /// and the cycle rests. `[weak self]` everywhere: when the hosting view+ /// discards the synchronizer the armed registration fires at most once+ /// more as a no-op and the cycle ends.+ private func synchronize() {+ let pass = withObservationTracking {+ computePass()+ } onChange: { [weak self] in+ // willSet callback (not actor-isolated by API contract); hop back+ // to the main actor. Tracking disarmed on this first fire, so a+ // same-turn burst of mutations coalesces into ONE scheduled pass,+ // which reads all of them fresh.+ Task { @MainActor [weak self] in+ self?.synchronize()+ }+ }+ dispatch(pass)+ }++ /// The tracked read: computes each domain's desired value, registering+ /// exactly the observable state the computations touch. Read-only — all+ /// mutations (pushes, target consumption) happen in `dispatch`.+ private func computePass() -> SyncPass {+ let blocks = session.parsedBlocks+ // ONE blocks→DOM id walk per pass, shared by every domain below and+ // carried into `dispatch` for navigation. `block.id` rebuilds the block's+ // content string and takes the global BlockIDCache mutex on each access,+ // so an unshared walk per lookup made a pass O(4N) on that hot path — and+ // a pass fires for any tracked mutation, not just note changes.+ let mapped = mapping(for: blocks)+ let notePayloads = WebDocumentControllerFactory.noteStatePayloads(+ session: session,+ notesManager: notesManager,+ settings: settings,+ mapped: mapped+ )+ return SyncPass(+ typography: WebDocumentControllerFactory.typographyVariables(settings: settings),+ commentVisibility: settings.showHTMLComments,+ noteIndicatorsJSON: notePayloads.indicatorsJSON,+ inlineNotesJSON: notePayloads.inlineNotesJSON,+ sectionCollapsedIDs: session.sections.collapsedSectionIds,+ detailsOpenDOMIDs: session.expansionCoordinator.openDetailsDOMIDs,+ tableModes: translatedTableModes(mapped: mapped),+ anchorTarget: session.pendingAnchorScroll,+ noteNavigationTarget: coordinator.noteNavigationTarget,+ searchScrollDOMID: currentSearchMatchDOMID(mapped: mapped),+ mapped: mapped+ )+ }++ /// The blocks→DOM id mapping for the current parse, cached across passes.+ ///+ /// `parsedBlocks` is assigned exactly once per parse and `parseRevision` is+ /// bumped in the same synchronous stretch, so the revision is a sound cache+ /// key — the same invariant the document-HTML cache uses (T-1681). Without+ /// this, every pass re-walked every block even though the blocks only change+ /// on a re-parse.+ private func mapping(for blocks: [MarkdownBlock]) -> BlockDOMID.Mapping {+ let revision = session.parseRevision+ if let cached = cachedMapping, cached.revision == revision {+ return cached.mapped+ }+ let mapped = BlockDOMID.map(blocks: blocks)+ cachedMapping = (revision, mapped)+ return mapped+ }++ /// Dirty-flag dispatch: pushes each state domain whose value changed since+ /// the last push, then consumes the navigation targets. Runs outside the+ /// tracked read, so pushes never register spurious dependencies.+ private func dispatch(_ pass: SyncPass) {+ if pass.typography != lastTypography {+ lastTypography = pass.typography+ controller.applyTypography(variables: pass.typography)+ }+ if pass.commentVisibility != lastCommentVisibility {+ lastCommentVisibility = pass.commentVisibility+ controller.setCommentVisibility(pass.commentVisibility)+ }+ // Note indicators + inline notes/banner (Req 5.2/5.6).+ if pass.noteIndicatorsJSON != lastNoteIndicatorsJSON {+ lastNoteIndicatorsJSON = pass.noteIndicatorsJSON+ controller.setNoteIndicators(json: pass.noteIndicatorsJSON)+ }+ if pass.inlineNotesJSON != lastInlineNotesJSON {+ lastInlineNotesJSON = pass.inlineNotesJSON+ controller.setInlineNotes(json: pass.inlineNotesJSON)+ }+ // Heading collapse state (Req 1.6): composite section ids, matching+ // the `data-prism-section-id` attributes the emitter stamps.+ if pass.sectionCollapsedIDs != lastSectionCollapsedIDs {+ lastSectionCollapsedIDs = pass.sectionCollapsedIDs+ controller.setSectionState(collapsedIDs: pass.sectionCollapsedIDs.sorted())+ }+ // The authoritative `<details>` open state (T-1719): occurrence-+ // qualified DOM section ids; prism-theme.js forces each `<details>`+ // open/closed by payload membership, so a recovery replays the exact+ // state.+ if pass.detailsOpenDOMIDs != lastDetailsOpenDOMIDs {+ lastDetailsOpenDOMIDs = pass.detailsOpenDOMIDs+ controller.setDetailsState(expandedIDs: pass.detailsOpenDOMIDs.sorted())+ }+ if pass.tableModes != lastTableModes {+ lastTableModes = pass.tableModes+ controller.setTableModes(pass.tableModes)+ }++ // Navigation. One-shot targets are consumed and cleared; the clears+ // write tracked state, costing exactly one settle pass (see above).+ // TOC sheet/sidebar and fragment-link targets (composite section ids).+ if let target = pass.anchorTarget {+ session.pendingAnchorScroll = nil+ scrollToTarget(target, pass: pass)+ }+ // Notes panel/sidebar targets (bare content hashes or row/item sub-ids).+ if let target = pass.noteNavigationTarget {+ coordinator.noteNavigationTarget = nil+ scrollToTarget(target, pass: pass)+ }+ // Search-match navigation (Req 6.3): level state, deduped by block DOM+ // id so navigating between matches inside one block does not re-jump.+ if pass.searchScrollDOMID != lastSearchScrollDOMID {+ lastSearchScrollDOMID = pass.searchScrollDOMID+ if let domID = pass.searchScrollDOMID {+ controller.scrollTo(blockID: domID)+ }+ }+ }++ // MARK: - Domain computations++ /// Table display modes keyed by DOM id (Req 1.5): native keys are either+ /// DOM ids (web toggles) or legacy composites (pre-cutover callers), the+ /// same two formats scroll restore accepts, so `restoreDOMID` translates.+ /// A key that no longer resolves is dropped from the push.+ private func translatedTableModes(mapped: BlockDOMID.Mapping) -> [String: String] {+ var modes: [String: String] = [:]+ for (key, mode) in session.tableDisplayModes {+ guard let domID = BlockDOMID.restoreDOMID(forStored: key, mapped: mapped) else { continue }+ modes[domID] = mode.webModeAttribute+ }+ return modes+ }++ /// The DOM id of the current search match's block, or nil when there is no+ /// current match. `session.currentMatch` is computed from the search+ /// coordinator's `currentGlobalMatchIndex` + `matchCountsPerBlock` and the+ /// parsed blocks; reading it registers all three.+ private func currentSearchMatchDOMID(mapped: BlockDOMID.Mapping) -> String? {+ guard let match = session.currentMatch else { return nil }+ guard match.blockIndex < mapped.count else { return nil }+ return mapped[match.blockIndex].domID+ }++ /// Translates a native navigation target to a DOM id and drives the web+ /// scroll. A stale/unresolvable target skips the scroll (T-1719).+ /// Collapsed sections hide their blocks in the DOM, so duplicate-content+ /// targets prefer a visible occurrence.+ private func scrollToTarget(_ target: String, pass: SyncPass) {+ // `visibleBlocks` is read here rather than in the tracked pass: it is+ // needed only when a navigation target is actually present (a small+ // minority of passes), and it is recomputed exactly when+ // `collapsedSectionIds` changes, which the pass already registers — so+ // reading it outside the tracked block loses no dependency.+ let visibleSourceIndices = Set(session.sections.visibleBlocks.map(\.sourceIndex))+ guard let domID = BlockDOMID.navigationDOMID(+ forTarget: target,+ mapped: pass.mapped,+ visibleSourceIndices: visibleSourceIndices+ ) else { return }+ controller.scrollTo(blockID: domID)+ }++ // MARK: - Production assembly++ /// Builds the full production web-document assembly for a session:+ /// controller + message router + synchronizer, wired exactly as the+ /// document view mounts them. `DocumentScrollContent` MUST create its+ /// assembly through here so production-assembly tests exercise the real+ /// wiring (the T-1719 regression class).+ static func makeAssembly(+ session: DocumentSession,+ settings: AppSettings,+ coordinator: DocumentLayoutCoordinator,+ notesManager: NotesManager,+ directoryAccessManager: DirectoryAccessManager? = nil,+ onImageAccessNeeded: (@Sendable (URL) -> Void)? = nil,+ routeLinkString: ((String) -> Void)? = nil,+ selectionAffordance: WebSelectionAffordanceState? = nil,+ presentSelectionDeclined: (() -> Void)? = nil+ ) -> (+ controller: WebDocumentController,+ router: WebDocumentMessageRouter,+ synchronizer: WebDocumentStateSynchronizer+ ) {+ let controller = WebDocumentControllerFactory.make(+ session: session,+ settings: settings,+ directoryAccessManager: directoryAccessManager,+ onImageAccessNeeded: onImageAccessNeeded+ )+ var router = WebDocumentMessageRouter(+ session: session,+ coordinator: coordinator,+ routeLinkString: routeLinkString,+ notesManager: notesManager+ )+ router.selectionAffordance = selectionAffordance+ router.presentSelectionDeclined = presentSelectionDeclined+ let synchronizer = WebDocumentStateSynchronizer(+ controller: controller,+ session: session,+ coordinator: coordinator,+ settings: settings,+ notesManager: notesManager+ )+ controller.onMessage = { router.handle($0) }+ return (controller, router, synchronizer)+ }+}
diff --git a/prism/Views/CompactDocumentLayout.swift b/prism/Views/CompactDocumentLayout.swiftindex d505b66e..cd7b8c26 100644--- a/prism/Views/CompactDocumentLayout.swift+++ b/prism/Views/CompactDocumentLayout.swift@@ -38,28 +38,15 @@ struct CompactDocumentLayout: View { /// Shared coordinator for note/scroll/reload state. Owned by DocumentReaderView /// so View-menu scroll commands can route through the active scroll controller.+ /// Also owns compact toolbar visibility (`isCompactToolbarVisible`), driven by+ /// the rendered document's `scrollDirectionChanged` bridge messages (T-1719). @Bindable var coordinator: DocumentLayoutCoordinator - /// Controls bottom toolbar visibility based on scroll direction.- @State private var isToolbarVisible = true-- /// Previous scroll offset for direction detection.- @State private var previousScrollOffset: CGFloat = 0- // MARK: - Sheet Presentation State /// Whether the search overlay sheet is presented. @State private var showSearchOverlay = false - /// Pending scroll target from search result selection.- @State private var pendingSearchScrollTarget: String?-- /// Target block ID for scroll navigation from notes panel.- @State private var notesScrollTarget: String?-- /// Pending scroll target from TOC navigation.- @State var pendingTOCScrollTarget: String?- /// Keyboard focus on the rendered document body. Owned here so the /// layout can re-acquire focus after layout-owned sheets dismiss /// and across scene-phase transitions (Decision 18).@@ -73,10 +60,6 @@ struct CompactDocumentLayout: View { @Environment(\.scenePhase) private var scenePhase #endif - private var typographyResolver: TypographyResolver {- TypographyResolver(from: settings)- }- private var layoutContext: LayoutContext { LayoutContext( session: session,@@ -112,9 +95,11 @@ struct CompactDocumentLayout: View { reduceMotion ? nil : .easeInOut(duration: 0.2), value: coordinator.showRawSource )- // Bottom toolbar with safeAreaInset (Task 12.2)+ // Bottom toolbar with safeAreaInset (Task 12.2). Visibility is native+ // truth on the coordinator, fed by the web bridge's direction flips+ // (hide-on-scroll, T-1719). .safeAreaInset(edge: .bottom) {- if isToolbarVisible {+ if coordinator.isCompactToolbarVisible { CompactBottomToolbar( isRawSourceActive: coordinator.showRawSource, isUnsaved: session.isUnsaved,@@ -137,6 +122,13 @@ struct CompactDocumentLayout: View { .padding(.bottom, 16) } }+ // Animates the inset change when the coordinator flips visibility —+ // the observation-driven equivalent of the withAnimation the retired+ // updateToolbarVisibility wrapped around its @State write.+ .animation(+ reduceMotion ? nil : .easeInOut(duration: 0.25),+ value: coordinator.isCompactToolbarVisible+ ) // Reload banner overlay .overlay(alignment: .top) { if session.fileObserver?.fileChangedExternally == true {@@ -175,7 +167,9 @@ struct CompactDocumentLayout: View { exportFlow: coordinator.exportNotesFlow, onRetryBanner: { coordinator.bannerMessage = $0 }, onNavigate: { blockId in- notesScrollTarget = blockId+ // Consumed by WebDocumentStateSynchronizer, which translates+ // the note anchor id to a DOM id and scrolls the web view.+ coordinator.noteNavigationTarget = blockId }, onDismiss: { showNotesSheet = false@@ -189,7 +183,9 @@ struct CompactDocumentLayout: View { collapsedSectionIds: session.sections.collapsedSectionIds, onNavigate: { sectionId in session.sections.expandSectionAndAncestors(for: sectionId)- pendingTOCScrollTarget = sectionId+ // Consumed by WebDocumentStateSynchronizer (same path as+ // fragment links).+ session.pendingAnchorScroll = sectionId showTOCSheet = false }, onToggleCollapse: { sectionId in@@ -213,9 +209,11 @@ struct CompactDocumentLayout: View { onDismiss: { showSearchOverlay = false },- onSelectResult: { matchIndex, scrollId in- session.search.currentGlobalMatchIndex = matchIndex- pendingSearchScrollTarget = scrollId+ onSelectResult: { matchIndex, _ in+ // navigateToMatch runs the selection side effects+ // (ancestor expansion, VoiceOver); the synchronizer+ // observes the current match and scrolls the web view.+ session.search.navigateToMatch(at: matchIndex) showSearchOverlay = false } )@@ -246,21 +244,9 @@ struct CompactDocumentLayout: View { .sheet(isPresented: $coordinator.showDocumentNoteSheet) { SharedBlockViews.documentNoteSheetContent(context: layoutContext) }- // Footnote popover sheet (iPhone uses sheet with .medium detent)- .sheet(- isPresented: Binding(- get: { coordinator.activeFootnoteId != nil },- set: { if !$0 { coordinator.dismissFootnote() } }- )- ) {- if let id = coordinator.activeFootnoteId {- FootnotePopoverView(- footnoteId: id,- data: session.footnoteData,- popoverPage: coordinator.footnotePopoverPage- )- }- }+ // Footnote popover, shared with the regular layout so the two presentation+ // hosts cannot drift apart (T-1893).+ .footnotePresentation(coordinator: coordinator, session: session) // Fullscreen/zoom for diagrams + images activated in the web surface (T-1542). .mediaZoomPresentation(coordinator: coordinator, session: session) // Sibling-image folder-access picker (iOS), presented here for reliability.@@ -277,17 +263,13 @@ struct CompactDocumentLayout: View { .hidden() } #endif- // Reset state when session changes+ // Reset state when session changes (resetSessionState also restores+ // the compact toolbar and clears the note navigation target). .onChange(of: session.id) { _, _ in coordinator.resetSessionState()- notesScrollTarget = nil showTOCSheet = false- pendingTOCScrollTarget = nil showSearchOverlay = false- pendingSearchScrollTarget = nil session.search.clearSearch()- isToolbarVisible = true- previousScrollOffset = 0 } // Clear search when toggling to raw source (Req 9.1) .onChange(of: coordinator.showRawSource) { _, isRawSource in@@ -361,99 +343,7 @@ struct CompactDocumentLayout: View { // MARK: - Rendered Content View private var renderedContentView: some View {- DocumentScrollContent(- context: layoutContext,- typographyResolver: typographyResolver,- keyboardScroll: coordinator.renderedScroll,- bodyHasFocus: $bodyHasFocus,- onTapInlineNote: { tappedBlock in- coordinator.notePopoverBlock = tappedBlock- coordinator.notePopoverListItemId = nil- },- onToggleSection: { toggleSection($0) },- scrollModifiers: { proxy in- Color.clear- // Hide-on-scroll tracking (Task 12.1)- .onScrollGeometryChange(for: CGFloat.self) { geometry in- geometry.contentOffset.y- } action: { oldOffset, newOffset in- updateToolbarVisibility(oldOffset: oldOffset, newOffset: newOffset)- }- .onChange(of: session.parsedBlocks) { _, newBlocks in- if !newBlocks.isEmpty, !session.scrollPositionID.isEmpty {- DocumentLayoutCoordinator.delayedScrollToID(- session.scrollPositionID,- scroll: coordinator.renderedScroll,- proxy: proxy- )- }- showTOCSheet = false- pendingTOCScrollTarget = nil- }- .onChange(of: notesScrollTarget) { _, newTarget in- if let target = newTarget,- let scrollId = SharedBlockViews.scrollIdForBlock(target, session: session) {- SharedBlockViews.delayedScroll(- to: scrollId,- proxy: proxy,- reduceMotion: reduceMotion- )- notesScrollTarget = nil- }- }- .onChange(of: pendingTOCScrollTarget) { _, newTarget in- if let scrollId = newTarget {- SharedBlockViews.delayedScroll(- to: scrollId,- proxy: proxy,- reduceMotion: reduceMotion- )- pendingTOCScrollTarget = nil- }- }- .onChange(of: pendingSearchScrollTarget) { _, newTarget in- if let scrollId = newTarget {- SharedBlockViews.delayedScroll(- to: scrollId,- anchor: .center,- proxy: proxy,- reduceMotion: reduceMotion- )- pendingSearchScrollTarget = nil- }- }- }- )- }-- // MARK: - Hide-on-Scroll (Task 12.1)-- /// Updates toolbar visibility based on scroll direction.- ///- /// Uses a threshold to prevent flickering on small movements.- /// Toolbar hides when scrolling down past initial content and- /// reappears immediately when scrolling up.- ///- /// Requirements: 4.1-4.2- private func updateToolbarVisibility(oldOffset: CGFloat, newOffset: CGFloat) {- let threshold: CGFloat = 10- let delta = newOffset - oldOffset-- // Only hide when scrolling down past initial content- if delta > threshold && newOffset > 50 {- if isToolbarVisible {- withAnimation(reduceMotion ? nil : .easeInOut(duration: 0.25)) {- isToolbarVisible = false- }- }- } else if delta < -threshold {- // Show immediately when scrolling up- if !isToolbarVisible {- withAnimation(reduceMotion ? nil : .easeInOut(duration: 0.25)) {- isToolbarVisible = true- }- }- }+ DocumentScrollContent(context: layoutContext) } // MARK: - Collapsible Sections@@ -464,7 +354,7 @@ struct CompactDocumentLayout: View { session: session, reduceMotion: reduceMotion, scrollPercentage: coordinator.scrollPercentage,- onScrollToSection: { id in pendingTOCScrollTarget = id }+ onScrollToSection: { id in session.pendingAnchorScroll = id } ) }
diff --git a/prism/Views/DocumentLayoutCoordinator.swift b/prism/Views/DocumentLayoutCoordinator.swiftindex ae6bc5ff..18d2ca75 100644--- a/prism/Views/DocumentLayoutCoordinator.swift+++ b/prism/Views/DocumentLayoutCoordinator.swift@@ -101,6 +101,46 @@ final class DocumentLayoutCoordinator { /// ViewModel for raw source processing. var rawSourceViewModel = RawSourceViewModel() + // MARK: - Web Navigation + Compact Toolbar (T-1719)++ /// One-shot navigation target set by the notes panel (compact) / notes+ /// sidebar (regular). A bare content hash or a sub-block id+ /// (`{hash}-row-N` / `{hash}-item-N`); consumed and cleared by+ /// `WebDocumentStateSynchronizer`, which translates it to a DOM id and+ /// drives the web controller's scroll.+ var noteNavigationTarget: String?++ /// Compact bottom-toolbar visibility, driven by the rendered document's+ /// scroll direction (hide-on-scroll, prism-v1 Req 4.1-4.2). Native truth:+ /// the bridge posts `scrollDirectionChanged` and the message router applies+ /// the threshold rules here; `CompactDocumentLayout` observes this.+ var isCompactToolbarVisible = true++ /// Applies the hide-on-scroll rules for a reported scroll direction:+ /// scrolling down past the initial content hides the toolbar, scrolling up+ /// shows it immediately (the pre-cutover `updateToolbarVisibility` rules).+ /// The ~10px movement hysteresis lives in prism-scroll.js, which posts+ /// only direction flips and the hide-threshold crossing.+ func applyScrollDirection(_ direction: ScrollDirection, offsetY: Double) {+ // Assignment through @Observable notifies on every write, equal or not, so+ // guard both branches: a scroll burst posts many flips and only the ones+ // that actually change visibility should invalidate the layout body.+ switch direction {+ case .down:+ // Only hide once scrolled past the initial content.+ guard isCompactToolbarVisible,+ offsetY > Self.compactToolbarHideThreshold else { return }+ isCompactToolbarVisible = false+ case .up:+ guard !isCompactToolbarVisible else { return }+ isCompactToolbarVisible = true+ }+ }++ /// The y offset a downward scroll must pass before the compact toolbar+ /// hides (the pre-cutover `newOffset > 50` rule).+ static let compactToolbarHideThreshold: Double = 50+ // MARK: - Keyboard Scroll Controllers /// Keyboard scroll controller for the rendered document body.@@ -215,6 +255,8 @@ final class DocumentLayoutCoordinator { showRawSource = false scrollPercentage = 0 pendingRestorePercentage = nil+ noteNavigationTarget = nil+ isCompactToolbarVisible = true rawSourceViewModel.reset() renderedScroll.resetForNewSession() rawSourceScroll.resetForNewSession()@@ -423,25 +465,16 @@ final class DocumentLayoutCoordinator { return true } - /// Restores scroll position from the session.+ /// Scrolls a SwiftUI scroll surface to `targetId` after a short layout+ /// delay. Used by `RawSourceView` (the only remaining ScrollViewProxy+ /// surface); the rendered web document scrolls through+ /// `WebDocumentController.scrollTo` instead (T-1719). /// /// Drives both the legacy `ScrollViewReader` proxy *and* the modern /// `ScrollPosition` binding because the ScrollView attaches /// `.scrollPosition($keyboardScroll.scrollPosition)` — that binding is /// the authoritative scroll driver in iOS 18+ and `proxy.scrollTo` /// alone may not take effect alongside it.- ///- /// Defers both calls by `Self.scrollRestoreDelay` so a freshly-mounted- /// scroll view has completed a layout pass and placed the target block.- func restoreScrollPosition(session: DocumentSession,- scroll: KeyboardScrollController,- proxy: ScrollViewProxy) {- guard !session.scrollPositionID.isEmpty else { return }- Self.delayedScrollToID(session.scrollPositionID, scroll: scroll, proxy: proxy)- }-- /// Convenience used by both `restoreScrollPosition` and the layout- /// `.onChange(of: parsedBlocks)` handlers, which restore after a re-parse. static func delayedScrollToID(_ targetId: String, scroll: KeyboardScrollController, proxy: ScrollViewProxy) {
diff --git a/prism/Views/DocumentScrollContent.swift b/prism/Views/DocumentScrollContent.swiftindex 4c1c7db2..60af6bc3 100644--- a/prism/Views/DocumentScrollContent.swift+++ b/prism/Views/DocumentScrollContent.swift@@ -3,31 +3,25 @@ // prism // // Shared document-body subview used by both compact and regular layouts.-// Hosts the web-rendered document surface (WebDocumentView) and owns the-// per-session WebDocumentController. The legacy SwiftUI block stack and the-// PRISM_DEBUG_LEGACY_RENDERER lever were removed at cutover (webview-rendering-// spec, task 37): the web renderer is the only document path.+// Hosts the web-rendered document surface (WebDocumentView) and mounts the+// per-session production assembly (controller + message router ++// WebDocumentStateSynchronizer) via `WebDocumentStateSynchronizer.makeAssembly`.+// The synchronizer owns every native→web state push and navigation route+// (T-1719); this view keeps only what genuinely needs the view world:+// load/reload triggering, scroll-position restore, the colorScheme-resolved+// theme feed, the iOS image-access banner, and footnote-popover routing. // import SwiftUI /// The shared document-body subview used by both compact and regular layouts. ///-/// Presents the web-rendered document surface and drives its load/theme/-/// typography/comment-visibility lifecycle. Layout-specific scroll handlers are-/// still injected via the `scrollModifiers` closure for source compatibility-/// with the layouts; the web path drives scroll through the controller, so the-/// closure is not attached here.-struct DocumentScrollContent<ScrollModifiers: View>: View {+/// Presents the web-rendered document surface, mounts the production web+/// assembly for the session, and drives its load lifecycle. State sync and+/// navigation live in `WebDocumentStateSynchronizer` (observation-driven, so+/// they exist independent of this view's mounting).+struct DocumentScrollContent: View { let context: LayoutContext- let typographyResolver: TypographyResolver- @Bindable var keyboardScroll: KeyboardScrollController- @FocusState.Binding var bodyHasFocus: Bool- let onTapInlineNote: SharedCollapsibleSections.InlineNoteTapHandler?- let onToggleSection: (String) -> Void- /// Retained for source compatibility with the layouts. The web path drives- /// scroll through `WebDocumentController`, so the closure is not attached.- @ViewBuilder let scrollModifiers: (ScrollViewProxy) -> ScrollModifiers /// The web-rendered document surface controller. Owned here so it shares the /// session's lifetime; created lazily once the session is available@@ -43,6 +37,9 @@ struct DocumentScrollContent<ScrollModifiers: View>: View { /// The message router, retained so the overlay's "Add note" tap can call its shared /// create path. @State private var webRouter: WebDocumentMessageRouter?+ /// The state synchronizer, retained so its observation pass stays armed for+ /// the session and the theme feed below can route through it (T-1719).+ @State private var synchronizer: WebDocumentStateSynchronizer? /// Routes activated links from their RAW href string, the way the SwiftUI path's /// `openURL` handler does but without a `URL(string:)` round-trip that would@@ -50,13 +47,13 @@ struct DocumentScrollContent<ScrollModifiers: View>: View { /// Req 2.4; T-1590). @Environment(\.documentLinkRouter) private var documentLinkRouter - /// Resolves the effective theme (light/dark) for the initial theme push.+ /// Resolves the effective theme (light/dark) for the theme pushes. @Environment(\.colorScheme) private var colorScheme /// Image services, for the iOS sibling-image folder-access grant flow (Req 3.x). @Environment(\.imageServices) private var imageServices - /// The web-rendered document surface. Creates the controller for the session,+ /// The web-rendered document surface. Creates the assembly for the session, /// loads the document, and reloads on parseRevision change (Req 2.5). var body: some View { Group {@@ -97,56 +94,42 @@ struct DocumentScrollContent<ScrollModifiers: View>: View { } } }- // One controller per session; recreated only when the session changes.- let controller = WebDocumentControllerFactory.make(+ // One assembly per session; recreated only when the session changes.+ // makeAssembly wires controller + router + synchronizer exactly as the+ // T-1719 production-assembly tests mount them; the synchronizer owns+ // every state push and navigation route from here on.+ let made = WebDocumentStateSynchronizer.makeAssembly( session: context.session, settings: context.settings,- directoryAccessManager: imageServices.directoryAccessManager,- onImageAccessNeeded: accessCallback- )- // Route accepted JS→native messages onto the existing native truth- // (scroll persistence, footnote/link routing, copy, toggles). The router- // reuses the session/coordinator state (native-as-truth).- //- // The selection affordance is a native overlay on both platforms (T-1542);- // fold selectionCandidate updates into the observable state the view positions- // from (Req 12.2), and retain the router so the overlay's "Add note" tap can- // call its shared create path.- var router = WebDocumentMessageRouter(- session: context.session, coordinator: context.coordinator,+ notesManager: context.notesManager,+ directoryAccessManager: imageServices.directoryAccessManager,+ onImageAccessNeeded: accessCallback, routeLinkString: { href in documentLinkRouter(href) },- notesManager: context.notesManager+ selectionAffordance: selectionAffordance )- router.selectionAffordance = selectionAffordance- webRouter = router- controller.onMessage = { message in router.handle(message) }- // Queue the initial theme/typography/comment-visibility state; it flushes- // on `ready` and replays after a WebContent recovery (Req 1.3/1.4/1.7/9.6).- WebDocumentControllerFactory.pushInitialState(- to: controller,- settings: context.settings,+ webRouter = made.router+ made.synchronizer.start()+ // The theme stays view-fed: the effective key depends on colorScheme.+ made.synchronizer.applyTheme( themeKey: context.settings.theme(for: colorScheme).rawValue )- // Push the note indicators + inline notes/banner from NotesManager (Req 5.2/- // 5.6). Like the other state pushes these queue until `ready` and replay after- // a WebContent recovery via the coalesced snapshot.- WebDocumentControllerFactory.pushNoteState(- to: controller,- session: context.session,- notesManager: context.notesManager,- settings: context.settings- )- // Push the current heading-collapse state (Req 1.6) so a web chevron tap and a- // TOC-sidebar collapse stay in sync, and so it replays after a WebContent- // recovery via the coalesced snapshot. The ids are composite section ids- // (MarkdownSection.id) the emitter stamps onto heading sections.- controller.setSectionState(- collapsedIDs: Array(context.session.sections.collapsedSectionIds)- )- // Push the search-highlight state from SearchCoordinator (Req 6.1/6.2/7.2,- // T-1680) so a search that is already active when the surface (re)mounts- // renders its highlights; re-pushed below on coordinator state changes.+ synchronizer = made.synchronizer+ // Keyboard/menu scrolling (T-1719): route the shared controller's+ // page/edge commands to the rendered document so the macOS View+ // menu and key handling work without SwiftUI scroll geometry.+ let controller = made.controller+ context.scrollController.attachWebBridge(.init(+ pageUp: { controller.scrollByPage(.up) },+ pageDown: { controller.scrollByPage(.down) },+ scrollToTop: { controller.scrollToEdge(.top) },+ scrollToBottom: { controller.scrollToEdge(.bottom) }+ ))+ // Note indicators, heading-collapse, details and table state are pushed by+ // the synchronizer's observation pass (T-1719) — they used to be pushed+ // here. Search highlights stay a view-fed seam: the effective payload+ // depends on the debounced coordinator state this view already observes+ // (T-1680), so push the current one on mount and re-push on change below. WebDocumentControllerFactory.pushSearchState( to: controller, session: context.session,@@ -154,43 +137,11 @@ struct DocumentScrollContent<ScrollModifiers: View>: View { ) webController = controller }- // Re-push notes when the document's notes change (creation, edit, delete, toggle,- // relocation on reload) or when imported notes load.- .onChange(of: context.notesManager.documentNotes) { _, _ in- pushNoteState()- }- .onChange(of: context.notesManager.importedNotes) { _, _ in- pushNoteState()- }- // Re-push when the note display settings change so the page reflects them without- // a reload: showInlineNotes gates bubbles, placement moves the banner.- .onChange(of: context.settings.showInlineNotes) { _, _ in- pushNoteState()- }- .onChange(of: context.settings.documentNotesBannerPlacement) { _, _ in- pushNoteState()- }- // Live theme / typography / comment-visibility updates without a reload.+ // Live theme updates without a reload (Req 1.3). Typography, comment+ // visibility, notes, sections, details, and table modes re-push from the+ // synchronizer's observation pass — no view .onChange needed. .onChange(of: context.settings.theme(for: colorScheme)) { _, theme in- webController?.applyTheme(- theme: theme.rawValue,- variables: [:],- mermaidConfig: WebDocumentControllerFactory.mermaidThemeConfigJSON(for: theme.rawValue)- )- }- .onChange(of: context.settings.textSizeScale) { _, _ in- webController?.applyTypography(- variables: WebDocumentControllerFactory.typographyVariables(settings: context.settings)- )- }- .onChange(of: context.settings.showHTMLComments) { _, visible in- webController?.setCommentVisibility(visible)- }- // Heading collapse (Req 1.6): a TOC-sidebar collapse or a web chevron tap both- // mutate session.sections.collapsedSectionIds; re-push so the web view reflects it- // without a reload (the web chevron already round-tripped through this state).- .onChange(of: context.session.sections.collapsedSectionIds) { _, ids in- webController?.setSectionState(collapsedIDs: Array(ids))+ synchronizer?.applyTheme(themeKey: theme.rawValue) } // Search highlights (Req 6.1/6.2/6.3/7.2, T-1680): re-push whenever the // coordinator's debounced query, per-block counts (recomputed on block or@@ -227,14 +178,6 @@ struct DocumentScrollContent<ScrollModifiers: View>: View { webController.load(documentURL: url, parseRevision: revision) restoreScrollPosition(with: webController) }- // Anchor scroll requests (TOC / fragment links) drive the web view's native- // scroll through the controller instead of a ScrollViewReader proxy.- .onChange(of: context.session.pendingAnchorScroll) { _, newTarget in- if let scrollId = newTarget {- webController?.scrollTo(blockID: scrollId)- context.session.pendingAnchorScroll = nil- }- } .onChange(of: context.session.pendingFootnoteId) { _, newId in if let identifier = newId { let blockId = context.session.pendingFootnoteBlockId@@ -243,6 +186,13 @@ struct DocumentScrollContent<ScrollModifiers: View>: View { context.session.pendingFootnoteBlockId = nil } }+ .onDisappear {+ // The raw-source toggle (and document close) unmounts this view and+ // discards the assembly; return the keyboard controller to its+ // geometry-driven disabled state. The raw-source view drives its own+ // controller (coordinator.rawSourceScroll), which is unaffected.+ context.scrollController.detachWebBridge()+ } #if DEBUG // T-1513 convergence harness over the web renderer: drive the scroll through the // bridge and fold the in-page rAF/LoAF perf probe into the verdict (Req 9.2).@@ -320,18 +270,6 @@ struct DocumentScrollContent<ScrollModifiers: View>: View { webController.scrollTo(blockID: restoreID) } - /// Recomputes and re-pushes the note indicators + inline notes/banner from current- /// NotesManager state and settings. No-op until the controller exists.- private func pushNoteState() {- guard let webController else { return }- WebDocumentControllerFactory.pushNoteState(- to: webController,- session: context.session,- notesManager: context.notesManager,- settings: context.settings- )- }- /// Recomputes and re-pushes the per-block search-highlight state from the /// session's SearchCoordinator (T-1680). No-op until the controller exists. private func pushSearchState() {
diff --git a/prism/Views/FootnotePresenter.swift b/prism/Views/FootnotePresenter.swiftnew file mode 100644index 00000000..108a2139--- /dev/null+++ b/prism/Views/FootnotePresenter.swift@@ -0,0 +1,61 @@+//+// FootnotePresenter.swift+// prism+//+// Presents the footnote popover for a footnote badge activated in the+// web-rendered document surface (footnotes spec; T-1893). A badge tap arrives+// as a `prism://footnote/{id}` link over the bridge, is routed by+// `WebDocumentMessageRouter` into `session.pendingFootnoteId`, and lands in+// `DocumentLayoutCoordinator.activeFootnoteId`; this modifier turns that state+// into the presentation.+//+// `FootnotePopoverView` sizes itself per platform (macOS panel frame, iOS+// medium detent + drag indicator), so both layouts present it the same way.+//+// Applied by both CompactDocumentLayout and RegularDocumentLayout so the+// presentation lives in one place. It used to be inline in the compact layout+// only, which is exactly why footnote taps did nothing on macOS and+// regular-width iPad (T-1893) — `DocumentReaderView` selects the regular+// layout there.+//++import SwiftUI++struct FootnotePresenter: ViewModifier {+ let coordinator: DocumentLayoutCoordinator+ let session: DocumentSession++ /// Presented whenever the coordinator holds an active footnote; dismissing+ /// clears the coordinator state so `coordinatorOwnsModalPresentation` does+ /// not stay true with nothing on screen.+ private var isPresented: Binding<Bool> {+ Binding(+ get: { coordinator.activeFootnoteId != nil },+ set: { if !$0 { coordinator.dismissFootnote() } }+ )+ }++ func body(content: Content) -> some View {+ content.sheet(isPresented: isPresented) {+ if let id = coordinator.activeFootnoteId {+ FootnotePopoverView(+ footnoteId: id,+ data: session.footnoteData,+ popoverPage: coordinator.footnotePopoverPage+ )+ }+ }+ }+}++extension View {+ /// Presents the footnote popover/sheet for footnote badges activated in the+ /// web document surface. Both document layouts must apply this — pinned by+ /// `FootnotePresentationHostTests`.+ func footnotePresentation(+ coordinator: DocumentLayoutCoordinator,+ session: DocumentSession+ ) -> some View {+ modifier(FootnotePresenter(coordinator: coordinator, session: session))+ }+}
diff --git a/prism/Views/RegularDocumentLayout.swift b/prism/Views/RegularDocumentLayout.swiftindex dcd8ef6e..a99fae67 100644--- a/prism/Views/RegularDocumentLayout.swift+++ b/prism/Views/RegularDocumentLayout.swift@@ -44,11 +44,6 @@ struct RegularDocumentLayout: View { /// Current right sidebar width (initialized from settings on appear). @State private var rightSidebarWidth: CGFloat = AppSettings.defaultSidebarWidth - // MARK: - Navigation State-- /// Target block ID for scroll navigation from sidebar or notes.- @State var scrollTarget: String?- /// Keyboard focus on the rendered document body. Owned here so /// the layout can re-acquire focus after layout-owned modals dismiss /// and across scene/window-active transitions (Decision 18).@@ -64,10 +59,6 @@ struct RegularDocumentLayout: View { @Environment(\.scenePhase) private var scenePhase #endif - private var typographyResolver: TypographyResolver {- TypographyResolver(from: settings)- }- private var layoutContext: LayoutContext { LayoutContext( session: session,@@ -139,7 +130,6 @@ struct RegularDocumentLayout: View { )) .onChange(of: session.id) { _, _ in coordinator.resetSessionState()- scrollTarget = nil } // Note popover: view/edit/delete/resolve/reply existing notes on a block // (Req 4.1-4.3 / 5.2 / 5.6, T-1542). Sheet on iPad/macOS, matching the@@ -163,6 +153,10 @@ struct RegularDocumentLayout: View { .sheet(isPresented: $coordinator.showDocumentNoteSheet) { SharedBlockViews.documentNoteSheetContent(context: layoutContext) }+ // Footnote popover for badges activated in the web surface (T-1893). Shared+ // with the compact layout; this layout previously had no footnote+ // presentation at all, so taps on macOS / wide iPad did nothing.+ .footnotePresentation(coordinator: coordinator, session: session) // Fullscreen/zoom for diagrams + images activated in the web surface (T-1542): // iPad presents a sheet; macOS opens the dedicated diagram/image window. .mediaZoomPresentation(coordinator: coordinator, session: session)@@ -220,7 +214,9 @@ struct RegularDocumentLayout: View { collapsedSectionIds: session.sections.collapsedSectionIds, onNavigate: { sectionId in session.sections.expandSectionAndAncestors(for: sectionId)- scrollTarget = sectionId+ // Consumed by WebDocumentStateSynchronizer (same path+ // as fragment links).+ session.pendingAnchorScroll = sectionId }, onToggleCollapse: { sectionId in toggleSection(sectionId)@@ -266,7 +262,10 @@ struct RegularDocumentLayout: View { session: session, exportFlow: coordinator.exportNotesFlow, onNavigateToNote: { blockId in- scrollTarget = blockId+ // Consumed by WebDocumentStateSynchronizer, which+ // translates the note anchor id to a DOM id and+ // scrolls the web view.+ coordinator.noteNavigationTarget = blockId } ) .frame(width: rightSidebarWidth)@@ -315,55 +314,7 @@ struct RegularDocumentLayout: View { // MARK: - Rendered Content View private var renderedContentView: some View {- DocumentScrollContent(- context: layoutContext,- typographyResolver: typographyResolver,- keyboardScroll: coordinator.renderedScroll,- bodyHasFocus: $bodyHasFocus,- onTapInlineNote: nil,- onToggleSection: { toggleSection($0) },- scrollModifiers: { proxy in- Color.clear- .onChange(of: session.parsedBlocks) { _, newBlocks in- if !newBlocks.isEmpty, !session.scrollPositionID.isEmpty {- DocumentLayoutCoordinator.delayedScrollToID(- session.scrollPositionID,- scroll: coordinator.renderedScroll,- proxy: proxy- )- }- }- // Scroll navigation from sidebar- .onChange(of: scrollTarget) { _, newTarget in- if let target = newTarget {- let scrollId = SharedBlockViews.scrollIdForBlock(target, session: session) ?? target-- SharedBlockViews.delayedScroll(- to: scrollId,- proxy: proxy,- reduceMotion: reduceMotion- )- scrollTarget = nil- }- }- // Scroll to current search match (Requirement 6.3)- .onChange(of: session.currentMatch?.blockId) { _, blockId in- if let blockId {- let sourceIndex = session.sections.visibleBlocks.first(where: { $0.block.id == blockId })?.sourceIndex- ?? session.parsedBlocks.firstIndex(where: { $0.id == blockId })- guard let sourceIndex else { return }-- let scrollId = "\(blockId)-\(sourceIndex)"- SharedBlockViews.delayedScroll(- to: scrollId,- anchor: .center,- proxy: proxy,- reduceMotion: reduceMotion- )- }- }- }- )+ DocumentScrollContent(context: layoutContext) } // MARK: - Collapsible Sections@@ -374,7 +325,7 @@ struct RegularDocumentLayout: View { session: session, reduceMotion: reduceMotion, scrollPercentage: coordinator.scrollPercentage,- onScrollToSection: { id in scrollTarget = id }+ onScrollToSection: { id in session.pendingAnchorScroll = id } ) }
diff --git a/prism/Views/SharedBlockViews.swift b/prism/Views/SharedBlockViews.swiftindex 7010291f..67eebfd7 100644--- a/prism/Views/SharedBlockViews.swift+++ b/prism/Views/SharedBlockViews.swift@@ -29,13 +29,16 @@ struct LayoutContext { let scrollController: KeyboardScrollController } -/// Namespace for shared sheet builders and scroll helpers used by both layout-/// views. The in-flow block/list/table row builders and the block-anchored note-/// popovers retired with the SwiftUI document path at cutover (webview-rendering-/// spec, task 37); rows are emitted by `BlockHTMLEmitter` and note interactions-/// arrive as bridge messages routed by `WebDocumentMessageRouter`. The add-note,-/// reply, and document-note sheets remain — they are presented at layout level-/// and driven by coordinator state the router sets.+/// Namespace for shared sheet builders used by both layout views. The in-flow+/// block/list/table row builders and the block-anchored note popovers retired+/// with the SwiftUI document path at cutover (webview-rendering spec, task 37);+/// rows are emitted by `BlockHTMLEmitter` and note interactions arrive as+/// bridge messages routed by `WebDocumentMessageRouter`. The ScrollViewProxy+/// helpers retired with T-1719: navigation now routes through+/// `WebDocumentStateSynchronizer` → `BlockDOMID.navigationDOMID` →+/// `WebDocumentController.scrollTo`. The add-note, reply, and document-note+/// sheets remain — they are presented at layout level and driven by+/// coordinator state the router sets. enum SharedBlockViews { // MARK: - Add Note Sheet Content@@ -122,67 +125,6 @@ enum SharedBlockViews { ) } - // MARK: - Delayed Scroll Helper-- /// Scrolls to a target after a short layout delay, respecting reduce-motion.- ///- /// Replaces the repeated pattern of `DispatchQueue.main.asyncAfter` +- /// `reduceMotion` check + `withAnimation` + `proxy.scrollTo` used by- /// both layout views for TOC, search, notes, and anchor scroll targets.- static func delayedScroll(- to id: String,- anchor: UnitPoint = .top,- proxy: ScrollViewProxy,- reduceMotion: Bool- ) {- DispatchQueue.main.asyncAfter(deadline: .now() + 0.1) {- if reduceMotion {- proxy.scrollTo(id, anchor: anchor)- } else {- withAnimation(.easeInOut(duration: 0.3)) {- proxy.scrollTo(id, anchor: anchor)- }- }- }- }-- /// Finds the scroll ID for a block by looking up visible blocks first, then parsed blocks.- ///- /// Handles sub-block IDs (e.g., `"{blockId}-row-0"`, `"{blockId}-item-2"`) by- /// extracting the parent block ID and scrolling to the containing block.- static func scrollIdForBlock(_ blockId: String, session: DocumentSession) -> String? {- // Try direct match first- if let visibleBlock = session.sections.visibleBlocks.first(where: { $0.block.id == blockId }) {- return "\(blockId)-\(visibleBlock.sourceIndex)"- }- if let index = session.parsedBlocks.firstIndex(where: { $0.id == blockId }) {- return "\(blockId)-\(index)"- }-- // Try extracting parent block ID from a sub-block ID (row or item suffix)- let parentId = parentBlockId(from: blockId)- guard parentId != blockId else { return nil }-- if let visibleBlock = session.sections.visibleBlocks.first(where: { $0.block.id == parentId }) {- return "\(parentId)-\(visibleBlock.sourceIndex)"- }- if let index = session.parsedBlocks.firstIndex(where: { $0.id == parentId }) {- return "\(parentId)-\(index)"- }- return nil- }-- /// Extracts the parent block ID from a sub-block ID by stripping known suffixes.- ///- /// `"{blockId}-row-0"` → `"{blockId}"`- /// `"{blockId}-row-header"` → `"{blockId}"`- /// `"{blockId}-item-2"` → `"{blockId}"`- /// `"{blockId}"` → `"{blockId}"` (unchanged)- private static func parentBlockId(from subBlockId: String) -> String {- let pattern = /-(row-(?:header|\d+)|item-\d+(?:-item-\d+)*)$/- guard let match = subBlockId.firstMatch(of: pattern) else { return subBlockId }- return String(subBlockId[subBlockId.startIndex..<match.range.lowerBound])- } } // MARK: - Document Navigation Title
diff --git a/prism/Views/SharedCollapsibleSections.swift b/prism/Views/SharedCollapsibleSections.swiftindex 8e241351..68fb90e7 100644--- a/prism/Views/SharedCollapsibleSections.swift+++ b/prism/Views/SharedCollapsibleSections.swift@@ -20,12 +20,6 @@ enum SharedCollapsibleSections { /// Sections smaller than this animate content collapse/expand transitions. static let animationBlockThreshold = 20 - /// Callback for heading-level inline note taps.- ///- /// Retained for source compatibility with `DocumentScrollContent` and the- /// layouts, which still thread an `onTapInlineNote` closure through.- typealias InlineNoteTapHandler = (MarkdownBlock) -> Void- // MARK: - Toggle Section /// Toggles collapse/expand for a section with animation and scroll compensation.
diff --git a/prismTests/FootnotePresentationHostTests.swift b/prismTests/FootnotePresentationHostTests.swiftnew file mode 100644index 00000000..3f78184c--- /dev/null+++ b/prismTests/FootnotePresentationHostTests.swift@@ -0,0 +1,107 @@+//+// FootnotePresentationHostTests.swift+// prismTests+//+// Regression tests for T-1893: footnote taps did nothing in the regular+// (iPad regular-width / macOS) layout.+//+// `DocumentLayoutCoordinator.activeFootnoteId` is set by the shared routing+// path for every layout, but only `CompactDocumentLayout` attached a+// presentation observing it. `DocumentReaderView` forces the regular layout on+// macOS and uses it for wide iPad, so on those platforms a footnote badge tap+// updated coordinator state and produced no UI — and left+// `coordinatorOwnsModalPresentation` true with no modal on screen.+//+// The presentation now lives in one shared modifier+// (`View.footnotePresentation(coordinator:session:)`) that BOTH layouts apply,+// so the two hosts cannot drift apart again. The structural tests below pin+// that: they fail if either layout stops applying it.+//++import Foundation+import Testing+@testable import prism++@Suite("Footnote presentation hosts (T-1893)")+struct FootnotePresentationHostTests {++ // MARK: - Source-structural wiring++ /// The layout sources, read from disk relative to this file (the same+ /// `#filePath` approach `ParityFixtureSupport` uses) so the check needs no+ /// bundle resource wiring.+ private static func layoutSource(_ fileName: String) throws -> String {+ let viewsDirectory = URL(fileURLWithPath: #filePath)+ .deletingLastPathComponent() // prismTests+ .deletingLastPathComponent() // repo root+ .appendingPathComponent("prism")+ .appendingPathComponent("Views")+ return try String(+ contentsOf: viewsDirectory.appendingPathComponent(fileName),+ encoding: .utf8+ )+ }++ @Test("Both document layouts apply the shared footnote presentation", arguments: [+ "CompactDocumentLayout.swift",+ "RegularDocumentLayout.swift"+ ])+ func bothLayoutsPresentFootnotes(fileName: String) throws {+ let source = try Self.layoutSource(fileName)+ #expect(+ source.contains(".footnotePresentation("),+ """+ \(fileName) must apply `.footnotePresentation(coordinator:session:)`.+ Without it, tapping a footnote badge sets coordinator.activeFootnoteId \+ but shows no popover (T-1893).+ """+ )+ }++ /// Guards the reason the bug was invisible: the presentation must be driven+ /// by the shared coordinator state, not by layout-local state that only one+ /// layout happens to own.+ @Test("The shared footnote presentation is driven by coordinator state")+ func presentationReadsCoordinatorState() throws {+ let source = try Self.layoutSource("FootnotePresenter.swift")+ #expect(source.contains("coordinator.activeFootnoteId"))+ #expect(source.contains("coordinator.dismissFootnote()"))+ #expect(source.contains("FootnotePopoverView("))+ }++ // MARK: - Coordinator contract the presentation binds to++ @MainActor+ @Test("showFootnote publishes presentation state and dismissFootnote clears it")+ func footnoteStateRoundTrips() {+ let coordinator = DocumentLayoutCoordinator()++ #expect(coordinator.activeFootnoteId == nil)+ #expect(coordinator.coordinatorOwnsModalPresentation == false)++ coordinator.showFootnote(identifier: "note-1", blockId: "block-a")++ #expect(coordinator.activeFootnoteId == "note-1")+ #expect(coordinator.activeFootnoteBlockId == "block-a")+ // The regular layout used to leave this true with nothing on screen.+ #expect(coordinator.coordinatorOwnsModalPresentation)++ coordinator.dismissFootnote()++ #expect(coordinator.activeFootnoteId == nil)+ #expect(coordinator.activeFootnoteBlockId == nil)+ #expect(coordinator.coordinatorOwnsModalPresentation == false)+ }++ @MainActor+ @Test("Switching sessions clears any active footnote presentation")+ func sessionResetClearsFootnote() {+ let coordinator = DocumentLayoutCoordinator()+ coordinator.showFootnote(identifier: "note-1", blockId: "block-a")++ coordinator.resetSessionState()++ #expect(coordinator.activeFootnoteId == nil)+ #expect(coordinator.activeFootnoteBlockId == nil)+ }+}
diff --git a/prismTests/TOCNavigationDetailsIndexTests.swift b/prismTests/TOCNavigationDetailsIndexTests.swiftnew file mode 100644index 00000000..a3280621--- /dev/null+++ b/prismTests/TOCNavigationDetailsIndexTests.swift@@ -0,0 +1,173 @@+//+// TOCNavigationDetailsIndexTests.swift+// prismTests+//+// Regression tests for T-1662: TOC and fragment navigation resolving to+// nothing once a `<details>` block appears in the document.+//+// TOCCoordinator numbers entries with a FLATTENED counter that also advances+// through a `<details>` block's nested children, but `.details` is a single+// TOP-LEVEL block (its children never occupy a top-level slot), and both+// `MarkdownSection.id` and `BlockDOMID` verify a composite's trailing index+// against the top-level `parsedBlocks` enumeration. After one `<details>`+// block the two counters diverge, so every later heading's composite named the+// wrong block — or ran past the end — and the tap silently did nothing.+//+// These go through the REAL producer (`toc.tocEntries`) rather than+// hand-building a composite. That is the gap the original T-1719 verification+// had: `WebStateSynchronizerAssemblyTests.tocNavigationTargetsWebDOMID` feeds+// the resolver an id it built itself, so it proved the resolver works and never+// exercised the producer.+//+// Deliberately synchronous — parser → TOCCoordinator → BlockDOMID, no+// DocumentSession — so there is no async/actor surface here.+//++import Foundation+import Testing+@testable import prism++@MainActor+@Suite("TOC navigation ids resolve with <details> present (T-1662)")+struct TOCNavigationDetailsIndexTests {++ /// A heading that follows a `<details>` block holding two children, plus a+ /// heading nested inside it.+ private static let withDetails = """+ # Intro++ <details>+ <summary>Collapsed</summary>++ ### Nested Heading++ Child paragraph.++ </details>++ ## Later Heading++ Trailing paragraph.+ """++ private static let withoutDetails = """+ # Intro++ Some paragraph.++ ## Later Heading++ Trailing paragraph.+ """++ private func toc(for markdown: String) -> (entries: [TOCEntry], blocks: [MarkdownBlock]) {+ let blocks = MarkdownBlockParser.parse(markdown)+ let coordinator = TOCCoordinator()+ coordinator.invalidateAndUpdate(blocks: blocks)+ return (coordinator.tocEntries, blocks)+ }++ private func entry(_ text: String, in entries: [TOCEntry]) throws -> TOCEntry {+ try #require(+ entries.first { $0.text.contains(text) },+ "fixture must produce a TOC entry containing \"\(text)\""+ )+ }++ // MARK: - The regression++ @Test("A heading after a <details> block resolves to a DOM id")+ func headingAfterDetailsResolves() throws {+ let (entries, blocks) = toc(for: Self.withDetails)+ let later = try entry("Later", in: entries)++ #expect(+ BlockDOMID.navigationDOMID(forTarget: later.scrollId, blocks: blocks) != nil,+ """+ scrollId "\(later.scrollId)" did not resolve. The flattened TOC \+ counter diverged from the top-level parsedBlocks index that \+ BlockDOMID verifies against (T-1662).+ """+ )+ }++ @Test("A heading after a <details> block resolves to that heading's own block")+ func headingAfterDetailsResolvesToTheRightBlock() throws {+ let (entries, blocks) = toc(for: Self.withDetails)+ let later = try entry("Later", in: entries)++ let resolved = try #require(+ BlockDOMID.navigationDOMID(forTarget: later.scrollId, blocks: blocks)+ )+ let expectedIndex = try #require(+ blocks.firstIndex { block in+ if case .heading(_, let text) = block { return text.contains("Later") }+ return false+ }+ )+ // Resolving to *a* DOM id is not enough — it must be the heading's own.+ #expect(resolved == BlockDOMID.map(blocks: blocks)[expectedIndex].domID)+ }++ @Test("A TOC scroll target's index is the heading's top-level parsedBlocks index")+ func scrollTargetUsesTopLevelIndex() throws {+ let (entries, blocks) = toc(for: Self.withDetails)+ let later = try entry("Later", in: entries)++ let parsedIndex = try #require(+ blocks.firstIndex { block in+ if case .heading(_, let text) = block { return text.contains("Later") }+ return false+ }+ )+ let trailingIndex = try #require(+ later.scrollId.split(separator: "-").last.flatMap { Int($0) }+ )++ #expect(+ trailingIndex == parsedIndex,+ "scroll target index \(trailingIndex) must equal top-level index \(parsedIndex)"+ )+ }++ // MARK: - Nested headings++ @Test("A heading inside <details> navigates to the containing details block")+ func nestedHeadingResolvesToItsDetailsBlock() throws {+ let (entries, blocks) = toc(for: Self.withDetails)+ let nested = try entry("Nested", in: entries)++ let resolved = try #require(+ BlockDOMID.navigationDOMID(forTarget: nested.scrollId, blocks: blocks),+ "a nested heading must still resolve to something scrollable"+ )+ let detailsIndex = try #require(+ blocks.firstIndex { block in+ if case .details = block { return true }+ return false+ }+ )+ // Nested headings get no section id of their own from the emitter, so+ // the containing details block is the correct nearest anchor.+ #expect(resolved == BlockDOMID.map(blocks: blocks)[detailsIndex].domID)+ }++ @Test("Entry identity stays unique even though scroll targets can repeat")+ func identityRemainsUnique() throws {+ let (entries, _) = toc(for: Self.withDetails)+ #expect(Set(entries.map(\.id)).count == entries.count)+ }++ // MARK: - Control++ @Test("Control — a heading resolves in a document with no <details>")+ func headingResolvesWithoutDetails() throws {+ let (entries, blocks) = toc(for: Self.withoutDetails)+ let later = try entry("Later", in: entries)++ #expect(BlockDOMID.navigationDOMID(forTarget: later.scrollId, blocks: blocks) != nil)+ // With no <details> the two index schemes agree, so id and scroll target+ // must still coincide — this is what the pre-existing tests relied on.+ #expect(later.scrollId == later.id)+ }+}
diff --git a/prismTests/WebRendering/WebScrollIntegrationContractTests.swift b/prismTests/WebRendering/WebScrollIntegrationContractTests.swiftnew file mode 100644index 00000000..3486ebc3--- /dev/null+++ b/prismTests/WebRendering/WebScrollIntegrationContractTests.swift@@ -0,0 +1,182 @@+//+// WebScrollIntegrationContractTests.swift+// prismTests+//+// T-1719 regression tests for the two scroll seams the ScrollViewProxy+// cutover severed and never rebuilt on the web path:+//+// 1. Compact hide-on-scroll: the only inbound scroll signal (`visibleBlock`)+// is a trailing debounce that fires when scrolling PAUSES, so the bridge+// contract gains a `scrollDirectionChanged` message (data-only,+// allowlisted, generation-tagged) that the message router folds into+// `DocumentLayoutCoordinator.isCompactToolbarVisible` using the+// pre-cutover threshold rules.+//+// 2. Keyboard/menu scrolling: `KeyboardScrollController` drives a SwiftUI+// ScrollPosition bound to nothing on the rendered path, and its geometry+// inputs never arrive, so `canScroll`/`hasContent` stay false and the+// macOS View-menu commands are permanently disabled. A web-bridge backend+// routes the commands to the controller's scrollByPage/scrollToEdge.+//+// Pre-fix, every expectation here fails (T-1719 red phase).+//++import Foundation+import Testing+@testable import prism++@MainActor+struct WebScrollIntegrationContractTests {++ // MARK: - scrollDirectionChanged bridge contract++ private func makeController() -> WebDocumentController {+ WebDocumentController(+ sessionID: "t1719",+ parseRevision: 1,+ schemeHandler: PrismDocSchemeHandler()+ )+ }++ @Test("scrollDirectionChanged decodes through the audited bridge allowlist")+ func scrollDirectionChangedDecodes() {+ let controller = makeController()+ let body: [String: Any] = [+ "type": "scrollDirectionChanged",+ "generation": controller.currentGeneration.argumentValue,+ "direction": "down",+ "offsetY": 120.0,+ ]+ let result = controller.receive(messageBody: body)+ #expect(result == .accepted(.scrollDirectionChanged(direction: .down, offsetY: 120)))+ }++ @Test("A malformed scrollDirectionChanged payload is dropped, not defaulted")+ func scrollDirectionChangedMalformedDropped() {+ let controller = makeController()+ let body: [String: Any] = [+ "type": "scrollDirectionChanged",+ "generation": controller.currentGeneration.argumentValue,+ "direction": "sideways",+ "offsetY": 120.0,+ ]+ let result = controller.receive(messageBody: body)+ #expect(result != .accepted(.scrollDirectionChanged(direction: .down, offsetY: 120)))+ if case .accepted = result {+ Issue.record("malformed direction must not decode to an accepted message")+ }+ }++ // MARK: - Router → coordinator hide-on-scroll routing++ @Test("scrollDirectionChanged drives the compact toolbar visibility")+ func scrollDirectionDrivesCompactToolbar() async {+ let session = DocumentSession(+ url: URL(fileURLWithPath: "/tmp/t1719-scroll.md"),+ content: "# Title\n\nBody."+ )+ await session.parseContent()+ let coordinator = DocumentLayoutCoordinator()+ let router = WebDocumentMessageRouter(session: session, coordinator: coordinator)++ #expect(coordinator.isCompactToolbarVisible)++ router.handle(.scrollDirectionChanged(direction: .down, offsetY: 300))+ #expect(!coordinator.isCompactToolbarVisible, "scrolling down past initial content hides the toolbar")++ router.handle(.scrollDirectionChanged(direction: .up, offsetY: 250))+ #expect(coordinator.isCompactToolbarVisible, "scrolling up shows the toolbar immediately")+ }++ // MARK: - Coordinator threshold rules (pre-cutover behaviour)++ @Test("Scrolling down near the top does not hide the toolbar")+ func downNearTopKeepsToolbar() {+ let coordinator = DocumentLayoutCoordinator()+ coordinator.applyScrollDirection(.down, offsetY: 30)+ #expect(coordinator.isCompactToolbarVisible, "hide only engages past the initial content (offset > 50)")+ }++ @Test("Scrolling down past the threshold hides; scrolling up restores")+ func downPastThresholdHidesUpRestores() {+ let coordinator = DocumentLayoutCoordinator()+ coordinator.applyScrollDirection(.down, offsetY: 120)+ #expect(!coordinator.isCompactToolbarVisible)+ coordinator.applyScrollDirection(.up, offsetY: 500)+ #expect(coordinator.isCompactToolbarVisible)+ }++ @Test("resetSessionState restores the toolbar for the next document")+ func resetSessionStateRestoresToolbar() {+ let coordinator = DocumentLayoutCoordinator()+ coordinator.applyScrollDirection(.down, offsetY: 120)+ #expect(!coordinator.isCompactToolbarVisible)+ coordinator.resetSessionState()+ #expect(coordinator.isCompactToolbarVisible)+ }++ // MARK: - Keyboard / menu scroll web backend++ @MainActor+ private final class CommandRecorder {+ var pageUp = 0+ var pageDown = 0+ var top = 0+ var bottom = 0++ var commands: KeyboardScrollController.WebCommands {+ KeyboardScrollController.WebCommands(+ pageUp: { self.pageUp += 1 },+ pageDown: { self.pageDown += 1 },+ scrollToTop: { self.top += 1 },+ scrollToBottom: { self.bottom += 1 }+ )+ }+ }++ @Test("Attaching the web bridge enables scrolling and routes commands to it")+ func webBridgeRoutesKeyboardCommands() {+ let controller = KeyboardScrollController()+ let recorder = CommandRecorder()++ #expect(!controller.canScroll)+ controller.attachWebBridge(recorder.commands)+ #expect(controller.hasContent, "menu Top/Bottom enable from hasContent (Req 6.6/6.7)")+ #expect(controller.canScroll, "page/arrow commands gate on canScroll")++ controller.pageDown(reduceMotion: true)+ controller.pageUp(reduceMotion: true)+ controller.scrollToTop(reduceMotion: true)+ controller.scrollToBottom(reduceMotion: true)+ #expect(recorder.pageDown == 1)+ #expect(recorder.pageUp == 1)+ #expect(recorder.top == 1)+ #expect(recorder.bottom == 1)+ }++ @Test("The suspended gate still applies with a web bridge attached")+ func suspendedGateAppliesToWebBridge() {+ let controller = KeyboardScrollController()+ let recorder = CommandRecorder()+ controller.attachWebBridge(recorder.commands)+ controller.suspended = true++ controller.pageDown(reduceMotion: true)+ controller.scrollToBottom(reduceMotion: true)+ #expect(recorder.pageDown == 0, "a coordinator-owned modal must keep suppressing scroll commands (T-1099)")+ #expect(recorder.bottom == 0)+ }++ @Test("Detaching the web bridge disables scrolling again")+ func detachWebBridgeDisables() {+ let controller = KeyboardScrollController()+ let recorder = CommandRecorder()+ controller.attachWebBridge(recorder.commands)+ #expect(controller.canScroll)++ controller.detachWebBridge()+ #expect(!controller.canScroll)+ controller.pageDown(reduceMotion: true)+ #expect(recorder.pageDown == 0, "commands must not route to a detached bridge")+ }+}
diff --git a/prismTests/WebRendering/WebSearchWiringTests.swift b/prismTests/WebRendering/WebSearchWiringTests.swiftindex 51785a8b..43a505f4 100644--- a/prismTests/WebRendering/WebSearchWiringTests.swift+++ b/prismTests/WebRendering/WebSearchWiringTests.swift@@ -56,24 +56,19 @@ struct WebSearchWiringTests { notesManager: NotesManager ) -> WebDocumentController { let coordinator = DocumentLayoutCoordinator()- let controller = WebDocumentControllerFactory.make(session: session, settings: settings)- let router = WebDocumentMessageRouter(session: session, coordinator: coordinator)- controller.onMessage = { message in router.handle(message) }-- WebDocumentControllerFactory.pushInitialState(- to: controller,- settings: settings,- themeKey: settings.theme(for: .light).rawValue- )- WebDocumentControllerFactory.pushNoteState(- to: controller,+ // Mirrors DocumentScrollContent's `.task(id: session.id)` block: one+ // assembly per session, started so the synchronizer's observation pass+ // owns the note/section/details/table pushes (T-1719), then the+ // view-fed theme and search pushes.+ let made = WebDocumentStateSynchronizer.makeAssembly( session: session,- notesManager: notesManager,- settings: settings- )- controller.setSectionState(- collapsedIDs: Array(session.sections.collapsedSectionIds)+ settings: settings,+ coordinator: coordinator,+ notesManager: notesManager )+ let controller = made.controller+ made.synchronizer.start()+ made.synchronizer.applyTheme(themeKey: settings.theme(for: .light).rawValue) WebDocumentControllerFactory.pushSearchState( to: controller, session: session,
diff --git a/prismTests/WebRendering/WebStateSynchronizerAssemblyTests.swift b/prismTests/WebRendering/WebStateSynchronizerAssemblyTests.swiftnew file mode 100644index 00000000..40599daf--- /dev/null+++ b/prismTests/WebRendering/WebStateSynchronizerAssemblyTests.swift@@ -0,0 +1,366 @@+//+// WebStateSynchronizerAssemblyTests.swift+// prismTests+//+// T-1719 regression tests: production-assembly coverage for the WebKit+// navigation/state integration seam the ScrollViewProxy cutover left dead.+//+// These tests mount the REAL production assembly — controller + message+// router + WebDocumentStateSynchronizer via+// `WebDocumentStateSynchronizer.makeAssembly` (the same entry point+// DocumentScrollContent uses) — and assert that native truth reaches the+// controller's coalesced snapshot. No live WebPage is needed: commands queue+// before `ready` and fold into `latestSnapshot`, which is exactly what a+// reload/WebContent recovery replays (Req 9.6).+//+// Pre-fix behaviour (the bug): navigation targets are set but never consumed,+// and setDetailsState/setTableModes/typography/note state have no production+// sender, so every expectation here fails — the components exist but the+// production wiring does not (T-1719).+//++import Foundation+import Testing+@testable import prism++@MainActor+struct WebStateSynchronizerAssemblyTests {++ // MARK: - Fixture++ /// Markdown exercising every navigation/state surface: headings (TOC /+ /// section collapse), paragraphs (search), a table (display modes), and+ /// two details blocks (open-state seeding + toggle tracking).+ private static let fixture = """+ # Alpha++ Intro paragraph with searchable words.++ ## Beta++ Second paragraph, also searchable.++ | Left | Right |+ | ---- | ----- |+ | a | b |++ <details open>+ <summary>Open by default</summary>++ Open body text.++ </details>++ <details>+ <summary>Closed by default</summary>++ Closed body text.++ </details>++ Final paragraph.+ """++ /// The full production assembly plus the native collaborators the tests+ /// mutate. Mirrors what DocumentScrollContent mounts for a session.+ private struct Assembly {+ let session: DocumentSession+ let coordinator: DocumentLayoutCoordinator+ let settings: AppSettings+ let notesManager: NotesManager+ let controller: WebDocumentController+ let router: WebDocumentMessageRouter+ let synchronizer: WebDocumentStateSynchronizer+ }++ private func makeAssembly(content: String = fixture) async -> Assembly {+ let session = DocumentSession(+ url: URL(fileURLWithPath: "/tmp/t1719-assembly-fixture.md"),+ content: content+ )+ await session.parseContent()+ let coordinator = DocumentLayoutCoordinator()+ let settings = AppSettings()+ let notesManager = NotesManager()+ let made = WebDocumentStateSynchronizer.makeAssembly(+ session: session,+ settings: settings,+ coordinator: coordinator,+ notesManager: notesManager+ )+ made.synchronizer.start()+ return Assembly(+ session: session,+ coordinator: coordinator,+ settings: settings,+ notesManager: notesManager,+ controller: made.controller,+ router: made.router,+ synchronizer: made.synchronizer+ )+ }++ // MARK: - Helpers++ /// Polls the main actor until `condition` holds or the timeout elapses.+ /// Observation-driven pushes land on later main-actor turns; polling keeps+ /// the assertions deterministic without coupling to the wiring internals.+ private func waitUntil(+ timeout: Duration = .seconds(2),+ _ condition: () -> Bool+ ) async -> Bool {+ let clock = ContinuousClock()+ let deadline = clock.now.advanced(by: timeout)+ while clock.now < deadline {+ if condition() { return true }+ await Task.yield()+ try? await Task.sleep(for: .milliseconds(10))+ }+ return condition()+ }++ /// The occurrence-qualified DOM id for the block at `sourceIndex`.+ private func domID(at sourceIndex: Int, in session: DocumentSession) -> String? {+ let mapped = BlockDOMID.map(blocks: session.parsedBlocks)+ guard sourceIndex < mapped.count else { return nil }+ return mapped[sourceIndex].domID+ }++ /// First block index matching `predicate`, with its block.+ private func firstBlock(+ in session: DocumentSession,+ where predicate: (MarkdownBlock) -> Bool+ ) -> (index: Int, block: MarkdownBlock)? {+ for (index, block) in session.parsedBlocks.enumerated() where predicate(block) {+ return (index, block)+ }+ return nil+ }++ // MARK: - Initial state pushes (single production owner)++ @Test("Assembly start pushes typography, comment visibility, and note state for replay")+ func initialStatePushedOnStart() async throws {+ let assembly = await makeAssembly()+ let expectedTypography = WebDocumentControllerFactory.typographyVariables(+ settings: assembly.settings+ )+ let pushed = await waitUntil {+ assembly.controller.latestSnapshot.typography == expectedTypography+ && assembly.controller.latestSnapshot.commentVisibility != nil+ && assembly.controller.latestSnapshot.noteIndicatorsJSON != nil+ }+ #expect(pushed, "start() must push initial typography/comment/note state so recovery replay has native truth")+ }++ @Test("applyTheme pushes the theme through the synchronizer")+ func applyThemePushes() async throws {+ let assembly = await makeAssembly()+ assembly.synchronizer.applyTheme(themeKey: "dark")+ let pushed = await waitUntil {+ assembly.controller.latestSnapshot.theme?.name == "dark"+ }+ #expect(pushed)+ }++ @Test("Settings changes re-push typography and comment visibility")+ func settingsChangesRePush() async throws {+ let assembly = await makeAssembly()+ let originalScale = assembly.settings.textSizeScale+ let originalComments = assembly.settings.showHTMLComments+ defer {+ assembly.settings.textSizeScale = originalScale+ assembly.settings.showHTMLComments = originalComments+ }++ assembly.settings.textSizeScale = originalScale == 120 ? 140 : 120+ let expectedTypography = WebDocumentControllerFactory.typographyVariables(+ settings: assembly.settings+ )+ let typographyPushed = await waitUntil {+ assembly.controller.latestSnapshot.typography == expectedTypography+ }+ #expect(typographyPushed, "text-size change must reach the web controller without a view remount")++ assembly.settings.showHTMLComments = !originalComments+ let commentsPushed = await waitUntil {+ assembly.controller.latestSnapshot.commentVisibility == !originalComments+ }+ #expect(commentsPushed)+ }++ // MARK: - Navigation routing (TOC / notes / search)++ @Test("A pending anchor scroll (TOC / fragment target) is translated and routed to the web controller")+ func tocNavigationTargetsWebDOMID() async throws {+ let assembly = await makeAssembly()+ let heading = try #require(firstBlock(in: assembly.session) { block in+ if case .heading(let level, _) = block { return level == 2 }+ return false+ })+ let composite = "\(heading.block.id)-\(heading.index)"+ let expectedDOMID = try #require(domID(at: heading.index, in: assembly.session))++ assembly.session.pendingAnchorScroll = composite++ let routed = await waitUntil {+ assembly.controller.latestSnapshot.scrollTargetBlockID == expectedDOMID+ && assembly.session.pendingAnchorScroll == nil+ }+ #expect(routed, "composite TOC target must translate to the occurrence-qualified DOM id and be consumed")+ }++ @Test("A notes-panel navigation target (bare content hash) scrolls to the block's first occurrence")+ func noteNavigationBareHashTargetsFirstOccurrence() async throws {+ let assembly = await makeAssembly()+ let paragraph = try #require(firstBlock(in: assembly.session) { block in+ if case .paragraph = block { return true }+ return false+ })+ let expectedDOMID = try #require(domID(at: paragraph.index, in: assembly.session))++ assembly.coordinator.noteNavigationTarget = paragraph.block.id++ let routed = await waitUntil {+ assembly.controller.latestSnapshot.scrollTargetBlockID == expectedDOMID+ && assembly.coordinator.noteNavigationTarget == nil+ }+ #expect(routed, "bare-hash note target must resolve to a DOM id and be consumed")+ }++ @Test("A sub-block note target (table row) scrolls to the parent table block")+ func noteNavigationSubBlockTargetsParent() async throws {+ let assembly = await makeAssembly()+ let table = try #require(firstBlock(in: assembly.session) { block in+ if case .table = block { return true }+ return false+ })+ let expectedDOMID = try #require(domID(at: table.index, in: assembly.session))++ assembly.coordinator.noteNavigationTarget = "\(table.block.id)-row-0"++ let routed = await waitUntil {+ assembly.controller.latestSnapshot.scrollTargetBlockID == expectedDOMID+ && assembly.coordinator.noteNavigationTarget == nil+ }+ #expect(routed, "row sub-block id must resolve to its parent block's DOM id")+ }++ @Test("Navigating search matches scrolls the rendered document to the current match")+ func searchMatchNavigationScrollsToMatchBlock() async throws {+ let assembly = await makeAssembly()+ let paragraph = try #require(firstBlock(in: assembly.session) { block in+ if case .paragraph = block { return true }+ return false+ })+ let expectedDOMID = try #require(domID(at: paragraph.index, in: assembly.session))++ assembly.session.search.updateMatchCount(for: paragraph.index, count: 2)+ assembly.session.search.navigateToMatch(at: 0)+ #expect(assembly.session.currentMatch?.blockIndex == paragraph.index)++ let routed = await waitUntil {+ assembly.controller.latestSnapshot.scrollTargetBlockID == expectedDOMID+ }+ #expect(routed, "search-match navigation must drive controller.scrollTo (Req 6.3)")+ }++ // MARK: - Details open-state (native authoritative, exact replay)++ @Test("Details open-by-default state is seeded so a recovery replay preserves it")+ func detailsOpenByDefaultSeeded() async throws {+ let assembly = await makeAssembly()+ let open = try #require(firstBlock(in: assembly.session) { block in+ if case .details(_, _, let isOpen, _) = block { return isOpen }+ return false+ })+ let closed = try #require(firstBlock(in: assembly.session) { block in+ if case .details(_, _, let isOpen, _) = block { return !isOpen }+ return false+ })+ let openDOMID = try #require(domID(at: open.index, in: assembly.session))+ let closedDOMID = try #require(domID(at: closed.index, in: assembly.session))++ let seeded = await waitUntil {+ let ids = assembly.controller.latestSnapshot.detailsExpandedIDs+ return ids?.contains(openDOMID) == true && ids?.contains(closedDOMID) == false+ }+ #expect(seeded, "setDetailsState replay must reflect isOpenByDefault, or a recovery collapses default-open details")+ }++ @Test("Details toggles from the page update native truth in both directions")+ func detailsToggleTracksBothDirections() async throws {+ let assembly = await makeAssembly()+ let closed = try #require(firstBlock(in: assembly.session) { block in+ if case .details(_, _, let isOpen, _) = block { return !isOpen }+ return false+ })+ let closedDOMID = try #require(domID(at: closed.index, in: assembly.session))++ assembly.router.handle(.detailsToggled(id: closedDOMID, expanded: true))+ let expanded = await waitUntil {+ assembly.controller.latestSnapshot.detailsExpandedIDs?.contains(closedDOMID) == true+ }+ #expect(expanded, "an expand toggle must be part of the replayable snapshot")++ assembly.router.handle(.detailsToggled(id: closedDOMID, expanded: false))+ let collapsed = await waitUntil {+ assembly.controller.latestSnapshot.detailsExpandedIDs?.contains(closedDOMID) == false+ }+ #expect(collapsed, "a collapse toggle must also update native truth (authoritative both ways)")+ }++ // MARK: - Table display modes++ @Test("A table-mode toggle from the page is pushed back for replay")+ func tableModeToggleIsPushedForReplay() async throws {+ let assembly = await makeAssembly()+ let table = try #require(firstBlock(in: assembly.session) { block in+ if case .table = block { return true }+ return false+ })+ let tableDOMID = try #require(domID(at: table.index, in: assembly.session))++ assembly.router.handle(.tableModeToggled(blockID: tableDOMID, mode: "readable"))++ let pushed = await waitUntil {+ assembly.controller.latestSnapshot.tableModes?[tableDOMID] == "readable"+ }+ #expect(pushed, "table modes must reach the snapshot or a reload loses them")+ }++ @Test("A natively-set table mode (composite key) is translated and pushed")+ func nativeTableModeIsTranslatedAndPushed() async throws {+ let assembly = await makeAssembly()+ let table = try #require(firstBlock(in: assembly.session) { block in+ if case .table = block { return true }+ return false+ })+ let tableDOMID = try #require(domID(at: table.index, in: assembly.session))++ assembly.session.tableDisplayModes["\(table.block.id)-\(table.index)"] = .wide++ let pushed = await waitUntil {+ assembly.controller.latestSnapshot.tableModes?[tableDOMID] == "scroll"+ }+ #expect(pushed, "composite-keyed native table modes must translate to DOM-id keys and the JS mode string")+ }++ // MARK: - Section collapse++ @Test("A section collapse is pushed to the web controller by the synchronizer")+ func sectionCollapseIsPushed() async throws {+ let assembly = await makeAssembly()+ let heading = try #require(firstBlock(in: assembly.session) { block in+ if case .heading(let level, _) = block { return level == 2 }+ return false+ })+ let sectionID = "\(heading.block.id)-\(heading.index)"+ _ = try #require(assembly.session.sections.toggleSection(sectionID))++ let pushed = await waitUntil {+ assembly.controller.latestSnapshot.sectionCollapsedIDs?.contains(sectionID) == true+ }+ #expect(pushed, "section collapse must be pushed without the SwiftUI view's .onChange being mounted")+ }+}
diff --git a/specs/bugfixes/footnote-popover-regular-layout/report.md b/specs/bugfixes/footnote-popover-regular-layout/report.mdnew file mode 100644index 00000000..e03b7070--- /dev/null+++ b/specs/bugfixes/footnote-popover-regular-layout/report.md@@ -0,0 +1,229 @@+# Bugfix Report: Footnote Popover Missing in the Regular Layout++**Date:** 2026-07-25+**Status:** Fixed+**Ticket:** T-1893++## Description of the Issue++Tapping or clicking a rendered footnote badge did nothing on macOS and on+regular-width iPad. The badge was rendered, the tap was received, and the+native state updated — but no popover or sheet ever appeared. On iPhone+(compact layout) the same tap worked correctly.++**Reproduction steps:**++1. Open a document containing a footnote reference and its definition on macOS,+ or on an iPad in regular-width layout.+2. Tap/click the rendered footnote reference badge.+3. Observe that no footnote sheet/popover appears.+4. Repeat on iPhone (or a compact-width iPad window); the footnote sheet appears.++**Impact:** Footnotes were effectively unusable on two of the three layouts —+every macOS window and every wide iPad window. Footnote content was reachable+only by scrolling to the definition manually. A secondary effect: the+activation left `DocumentLayoutCoordinator.coordinatorOwnsModalPresentation`+`true` with nothing on screen, which misreports modal ownership to focus+restoration.++## Investigation Summary++- **Symptoms examined:** badge taps produced no UI on macOS/regular iPad, but the+ same routing worked on iPhone — pointing at the presentation layer rather+ than the bridge, the router, or the emitter.+- **Code inspected:**+ - `prism/Views/DocumentScrollContent.swift` — the `pendingFootnoteId` observer+ calls `coordinator.showFootnote(...)`. Shared by both layouts, so activation+ was not the problem.+ - `prism/Views/DocumentLayoutCoordinator.swift` — `showFootnote` only assigns+ `activeFootnoteId` / `activeFootnoteBlockId`. It publishes state; it does not+ present anything.+ - `prism/Views/CompactDocumentLayout.swift` — attached a `.sheet` bound to+ `coordinator.activeFootnoteId != nil` rendering `FootnotePopoverView`.+ - `prism/Views/RegularDocumentLayout.swift` — **no footnote presentation at+ all.** It has sheets for note popovers, add-note, replies and document notes,+ but nothing observing `activeFootnoteId`.+ - `prism/Views/DocumentReaderView.swift` — `useCompact` is always `false` on+ macOS and `false` for sufficiently wide iPad layouts.+- **Hypotheses ruled out:** bridge message not arriving (the compact layout+ proves it does); `FootnotePopoverView` failing to render (same view works in+ compact); footnote data missing from the session (`session.footnoteData` is+ layout-independent).++## Discovered Root Cause++The footnote presentation was written inline in `CompactDocumentLayout` only.+`DocumentLayoutCoordinator.activeFootnoteId` is shared state with exactly one+observer, and that observer lives in the layout that macOS and wide iPad never+mount.++**Defect type:** Missing wiring / duplicated-presentation drift.++**Why it occurred:** The coordinator's footnote state is layout-agnostic, but its+presentation was not extracted into anything shared. Nothing in the type system+or the test suite required the second layout to present it, so the regular+layout could exist without a footnote host and still compile and pass.++**Contributing factors:** The codebase already had the right pattern for exactly+this problem — `mediaZoomPresentation` and `imageAccessFolderPicker` are shared+`ViewModifier`s applied by both layouts precisely so presentations cannot drift+per layout. The footnote popover simply never adopted it.++## Resolution for the Issue++**Changes made:**++- `prism/Views/FootnotePresenter.swift` (new) — a `FootnotePresenter`+ `ViewModifier` plus a `View.footnotePresentation(coordinator:session:)`+ extension holding the single footnote presentation, following the+ `MediaZoomPresenter` pattern.+- `prism/Views/CompactDocumentLayout.swift` — the inline footnote `.sheet` is+ replaced by `.footnotePresentation(coordinator:session:)`. Behaviour is+ unchanged: same binding, same `FootnotePopoverView`.+- `prism/Views/RegularDocumentLayout.swift` — applies+ `.footnotePresentation(coordinator:session:)`, which it previously lacked+ entirely. This is the actual fix.++`FootnotePopoverView` already sizes itself per platform (`#if os(macOS)` panel+frame vs. iOS `.presentationDetents([.medium])` + drag indicator), so one shared+sheet presentation is correct for both layouts with no per-layout branching.++**Approach rationale:** Fixing the symptom would have meant copying the compact+layout's `.sheet` into the regular layout — restoring the duplication that caused+the bug. Extracting one shared modifier fixes the bug and removes the drift that+produced it, matching the pattern the neighbouring presentations already use.++**Alternatives considered:**++- **Copy the `.sheet` into `RegularDocumentLayout`** — smallest diff, but leaves+ two independent copies of the same presentation, which is the root cause. A+ third layout, or any future change to footnote presentation, reintroduces the+ bug.+- **Present a true `.popover` anchored to the badge on iPad/macOS** — closer to+ the original SwiftUI-era design, but the regular layout deliberately uses+ sheets for all its note editors ("Sheet on iPad/macOS, matching the other note+ editors in this layout"), and the web-rendered badge has no SwiftUI anchor+ view to attach a popover to. Rejected as inconsistent and more invasive.+- **Move the presentation into `DocumentScrollContent`** (already shared by both+ layouts) — would work, but presentations in this codebase live at layout level+ deliberately (see the `imageAccessFolderPicker` comment: presented at layout+ level "for reliability"), and `DocumentScrollContent` is the document body,+ not a presentation host.++## Regression Test++**Test file:** `prismTests/FootnotePresentationHostTests.swift`++**Test names:**++- `bothLayoutsPresentFootnotes(fileName:)` — parameterised over+ `CompactDocumentLayout.swift` and `RegularDocumentLayout.swift`; asserts each+ applies `.footnotePresentation(`. This is the test that fails without the fix.+- `presentationReadsCoordinatorState()` — asserts the shared presenter is driven+ by `coordinator.activeFootnoteId` / `dismissFootnote()` and renders+ `FootnotePopoverView`, so the presentation cannot be re-pointed at+ layout-local state.+- `footnoteStateRoundTrips()` — pins the coordinator contract the presentation+ binds to, including `coordinatorOwnsModalPresentation` returning to `false` on+ dismissal.+- `sessionResetClearsFootnote()` — a session switch clears any active footnote.++**What it verifies:** that both presentation hosts stay wired. The wiring checks+are source-structural (reading the layout sources from disk relative to+`#filePath`, the approach `ParityFixtureSupport` already uses) because the defect+is a missing view modifier: no runtime unit assertion over the coordinator can+observe whether a layout applied it, and evaluating layout view bodies in tests+is a known suite-crasher in this project (T-1541).++**Red/green confirmed:** before the fix, `bothLayoutsPresentFootnotes` failed for+*both* layouts and `presentationReadsCoordinatorState` failed (no presenter+existed); the two coordinator tests passed, matching the ticket's observation+that state was set correctly but nothing was presented. After the fix all four+pass.++**Run command:**++```bash+xcodebuild test -project prism.xcodeproj -scheme prism \+ -destination 'platform=macOS' \+ -only-testing:prismTests/FootnotePresentationHostTests+```++## Affected Files++| File | Change |+|------|--------|+| `prism/Views/FootnotePresenter.swift` | New — shared footnote presentation modifier |+| `prism/Views/CompactDocumentLayout.swift` | Inline footnote sheet replaced by the shared modifier |+| `prism/Views/RegularDocumentLayout.swift` | Applies the shared modifier (the fix) |+| `prismTests/FootnotePresentationHostTests.swift` | New — regression tests pinning both hosts |++## Verification++**Automated:**++- [x] Regression test passes (`FootnotePresentationHostTests`, 4 tests)+- [x] `make build-macos` — zero warnings, zero errors+- [x] `make build-ios` — zero warnings, zero errors+- [x] `make lint` — 0 violations in 491 files+- [x] Full macOS unit suite: no new failures attributable to this change,+ established against an `origin/main` baseline run (see below)++**Full-suite baseline comparison.** `make test-quick` is not a usable pass/fail+signal on this project: a single trap cascades into thousands of 0.000s+"failures" (T-1541), and the parallel locale-matrix processes race on shared+`UserDefaults`/pasteboard state (T-1652). Both runs were therefore compared+set-to-set rather than by verdict:++| | `origin/main` (5d33c13) | this branch |+|---|---|---|+| distinct failing tests | 3298 | 3358 |+| shared with the other run | 3296 | 3296 |++The 62 branch-only entries decompose into:++- **3 test classes that do not exist on `main`** — `FootnotePresentationHostTests`+ (new here), `WebStateSynchronizerAssemblyTests` and+ `WebScrollIntegrationContractTests` (added by T-1719). They cannot appear in+ `main`'s failure set; all pass when run targeted.+- **20 pre-existing classes swept into the cascade** in this run but not in+ `main`'s. Re-running that exact set in isolation inverts the result — `main`+ fails **more** (12 distinct) than this branch (4), and 3 of the branch's 4 are+ in `main`'s set too.++The single remaining branch-only isolated failure,+`CopyNotesButtonTests.copiedPayloadMatchesExportMarkdownAcrossNoteKinds`, was run+alone twice on this branch: run 1 `** TEST SUCCEEDED **`, run 2 `** TEST FAILED **`+— flaky by race, and in the notes-copy path, which this change does not touch.++No suite covering code this change touches fails: the four web-rendering suites+and the new footnote suite all pass when run targeted.++**Manual verification:** not performed on-device. The fix restores a presentation+modifier; the presented view (`FootnotePopoverView`) is unchanged and already+exercised by the compact layout and `FootnotePopoverContentTests`.++## Prevention++**Recommendations to avoid similar bugs:**++- Presentations driven by shared `DocumentLayoutCoordinator` state belong in a+ shared `ViewModifier` applied by every layout, never inline in one layout.+ `MediaZoomPresenter` is the reference pattern.+- When adding coordinator state that drives UI, add it to the shared presenter+ rather than to a layout body.+- The remaining coordinator-driven sheets in `RegularDocumentLayout`+ (`notePopoverBlock`, `addNoteBlock`, `replyToNote`, `showDocumentNoteSheet`)+ are still duplicated per layout and carry the same drift risk. Consolidating+ them the same way would close the class rather than this one instance — worth+ a follow-up chore.++## Related++- T-1893 — this bug+- T-1719 — WebKit navigation and state integration (same branch; the regular+ layout's state wiring was reworked there)+- T-1542 — the WebKit rendering cutover that moved footnote activation onto the+ bridge+- `specs/footnotes/` — footnote system spec+- `specs/bugfixes/webkit-state-integration/report.md` — T-1719 report
diff --git a/specs/bugfixes/webkit-state-integration/report.md b/specs/bugfixes/webkit-state-integration/report.mdnew file mode 100644index 00000000..4727fd1f--- /dev/null+++ b/specs/bugfixes/webkit-state-integration/report.md@@ -0,0 +1,378 @@+# Bugfix Report: WebKit Navigation and State Integration (ScrollViewProxy Cutover Debt)++**Date:** 2026-07-10 (fixed 2026-07-11)+**Status:** Fixed+**Ticket:** T-1719++## Description of the Issue++The WebKit cutover (T-1542) replaced the SwiftUI document renderer with a+`WebView`/`WebPage` surface, but both layout views still put core behaviour+inside a `ScrollViewProxy` modifier closure that `DocumentScrollContent`+retains only "for source compatibility" and never attaches. Everything inside+that closure — and every state push that had no web-path sender — silently+stopped running.++**Reproduction steps:**+1. Open a document on iPhone (compact layout) and scroll down — the bottom+ toolbar never hides (hide-on-scroll dead).+2. Open the notes panel/sidebar and tap a note's "go to block" — nothing+ scrolls (target set into dead `@State`).+3. Search, then navigate matches on iPad/macOS — the document does not scroll+ to the match.+4. Toggle a table's display mode in the page, reload the document (or let the+ WebContent process recover) — the mode resets (never pushed back natively).+5. On macOS, use View-menu Page Down / Top / Bottom on a rendered document —+ the items are disabled (`canScroll`/`hasContent` never become true).++**Impact:** High. TOC/notes/search navigation, compact hide-on-scroll,+keyboard/menu scrolling, and exact details/table state replay after+reload/WebContent recovery are all broken on the only render path the app has.++## Investigation Summary++Systematic inspection (Fagan) of the cutover seam; full analysis in the+conversation and condensed here.++- **Symptoms examined:** dead `scrollModifiers` closures in both layouts;+ unused `DocumentScrollContent` inputs; production senders missing for+ `setDetailsState`/`setTableModes`/`setSearchState` and+ `scrollByPage`/`scrollToEdge`.+- **Code inspected:** `DocumentScrollContent`, `CompactDocumentLayout`,+ `RegularDocumentLayout`, `WebDocumentController(+Factory)`,+ `WebDocumentMessageRouter`, `BridgeMessageRouter`, `WebBridgeContract`,+ `KeyboardScrollController`, `DocumentSession` (search/sections/expansion/+ table-mode state), `BlockDOMID`, `SharedBlockViews`, `prism-bridge.js`,+ `prism-scroll.js`, `prism-theme.js`.+- **Hypotheses tested:** `visibleBlock` as a hide-on-scroll signal — ruled+ out: it is a 120 ms *trailing* debounce that only fires when scrolling+ pauses, so it cannot carry direction. WebView-native SwiftUI scroll+ observation — ruled out: the WebView's scroller is not a SwiftUI ScrollView,+ so `onScrollGeometryChange` never fires.++## Discovered Root Cause++**Defect type:** Missing integration (cutover left behaviours in an+unattached closure; no web-path owner for native→web state sync).++**Why it occurred:** The cutover replaced the scroll surface but kept the+layouts' API shape for source compatibility instead of migrating the+behaviours to a web-path owner. Nothing owns "keep the rendered page in sync+with native truth and route navigation into it".++**Contributing factors:**+- Component tests exercise bridge commands directly, so senders-that-don't-+ exist were invisible; no test mounted the production assembly.+- ID-format split: JS handlers key on occurrence-qualified DOM ids+ (`b-{hash}-{occurrence}`), native truth uses composite ids+ (`{hash}-{sourceIndex}`), bare hashes, and sub-block ids+ (`{hash}-row-N`) — the only translator (`BlockDOMID.restoreDOMID`, T-1639)+ was used solely for scroll restore.+- `DetailsExpansionCoordinator` is additive-only (expansion requests, no+ collapse), so exact replay was impossible by design.++## Resolution for the Issue++Implemented via three competing implementations from the same checkpoint;+the winning solution (Agent 2) was cherry-picked as `aed809f`. Full+evaluation: `solution-comparison.md`.++**Changes made:**+1. `WebDocumentStateSynchronizer` (`prism/ViewModels/`) — the single+ production owner beside `WebDocumentController`. One coalescing+ `withObservationTracking` pass computes every domain's desired value+ inside a single tracked read (dependencies self-register from the+ computation; no hand-maintained list to drift), re-arms on the first+ mutation, then dirty-diffs against last-pushed values and dispatches only+ changes. Consumes `session.pendingAnchorScroll` and+ `coordinator.noteNavigationTarget` (translate → `controller.scrollTo` →+ clear) and scrolls to the current search match. Search-highlight state is+ an explicitly named seam left for T-1680. `DocumentScrollContent` mounts+ the assembly via `WebDocumentStateSynchronizer.makeAssembly` — the same+ entry point the regression tests use — and feeds theme through+ `applyTheme(themeKey:)` (colorScheme is view-world).+2. `BlockDOMID.navigationDOMID(forTarget:blocks:visibleSourceIndices:)` —+ the one translation seam: DOM ids pass through, composite ids verify+ hash-at-index, sub-block ids (`-row-N`/`-row-header`/`-item-N…`) strip to+ the parent, bare hashes resolve to the first (preferably visible)+ occurrence.+3. Details open-state is native-authoritative both ways:+ `DetailsExpansionCoordinator` gains a DOM-keyed open set seeded from each+ `.details` block's `isOpenByDefault` at parse (nested details carry the+ emitter's `-d{childIndex}` section ids individually), folded with+ composite expansion requests, and updated in both directions by+ `detailsToggled` (last write wins) — so `setDetailsState` replays exactly+ after reload/WebContent recovery and the raw-source remount.+4. Table modes push `session.tableDisplayModes` (mixed DOM/composite keys+ translated) via the shared `TableDisplayMode.webModeAttribute` mapping the+ emitter now also uses.+5. New inbound `scrollDirectionChanged` bridge message (data-only,+ allowlisted, generation-tagged). `prism-scroll.js` posts it on direction+ flips **plus the one hide-threshold crossing** (a descent starting at the+ top otherwise never re-posts and the toolbar would never hide — the+ investigation brief's "flips only" was defective), suppressed during+ programmatic scrolls. Routed to+ `DocumentLayoutCoordinator.applyScrollDirection`, which applies the+ pre-cutover threshold rules to `isCompactToolbarVisible`;+ `CompactDocumentLayout` observes it.+6. `KeyboardScrollController.attachWebBridge/detachWebBridge` route+ page/edge commands to `scrollByPage`/`scrollToEdge` and flip+ `hasContent`/`canScroll`, so macOS View-menu scrolling works on the+ rendered path; `resetForNewSession` detaches.+7. Dead API removed: `scrollModifiers` + unused `DocumentScrollContent`+ inputs, both layouts' dead closures and orphaned `@State` targets+ (rewired to `session.pendingAnchorScroll` / `coordinator.noteNavigationTarget`+ / `search.navigateToMatch`), `SharedBlockViews.delayedScroll`/+ `scrollIdForBlock`/`parentBlockId` (grammar moved into `BlockDOMID`).++**Approach rationale:** the synchronizer observes through the Observation+framework rather than SwiftUI `.onChange` so the wiring exists independent of+any view being mounted — the failure mode this ticket fixes — and the+self-registering dependency design removes the drift risk of a separate+registration list.++**Alternatives considered:**+- Per-domain observation re-arm loops (Agent 1) — equivalent behaviour, but+ a hand-maintained dependency list can silently miss a future domain.+- Kiro's independent implementation — rejected for flips-only scroll+ reporting, which leaves hide-on-scroll broken in the primary gesture.++**Scope boundaries:**+- T-1680 (in progress in parallel) owns `setSearchState`/search highlights;+ this fix wires search *navigation* only and leaves a named seam.+- T-1662 is the canonical ticket for the malformed TOC/fragment ids; the+ translation seam introduced here resolves the ids for the paths this fix+ wires, and T-1662 should be re-verified against it.++## Regression Test++**Test files:**+- `prismTests/WebRendering/WebStateSynchronizerAssemblyTests.swift`+- `prismTests/WebRendering/WebScrollIntegrationContractTests.swift`++**What they verify:** the production assembly (controller + router ++synchronizer via `makeAssembly`) pushes typography/comments/notes/theme,+routes TOC/notes/search navigation to translated DOM ids, tracks details+open-state both directions (including `isOpenByDefault` seeding), pushes+table modes (DOM-id and composite-key), pushes section collapse — all+asserted against the controller's coalesced replay snapshot. Plus: the+`scrollDirectionChanged` bridge contract, the coordinator's hide-on-scroll+threshold rules, and the keyboard web-bridge routing.++**Red run (2026-07-10, pre-fix):** 18 of 21 test cases fail, 3 negative+guards pass — confirming the missing production wiring. All 21 pass with the+fix.++**Run command:**+```bash+xcodebuild test -project prism.xcodeproj -scheme prism \+ -destination 'platform=macOS' \+ -only-testing:prismTests/WebStateSynchronizerAssemblyTests \+ -only-testing:prismTests/WebScrollIntegrationContractTests+```++## Affected Files++| File | Change |+|------|--------|+| `prism/ViewModels/WebDocumentStateSynchronizer.swift` | New: coalescing observation pass, navigation routing, `makeAssembly` |+| `prism/Services/WebRendering/BlockDOMID.swift` | `navigationDOMID` translation seam (+ sub-block grammar from SharedBlockViews) |+| `prism/Services/DetailsExpansionCoordinator.swift` | DOM-keyed authoritative open-state, `isOpenByDefault` seeding, both-way toggles |+| `prism/Models/MarkdownBlock.swift` | Shared `TableDisplayMode.webModeAttribute` |+| `prism/Services/WebRendering/BlockHTMLEmitter.swift` | Uses the shared mode mapping |+| `prism/ViewModels/WebBridgeContract.swift` | `scrollDirectionChanged` inbound message + allowlist entry |+| `prism/ViewModels/BridgeMessageRouter.swift` | `scrollDirectionChanged` decode |+| `prism/ViewModels/WebDocumentMessageRouter.swift` | Routes scroll direction to the coordinator; both-way details toggle |+| `prism/ViewModels/WebDocumentControllerFactory.swift` | `pushNoteState` → diffable `noteStatePayloads` |+| `prism/Resources/WebRenderer/prism-scroll.js` | Direction-flip + hide-threshold-crossing reporting |+| `prism/Services/KeyboardScrollController.swift` | Web-bridge backend (attach/detach, command routing) |+| `prism/Views/DocumentLayoutCoordinator.swift` | `noteNavigationTarget`, `isCompactToolbarVisible`, `applyScrollDirection`, session reset |+| `prism/Views/DocumentScrollContent.swift` | Mounts `makeAssembly`; dead inputs/`scrollModifiers` removed |+| `prism/Views/CompactDocumentLayout.swift` | Dead closure/@State removed; toolbar observes coordinator; navigation rewired |+| `prism/Views/RegularDocumentLayout.swift` | Dead closure/@State removed; navigation rewired |+| `prism/Views/SharedBlockViews.swift`, `SharedCollapsibleSections.swift` | Retired scroll helpers removed |+| `prismTests/WebRendering/WebStateSynchronizerAssemblyTests.swift` | New production-assembly regression suite |+| `prismTests/WebRendering/WebScrollIntegrationContractTests.swift` | New contract/toolbar/keyboard regression suite |++## Verification++**Automated:**+- [x] Regression tests pass (21/21 on the integration branch)+- [x] Adjacent suites pass (WebDocumentController/MessageRouter/ThemeStateSync/+ ScrollNavigation live suite, DetailsExpansionCoordinator/ScrollIntegration —+ 984 case-runs, 0 failures). Pre-existing failures at base c721623 are+ documented in `solution-comparison.md`; none are touched by this change.+- [x] `make build-macos` and `make build-ios` pass at the zero-warning gate+- [x] `make lint` clean++**Manual verification:**+- Device pass recommended at next release gate: compact hide-on-scroll feel,+ TOC/notes/search navigation, details/table state across a WebContent+ recovery (the live suites cover the command round-trips; the scroll feel is+ a human check).++## Rebase reconciliation (2026-07-25)++The branch sat unmerged as a conflicting draft (PR #315) while T-1680 (#314)+and T-1681 (#316) landed on `main`. Rebasing produced two conflicts, both in+already-understood territory:++- `prism/Views/DocumentScrollContent.swift` — `main` had added the T-1680+ search-highlight push in the very block this change deletes (the view-level+ initial state pushes that moved into the synchronizer). Resolved by keeping+ the synchronizer as the owner of note/section/details/table pushes while+ leaving search highlights as a deliberate view-fed seam: the effective+ payload depends on the debounced `SearchCoordinator` state this view already+ observes, so it is pushed on mount and re-pushed via `.onChange(of:+ searchStateKey)`. This is the seam the original T-1719 work explicitly left+ for T-1680; the two designs agree.+- `CHANGELOG.md` — additive on both sides; both entries kept. The T-1719 entry's+ closing "search highlighting remains tracked separately" clause was dropped+ because T-1680 has since shipped.++One follow-on fix was required: `prismTests/WebRendering/WebSearchWiringTests.swift`+(added on `main`) assembles a controller to mirror `DocumentScrollContent`+"call for call" and called `WebDocumentControllerFactory.pushNoteState`, which+this change removes in favour of the synchronizer. Its helper now builds the+assembly through `WebDocumentStateSynchronizer.makeAssembly` + `start()`, which+is what its own docstring requires.++Post-rebase verification: `WebStateSynchronizerAssemblyTests`,+`WebScrollIntegrationContractTests`, `WebSearchWiringTests` and `OffMainEmitTests`+all pass together; `make build-macos`, `make build-ios` and `make lint` are clean.++## T-1662 (TOC / fragment anchor DOM ids) — fixed++> **Correction history (2026-07-25).** An earlier revision of this section+> claimed T-1662 was resolved by the T-1719 wiring alone and needed no fix.+> **That was wrong**, and PR review caught it. The tracing below is accurate as+> far as it goes — every producer does reach the translation seam — but it+> verified only that the *plumbing* connects, never that the *ids the producers+> emit* satisfy the seam's verification. They did not. The defect and its fix+> are recorded under "The index-space defect" below.++What this change does fix: every navigation target now flows through one+translation seam instead of being passed raw to a DOM that uses a different id+scheme. Traced producers:++- ToC entry taps — `RegularDocumentLayout.swift:215`, `:324` and+ `CompactDocumentLayout.swift:188`, `:369` set `session.pendingAnchorScroll`.+- In-document `#fragment` links — `WebDocumentMessageRouter.swift:296` calls+ `session.scrollToAnchor(_:)`, which resolves the slug against the ToC and sets+ the same `pendingAnchorScroll` (`DocumentSession.swift:359`).+- Restore-from-recent uses the same field.++All of them are consumed by `WebDocumentStateSynchronizer.scrollToTarget`, which+routes through `BlockDOMID.navigationDOMID` — the seam that verifies composite+`{hash}-{sourceIndex}` ids, resolves bare hashes to a visible first occurrence,+maps sub-block ids to their parent, and drops stale targets. Covered by+`WebStateSynchronizerAssemblyTests.tocNavigationTargetsWebDOMID` (composite ToC+target → occurrence-qualified DOM id, target consumed) plus the existing+`AnchorNavigationDuplicateHeadingsTests` for the slug→`pendingAnchorScroll` half.++### The index-space defect (fixed)++The two id schemes disagreed for any document containing a `<details>`+block with children:++- `TOCCoordinator.buildTOCEntries` builds `TOCEntry.id` as+ `"{block.id}-{runningIndex}"` where `runningIndex` is a **flattened** counter:+ it advances for the `.details` block *and again for each of its nested+ children* (the recursion in `TOCCoordinator.swift`).+- `.details` is a single **top-level** block that holds its children nested+ (`MarkdownBlock.swift:552`, built at `DetailsBlockParser.swift:58`), so those+ children never occupy a top-level index.+- `MarkdownSectionBuilder` and `BlockHTMLEmitter` index by the top-level+ `blocks.enumerated()` position, and `BlockDOMID.restoreDOMID` verifies+ `mapped[sourceIndex].block.id == hash` against that same top-level mapping.++Worked example — `# Intro`, `<details>` with two child paragraphs,+`## Later Heading`, trailing paragraph:++| | value |+|---|---|+| `parsedBlocks` | `[h1, details, h2, p]` — `h2` at top-level index **2** |+| TOC `runningIndex` at `h2` | `h1`→0, `details`→1, children→2 and 3, so `h2` gets **4** |+| `restoreDOMID("{hash}-4")` | `sourceIndex 4 < mapped.count 4` is false → **nil** |++So the TOC entry — and any `#fragment` resolving to that heading, since+`scrollToAnchor` sets the same `entry.scrollId` — resolves to nothing and the tap+silently does not scroll. That is the original T-1662 symptom, for every heading+following a `<details>` block. Headings *inside* `<details>` are worse: the+emitter gives non-details children no section id, so they have no anchor at all.++Why the existing tests miss it: `tocNavigationTargetsWebDOMID` hand-builds its+composite as `"\(heading.block.id)-\(heading.index)"` rather than reading+`session.toc.tocEntries[…].scrollId`, so it exercises the resolver against a+*correct* id and never the producer. `AnchorNavigationDuplicateHeadingsTests`+asserts only that `pendingAnchorScroll == entry.scrollId`, never that the id+resolves. That is the untested composition seam an earlier revision of this+report claimed did not exist.++### The fix++The defect was one field serving two incompatible purposes. `TOCEntry.id` has to+be unique for `Identifiable`, which the flattened counter guarantees; it was also+being used as the navigation target, which requires the top-level index. Those+are now separate:++- `TOCEntry.scrollTargetId` (new, stored) carries the resolvable composite built+ from the **top-level** `parsedBlocks` index, and `scrollId` returns it.+- `TOCEntry.id` keeps the flattened counter unchanged, so list identity and every+ existing consumer are untouched.+- `TOCCoordinator.buildTOCEntries` threads a `topLevelAnchor` through its+ recursion: at depth 0 a heading targets its own top-level composite; inside+ `<details>` every heading targets the enclosing top-level details block.++That last part also repairs a case that never worked: headings **inside**+`<details>` previously resolved to nothing at all (the emitter gives non-details+children no section id), and now scroll to the details block containing them —+the nearest ancestor that has a DOM anchor.++The same correction fixes section expansion as well as scrolling.+`DocumentSession.scrollToAnchor` passes `entry.scrollId` to+`sections.expandSectionAndAncestors(for:)`, which matches against+`MarkdownSection.id` — and that is `"\(heading.id)-\(headingSourceIndex)"` using+the same top-level index. Both consumers were failing on the same wrong id.++### Regression test++`prismTests/TOCNavigationDetailsIndexTests.swift`, which goes through the real+producer (`TOCCoordinator.tocEntries`) rather than a hand-built composite —+precisely the gap that let this through. It also asserts the resolved id is the+*correct* block, not merely non-nil.++Red/green confirmed by reverting the fix and re-running:++| | fix reverted | fix applied |+|---|---|---|+| `headingAfterDetailsResolves` | **failed** | passed |+| `headingAfterDetailsResolvesToTheRightBlock` | **failed** | passed |+| `scrollTargetUsesTopLevelIndex` | **failed** | passed |+| `nestedHeadingResolvesToItsDetailsBlock` | **failed** | passed |+| `headingResolvesWithoutDetails` (control) | passed | passed |+| `identityRemainsUnique` | passed | passed |++The control and identity tests passing in both columns is the point: they are not+sensitive to the defect, so the four that flip are pinning it specifically.+`TOCEntryTests` (35 cases) passes unchanged.++## Prevention++**Recommendations to avoid similar bugs:**+- When a cutover retires a view surface, delete its integration API in the+ same change — "retained for source compatibility" closures hide dead+ behaviour from the compiler and reviewers.+- Keep cross-component wiring in a non-view owner (observation-framework+ driven) so it exists independent of view mounting, and cover it with+ production-assembly tests that mount the real factory path.+- When two id schemes must coexist, funnel every translation through one+ named seam (`BlockDOMID`) and grep for raw format assumptions at review.++## Related++- Transit T-1719 (this fix), T-1680 (search highlights — parallel, owns+ `setSearchState`), T-1662 (TOC/fragment DOM ids — superseded in part by the+ translation seam here), T-1639 (scroll restore, introduced `restoreDOMID`),+ T-1542 (the WebKit cutover).+- `specs/webview-rendering/` (spec), `docs/agent-notes/webview-rendering-status.md`.
diff --git a/specs/bugfixes/webkit-state-integration/solution-comparison.md b/specs/bugfixes/webkit-state-integration/solution-comparison.mdnew file mode 100644index 00000000..3431b50c--- /dev/null+++ b/specs/bugfixes/webkit-state-integration/solution-comparison.md@@ -0,0 +1,78 @@+# Solution Comparison: webkit-state-integration (T-1719)++Three competing implementations were produced from the same investigation+checkpoint (c721623: failing regression tests + API skeletons) and evaluated+on correctness, minimality, code quality, safety, and maintainability.++## Candidates++### Agent 1 (primary — Observation re-arm approach, evolved to a coalescing pass)+- **Files changed:** 16 (+585 / −451)+- **Tests:** 21/21 acceptance pass; adjacent suites green; pre-existing+ failures verified identical at base via stash + baseline run+- **Approach:** single coalescing `withObservationTracking` pass with a+ hand-maintained `registerDependencies()` read list; dirty-diff dispatch;+ details truth as `DetailsExpansionCoordinator.openDetailsDOMIDs` with a+ composite-path → DOM id map; JS posts direction flips plus the one+ hide-threshold crossing++### Agent 2 (alternative mechanism) — SELECTED+- **Files changed:** 16 (+618 / −457)+- **Tests:** 21/21 acceptance pass; the broadest adjacent sweep of the three+ (also surfaced latent suite defects: `bridgePostsReady`'s unconditional+ `Issue.record`, retention tests unsafe under parallel runs); pre-existing+ failures verified at base+- **Approach:** single coalescing observation pass where each `synchronize()`+ computes every domain's desired value **inside** one tracked read — the+ dependency set is derived from the computation itself, with no separate+ registration list — then dirty-diffs and dispatches outside the read.+ Same details/table/JS design as Agent 1 (independently converged).++### Agent 3 (Kiro — independent perspective)+- **Files changed:** 16 (+530 / −394)+- **Tests:** commit message reports 22/22 (actual case count 21) + adjacent+ green with the two pre-existing details failures correctly identified+- **Approach:** per-domain `withObservationTracking` re-arm loops; session-owned+ `detailsOpenStateByDOMID`; **JS posts direction flips only**++## Selected: Agent 2++**Reason:** Correctness and maintainability. Kiro's flips-only scroll+reporting breaks the ticket's headline symptom in the most common gesture — a+descent that starts at the top posts its single flip below the 50pt hide+threshold and never posts again, so the compact toolbar still never hides+(invisible to the suite, which does not exercise the JS posting cadence).+Agents 1 and 2 both caught this and post the one extra threshold-crossing+message. Between those two near-equivalent solutions, Agent 2's dependency+tracking is self-registering (whatever `computePass()` reads is what re-arms),+where Agent 1 keeps a hand-maintained dependency list that a future domain+could silently miss — the exact "wiring exists but nothing attaches it"+failure class T-1719 fixes. Agent 2 also shipped the broadest verification.+Cost accepted: one documented no-op settle pass after a navigation target is+consumed.++Integrated as cherry-pick `aed809f` onto `T-1719/bugfix-webkit-state-integration`+and re-verified there: acceptance + adjacent classes green (984 case-runs, 0+failures, including the live `WebScrollNavigationTests` over the changed JS),+`make build-macos` / `make build-ios` at the zero-warning gate, `make lint`+clean.++## Shared discoveries worth keeping (all three candidates)+- The brief's "post only on direction flips" was defective; the JS must also+ post the one hide-threshold crossing (Agents 1/2), or hide-on-scroll never+ engages from a top-of-page descent (Kiro).+- The brief's "strip nested details paths to the root composite" was also+ wrong: the emitter gives every nested `<details>` its own `<section>` with a+ `-d{childIndex}` id, and `setDetailsState` forces open/closed by payload+ membership — nested ids must be carried individually or a replay would+ force nested default-open details closed.+- Pre-existing failures documented (fail identically at base c721623):+ `DetailsStateCoordinationTests/tocNavigationExpandsAncestors` and+ `DetailsSearchIntegrationTests/ancestorMapTracksNesting` (both key the+ T-300 composite-id ancestorMap with bare block ids),+ `SectionCollapseManagerTests/visibleBlocksCountChanges`, seven+ `KeyboardScrollControllerTests` ScrollPosition-math cases (unbound+ `ScrollPosition.point` is nil on macOS 26.5.1),+ `WebDocumentBridgeLiveTests/bridgePostsReady` (unconditional+ `Issue.record` since PR #289), and retention/export flakes under parallel+ runs. Candidates for follow-up tickets; none touched by this change.
scrollTargetId reverted to its old value, headingAfterDetailsResolves, headingAfterDetailsResolvesToTheRightBlock, scrollTargetUsesTopLevelIndex and nestedHeadingResolvesToItsDetailsBlock all fail; with it applied all six pass. The control (no <details>) and identity tests pass in both states, confirming the four that flip are pinning the defect specifically rather than passing vacuously.make lint (0 violations), make build-macos and make build-ios all pass cleanly.origin/main: one trap cascades into thousands of 0.000s failures (T-1541), and parallel locale-matrix processes race on shared UserDefaults/pasteboard state (T-1652). Compare failure sets against a baseline rather than reading the verdict.