Pre-push review: T-1851 collapsed-heading scroll position

diff --git a/prism/Resources/WebRenderer/prism-scroll.js b/prism/Resources/WebRenderer/prism-scroll.jsindex 76f0483..4efdda2 100644--- a/prism/Resources/WebRenderer/prism-scroll.js+++ b/prism/Resources/WebRenderer/prism-scroll.js@@ -66,20 +66,46 @@     }      // The topmost block section currently intersecting the viewport top.+    //+    // A display:none element's bounding rect is ALL ZEROS. A top of 0 satisfies the+    // "closest to the viewport top" test and beats every genuinely visible section,+    // which starts ABOVE the fold at a negative top — so without this filter a+    // non-laid-out section wins and Prism persists a block the reader cannot see as+    // the reading position (T-1851). Two kinds of section are display:none:+    //   - content under a collapsed heading (prism-theme.js marks it with+    //     data-prism-section-hidden; document.css hides it), and+    //   - the inert YAML-frontmatter carrier, which BlockHTMLEmitter emits with the+    //     bare `hidden` attribute — so it is sections[0] and used to win at EVERY+    //     scroll offset in any document with frontmatter.+    // Skip anything that is not laid out, in the scan AND in the fallback. The rect+    // check alone covers both cases today; the attribute check is kept as defence in+    // depth, because it keeps working if the collapse rule ever moves to+    // visibility/content-visibility, which leave the rect non-zero.     function topmostBlockID() {         var sections = document.querySelectorAll("section[data-prism-block-id]");         var best = null;         var bestTop = -Infinity;+        var firstRendered = null;         for (var i = 0; i < sections.length; i++) {-            var rect = sections[i].getBoundingClientRect();+            var section = sections[i];+            if (section.getAttribute("data-prism-section-hidden") === "true") { continue; }+            var rect = section.getBoundingClientRect();+            if (rect.width === 0 && rect.height === 0) { continue; }+            if (!firstRendered) { firstRendered = section; }             // The block whose top is closest to (but not far below) the viewport top.             if (rect.top <= 1 && rect.top > bestTop) {                 bestTop = rect.top;-                best = sections[i];+                best = section;             }         }-        // If nothing is above the fold yet, use the first section.-        if (!best && sections.length > 0) { best = sections[0]; }+        // If nothing is above the fold yet, use the first RENDERED section (before+        // T-1851 this was sections[0], which is the hidden frontmatter carrier in+        // any document with frontmatter). When NOTHING is rendered — a+        // frontmatter-only document, or a zero-size viewport — this deliberately+        // returns null and reportVisibleBlock posts nothing, keeping the last known+        // position rather than persisting an unreachable id. Silence is the intended+        // behaviour there, not a missing fallback.+        if (!best) { best = firstRendered; }         return best ? best.id : null;     } 

