PR #351 — a TOC entry, note, #fragment or search match chosen while an in-place re-parse is still emitting HTML must survive the reload it lands in. Three review rounds already applied; this pass audits the final design.
undeliveredNavigationTarget, T-1918 hasUndeliveredSearchReveal) are raised only when a command cannot dispatch. During the off-main HTML emit the outgoing page was still isReady, so a navigation dispatched there, released the claim, and the reload's stored-position restore had nothing to yield to.WebDocumentController.beginLoad() marks the visible page superseded before the emit is awaited, and WebDocumentControllerFactory.loadDocument becomes the single production load sequence that owns both ends of the window.isReady means "a page is up"; isSuperseded means "the page that is up is not the one this navigation is for". Conflating them (review round 1) made Page Up/Down and Scroll to Top/Bottom dead keys for the whole emit and rested the window on a premise crash recovery falsifies.loadDocument calls overlap whenever a second re-parse lands mid-emit: .task(id:) cancels the first without awaiting it and the emit is not interruptible, so an unconditional abandonLoad closed the successor's window — the original bug reached through its own fix..scrollToEdge requires layoutSettled, so after load it now hits the new drop branch instead of the queue — a live-page window the docstring says the drop does not apply to.Ready to push
The fix is sound and unusually well pinned. The three-state canDispatch, the announcement-id-owned window, and the single production load sequence hold together, and every load-bearing claim in the docstrings has a test behind it — including the two paths that bring a page up inside the window and the overlapping-load ordering that the second review round found. Lint clean, iOS build clean, 172 targeted tests green across six web suites, CI green on 4c0de5a with mergeStateStatus: CLEAN.
One minor finding is left for the author rather than fixed here (this pass is report-only): the new drop branch also swallows scrollToEdge in the post-load gap between ready and layoutSettled, where a page is live — a window this ticket was not about, and where the emitted log line ("no live page to scroll") is factually wrong. It is a one-line gate change if the author agrees; it does not block the push.
36f26f2 Fix T-1975: Navigation During Reload Is Lost to Persisted Scroll Restore d1ef3d7 Fix T-1975 review: hold the emit window by supersession, not by readiness c5e49de Fix T-1975 review: own the emit window by announcement, not by flag 4c0de5a Merge origin/main into T-1975/bugfix-navigation-during-reload When a markdown file you have open changes on disk, Prism re-reads it and rebuilds the page. Building that page takes time — on a big document, seconds. During that time the old page is still on your screen.
If you picked a table-of-contents entry, tapped a note, followed a link to a heading, or stepped to a search match in that gap, Prism sent you there on the page it was about to throw away. When the new page arrived it put you back at your saved reading position instead, so your choice was lost.
The fix is to tell Prism at the start of a reload that the page on screen is on its way out. Anything you choose from then on is held for the page that is coming, and takes priority over the saved position — exactly as it already did if you had chosen a moment later.
The bigger the document, the longer the gap, so the bug hit hardest on exactly the documents where losing your place costs the most.
The document surface renders in WebKit; native stays the source of truth and pushes state over a bridge. DocumentScrollContent's .task(id:) is re-keyed by every parseRevision bump and previously did: await precomputeDocumentHTML → controller.load → restoreScrollPosition. The await is a real suspension (the emit runs on a detached task, T-1681), and throughout it the controller still described the outgoing page as isReady and isLayoutSettled.
Both precedence claims are raised in scrollTo / pushSearchState as !canDispatch(command) and released in dispatch. So "undelivered" was modelled as "could not be dispatched" — correct only while the page a command reaches is the page the reader will see. During the emit it was not, and all four navigation kinds were affected.
beginLoad() returns a monotonic UInt64; load closes unconditionally (it is only reached on the announcement's own non-cancelled path) and abandonLoad(id) closes only if that id still holds the window.canDispatch went from ready/not-ready to nothing (no page) / transient page scrolls only (superseded) / everything (current, subject to layoutSettled).WebDocumentControllerFactory.loadDocument, with the emit step injectable so a test can drive a navigation inside the window without racing a real emit.OutboundBridgeCommand.isTransientPageScroll mirrors what WebDocumentStateSnapshot.apply refuses to retain, so "not state" is expressed once.precomputeDocumentHTML declines to store it. Making the emit interruptible would not remove the need for the announcement id, so it was left alone.reloadDocument declines while a window is open, so the recovery banner's Reload can be a no-op for the length of an emit. Both declines log, and the announced load clears recoveryAbandoned — and the banner — moments later.Round 1 lowered isReady in beginLoad. That failed twice. First, scrollByPage / scrollToEdge queued behind the lowered readiness and were then wiped by load's resetForNavigation, so the keyboard and View menu were dead for the whole emit while trackpad scrolling kept working — and scrollabilityChanged is inbound, so the menu kept offering them. Second, the window rested on "nothing re-raises ready inside it", which prism-bridge.js guarantees per page evaluation, not per wall-clock window: handleProcessTermination and a same-revision reload each start an evaluation that posts its own ready. isSuperseded survives resetForNavigation, so neither can re-open dispatch.
Announcements overlap. .task(id:) is re-keyed per parseRevision and SwiftUI cancels the outgoing task without awaiting it, while await Task.detached { … }.value is not cancellation-aware — so load A resumes after load B has announced. An unconditional abandon then closed B's window and flushed the reader's navigation to the page B was about to replace: the ticket's symptom via its own fix. load needs no id because a cancelled load returns through abandonLoad and never reaches it; the stale-close is specific to the path a cancelled load takes, late. The counter uses +=, not &+=, deliberately — a stale abandon must fail to match forever, not until a wrap.
beginLoad deliberately does not bump the generation, so visibleBlock reports from the still-visible page keep updating native truth and the post-emit restore targets where the reader actually is. Any stale-report fix must therefore key on which page sent the message, not on readiness. Pinned by visibleBlockReportsStillLandDuringTheEmit.selectionCandidate is the one inbound class dropped in the window; the gate had to become !isReady || isSuperseded because readiness alone no longer describes it. beginLoad also clears the affordance, moving the overlay's death from page swap to announcement.isLayoutSettled and brings a page up that posts ready while isSuperseded is still true, so the superseded branch would dispatch a scrollToEdge to an unsettled page. Extremely narrow; noted below.reloadDocument declining forever. BlockHTMLEmitter.emit is bounded and synchronous, so this is currently unreachable by construction rather than by guard.abandonLoad that fires between two announcements flushes the queued navigation to the visible page, which releases the claim before the successor's restore — coherent (the scroll happens where the reader is looking, and its visibleBlock report feeds the restore) but dependent on that report beating the successor's load.WebDocumentController.swift
Why it matters. The whole fix. It is the state that makes 'undelivered' true for the length of the emit, and it is deliberately NOT isReady — the page is still up, still scrollable, still reporting.
What to look at. WebDocumentController.swift:77-127 (state), 724-765 (beginLoad / abandonLoad)
WebDocumentController.swift
Why it matters. This is what keeps the fix from trading one dropped intent for another. A superseded page still takes Page Up/Down and Scroll to Top/Bottom, because those aim at pixels that are still on screen; nothing revision-shaped may reach it, because dispatch is what releases the claims.
What to look at. WebDocumentController.swift:389-424 (send / canDispatch), WebBridgeContract.swift:281-301 (isTransientPageScroll)
WebDocumentControllerFactory.swift
Why it matters. The ordering (announce -> emit -> load -> restore) is the bug. Moving it out of the view makes it one testable thing and gives beginLoad an owner for both of its ends.
What to look at. WebDocumentControllerFactory.swift:213-278; DocumentScrollContent.swift:218-231
WebDocumentControllerFactory.swift
Why it matters. The same-revision reload (iOS folder-access grant, recovery banner) would call load, which closes the window early and hands the reader's navigation to a page that is about to be replaced.
What to look at. WebDocumentControllerFactory.swift:280-324; DocumentScrollContent.swift:329-347
WebReloadNavigationClaimTests.swift
Why it matters. Every load-bearing sentence in the new docstrings has a test behind it, including the two paths that re-open the window and the overlapping-load ordering that is otherwise pure prose.
What to look at. WebReloadNavigationClaimTests.swift:169-704; harness at WebNavigationPrecedenceHarness.swift
isReady answers 'is there a page to talk to'; isSuperseded answers 'is the page that is up the one this navigation is for'. Conflating them cost the page-scroll contract and rested the window on a premise two production paths falsify.
beginLoad returns a UInt64; abandonLoad no-ops unless it still holds the window. Overlapping loads are ordinary on large documents — .task(id:) cancels without awaiting, the emit is not interruptible — so 'close the window' and 'close my window' are different operations.
Sound because a load only reaches load on its announcement's own non-cancelled path: loadDocument returns through abandonLoad when cancelled. The stale-close the id exists to prevent is specific to abandon.
Crash recovery calls it and then brings up a page that posts its own ready. If that re-opened dispatch, the queued navigation would flush to a page the announced load is about to replace.
They are the two commands WebDocumentStateSnapshot.apply refuses to retain, so a queued one is either wiped by the next load or lands on a different document and fights the restore.
Making it interruptible would not remove the need for the announcement id (the calls can still interleave) and would cost a polling race on the load path; precomputeDocumentHTML already declines to store what a cancelled emit produced.
Unlike the test_ mutators on the controller, it stores nothing and has a production default that every production call site takes, so release behaviour is fixed at checked-in call sites rather than at runtime.
Diverges from processGeneration's wrapping increment on purpose: a stale abandon must fail to match forever, not until a counter wraps back onto its value. Overflow is unreachable at UInt64.
Consistent with the last five bugfix PRs on main (#323, #327, #328, #329, #330), none of which added one; the CHANGELOG entry plus docs/agent-notes/scroll-persistence.md carry the record now.
| Severity | Area | Finding | Resolution |
|---|---|---|---|
| minor | WebDocumentController.send / canDispatch | The new drop branch also swallows .scrollToEdge in the post-load gap between `ready` and `layoutSettled`, where a page IS live. `.scrollToEdge` has requiresLayoutSettled == true, so in that gap canDispatch is false and `else if command.isTransientPageScroll` drops it; before this PR it was queued and flushed at markLayoutSettled. The gap is not theoretical: prism-bridge.js posts `ready` on a setTimeout(0) at documentEnd while `layoutSettled` waits on fonts + the first mermaid/highlight passes + near-viewport images, and WebDocumentMessageRouter deliberately keeps Top/Bottom enabled in the View menu throughout ('hasContent is deliberately untouched'). So on exactly the large documents this ticket targets, Cmd-Down right after a reload can now silently do nothing where it used to land late. The emitted log line, 'Page scroll dropped: no live page to scroll', is also factually wrong there — a page is live — which undercuts the diagnosis the log exists for, and the isTransientPageScroll docstring says the drop applies 'when there is no live page'. | Reported, not fixed (report-only pass). One-line gate if the author agrees the queue was the better behaviour: `} else if command.isTransientPageScroll, !isReady {` — that confines the drop to exactly the state both docstrings describe, leaves the superseded path untouched (it dispatches, so it never reaches this branch), and makes the log line true. If the author instead considers a late edge-scroll landing next to the restore worse than nothing, the code is right and only the log line and the docstring's 'no live page' wording need to change. |
| nit | WebDocumentController.canDispatch (superseded branch) | The superseded branch returns isTransientPageScroll without consulting requiresLayoutSettled, justified by 'the superseded page's layout has long since settled'. That is false on one reachable path: a crash recovery inside the window calls resetForNavigation (clearing isLayoutSettled) and brings up a page that posts its own `ready` while isSuperseded is still true, so a Scroll to Bottom then dispatches to an unsettled page and scrolls to a provisional bottom. | Noted only — extremely narrow (crash recovery inside an emit window, plus a keystroke in the same window), and the failure mode is a mis-aimed scroll rather than lost state. |
| nit | WebDocumentControllerFactory.loadDocument (window lifetime) | Nothing watchdogs the window itself. If a loadDocument never resumed from prepareHTML, the page would accept no navigation, no restore and no state push for the rest of the session, and reloadDocument would decline forever — silently, which is the exact failure mode the docstrings say must not happen. Unreachable today because BlockHTMLEmitter.emit is bounded and synchronous, so the invariant is held by the emit's termination rather than by a guard. | Noted only. The recovery path has its own watchdog (defaultRecoveryWatchdogTimeout); this one is by construction. |
| info | Test flake (pre-existing, unrelated) | WebScrollPositionRetentionTests/initialLoadRestoreSurvivesAutoReport() failed twice with 30s timeouts before passing on retry, and WebContentTerminationWiringTests showed three 0.000s failures followed by a clean retry (the known crash-cascade shape, T-2096). Both suites are untouched by this branch and the result bundles report 102/102 and 24/24 passed respectively. | No action. Counts verified through Tools/check-test-results.sh rather than console text, per the project's own guidance. |
Click to expand.
diff --git a/prism/ViewModels/WebDocumentController.swift b/prism/ViewModels/WebDocumentController.swiftindex 8cfbb11..4b0f68f 100644--- a/prism/ViewModels/WebDocumentController.swift+++ b/prism/ViewModels/WebDocumentController.swift@@ -77,6 +77,52 @@ final class WebDocumentController { /// Unblocks scroll restore (Req 2.2/9.6). private(set) var isLayoutSettled = false + /// Raised by `beginLoad` and lowered by `load` / `abandonLoad`: a load has been+ /// ANNOUNCED for this surface but not yet issued, so whatever page is on screen+ /// belongs to the outgoing revision and is about to be replaced (T-1975).+ ///+ /// Deliberately separate from `isReady`, which answers a different question.+ /// `isReady` means "a page is up and can be talked to"; this means "the page that+ /// is up is not the one the reader's navigation is for". The two were conflated in+ /// the first cut of this fix — `beginLoad` simply lowered `isReady` — and that cost+ /// more than it bought:+ ///+ /// - It made the surface unscrollable. Page Up/Down and Scroll to Top/Bottom are+ /// not state; they act on the pixels on screen, and those pixels are still there+ /// for the whole emit. Lowering `isReady` sent them to the queue, where `load`+ /// then wiped them — so for the seconds a large document takes to emit, the+ /// keyboard and the View menu did nothing while trackpad scrolling carried on+ /// working. Keeping readiness truthful and supersession separate lets+ /// `canDispatch` say precisely what it means: transient page scrolls still go+ /// out, everything revision-shaped queues and claims.+ /// - It rested the whole window on `isReady` never being re-raised inside it. That+ /// premise fails for a page evaluation that starts DURING the window — a crash+ /// recovery (`handleProcessTermination`) or a same-revision reload — because+ /// each brings up a page that posts `ready` of its own. This flag survives both+ /// (`resetForNavigation` does not touch it), so the reader's queued navigation+ /// cannot be flushed to a page the announced load is about to discard.+ ///+ /// Only `load` (the announced load arriving) and `abandonLoad` (the announcement+ /// holding the window will never arrive) lower it. That is why a second, unannounced+ /// `load` must not be issued inside the window — see+ /// `WebDocumentControllerFactory.reloadDocument`.+ @ObservationIgnored private(set) var isSuperseded = false++ /// Identifies WHICH announcement raised `isSuperseded`, so an abandon can only lower+ /// the window it opened (T-1975 review).+ ///+ /// Announcements overlap in production. `.task(id:)` is re-keyed by every+ /// `parseRevision` bump, and SwiftUI cancels the outgoing task WITHOUT awaiting it:+ /// a second re-parse landing while the first is still emitting leaves load A parked+ /// in an emit that no cancellation can interrupt (`Task.detached`'s `value` ignores+ /// the awaiter's cancellation), so A resumes AFTER B has announced. Without an id,+ /// A's abandon would lower B's window and flush the reader's navigation to the page+ /// B is about to replace — the T-1975 bug, reached through its own fix.+ ///+ /// Monotonic and never reset: a stale abandon must fail to match forever, not until+ /// a counter wraps back onto its value.+ @ObservationIgnored private var loadAnnouncement: UInt64 = 0+ // MARK: - Command queue /// Commands awaiting `ready` (or `layoutSettled` for scroll restore). On@@ -253,13 +299,21 @@ final class WebDocumentController { // matched for the entire load and could otherwise re-arm the overlay at its // own rect after the clear. //- // "Only the INCOMING page's `ready`" is the load-bearing premise, and it is- // pinned at its source: prism-bridge.js posts `ready` EXACTLY ONCE per- // evaluation of the script (a single unconditional `setTimeout` registered- // at injection, no re-post path), so the outgoing page — already ready,- // already generation-matched — cannot raise `isReady` again after the reset.- // If that ever stops holding, this gate degrades from closing the window to- // merely narrowing it.+ // `isSuperseded` extends the same drop across the announce-to-load window+ // (T-1975), where `isReady` is deliberately left TRUE so the reader can still+ // page-scroll the document in front of them. A selection made there points at+ // a page that is going away regardless, so it is dropped on exactly the+ // T-1852 reasoning; it must be checked explicitly, because readiness alone no+ // longer describes the window.+ //+ // "Only the INCOMING page's `ready`" is the load-bearing premise for the+ // reload window, and it is pinned at its source: prism-bridge.js posts `ready`+ // EXACTLY ONCE per evaluation of the script (a single unconditional+ // `setTimeout` registered at injection, no re-post path), so the outgoing page+ // — already ready, already generation-matched — cannot raise `isReady` again+ // after the reset. A page evaluation that STARTS inside the window (crash+ // recovery, or a reload of the same revision) does post its own `ready`, which+ // is why the window is held by `isSuperseded` rather than by readiness. // // Safe in both directions. Nothing legitimate is lost: a genuine candidate // needs user interaction on a rendered page, and the incoming page's own@@ -274,7 +328,7 @@ final class WebDocumentController { // generation + allowlist verdict, which this message genuinely passes. This // is a lifecycle gate on routing, deliberately kept out of the value-type // router, which has no notion of readiness.- if case .selectionCandidate = message, !isReady { return }+ if case .selectionCandidate = message, !isReady || isSuperseded { return } switch message { case .ready: markReady()@@ -336,14 +390,35 @@ final class WebDocumentController { latestSnapshot.apply(command) if canDispatch(command) { dispatch(command)+ } else if command.isTransientPageScroll {+ // Dropped, never queued (T-1975). A page scroll acts on the page in front+ // of the reader or it does not happen: the snapshot deliberately does not+ // retain these, so a queued one is wiped by the next `load` in the common+ // case and, in the uncommon one, arrives on a DIFFERENT document seconds+ // later and fights the stored-position restore. Queueing them only ever+ // bought the latter.+ Self.logger.debug("Page scroll dropped: no live page to scroll") } else { pendingCommands.append(command) } } - /// Whether a command can be dispatched right now given readiness.+ /// Whether a command can be dispatched right now.+ ///+ /// Three states, not two. A page that has not come up takes nothing. A page that is+ /// up but SUPERSEDED — a load has been announced for it and is still emitting HTML+ /// (T-1975) — takes only transient page scrolls: it is still the document the+ /// reader is looking at and still scrolls under their trackpad, so Page Down must+ /// work there too, but nothing that belongs to the incoming revision may be+ /// delivered to it, because dispatch is what releases the T-1775 / T-1918+ /// navigation claims. A page that is up and current takes everything, subject to+ /// the layout-settled gate for the commands that scroll. func canDispatch(_ command: OutboundBridgeCommand) -> Bool { guard isReady else { return false }+ // The superseded page's layout has long since settled, so a page scroll needs+ // no further gate — and the incoming revision's `layoutSettled` is precisely+ // what it must NOT wait for.+ if isSuperseded { return command.isTransientPageScroll } if command.requiresLayoutSettled { return isLayoutSettled } return true }@@ -598,12 +673,117 @@ final class WebDocumentController { // MARK: - Load / reload / recovery + /// Marks the page currently on screen superseded because a reload for it has+ /// STARTED — called before the caller awaits the off-main HTML emit, not after+ /// (T-1975).+ ///+ /// `load` cannot cover this itself: the document surface must emit the new+ /// revision's HTML before it can navigate, and that emit is a suspension —+ /// seconds long on a large document. Until this call existed the controller kept+ /// treating the OLD page as the reader's current one throughout it, so a+ /// navigation raised in that window (a TOC entry, a note, a cross-document+ /// `#fragment`, a search match) dispatched straight to a page about to be+ /// discarded. Dispatch is exactly what releases the T-1775 / T-1918 claims, so the+ /// reload's stored-position restore had nothing to yield to and the reader was+ /// dropped back where they started.+ ///+ /// From here until `load` or `abandonLoad`, everything revision-shaped queues and+ /// claims precisely as it does for a page that has not come up yet — the precedence+ /// rules need no special case for the window, and all four navigation kinds are+ /// covered by the machinery that already exists. What does NOT stop is the page in+ /// front of the reader: it is still on screen, still scrolls under their trackpad,+ /// and still takes Page Up/Down and Scroll to Top/Bottom, which are aimed at those+ /// pixels rather than at a revision (`OutboundBridgeCommand.isTransientPageScroll`).+ /// The resulting `visibleBlock` reports feed the restore, which is the same model+ /// the trackpad has always had. Anything that made the window quietly unscrollable+ /// would trade one dropped intent for another, and leave the View menu offering+ /// commands that do nothing.+ ///+ /// Inbound messages are deliberately NOT cut off here: the parse revision moves in+ /// `load`, so the generation still matches and the reader's scroll reports keep+ /// updating native truth while the old page is still on screen — a restore issued+ /// after a long emit then targets where they actually are. That matters for T-2043+ /// (stale `visibleBlock` writes from an outgoing page): the fix for it must key on+ /// WHICH page sent the message, not on readiness, because a superseded page is+ /// still on screen and still the reader's. The one message class that IS dropped in+ /// this window is `selectionCandidate` (T-1852) — correctly, since that selection+ /// dies with the page being replaced.+ ///+ /// Every call must be paired with `load` (the announced load arrived) or+ /// `abandonLoad` (it never will) — see `WebDocumentControllerFactory.loadDocument`,+ /// which owns both ends. Without the pairing, an early return between the two would+ /// leave a live page permanently superseded: every navigation queued forever, every+ /// restore skipped, no state push delivered, and no signal anywhere.+ ///+ /// Idempotent in effect but NOT anonymous: the returned announcement id is what the+ /// caller must hand back to `abandonLoad`, because announcements overlap. A second+ /// re-parse announces over a first that is still emitting, and the first's+ /// cancellation lands after that — so "close the window" and "close MY window" are+ /// different operations, and only the second is safe (see `loadAnnouncement`).+ @discardableResult+ func beginLoad() -> UInt64 {+ loadAnnouncement += 1+ isSuperseded = true+ // The selection the native "Add note" overlay describes belongs to the page+ // being replaced, so it dies with the announcement rather than with the+ // navigation (T-1852): the reader is looking at a document whose replacement is+ // already being prepared, and the overlay's block id and rect describe the one+ // going away.+ selectionAffordance?.clear()+ return loadAnnouncement+ }++ /// The paired abandon for `beginLoad`: the announcement `announcement` will not be+ /// issued, so if it is still the one holding the window, the page on screen is the+ /// reader's current page again (T-1975 review).+ ///+ /// Reachable when the load task is cancelled during the emit — a further revision+ /// bump re-keys `.task(id:)`, or the surface is torn down. Neither is a reason to+ /// leave the state latched: `beginLoad`'s window is only sound because something+ /// always closes it, and an invariant that depends on today's call sites is one edit+ /// away from a page that silently accepts nothing for the rest of the session.+ ///+ /// **Which** window closes is the whole point of the id. The revision-bump case does+ /// not simply re-open the window a moment later, as the first cut of this assumed:+ /// the successor announces BEFORE the cancelled load resumes (its emit is not+ /// interruptible), so an unconditional abandon would close a window that opened after+ /// it and hold it closed for the successor's entire emit. A non-matching id is+ /// therefore a no-op, not a fallback — the successor owns the window and will close+ /// it itself, through `load` or through its own abandon.+ ///+ /// When it does match, everything the window held flushes to the page it is now+ /// allowed to reach, which is the coherent outcome rather than a merely safe one: the+ /// reader's TOC entry or note target scrolls the document actually in front of them,+ /// and dispatching it releases its claim, so the next reload restores normally.+ /// Nothing needs re-announcing and no banner is warranted — the page never went+ /// anywhere.+ func abandonLoad(_ announcement: UInt64) {+ guard isSuperseded, announcement == loadAnnouncement else { return }+ isSuperseded = false+ flushPending()+ }+ /// Loads (or reloads) the document for a given parse revision. Resets the /// readiness milestones, bumps nothing (same process), and discards the /// pending queue: native truth is replayed from the coalesced snapshot once /// the new page is ready (design `load` ordering).+ ///+ /// This is the announced load arriving, so it closes any `beginLoad` window: the+ /// page it brings up IS the one the reader's queued navigation is for. A load+ /// issued from anywhere else while a window is open would close it early and hand+ /// that navigation to a page the announced load is about to replace — the T-1975+ /// bug, one caller over. `WebDocumentControllerFactory.reloadDocument` is the+ /// production reload path and it declines to run inside the window for that reason.+ ///+ /// Unlike `abandonLoad` this takes no announcement id and closes unconditionally,+ /// which is sound because a load only reaches here on the announcement's own+ /// non-cancelled path: `loadDocument` returns through `abandonLoad` when its task was+ /// cancelled, so a superseded announcement never gets this far. The stale-close the id+ /// exists to stop is specific to abandon, which is exactly the path a cancelled load+ /// takes, late. func load(documentURL: URL, parseRevision: UInt64) { self.parseRevision = parseRevision+ isSuperseded = false loadedDocumentURL = documentURL // A user-initiated navigation (re-parse, file-change reload, URL refresh, the // iOS folder-access retry, or the reload the abandonment banner offers) is@@ -1030,6 +1210,12 @@ final class WebDocumentController { /// Clears the per-navigation readiness + pending queue. The snapshot survives /// so it can be replayed once the reloaded page reports ready.+ ///+ /// Deliberately does NOT touch `isSuperseded` (T-1975 review). Crash recovery calls+ /// this and then brings up a page of its own, which posts its own `ready`; if that+ /// re-opened dispatch, a navigation the reader raised during the emit would be+ /// flushed to a page the announced load is about to replace. The window belongs to+ /// the announcement, not to any one page, so only `load` and `abandonLoad` end it. private func resetForNavigation() { isReady = false isLayoutSettled = false
diff --git a/prism/ViewModels/WebDocumentControllerFactory.swift b/prism/ViewModels/WebDocumentControllerFactory.swiftindex 4c1a35b..0c10232 100644--- a/prism/ViewModels/WebDocumentControllerFactory.swift+++ b/prism/ViewModels/WebDocumentControllerFactory.swift@@ -211,6 +211,117 @@ enum WebDocumentControllerFactory { RenderSettings(showHTMLComments: settings.showHTMLComments, strings: catalogStrings()) } + /// Runs the whole load sequence for the session's current parse revision, in the+ /// order the scroll-precedence rules require. `DocumentScrollContent`'s load task is+ /// exactly this call, so the ordering is one testable thing rather than a handful of+ /// view lines nothing drives:+ ///+ /// 1. `beginLoad` — the visible page is superseded from here on, BEFORE the emit is+ /// awaited (T-1975). Skipping this is the whole bug: during the await the old page+ /// is still the controller's current one, so a navigation raised by the reader+ /// dispatches there and neither claim is raised for the revision about to load.+ /// 2. the off-main HTML emit (T-1681) — a real suspension, seconds long on a large+ /// document, and the window the reader navigates in.+ /// 3. `load` + `restoreScrollPosition` — the reload, then the stored reading position,+ /// which yields to any navigation claimed in step 2 (T-1775/T-1918).+ ///+ /// This owns BOTH ends of `beginLoad`: step 3 closes the window, and a cancellation+ /// between the two closes it through `abandonLoad` instead. The window must never+ /// outlive this function — a page left superseded accepts no navigation, no restore+ /// and no state push for the rest of the session, silently.+ ///+ /// It owns both ends of ITS OWN announcement, which is not the same thing, because+ /// two of these overlap whenever a second re-parse lands during the first's emit+ /// (T-1975 review). The `.task(id:)` re-key cancels the first WITHOUT awaiting it,+ /// and step 2 is not interruptible — `Task.detached`'s `value` ignores the awaiter's+ /// cancellation, and `BlockHTMLEmitter.emit` is a synchronous run to completion — so+ /// the cancelled call resumes only after the successor has announced. Hence the+ /// announcement id: the abandon below closes the window this call opened or nothing+ /// at all. Making the emit interruptible would not remove the need for it (the two+ /// calls can still interleave, just less often), and would cost a polling race on the+ /// load path; the late resume is only wasted work, and `precomputeDocumentHTML`+ /// already declines to store what it produced.+ ///+ /// It is the only path that opens the window, not the only path that loads: the+ /// same-revision reload (`reloadDocument`) has no emit to span, so it needs none —+ /// and declines to run inside one, since a second load would close the window early.+ ///+ /// `prepareHTML` is the emit step, injected so tests can drive a navigation inside the+ /// window without racing a real emit. Unlike the mutating test seams on+ /// `WebDocumentController` (`test_`-prefixed and `#if DEBUG`-walled so a release build+ /// cannot have its behaviour moved), this one needs no wall: it is a parameter with a+ /// production default, it stores nothing, and every production call site omits it — so+ /// a release build's behaviour is fixed at the call sites, which are checked into this+ /// repo, rather than at runtime.+ static func loadDocument(+ controller: WebDocumentController,+ session: DocumentSession,+ settings: AppSettings,+ prepareHTML: (DocumentSession, AppSettings) async -> Void = { session, settings in+ await precomputeDocumentHTML(for: session, settings: settings)+ }+ ) async {+ let revision = session.parseRevision+ guard revision > 0 else { return }+ let announcement = controller.beginLoad()+ await prepareHTML(session, settings)+ guard !Task.isCancelled else {+ controller.abandonLoad(announcement)+ return+ }+ controller.load(+ documentURL: documentURL(session: session, parseRevision: revision),+ parseRevision: revision+ )+ restoreScrollPosition(to: controller, session: session)+ }++ /// Re-fetches the CURRENT revision and offers the stored reading position: the iOS+ /// folder-access grant flow (images retry through the scheme handler once access+ /// exists) and the recovery-abandoned banner's **Reload** button.+ ///+ /// Not `loadDocument`, and deliberately so: the revision has not moved, so the+ /// emitted HTML is already cached and there is no suspension for the reader to+ /// navigate inside. With no window to open, opening one would only mean announcing a+ /// load and closing it in the same turn.+ ///+ /// It does, however, have to respect a window someone else opened. Returns `false`+ /// without loading when a `loadDocument` has announced a load it has not yet issued+ /// (T-1975 review): that load renders this very revision and is moments away, so the+ /// reload is redundant — and issuing it would call `load`, which closes the window.+ /// The page it brought up would then post `ready` and flush the reader's queued+ /// navigation to a page the announced load is about to replace, which is exactly the+ /// bug T-1975 closes. Skipping costs nothing; running costs the fix.+ ///+ /// Both declines log, and the return value is `false` rather than discarded, because+ /// the user-visible path is a button: the recovery-abandoned banner's **Reload**+ /// (T-1975 review). Tapping it inside the window does nothing the reader can see, and+ /// a decline nobody records is a "the Reload button is broken" report with no way to+ /// tell the two reasons apart. The banner deliberately does NOT retry afterwards —+ /// the announced load is moments away and `load` clears `recoveryAbandoned` itself, so+ /// the banner goes away on its own; a queued retry would be a second load racing the+ /// one already announced, to reach a state that arrives regardless.+ static func reloadDocument(+ controller: WebDocumentController,+ session: DocumentSession+ ) -> Bool {+ let revision = session.parseRevision+ guard revision > 0 else {+ logger.debug("Reload skipped: nothing parsed yet")+ return false+ }+ guard !controller.isSuperseded else {+ logger.debug("Reload skipped: a load is already announced for revision \(revision)")+ return false+ }+ controller.load(+ documentURL: documentURL(session: session, parseRevision: revision),+ parseRevision: revision+ )+ restoreScrollPosition(to: controller, session: session)+ return true+ }+ /// Builds the document HTML off the MainActor and caches it on `session`, keyed by the /// session's current `parseRevision` (T-1681). Called by the document surface before it /// navigates so the scheme handler serves cached HTML instead of running the full emit +
diff --git a/prism/ViewModels/WebBridgeContract.swift b/prism/ViewModels/WebBridgeContract.swiftindex f0860b0..d24726d 100644--- a/prism/ViewModels/WebBridgeContract.swift+++ b/prism/ViewModels/WebBridgeContract.swift@@ -277,6 +277,27 @@ enum OutboundBridgeCommand: Equatable, Sendable { return false } }++ /// Whether this command scrolls the page in front of the reader RIGHT NOW rather+ /// than carrying native truth for a revision.+ ///+ /// These two are exactly the commands `WebDocumentStateSnapshot.apply` refuses to+ /// retain: **Page Up/Down** and **Scroll to Top/Bottom** are a keystroke aimed at+ /// the pixels on screen, not state to replay onto whatever page comes next. The+ /// controller treats them accordingly (T-1975): they are delivered to any live+ /// page — including one a load has already superseded, where they are the exact+ /// keyboard counterpart of the trackpad scrolling that keeps working there — and+ /// DROPPED when there is no live page, never queued for a later one. A queued page+ /// scroll would arrive on a different document seconds later and fight the+ /// stored-position restore, which is the opposite of what the reader asked for.+ var isTransientPageScroll: Bool {+ switch self {+ case .scrollToEdge, .scrollByPage:+ return true+ default:+ return false+ }+ } } /// A document edge for scroll-to-edge commands.
diff --git a/prism/Views/DocumentScrollContent.swift b/prism/Views/DocumentScrollContent.swiftindex e4740e7..7304962 100644--- a/prism/Views/DocumentScrollContent.swift+++ b/prism/Views/DocumentScrollContent.swift@@ -218,22 +218,17 @@ struct DocumentScrollContent: View { hasController: webController != nil, revision: context.session.parseRevision )) {- guard let webController, context.session.parseRevision > 0 else { return }- let revision = context.session.parseRevision- // Build the document HTML off the MainActor and cache it before navigating, so a- // large/HTML-heavy document is emitted without freezing the main thread (T-1681).- // The scheme handler then serves the cached HTML; a cache miss falls back to a- // synchronous emit in WebDocumentControllerFactory.emitHTML.- await WebDocumentControllerFactory.precomputeDocumentHTML(- for: context.session, settings: context.settings- )- guard !Task.isCancelled else { return }- let url = WebDocumentControllerFactory.documentURL(+ guard let webController else { return }+ // The whole sequence — supersede the visible page, emit the HTML off the+ // MainActor, navigate, then offer the stored reading position — lives on the+ // factory so its ORDER is one testable thing (T-1975). Keep this call site a+ // call site: anything inserted before `loadDocument` re-opens the window in+ // which a navigation is dispatched to the page being replaced.+ await WebDocumentControllerFactory.loadDocument(+ controller: webController, session: context.session,- parseRevision: revision+ settings: context.settings )- webController.load(documentURL: url, parseRevision: revision)- restoreScrollPosition(with: webController) } .onChange(of: context.session.pendingFootnoteId) { _, newId in if let identifier = newId {@@ -334,24 +329,19 @@ struct DocumentScrollContent: View { /// Reloads the current document at the SAME revision — a re-fetch, not a re-parse. /// Used by the iOS folder-access grant flow (images re-request through the scheme /// handler once access exists) and by the recovery-abandoned banner.+ ///+ /// A call site, like the load task above: the reload's own contract — including when+ /// it must decline, because a `loadDocument` has announced a load it has not issued+ /// yet (T-1975) — lives on the factory next to the sequence it has to stay out of the+ /// way of. private func reloadWebDocument() {- guard let webController, context.session.parseRevision > 0 else { return }- let revision = context.session.parseRevision- let url = WebDocumentControllerFactory.documentURL(- session: context.session,- parseRevision: revision- )- webController.load(documentURL: url, parseRevision: revision)- restoreScrollPosition(with: webController)- }-- /// Replays the session's reading position into the freshly-loaded document- /// (T-1639), yielding to an undelivered navigation target (T-1775). The- /// contract lives on `WebDocumentControllerFactory.restoreScrollPosition`;- /// this is the view-side call site for it.- private func restoreScrollPosition(with webController: WebDocumentController) {- WebDocumentControllerFactory.restoreScrollPosition(- to: webController,+ guard let webController else { return }+ // The decline is deliberate and self-healing, so there is nothing for this+ // surface to do with it beyond not swallowing it: the factory logs which decline+ // it was, and the announced load that caused it clears `recoveryAbandoned` — and+ // so the banner — moments later (T-1975 review).+ _ = WebDocumentControllerFactory.reloadDocument(+ controller: webController, session: context.session ) }
diff --git a/prismTests/WebRendering/WebReloadNavigationClaimTests.swift b/prismTests/WebRendering/WebReloadNavigationClaimTests.swiftnew file mode 100644index 0000000..31acc57--- /dev/null+++ b/prismTests/WebRendering/WebReloadNavigationClaimTests.swift@@ -0,0 +1,704 @@+//+// WebReloadNavigationClaimTests.swift+// prismTests+//+// T-1975 regression tests: a navigation raised while an in-place re-parse is+// still emitting HTML must survive the reload it lands in.+//+// The window T-1775/T-1918 did NOT cover: those claims are raised only when the+// command cannot dispatch yet. During the off-main HTML precompute (T-1681) the+// OLD page is still on screen and still current, so a TOC entry / note target /+// fragment / search navigation dispatches straight to a page that is about to be+// replaced — no claim is raised, and the stored-position restore that follows+// `load` wins on the new revision.+//+// These tests drive the real production load sequence+// (`WebDocumentControllerFactory.loadDocument`) over the real production assembly+// (`WebDocumentStateSynchronizer.makeAssembly`), injecting the navigation into the+// emit step so it happens exactly inside that window.+//+// Four groups, and the last three are as load-bearing as the first:+//+// 1. The regression itself, per navigation kind, plus two guards against fixing it+// too hard (a re-parse nobody navigated during must still restore).+// 2. What the window must NOT stop. The page is still on screen: its scroll reports+// still land, and it still takes Page Up/Down and Scroll to Top/Bottom. The fix+// is only acceptable because those keep working; nothing else pins that.+// 3. What must not re-open the window. The whole thing rests on the reader's queued+// navigation not being dispatched to a doomed page — and two production paths+// bring a page up INSIDE the window (crash recovery, same-revision reload), each+// posting a `ready` of its own.+// 4. The pairing. A `beginLoad` with no `load` after it would latch a live page into+// accepting nothing, silently and for the rest of the session.+//++import Foundation+import SwiftUI+import Testing+import WebKit+@testable import prism++@MainActor+struct WebReloadNavigationClaimTests {++ // MARK: - Fixture++ /// The base fixture, the production assembly, and the waiting helpers are shared+ /// with `WebFragmentNavigationPrecedenceTests` (`WebNavigationPrecedenceHarness`).+ private typealias Assembly = WebNavigationPrecedenceHarness.Assembly++ /// The same document after an external edit — the intro paragraph changes, so+ /// the re-parse produces a genuinely new revision while the blocks the tests+ /// navigate to and restore to survive it.+ private static let reparsedFixture = """+ # Getting Started++ Intro paragraph, revised.++ ## Installation++ Install paragraph.++ ## Configuration++ Configuration paragraph.++ ## Troubleshooting++ Troubleshooting paragraph.+ """++ /// A SECOND external edit, for the overlapping-load test: it moves the revision+ /// again while the first load is still emitting, which is what re-keys+ /// `.task(id:)` and starts a second `loadDocument` over the first.+ private static let reparsedAgainFixture = """+ # Getting Started++ Intro paragraph, revised twice.++ ## Installation++ Install paragraph.++ ## Configuration++ Configuration paragraph.++ ## Troubleshooting++ Troubleshooting paragraph.+ """++ // MARK: - Helpers++ private func domIDs(for session: DocumentSession) -> [String] {+ WebNavigationPrecedenceHarness.domIDs(for: session)+ }++ private func waitUntil(+ timeout: Duration = .seconds(2),+ _ condition: () -> Bool+ ) async -> Bool {+ await WebNavigationPrecedenceHarness.waitUntil(timeout: timeout, condition)+ }++ private func settleSynchronizer() async {+ await WebNavigationPrecedenceHarness.settle()+ }++ /// The DOM id of the `## Troubleshooting` heading — the navigation target.+ private func anchorDOMID(in session: DocumentSession) throws -> String {+ try WebNavigationPrecedenceHarness.anchorDOMID(in: session)+ }++ /// Opens the document, renders it (page ready + laid out), and applies the+ /// external edit that starts an in-place re-parse.+ ///+ /// `withInitialLoad` additionally runs a real `load` first, which the recovery+ /// tests need for two reasons: `attemptRecovery` has nothing to reload until+ /// `loadedDocumentURL` is set, and the observation loop has to be parked. The+ /// second is the trap `WebContentTerminationWiringTests` documents — a real+ /// `page.navigations` is live from `init`, and navigating a real page to a+ /// `prism-doc://` URL can fail, which is charged and retried while a recovery is in+ /// flight. Parking the loop leaves the test's own `applyNavigationOutcome` call the+ /// only thing that can move recovery state, which is what these tests measure.+ private func reparsedAssembly(withInitialLoad: Bool = false) async -> Assembly {+ let assembly = await WebNavigationPrecedenceHarness.makeAssembly(namespace: "t1975")+ if withInitialLoad {+ assembly.controller.test_setInjectedNavigationStream {+ AsyncThrowingStream { _ in }+ }+ assembly.controller.test_rearmNavigationObservation()+ assembly.controller.load(+ documentURL: documentURL(for: assembly.session),+ parseRevision: assembly.session.parseRevision+ )+ }+ assembly.controller.test_markReady()+ assembly.controller.test_markLayoutSettled()+ await assembly.session.reloadContent(markdownString: Self.reparsedFixture)+ await settleSynchronizer()+ return assembly+ }++ /// The document URL the production load sequence navigates to for the session's+ /// current revision.+ private func documentURL(for session: DocumentSession) -> URL {+ WebDocumentControllerFactory.documentURL(+ session: session, parseRevision: session.parseRevision+ )+ }++ /// Runs the production load sequence for the session's current revision,+ /// performing `duringEmit` while the HTML emit is still in flight — i.e. with+ /// the previous revision's page still on screen.+ private func load(_ assembly: Assembly, duringEmit: @escaping @MainActor () async -> Void) async {+ await WebDocumentControllerFactory.loadDocument(+ controller: assembly.controller,+ session: assembly.session,+ settings: assembly.settings,+ prepareHTML: { session, settings in+ await duringEmit()+ await WebDocumentControllerFactory.precomputeDocumentHTML(+ for: session, settings: settings+ )+ }+ )+ }++ // MARK: - The regression, per navigation kind++ @Test("A TOC/fragment navigation during the re-parse emit survives the reload")+ func anchorNavigationDuringEmitSurvivesReload() async throws {+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let controller = assembly.controller+ let savedBlockID = domIDs(for: session)[3] // where the reader was reading+ let anchorID = try anchorDOMID(in: session) // the heading they asked for+ session.scrollPositionID = savedBlockID++ await load(assembly) {+ // The reader picks a TOC entry while the new revision is still being+ // emitted. The old page is still up, so pre-fix this dispatched there+ // and left no claim for the revision about to load.+ session.scrollToAnchor("troubleshooting")+ _ = await self.waitUntil { session.pendingAnchorScroll == nil }+ }++ #expect(+ controller.latestSnapshot.scrollTargetBlockID == anchorID,+ "the requested heading must outrank the saved reading position on the reloaded revision"+ )+ #expect(+ !controller.pendingCommands.contains(.scrollToBlock(domID: savedBlockID)),+ "no queued scroll may still land on the saved position"+ )+ }++ @Test("A note navigation during the re-parse emit survives the reload")+ func noteNavigationDuringEmitSurvivesReload() async throws {+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let controller = assembly.controller+ let savedBlockID = domIDs(for: session)[3]+ let anchorID = try anchorDOMID(in: session)+ let targetHash = try #require(+ BlockDOMID.map(blocks: session.parsedBlocks)+ .first { $0.domID == anchorID }?.block.id,+ "the target block must have a content hash"+ )+ session.scrollPositionID = savedBlockID++ await load(assembly) {+ // The reader taps a note in the notes panel during the emit.+ assembly.coordinator.noteNavigationTarget = targetHash+ _ = await self.waitUntil { assembly.coordinator.noteNavigationTarget == nil }+ }++ #expect(+ controller.latestSnapshot.scrollTargetBlockID == anchorID,+ "the note's block must outrank the saved reading position on the reloaded revision"+ )+ }++ @Test("A search-match navigation during the re-parse emit survives the reload")+ func searchNavigationDuringEmitSurvivesReload() async throws {+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let controller = assembly.controller+ session.scrollPositionID = domIDs(for: session)[3]++ // "paragraph" matches once in each of the fixture's four paragraphs.+ session.search.setActiveSearchQueryForTesting("paragraph")+ session.search.navigateToMatch(at: 0)+ try #require(session.currentMatch != nil, "search must select a match")+ await settleSynchronizer()++ await load(assembly) {+ // The reader steps to another match during the emit — both sides of the+ // diff are the new revision, so this classifies as a real navigation.+ let oldKey = WebSearchStateKey(session: session)+ session.search.navigateToMatch(at: session.search.totalMatchCount - 1)+ let newKey = WebSearchStateKey(session: session)+ #expect(+ WebSearchStateKey.isNavigation(from: oldKey, to: newKey),+ "a match step within one revision must classify as a navigation"+ )+ WebDocumentControllerFactory.pushSearchState(+ to: controller, session: session, settings: assembly.settings, reveal: true+ )+ }++ #expect(+ controller.latestSnapshot.scrollTargetBlockID == nil,+ "the saved reading position must not replace the search navigation"+ )+ guard case .setSearchState(_, let reveal)? = controller.pendingCommands.last else {+ Issue.record("the undelivered reveal must be re-queued last for the load")+ return+ }+ #expect(reveal, "the replayed search push must keep its navigation intent")+ }++ // MARK: - Guard against over-fixing++ @Test("A re-parse with no navigation still restores the saved reading position")+ func reparseWithoutNavigationStillRestores() async throws {+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let savedBlockID = domIDs(for: session)[3]+ session.scrollPositionID = savedBlockID++ await load(assembly) {}++ #expect(+ assembly.controller.latestSnapshot.scrollTargetBlockID == savedBlockID,+ "the reading position must survive a re-parse nobody navigated during"+ )+ }++ @Test("After the reloaded page takes the navigation, the next reload restores again")+ func restoreResumesOnTheReloadAfterTheNavigation() async throws {+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let controller = assembly.controller+ let anchorID = try anchorDOMID(in: session)+ session.scrollPositionID = domIDs(for: session)[3]++ await load(assembly) {+ session.scrollToAnchor("troubleshooting")+ _ = await self.waitUntil { session.pendingAnchorScroll == nil }+ }+ #expect(controller.latestSnapshot.scrollTargetBlockID == anchorID)++ // The reloaded page comes up and takes the queued navigation scroll.+ controller.test_markReady()+ controller.test_markLayoutSettled()++ // The reader reads on; the next reload must restore where they are now.+ let newPosition = domIDs(for: session)[5]+ session.scrollPositionID = newPosition+ await load(assembly) {}++ #expect(+ controller.latestSnapshot.scrollTargetBlockID == newPosition,+ "a delivered navigation must not keep blocking restores"+ )+ }++ // MARK: - What the window must NOT stop++ // The fix supersedes the page on screen, and the argument that this is safe rests+ // entirely on the page still being THERE: the reader is looking at it, scrolling it,+ // and its reports are still native truth. Both halves of that were prose in a+ // docstring; these two tests are the halves.++ @Test("Scroll reports from the superseded page still land, and still feed the restore")+ func visibleBlockReportsStillLandDuringTheEmit() async throws {+ // The single invariant the T-2043 correction depends on. The argument for NOT+ // bumping the generation in `beginLoad` — and for T-2043 keying on WHICH page+ // sent a message rather than on readiness — is that the reader's scroll reports+ // keep arriving throughout the emit and keep the restore target current. If+ // T-2043 is ever "fixed" by gating inbound messages on readiness, this goes red,+ // which is its purpose.+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let controller = assembly.controller+ session.scrollPositionID = domIDs(for: session)[1] // where the reader started+ let scrolledTo = domIDs(for: session)[5] // where they scroll during the emit++ await load(assembly) {+ let result = controller.receive(messageBody: [+ "type": "visibleBlock",+ "generation": controller.currentGeneration.argumentValue,+ "domID": scrolledTo,+ "fraction": 0.5,+ ])+ guard case .accepted = result else {+ Issue.record("a scroll report from the page on screen must still be accepted")+ return+ }+ #expect(+ session.scrollPositionID == scrolledTo,+ "the report must reach native truth, not just the router"+ )+ }++ #expect(+ controller.latestSnapshot.scrollTargetBlockID == scrolledTo,+ """+ The restore must land where the reader scrolled to DURING the emit, not \+ where they were when it started — the same contract trackpad scrolling has \+ always had.+ """+ )+ }++ @Test("Page and edge scrolls still reach the superseded page; revision-shaped commands do not")+ func pageScrollsStillReachTheSupersededPage() async throws {+ // Page Up/Down and Scroll to Top/Bottom act on the pixels in front of the+ // reader, not on a revision, so the emit window must not swallow them: the View+ // menu goes on offering them (`scrollabilityChanged` is inbound and unaffected),+ // and a keystroke that does nothing for the seconds a large document takes to+ // emit is the T-1932 contract going quietly false. Asserted through+ // `canDispatch`, which is the decision itself — the dispatch that follows is a+ // `callJavaScript` with nothing native to observe.+ let assembly = await reparsedAssembly()+ let controller = assembly.controller+ let anchorID = try anchorDOMID(in: assembly.session)++ await load(assembly) {+ #expect(+ controller.isSuperseded,+ "the window must actually be open, or this test proves nothing"+ )+ #expect(controller.canDispatch(.scrollByPage(direction: .down)))+ #expect(controller.canDispatch(.scrollByPage(direction: .up)))+ #expect(controller.canDispatch(.scrollToEdge(edge: .bottom)))+ #expect(controller.canDispatch(.scrollToEdge(edge: .top)))+ #expect(+ !controller.canDispatch(.scrollToBlock(domID: anchorID)),+ """+ …while a navigation scroll must NOT reach it: dispatching is what \+ releases the claim, which is the bug this ticket fixes.+ """+ )+ #expect(+ !controller.canDispatch(.setSearchState(json: "{}", reveal: false)),+ "and no state push belongs to a page that is being replaced"+ )+ }+ }++ @Test("A page scroll with no live page to scroll is dropped, not queued for a later one")+ func pageScrollWithNoLivePageIsDropped() async throws {+ // The other half of the transient contract. These commands are the two+ // `WebDocumentStateSnapshot.apply` refuses to retain, so a queued one has no+ // future: either the next `load` wipes it, or it arrives on a document the+ // reader never aimed it at and fights the stored-position restore.+ let assembly = await reparsedAssembly()+ let controller = assembly.controller+ controller.load(+ documentURL: documentURL(for: assembly.session),+ parseRevision: assembly.session.parseRevision+ )+ #expect(!controller.isSuperseded, "the load closed the window; the page is simply not up yet")++ controller.scrollByPage(.down)+ controller.scrollToEdge(.bottom)++ // The queue is not empty — `load` re-queued the coalesced snapshot for the+ // incoming page — but nothing in it may be a page scroll.+ #expect(+ !controller.pendingCommands.contains { $0.isTransientPageScroll },+ "a page scroll must never be held for a page that has not come up"+ )+ #expect(+ controller.latestSnapshot.scrollTargetBlockID == nil,+ "…nor be mistaken for a scroll position to restore"+ )+ }++ @Test("A selection made during the emit is dropped")+ func selectionCandidateDuringTheEmitIsDropped() async throws {+ // The T-1852 pairing, in the wider window this ticket opened. The selection the+ // native "Add note" overlay describes belongs to the page being replaced, so it+ // is the one message class the window deliberately sacrifices — and readiness+ // alone no longer expresses that, since the page is deliberately still ready.+ let assembly = await reparsedAssembly()+ let controller = assembly.controller+ let blockID = domIDs(for: assembly.session)[1]++ await load(assembly) {+ let result = controller.receive(messageBody: [+ "type": "selectionCandidate",+ "generation": controller.currentGeneration.argumentValue,+ "state": "available",+ "blockID": blockID,+ "range": ["start": 0, "length": 4],+ "rect": ["x": 0, "y": 0, "width": 10, "height": 10],+ ])+ // Accepted by the router — it is generation-matched and allowlisted — and+ // dropped by the controller's lifecycle gate, which is where the decision+ // belongs. The distinction is why the assertion is on the routing, not the+ // verdict.+ guard case .accepted = result else {+ Issue.record("the router's verdict is about generation, not lifecycle")+ return+ }+ #expect(+ !controller.receivedMessages.contains { message in+ if case .selectionCandidate = message { return true }+ return false+ },+ "a selection on a page about to be replaced must not be routed"+ )+ }+ }++ // MARK: - What must NOT re-open the window++ // `beginLoad`'s guarantee is that the reader's queued navigation cannot be handed to+ // a page the announced load is about to discard. Holding that on "no page raises+ // readiness inside the window" would be false: two production paths bring a page up+ // in there, each posting a `ready` of its own. The window is held by the+ // ANNOUNCEMENT instead, and these two tests are why it has to be.++ @Test("A WebContent crash during the emit does not release the navigation")+ func crashRecoveryDuringTheEmitDoesNotReleaseTheNavigation() async throws {+ let assembly = await reparsedAssembly(withInitialLoad: true)+ let session = assembly.session+ let controller = assembly.controller+ let savedBlockID = domIDs(for: session)[3]+ let anchorID = try anchorDOMID(in: session)+ session.scrollPositionID = savedBlockID++ await load(assembly) {+ session.scrollToAnchor("troubleshooting")+ _ = await self.waitUntil { session.pendingAnchorScroll == nil }++ // The renderer dies mid-emit. `applyNavigationOutcome` is the decision+ // function the observation loop calls, given the outcome the loop+ // classifies a real termination as — the stream→outcome wiring above it is+ // pinned by WebContentTerminationWiringTests, so driving the decision+ // directly keeps this test about the claims rather than about timing.+ #expect(controller.applyNavigationOutcome(.webContentTerminated))++ // The recovery reload brings a page up, and it posts its own `ready`. This+ // is the exact moment the "nothing re-raises readiness in the window"+ // premise fails.+ controller.test_markReady()+ controller.test_markLayoutSettled()+ }++ #expect(+ controller.latestSnapshot.scrollTargetBlockID == anchorID,+ """+ A page brought up by crash recovery is not the page the reader's navigation \+ was for; flushing it there releases the claim and the restore takes the \+ revision — the original bug, reached through recovery.+ """+ )+ }++ @Test("A same-revision reload requested during the emit declines, and the navigation survives")+ func sameRevisionReloadDuringTheEmitDeclines() async throws {+ let assembly = await reparsedAssembly(withInitialLoad: true)+ let session = assembly.session+ let controller = assembly.controller+ session.scrollPositionID = domIDs(for: session)[3]+ let anchorID = try anchorDOMID(in: session)++ await load(assembly) {+ session.scrollToAnchor("troubleshooting")+ _ = await self.waitUntil { session.pendingAnchorScroll == nil }++ // The iOS folder-access grant lands, or the reader takes the+ // recovery-abandoned banner's Reload, while the emit is still running.+ #expect(+ !WebDocumentControllerFactory.reloadDocument(+ controller: controller, session: session+ ),+ """+ The announced load renders this very revision and is moments away, so \+ a second load buys nothing and closes the window early.+ """+ )+ // Whatever page is up posts ready; the window must hold regardless.+ controller.test_markReady()+ controller.test_markLayoutSettled()+ }++ #expect(+ controller.latestSnapshot.scrollTargetBlockID == anchorID,+ "the reader's navigation must not be flushed to a page the announced load replaces"+ )+ }++ @Test("A same-revision reload outside the window still runs")+ func sameRevisionReloadOutsideTheWindowRuns() async throws {+ // The guard is about the window, not about the reload: the folder-access grant+ // and the abandonment banner must still work in the ordinary case.+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let savedBlockID = domIDs(for: session)[3]+ session.scrollPositionID = savedBlockID++ #expect(+ WebDocumentControllerFactory.reloadDocument(+ controller: assembly.controller, session: session+ )+ )+ #expect(assembly.controller.latestSnapshot.scrollTargetBlockID == savedBlockID)+ }++ // MARK: - The pairing++ @Test("A load cancelled during the emit hands the page back rather than latching it")+ func cancelledLoadAbandonsTheWindow() async throws {+ // `beginLoad` is only sound because something always closes its window. Today+ // that is `loadDocument`, at both ends. Without the abandon, a cancellation+ // between the two would leave a page that is on screen and perfectly healthy+ // accepting no navigation, no restore and no state push for the rest of the+ // session — silently, since nothing reports it.+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let controller = assembly.controller+ let savedBlockID = domIDs(for: session)[3]+ let anchorID = try anchorDOMID(in: session)+ session.scrollPositionID = savedBlockID++ let insideEmit = EmitGate()+ let task = Task { @MainActor in+ await WebDocumentControllerFactory.loadDocument(+ controller: controller,+ session: session,+ settings: assembly.settings,+ prepareHTML: { _, _ in+ insideEmit.entered = true+ // A real emit is a suspension; this one hangs there until the load+ // task is cancelled, which is what a revision bump re-keying+ // `.task(id:)` does to it.+ while !Task.isCancelled { await Task.yield() }+ }+ )+ }+ #expect(await waitUntil { insideEmit.entered })++ // The reader navigates while the emit is still in flight, so the claim is up.+ session.scrollToAnchor("troubleshooting")+ _ = await waitUntil { session.pendingAnchorScroll == nil }+ #expect(controller.isSuperseded)+ #expect(controller.pendingCommands.contains(.scrollToBlock(domID: anchorID)))++ task.cancel()+ _ = await task.value++ #expect(+ !controller.isSuperseded,+ "the announced load will never arrive, so the page on screen is current again"+ )+ #expect(+ !controller.pendingCommands.contains(.scrollToBlock(domID: anchorID)),+ "…and what the window was holding goes to the page it may now reach"+ )++ // Dispatching released the claim, so the surface is coherent rather than merely+ // unlatched: the next restore lands instead of being skipped forever.+ WebDocumentControllerFactory.restoreScrollPosition(to: controller, session: session)+ #expect(controller.latestSnapshot.scrollTargetBlockID == savedBlockID)+ }++ @Test("A cancelled load does not close the window a later load announced")+ func cancelledLoadDoesNotCloseTheSuccessorsWindow() async throws {+ // The other half of the pairing, and the reason the window is owned by an+ // ANNOUNCEMENT id rather than by a bare flag (T-1975 review). Two re-parses+ // overlapping is the ordinary case on the large documents this ticket exists+ // for — two external writes, or the file-change banner taken twice:+ //+ // A: beginLoad ─── emit ──────────────────────────┐ resume, cancelled → abandon+ // B: beginLoad ─── emit ────────────────────────────── load+ //+ // `.task(id:)` re-keys on the revision bump, and SwiftUI cancels A WITHOUT+ // awaiting it. A is parked on `await Task.detached { … }.value`, which is not+ // cancellation-aware, so it resumes only when its own emit finishes — reliably+ // after B has announced. An abandon that lowers whatever window it finds+ // therefore closes B's, flushing the reader's navigation to the page B is about+ // to replace: the ticket's original symptom, reached through the fix.+ let assembly = await reparsedAssembly()+ let session = assembly.session+ let controller = assembly.controller+ let savedBlockID = domIDs(for: session)[3]+ let anchorID = try anchorDOMID(in: session)+ session.scrollPositionID = savedBlockID++ // Load A announces, then parks in its emit.+ let firstEmit = EmitGate()+ let first = Task { @MainActor in+ await WebDocumentControllerFactory.loadDocument(+ controller: controller,+ session: session,+ settings: assembly.settings,+ prepareHTML: { _, _ in+ firstEmit.entered = true+ while !Task.isCancelled { await Task.yield() }+ }+ )+ }+ #expect(await waitUntil { firstEmit.entered })++ // A second external write lands: the revision moves, so `.task(id:)` re-keys.+ await session.reloadContent(markdownString: Self.reparsedAgainFixture)+ await settleSynchronizer()++ // Load B announces for the new revision and parks in ITS emit.+ let secondEmit = EmitGate()+ let second = Task { @MainActor in+ await WebDocumentControllerFactory.loadDocument(+ controller: controller,+ session: session,+ settings: assembly.settings,+ prepareHTML: { _, _ in+ secondEmit.entered = true+ while !secondEmit.released { await Task.yield() }+ }+ )+ }+ #expect(await waitUntil { secondEmit.entered })++ // The reader picks a TOC entry inside B's window, so B's window holds it.+ session.scrollToAnchor("troubleshooting")+ _ = await waitUntil { session.pendingAnchorScroll == nil }+ #expect(controller.pendingCommands.contains(.scrollToBlock(domID: anchorID)))++ // Only now does A's emit come back and find itself cancelled.+ first.cancel()+ _ = await first.value++ #expect(+ controller.isSuperseded,+ "B's window must still be open: A abandoned its OWN announcement, not B's"+ )+ #expect(+ controller.pendingCommands.contains(.scrollToBlock(domID: anchorID)),+ "the navigation must still be held, not flushed to the page B is replacing"+ )++ // B's emit finishes and its load lands, which is where the navigation belongs.+ secondEmit.released = true+ await second.value+ #expect(+ controller.latestSnapshot.scrollTargetBlockID == anchorID,+ "the navigation must outrank the saved reading position on B's revision"+ )+ }+}++/// A main-actor flag pair the cancellation tests' injected emits drive from inside the+/// load task: `entered` lets the test wait for the window to be open, `released` lets it+/// choose when a parked emit finishes.+@MainActor+private final class EmitGate {+ var entered = false+ var released = false+}
diff --git a/prismTests/WebRendering/WebNavigationPrecedenceHarness.swift b/prismTests/WebRendering/WebNavigationPrecedenceHarness.swiftnew file mode 100644index 0000000..59be505--- /dev/null+++ b/prismTests/WebRendering/WebNavigationPrecedenceHarness.swift@@ -0,0 +1,144 @@+//+// WebNavigationPrecedenceHarness.swift+// prismTests+//+// The shared fixture and mounting helpers for the scroll-precedence suites+// (`WebFragmentNavigationPrecedenceTests` — T-1775/T-1918 — and+// `WebReloadNavigationClaimTests` — T-1975).+//+// Both suites assert the same rule from different angles: a user navigation+// outranks the stored reading position for the load it is queued on. They+// therefore need the same setup — a parsed, file-backed session with the REAL+// production assembly mounted over it (`WebDocumentStateSynchronizer.makeAssembly`,+// exactly as `DocumentScrollContent` mounts it) — and the same way of waiting for+// the synchronizer's observation pass, which lands on later main-actor turns.+//+// It lives here because the second copy was written by hand: ~120 lines duplicated+// verbatim, which is how two suites over one subsystem quietly stop describing the+// same subsystem. Each suite keeps its own re-parsed fixture, because what the edit+// has to disturb differs between them — that is the part that is genuinely per-suite.+//++import Foundation+import SwiftUI+import Testing+@testable import prism++@MainActor+enum WebNavigationPrecedenceHarness {++ // MARK: - Fixture++ /// Four sections with one paragraph each: enough for a saved reading position, a+ /// navigation target, and a search term that matches once per paragraph.+ static let fixture = """+ # Getting Started++ Intro paragraph.++ ## Installation++ Install paragraph.++ ## Configuration++ Configuration paragraph.++ ## Troubleshooting++ Troubleshooting paragraph.+ """++ // MARK: - Mounting++ /// A session plus the production assembly mounted over it.+ struct Assembly {+ let session: DocumentSession+ let coordinator: DocumentLayoutCoordinator+ let settings: AppSettings+ let controller: WebDocumentController+ let synchronizer: WebDocumentStateSynchronizer+ }++ /// A parsed file-backed session with the production assembly mounted, at a unique+ /// path so `ScrollPositionStore` entries never collide across tests. `namespace`+ /// only makes a failing path easier to attribute to its suite.+ static func makeAssembly(namespace: String) async -> Assembly {+ let session = DocumentSession(+ url: URL(fileURLWithPath: "/tmp/\(namespace)-\(UUID().uuidString).md"),+ content: fixture+ )+ await session.parseContent()+ return remount(session: session)+ }++ /// Mounts a SECOND production assembly over an existing session — exactly what a+ /// raw→rendered toggle does: `DocumentScrollContent` is unmounted and re-created,+ /// so controller and synchronizer are fresh while the session (and its search+ /// state) survives.+ static func remount(session: DocumentSession) -> Assembly {+ let coordinator = DocumentLayoutCoordinator()+ let settings = AppSettings()+ let made = WebDocumentStateSynchronizer.makeAssembly(+ session: session,+ settings: settings,+ coordinator: coordinator,+ notesManager: NotesManager()+ )+ made.synchronizer.start(dynamicTypeSize: .large)+ return Assembly(+ session: session,+ coordinator: coordinator,+ settings: settings,+ controller: made.controller,+ synchronizer: made.synchronizer+ )+ }++ // MARK: - Helpers++ /// The occurrence-qualified DOM ids the emitter stamps for this session.+ static func domIDs(for session: DocumentSession) -> [String] {+ BlockDOMID.map(blocks: session.parsedBlocks).map(\.domID)+ }++ /// The DOM id of the `## Troubleshooting` heading — the navigation target both+ /// suites aim at.+ static func anchorDOMID(in session: DocumentSession) throws -> String {+ let index = try #require(+ session.parsedBlocks.firstIndex { block in+ if case .heading(_, let text) = block { return text == "Troubleshooting" }+ return false+ },+ "fixture must contain the Troubleshooting heading"+ )+ return domIDs(for: session)[index]+ }++ /// Polls the main actor until `condition` holds or the timeout elapses. The+ /// synchronizer's pushes land on later main-actor turns.+ static 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()+ }++ /// Lets the synchronizer's scheduled observation pass run to completion.+ ///+ /// Unlike `waitUntil`, this needs no positive signal — the fixed behaviour pushes+ /// *nothing* on a re-parse, so there is nothing to poll for. The observation+ /// callback enqueues its pass as `Task { @MainActor … }` during the mutation+ /// itself, so yielding the main actor runs it (and any settle pass it schedules)+ /// before the assertion.+ static func settle() async {+ for _ in 0..<8 { await Task.yield() }+ }+}
diff --git a/prismTests/WebRendering/WebFragmentNavigationPrecedenceTests.swift b/prismTests/WebRendering/WebFragmentNavigationPrecedenceTests.swiftindex 5547c81..3c11026 100644--- a/prismTests/WebRendering/WebFragmentNavigationPrecedenceTests.swift+++ b/prismTests/WebRendering/WebFragmentNavigationPrecedenceTests.swift@@ -13,8 +13,9 @@ // 1. DocumentReaderView restores `session.scrollPositionID` (block B), awaits // `parseContent()`, then consumes `pendingFragment` into // `pendingAnchorScroll` (heading A).-// 2. The parseRevision bump starts DocumentScrollContent's load task, which-// now awaits `precomputeDocumentHTML` BEFORE calling `load` + restore.+// 2. The parseRevision bump starts DocumentScrollContent's load task — one call+// to `WebDocumentControllerFactory.loadDocument` (T-1975), which awaits+// `precomputeDocumentHTML` BEFORE calling `load` + restore. // 3. While that await is suspended, the synchronizer's observation pass // consumes `pendingAnchorScroll` and sends `scrollTo(A)` — so by the time // the load task resumes, `pendingAnchorScroll` is already nil.@@ -39,23 +40,11 @@ struct WebFragmentNavigationPrecedenceTests { // MARK: - Fixture - private static let fixture = """- # Getting Started-- Intro paragraph.-- ## Installation-- Install paragraph.-- ## Configuration-- Configuration paragraph.-- ## Troubleshooting-- Troubleshooting paragraph.- """+ /// The base fixture, the production assembly, and the waiting helpers are shared+ /// with `WebReloadNavigationClaimTests` (`WebNavigationPrecedenceHarness`). Only+ /// the re-parsed fixture is per-suite: what the external edit has to disturb+ /// differs between the two.+ private typealias Assembly = WebNavigationPrecedenceHarness.Assembly /// The fixture after an external edit: the intro paragraph now also mentions /// "Troubleshooting", so global match index 0 — which `recomputeMatchCounts`@@ -79,92 +68,34 @@ struct WebFragmentNavigationPrecedenceTests { Troubleshooting paragraph. """ - private struct Assembly {- let session: DocumentSession- let coordinator: DocumentLayoutCoordinator- let settings: AppSettings- let controller: WebDocumentController- let synchronizer: WebDocumentStateSynchronizer- }-- /// A parsed file-backed session with the production assembly mounted, at a- /// unique path so ScrollPositionStore entries never collide across tests. private func makeAssembly() async -> Assembly {- let session = DocumentSession(- url: URL(fileURLWithPath: "/tmp/t1775-\(UUID().uuidString).md"),- content: Self.fixture- )- await session.parseContent()- return remount(session: session)+ await WebNavigationPrecedenceHarness.makeAssembly(namespace: "t1775") } - /// Mounts a SECOND production assembly over an existing session — exactly what- /// a raw→rendered toggle does: `DocumentScrollContent` is unmounted and- /// re-created, so controller and synchronizer are fresh while the session (and- /// its search state) survives. private func remount(session: DocumentSession) -> Assembly {- let coordinator = DocumentLayoutCoordinator()- let settings = AppSettings()- let made = WebDocumentStateSynchronizer.makeAssembly(- session: session,- settings: settings,- coordinator: coordinator,- notesManager: NotesManager()- )- made.synchronizer.start(dynamicTypeSize: .large)- return Assembly(- session: session,- coordinator: coordinator,- settings: settings,- controller: made.controller,- synchronizer: made.synchronizer- )+ WebNavigationPrecedenceHarness.remount(session: session) } // MARK: - Helpers - /// The occurrence-qualified DOM ids the emitter stamps for this session. private func domIDs(for session: DocumentSession) -> [String] {- BlockDOMID.map(blocks: session.parsedBlocks).map(\.domID)+ WebNavigationPrecedenceHarness.domIDs(for: session) } - /// Polls the main actor until `condition` holds or the timeout elapses.- /// The synchronizer's pushes land on later main-actor turns. 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()+ await WebNavigationPrecedenceHarness.waitUntil(timeout: timeout, condition) } - /// Lets the synchronizer's scheduled observation pass run to completion.- ///- /// Unlike `waitUntil`, this needs no positive signal — the fixed behaviour- /// pushes *nothing* on a re-parse, so there is nothing to poll for. The- /// observation callback enqueues its pass as `Task { @MainActor … }` during- /// the mutation itself, so yielding the main actor runs it (and any settle- /// pass it schedules) before the assertion. private func settleSynchronizer() async {- for _ in 0..<8 { await Task.yield() }+ await WebNavigationPrecedenceHarness.settle() } /// The DOM id of the `## Troubleshooting` heading — the fragment target. private func anchorDOMID(in session: DocumentSession) throws -> String {- let index = try #require(- session.parsedBlocks.firstIndex { block in- if case .heading(_, let text) = block { return text == "Troubleshooting" }- return false- },- "fixture must contain the Troubleshooting heading"- )- return domIDs(for: session)[index]+ try WebNavigationPrecedenceHarness.anchorDOMID(in: session) } // MARK: - The ordering regression
diff --git a/docs/agent-notes/scroll-persistence.md b/docs/agent-notes/scroll-persistence.mdindex eb297a4..cd11c19 100644--- a/docs/agent-notes/scroll-persistence.md+++ b/docs/agent-notes/scroll-persistence.md@@ -61,6 +61,55 @@ The document body renders in a WebView, so the scroll contract has two halves: snapshot replays search state with the reveal stripped — unless it is still undelivered, in which case it is re-queued reveal-carrying and LAST (after the restore target) so its scroll still wins.+- **Both claims depend on nothing revision-shaped being deliverable during the emit+ (T-1975):** a claim is raised only when the command *cannot dispatch yet*, so it+ relies on the controller not still treating the OUTGOING page as the reader's+ current one while the next revision is being emitted.+ `WebDocumentControllerFactory.loadDocument` runs that sequence, and it calls+ `controller.beginLoad()` FIRST — before the `precomputeDocumentHTML` await — for+ exactly that reason. Before it, a TOC entry, note target, `#fragment`, or search+ match chosen during the emit dispatched to the page about to be discarded, which+ RELEASED the claim, and the reload's restore then had nothing to yield to. All four+ navigation kinds were affected, not just the non-search ones. Anything inserted+ ahead of `beginLoad` in that sequence re-opens the window; keep+ `DocumentScrollContent`'s load task a bare call to it. Four things about the window+ are load-bearing and each is pinned by a test in `WebReloadNavigationClaimTests`:+ - It is held by `controller.isSuperseded`, NOT by lowering `isReady`. Readiness+ means "a page is up"; supersession means "the page that is up is not the one this+ navigation is for". Conflating them cost both of the next two points.+ - The page stays scrollable. `OutboundBridgeCommand.isTransientPageScroll`+ (`scrollByPage` / `scrollToEdge` — exactly what the snapshot refuses to retain)+ still dispatches to a superseded page, because it is still on screen and still+ scrolls under the trackpad; Page Up/Down and Scroll to Top/Bottom must not become+ dead keys for the seconds a large document takes to emit (that would also falsify+ T-1932, since `scrollabilityChanged` is inbound and keeps the menu enabled). Those+ commands are dropped rather than queued when no live page exists: they are aimed+ at pixels, and a queued one lands on a different document and fights the restore.+ - Only `load` and `abandonLoad` end the window. A page evaluation that STARTS inside+ it — crash recovery, or a same-revision reload — posts its own `ready`, so+ "nothing re-raises readiness in the window" would be false. `resetForNavigation`+ therefore leaves `isSuperseded` alone, and+ `WebDocumentControllerFactory.reloadDocument` (the folder-access grant and the+ abandonment banner) declines while a load is announced — its `load` would+ otherwise close the window early. That is also why `loadDocument`, not+ `reloadDocument`, is the only path that opens one.+ - `beginLoad` must be paired, and paired with ITS OWN announcement. `loadDocument`+ owns both ends: `load` on the happy path, `abandonLoad` on cancellation. An+ unpaired `beginLoad` latches a healthy, visible page into accepting no navigation,+ no restore and no state push for the rest of the session, with nothing reporting+ it. But announcements overlap — a second re-parse re-keys `.task(id:)`, and SwiftUI+ cancels the first load WITHOUT awaiting it while its emit is not interruptible+ (`Task.detached`'s `value` ignores the awaiter's cancellation), so the cancelled+ call resumes *after* the successor announced. `beginLoad` therefore returns a+ monotonic announcement id and `abandonLoad(_:)` no-ops unless it still holds the+ window; an unconditional abandon reopened the original T-1975 bug through the fix+ itself. `load` needs no id: a cancelled load returns through `abandonLoad` and+ never reaches it.++ `beginLoad` deliberately does not touch the generation — the old page is still on+ screen and its `visibleBlock` reports are still the reader's truth, which is also+ why T-2043's stale-report fix must key on which page sent the message rather than+ on readiness. - **Neither a remount nor a re-parse is a navigation (T-1775):** only a *user* navigation may raise either claim. `pendingAnchorScroll` / `noteNavigationTarget` are genuine one-shot events the session hands over
diff --git a/CHANGELOG.md b/CHANGELOG.mdindex 4318ac1..5224d5a 100644--- a/CHANGELOG.md+++ b/CHANGELOG.md@@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Choosing where to go while a document reloads now takes you there (T-1975). If a file changed on disk — or a URL document was refreshed — while you had it open, and during the moment the app spends preparing the new version you picked a table-of-contents entry, tapped a note, followed a link to a heading, or stepped to a search match, the reloaded document appeared at your saved reading position instead. What you chose was handed to the copy still on screen, which was about to be replaced, so nothing was left to say where you had asked to go and restoring your place won — and on a large document, where preparing the new version takes longest, that window is at its widest. The document on screen is now treated as superseded from the moment a reload starts rather than from the moment the new version is ready, so anything you choose in between is held for the version that is coming and takes precedence over your saved place, exactly as it already did when you chose a moment later. This holds when a file changes twice in quick succession, so a second reload beginning before the first has finished preparing still takes you where you asked rather than back to your saved place. Reloads you did not navigate during still return you to where you were reading, and once the reloaded document has taken you where you asked, the next reload restores your place normally. Scrolling while a reload prepares still counts too, however you do it — dragging, a trackpad or wheel, **Page Up** and **Page Down**, or **Scroll to Top** and **Scroll to Bottom** — because the document stays in front of you and stays scrollable the whole time: the place you scroll to is the place you are returned to. - Changing the reading font or text size no longer moves you somewhere else in the document (T-1965). Both settings already applied without reloading, but they reflow the whole document and nothing put you back afterwards: raising **Larger Text** to an accessibility size makes every block roughly three times as tall, so the text you were reading slid off the bottom of the screen and left you looking at something you had already been through. The app then recorded that new spot as where you were reading, so closing and reopening the document returned you to it as well. Your place is now kept across the change — including how far into a paragraph you were, so the same words stay in front of you rather than merely the same paragraph starting at the top — and a place you were never reading can no longer be saved while the document settles. Jumping somewhere while the change is settling wins: a table-of-contents entry, a link, a note, or a search match all take you where you asked, and the re-anchoring steps aside. Collapsing the section you were reading during the change leaves you at its heading rather than at content that is no longer shown. - A document that goes blank because its rendering process stopped now restores itself (T-1943). The app has always been able to recover from this — it reloads the document and puts back your theme, your reading position, your note markers, and any active search highlights — but nothing was ever watching for the rendering process to stop, so the recovery never actually ran. A large or image-heavy document whose renderer was shut down under memory pressure therefore showed an empty page, with no error and no way back except closing the file and opening it again. The app now watches for it and recovers on the spot. An ordinary failure to load — a link that goes nowhere, an image that cannot be fetched — is told apart from a stopped renderer, so it neither causes a needless reload nor stops the app watching for a real one afterwards. The recovery also covers its own failure: if the reload it starts cannot itself load the document, that counts as the recovery failing and is tried again, instead of leaving the page blank with nothing running. A reload that neither succeeds nor fails — one that simply never finishes — is covered too: it is given a generous time limit, well beyond what even a large document takes to appear, and is then treated as a failed recovery and tried again rather than leaving the page blank indefinitely. If reloading repeatedly fails to bring the document back, the app stops retrying rather than reloading over and over — and says so, with a banner offering to reload. Taking that reload also restores the document's ability to recover on its own again, so giving up is never permanent while the file stays open. - Scrolling the raw source back to the top now takes the rendered view with it (T-1701). Reading part-way down a document, switching to raw source, scrolling that back to the start and switching to the rendered view again returned you to where you had been reading rather than to the top: the app kept the earlier position and replayed it, so the one thing you had just asked for was the one thing it ignored. Where you leave the raw source is now where the rendered view returns to, the top included, and that holds even when you switched over so soon after opening the document that the app had not yet worked out where you were reading. Switching straight back without having moved the raw source at all still returns you to where you were, so a document reopened at your saved place — or an accidental double-tap on the toggle — does not cost you that place. In a file that opens with a block of metadata, returning to the top now records the document's first line rather than that hidden block, so the position is one the app can still return to after you close and reopen the file.
If abandonLoad lands in the gap between two announcements (A resumes before B announces), the queued navigation flushes to the visible page and releases its claim, so B's restore proceeds normally. The reader still ends up in the right place only because the flushed scroll's visibleBlock report updates scrollPositionID before B's load. That is the same model trackpad scrolling has always had, but it is a race rather than an ordering guarantee — worth remembering if a future change makes the restore fire earlier.
Inside the window the button does nothing the reader can see. Self-healing (the announced load clears recoveryAbandoned), logged, and tested — but if the emit is long and the reader taps twice, there is no UI feedback at all. Acceptable as designed; flagging in case a spinner or disabled state is wanted later.
A navigation raised in the window resolves against the incoming revision's blocks, so the queued scrollToBlock carries a new-revision DOM id — correct. If the reader's target block is itself the block the external edit changed, its content-hash id moves and the queued scroll no-ops on the new page. Pre-existing to the claim mechanism, not introduced here, and not covered by a test.