diff --git a/prismTests/WebRendering/WebCollapsedSectionScrollTests.swift b/prismTests/WebRendering/WebCollapsedSectionScrollTests.swiftnew file mode 100644index 0000000..0ebb780--- /dev/null+++ b/prismTests/WebRendering/WebCollapsedSectionScrollTests.swift@@ -0,0 +1,287 @@+//+//  WebCollapsedSectionScrollTests.swift+//  prismTests+//+//  T-1851 regression tests: a section the reader cannot see must never be+//  reported as the reading position.+//+//  A display:none element's bounding rect is ALL ZEROS, so its `top` of 0+//  satisfies prism-scroll.js's "closest to the viewport top" test and beats every+//  genuinely visible section scrolled above the fold (negative top). Prism then+//  persists that block as the reading position and the restore either lands on the+//  wrong block or silently no-ops (scrollIntoView on a display:none element does+//  nothing). Two kinds of section are display:none: content under a collapsed+//  heading (`prism-theme.js` marks it `data-prism-section-hidden="true"`,+//  document.css hides it) and the inert YAML-frontmatter carrier that+//  `BlockHTMLEmitter` emits with the bare `hidden` attribute.+//+//  All four tests drive a real WebPage with the bundled document.css injected, so+//  the real cascade — not a hand-rolled inline style — governs visibility.+//++import Foundation+import Testing+import WebKit+@testable import prism++@MainActor+struct WebCollapsedSectionScrollTests {++    // MARK: - Harness++    /// The harness scheme handler cannot serve the linked stylesheet, so the+    /// bundled document.css is injected directly (mirrors+    /// `HTMLCommentVisibilityLiveTests.makeStyledHarness`).+    private static func makeStyledHarness(blocks: [MarkdownBlock]) async throws -> WebDocumentLiveHarness {+        let harness = try await WebDocumentLiveHarness.make(blocks: blocks)+        let cssURL = try #require(+            Bundle.main.url(forResource: "document", withExtension: "css"),+            "bundled document.css must be present in the test host"+        )+        let css = try String(contentsOf: cssURL, encoding: .utf8)+        _ = try await harness.page.callJavaScript(+            "var s = document.createElement('style'); s.textContent = css;"+                + " document.head.appendChild(s); return null;",+            arguments: ["css": css],+            contentWorld: harness.bridgeWorld+        )+        return harness+    }++    /// A document with an early collapsible H2 span followed by a tall visible one.+    /// Index 1 is the H2 whose span (indices 2...21) collapses.+    private static func collapsibleBlocks() -> [MarkdownBlock] {+        var blocks: [MarkdownBlock] = [+            .heading(level: 1, text: "Document"),+            .heading(level: 2, text: "Early section"),+        ]+        for index in 0..<20 {+            blocks.append(.paragraph(markdown: "Early paragraph \(index) with running text for height."))+        }+        blocks.append(.heading(level: 2, text: "Later section"))+        for index in 0..<40 {+            blocks.append(.paragraph(markdown: "Later paragraph \(index) with running text for height."))+        }+        return blocks+    }++    private func domIDs(_ blocks: [MarkdownBlock]) -> [String] {+        BlockDOMID.map(blocks: blocks).map(\.domID)+    }++    /// Waits for a visibleBlock report beyond the ones already recorded and+    /// returns its DOM id.+    private func waitForVisibleBlock(+        _ harness: WebDocumentLiveHarness, after count: Int+    ) async throws -> String? {+        for _ in 0..<40 {+            let reports = harness.messages(type: "visibleBlock")+            if reports.count > count { return reports.last?["domID"] as? String }+            try await Task.sleep(for: .milliseconds(50))+        }+        return nil+    }++    /// `"hidden"` / `"visible"` / `"missing"` for the section with `domID`, read+    /// from the live DOM under the real stylesheet.+    private func visibility(+        _ harness: WebDocumentLiveHarness, of domID: String+    ) async throws -> String? {+        let result = try await harness.page.callJavaScript(+            "var el = document.getElementById(id); if (!el) { return 'missing'; }"+                + " var hidden = el.getAttribute('data-prism-section-hidden') === 'true'"+                + " || getComputedStyle(el).display === 'none';"+                + " return hidden ? 'hidden' : 'visible';",+            arguments: ["id": domID],+            contentWorld: harness.bridgeWorld+        )+        return result as? String+    }++    /// The DOM ids of every section the `rect.width === 0 && rect.height === 0`+    /// filter in `topmostBlockID()` would skip, read from the live DOM.+    private func zeroRectSectionIDs(_ harness: WebDocumentLiveHarness) async throws -> [String] {+        let result = try await harness.evalString(+            "var out = [];"+                + " var sections = document.querySelectorAll('section[data-prism-block-id]');"+                + " for (var i = 0; i < sections.length; i++) {"+                + "   var r = sections[i].getBoundingClientRect();"+                + "   if (r.width === 0 && r.height === 0) { out.push(sections[i].id); }"+                + " }"+                + " return out.join(',');"+        )+        guard let result, !result.isEmpty else { return [] }+        return result.components(separatedBy: ",")+    }++    private func hiddenSectionCount(_ harness: WebDocumentLiveHarness) async throws -> Int {+        let result = try await harness.evalString(+            "return String(document.querySelectorAll("+                + "'section[data-prism-section-hidden=\"true\"]').length);"+        )+        return Int(result ?? "0") ?? 0+    }++    // MARK: - Tests++    // The reproduction from the ticket: an early collapsed heading, the reader+    // scrolled into a later visible section. The hidden sections' zero rects+    // must not win the topmost-block contest.+    @Test("visibleBlock never reports a section hidden by a collapsed heading")+    func collapsedSectionIsNotReportedAsVisibleBlock() async throws {+        let blocks = Self.collapsibleBlocks()+        let harness = try await Self.makeStyledHarness(blocks: blocks)+        let ids = domIDs(blocks)++        // Collapse the early H2 (block index 1); its section id is "{hash}-{sourceIndex}".+        try await harness.send(.setSectionState(collapsedIDs: ["\(blocks[1].id)-1"]))+        #expect(try await hiddenSectionCount(harness) > 0, "the collapse did not hide any section")++        // Scroll a later, visible paragraph to the viewport top — a user scroll,+        // so no programmatic-scroll suppression is in play.+        let targetID = ids[27]+        let seen = harness.messages(type: "visibleBlock").count+        _ = try await harness.page.callJavaScript(+            "var el = document.getElementById(id); el.scrollIntoView({ block: 'start' }); return null;",+            arguments: ["id": targetID],+            contentWorld: harness.bridgeWorld+        )++        let reported = try #require(+            try await waitForVisibleBlock(harness, after: seen), "no visibleBlock was reported"+        )+        #expect(+            try await visibility(harness, of: reported) == "visible",+            "a hidden section was persisted as the reading position: \(reported)"+        )+        #expect(reported == targetID)+    }++    // The fallback ("nothing above the fold yet, use the first section") must+    // skip hidden sections too: with the only heading collapsed, every following+    // section is display:none and the heading itself sits below the 1px probe+    // line because of the document's top padding.+    @Test("the topmost-block fallback skips sections hidden by a collapsed heading")+    func fallbackSkipsHiddenSections() async throws {+        var blocks: [MarkdownBlock] = [.heading(level: 1, text: "Document")]+        for index in 0..<20 {+            blocks.append(.paragraph(markdown: "Paragraph \(index) with running text for height."))+        }+        let harness = try await Self.makeStyledHarness(blocks: blocks)+        let ids = domIDs(blocks)++        try await harness.send(.setSectionState(collapsedIDs: ["\(blocks[0].id)-0"]))+        #expect(try await hiddenSectionCount(harness) > 0, "the collapse did not hide any section")++        let seen = harness.messages(type: "visibleBlock").count+        _ = try await harness.page.callJavaScript(+            "window.dispatchEvent(new Event('scroll')); return null;",+            contentWorld: harness.bridgeWorld+        )++        let reported = try #require(+            try await waitForVisibleBlock(harness, after: seen), "no visibleBlock was reported"+        )+        #expect(+            try await visibility(harness, of: reported) == "visible",+            "a hidden section was persisted as the reading position: \(reported)"+        )+        #expect(reported == ids[0])+    }++    // The negative case for the zero-rect heuristic: nothing is collapsed, and the+    // document is built from the sparsest blocks that still render (a thematic+    // break, a one-character paragraph). None of them may be mistaken for+    // display:none, and the minimal section at the viewport top must still be the+    // one reported. Sections are in-flow block boxes, so a visible one always has+    // a non-zero width even when its content is nearly empty — this pins that+    // assumption down rather than leaving it implicit.+    @Test("the zero-rect filter never skips a genuinely visible in-flow section")+    func minimalVisibleSectionIsNotSkipped() async throws {+        var blocks: [MarkdownBlock] = [.heading(level: 1, text: "Document")]+        for index in 0..<20 {+            blocks.append(.paragraph(markdown: "Filler paragraph \(index) with running text for height."))+        }+        // The sparsest renderable blocks, placed where a scroll can put them at the+        // viewport top.+        blocks.append(.thematicBreak)+        blocks.append(.paragraph(markdown: "x"))+        for index in 0..<20 {+            blocks.append(.paragraph(markdown: "Trailing paragraph \(index) with running text for height."))+        }+        let harness = try await Self.makeStyledHarness(blocks: blocks)+        let ids = domIDs(blocks)++        // Nothing is collapsed, so no section may be filtered out as non-rendered.+        #expect(try await hiddenSectionCount(harness) == 0, "no section should be hidden here")+        let skipped = try await zeroRectSectionIDs(harness)+        #expect(skipped.isEmpty, "the zero-rect filter would skip visible sections: \(skipped)")++        // The minimal one-character paragraph, scrolled to the viewport top, must+        // still win the topmost-block contest.+        let targetID = ids[22]+        let seen = harness.messages(type: "visibleBlock").count+        _ = try await harness.page.callJavaScript(+            "var el = document.getElementById(id); el.scrollIntoView({ block: 'start' }); return null;",+            arguments: ["id": targetID],+            contentWorld: harness.bridgeWorld+        )++        let reported = try #require(+            try await waitForVisibleBlock(harness, after: seen), "no visibleBlock was reported"+        )+        #expect(reported == targetID, "a minimal visible section was skipped; reported \(reported)")+    }++    // The collapse-free instance of the same defect, and the only case that+    // exercises the fallback change (`sections[0]` → the first *rendered*+    // section). `BlockHTMLEmitter` emits a `.metadata` (YAML frontmatter) block as+    // an inert `<section … data-prism-metadata hidden>` carrier so the block keeps+    // a stable DOM node. Nothing in document.css overrides `[hidden]`, so the UA+    // rule makes it display:none — an all-zero rect on `sections[0]`, carrying no+    // `data-prism-section-hidden`. Before the fix its top of 0 won the contest at+    // EVERY scroll offset, so any document with frontmatter always reported the+    // frontmatter carrier as the reading position and the restore silently+    // no-opped. Only the zero-rect half of the filter catches this one.+    @Test("visibleBlock never reports the hidden frontmatter carrier")+    func frontmatterCarrierIsNotReportedAsVisibleBlock() async throws {+        var blocks: [MarkdownBlock] = [+            .metadata(content: "title: Document"),+            .heading(level: 1, text: "Document"),+        ]+        for index in 0..<60 {+            blocks.append(.paragraph(markdown: "Paragraph \(index) with running text for height."))+        }+        let harness = try await Self.makeStyledHarness(blocks: blocks)+        let ids = domIDs(blocks)++        // No collapse anywhere — the carrier is hidden by the UA `[hidden]` rule,+        // not by `data-prism-section-hidden`.+        #expect(try await hiddenSectionCount(harness) == 0, "no section should be marked collapsed here")+        #expect(+            try await zeroRectSectionIDs(harness) == [ids[0]],+            "the frontmatter carrier should be the only zero-rect section"+        )++        // At the top of the document the fallback must pick the first RENDERED+        // section (the H1), not the hidden carrier.+        let atTop = try #require(+            try await waitForVisibleBlock(harness, after: 0), "no visibleBlock was reported at the top"+        )+        #expect(atTop == ids[1], "the hidden frontmatter carrier was reported at scroll 0: \(atTop)")++        // And after scrolling, the genuinely visible section must win the scan.+        let targetID = ids[20]+        let seen = harness.messages(type: "visibleBlock").count+        _ = try await harness.page.callJavaScript(+            "var el = document.getElementById(id); el.scrollIntoView({ block: 'start' }); return null;",+            arguments: ["id": targetID],+            contentWorld: harness.bridgeWorld+        )++        let reported = try #require(+            try await waitForVisibleBlock(harness, after: seen), "no visibleBlock was reported after scrolling"+        )+        #expect(reported == targetID, "the hidden frontmatter carrier beat a visible section: \(reported)")+    }+}

diff --git a/docs/agent-notes/scroll-persistence.md b/docs/agent-notes/scroll-persistence.mdindex f8dcba0..90a2e8e 100644--- a/docs/agent-notes/scroll-persistence.md+++ b/docs/agent-notes/scroll-persistence.md@@ -35,6 +35,22 @@ The document body renders in a WebView, so the scroll contract has two halves:   `layoutSettled`. The stored id goes through   `BlockDOMID.restoreDOMID(forStored:blocks:)`, which passes DOM-format ids   through and migrates legacy pre-cutover composite ids (`{hash}-{sourceIndex}`).+- **Non-laid-out sections must be filtered out of the scan (T-1851):**+  `topmostBlockID()` picks the section with the greatest `rect.top <= 1`. A+  `display:none` element's rect is **all zeros**, so its `top` of 0 beats every+  visible section (which sits at a *negative* top once scrolled past). Two kinds+  of section are `display:none`: content under a collapsed heading+  (`data-prism-section-hidden`, set by `applySectionVisibility` in+  `prism-theme.js`, hidden by `document.css`) and the inert YAML-frontmatter+  carrier (`BlockHTMLEmitter` emits it with the bare `hidden` attribute, so the+  UA rule applies and it is `sections[0]`). Before the filter, any collapsed+  heading — or merely having frontmatter — pinned the reported id to an+  unreachable block, and the restore silently no-opped (`scrollIntoView` does+  nothing on `display:none`). **The same zero-rect trap is still unguarded in+  `sectionInWindow` in `prism-search.js`** — an all-zero rect passes both bounds,+  so collapsed sections count as in-window; benign today, but the same reasoning+  error. Any new geometry check over `section[data-prism-block-id]` needs the+  same filter. - **Clobber guards in `prism-scroll.js`:** reports are suppressed for 600ms   around a programmatic scroll; `beginProgrammaticScroll` disarms any pending   debounced report AND the debounce callback re-checks the flag (a report

diff --git a/CHANGELOG.md b/CHANGELOG.mdindex c52d7e6..c78d7e9 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 +- The saved reading position is no longer corrupted by content the document does not show (T-1851). Hidden content measures as sitting exactly at the top of the window, which beat every genuinely visible block, so the document reported a block the reader could not see as the block being read. That happened with any heading collapsed, and — because the carrier holding a document's YAML frontmatter is hidden the same way — on every document that starts with frontmatter, collapsed or not. Reopening the document, returning from raw source, or recovering from a rendering-process restart then landed on the wrong block or did nothing at all. Content the document does not lay out is now skipped when working out the reading position. - 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`).