Skip to content

feat: find in page — match count, highlight-all, next/prev#16

Merged
jnahian merged 8 commits into
mainfrom
feat/find-in-page
Jul 9, 2026
Merged

feat: find in page — match count, highlight-all, next/prev#16
jnahian merged 8 commits into
mainfrom
feat/find-in-page

Conversation

@jnahian

@jnahian jnahian commented Jul 9, 2026

Copy link
Copy Markdown
Owner

Implements the find-in-page spec. Covers original request item 1.

Why native find had to go

WKFindConfiguration/WKFindResult expose only matchFound: Bool — no count — and native find highlights just the current match. Any solution keeping it would compute the count in JS anyway, leaving two sources of truth for "which match is current". So find moves into bridge.js entirely.

The crux: find needs its own text base

contentEl.textContent is not what the reader sees. After render(), it also contains:

Injected by Text
addHeadingAnchors() #, first child of every h1–h4
addCodeCopyButtons() Copy, inside every <pre>
renderMath() the LaTeX source, in KaTeX's hidden <annotation> MathML
renderMermaid() <text> nodes inside an inline <svg>

Searching that would inflate the count with invisible text, and wrapping a match inside the Mermaid subtree would insert an HTML <mark> into an <svg>. So find gets its own filtered walker (FIND_EXCLUDE) and a dedicated wrapFindMatch().

The marks feature is deliberately left alone. It is tempting to call resolveAnchor equally buggy and unify the two walkers. It isn't: a mark's quote/offsets are captured on mouseup, after the same injections have run, so the pollution is symmetric across write and read and cancels out. Marks and find have different correct text bases. Refactoring the anchoring would risk re-anchoring every saved highlight for zero gain. git diff confirms resolveAnchor, offsetsFromRange, rangeFromOffsets, and wrapRange are byte-unchanged; bridge.js is purely additive (138 insertions, 0 deletions).

Two subtleties worth reviewing

  • Occurrences are wrapped back-to-front. surroundContents splits the text node and leaves the original as the prefix, so wrapping an early match first invalidates the offsets of a later one sharing that node.
  • find() vs refind(). Re-application after a DOM rebuild (FSEvents re-render, marks re-wrap, PDF export) must keep the current match and not scroll — render() preserves scrollY, and a naive re-find() would fight it, yanking the viewport to match 1 mid-read. refind() preserves focus, clamps the index, and doesn't scroll. Conversely render() resets findFocus when keepScroll is false, so opening a new file restarts at match 1 rather than inheriting the old index.

Also: clearFind() runs before createPDF (otherwise highlights bake into the exported PDF), and ⌘F/⌘⇧F swap — Find in Page is now ⌘F, Filter Files is ⌘⇧F. That's a muscle-memory change; noted in SHORTCUTS.md and CHANGELOG.md.

Verification status

Verified mechanically: swift build passes; swift test green; WKFindConfiguration gone from the tree; marks plumbing untouched; both re-apply sites call refind(), and the only find( call is the user-initiated entry point.

Not verified — requires a human at the GUI. All 13 spec verification items are visual. The two that guard the trickiest bugs:

  • With find active and the 3rd match current, edit the file on disk → highlights re-apply, the 3rd match stays current, and the viewport does not move
  • With the 3rd match current, ⌘E → the PDF contains no find highlights, and afterwards you're still on match 3
  • Search a term appearing in a Mermaid node label → diagram intact, term not counted
  • Search a term inside a LaTeX expression → not counted, equation intact
  • Search Copy with a code block present → the copy-button label is not counted
  • Search # with headings present → heading anchors not counted
  • A term overlapping an existing user highlight → both render, mark popover still opens
  • 1 of 5 / wrap-around / No results / disabled chevrons
  • With find active on match 5, open a different file → restarts at match 1

jnahian added 6 commits July 9, 2026 17:31
Replace native WKWebView.find (no count, current-match-only highlight) with
a bridge.js find engine over its own filtered text walker (excludes heading
anchors, copy buttons, Mermaid SVG, KaTeX). Reports count/index via a new
findResult message into AppState.findCount/findIndex. clearFind is sequenced
around PDF export so highlights don't bake in.
Show "N of M" / "No results" between the field and the chevrons; disable
next/prev when there are zero matches instead of on empty query.
magnifyingglass button before the reload divider, disabled when no file is
open, opening the find bar.
Swap the two find shortcuts so in-page find takes the conventional Cmd-F.
Update the shortcut cheatsheet and changelog.
findFocus is module-level and survived a document switch, so opening a new
file with find active left refind() clamped to the previous file's match
index -- and because refind() deliberately does not scroll, the current
match landed off-screen. render() already distinguishes the two cases:
loadMarkdown passes keepScroll=false, reloadMarkdown and setTheme pass true.
The new topbar find button is gated on selectedFile, but the menu command and
its Cmd-F shortcut were not, so Cmd-F with no document open raised an empty
find bar over an empty content pane.
@jnahian

jnahian commented Jul 9, 2026

Copy link
Copy Markdown
Owner Author

Review pass — no correctness blocker

Every high-risk claim in this PR was checked against the DOM spec and survived. That's worth stating plainly, because the two sibling PRs each turned up real defects.

Reverse-order wrapping is correct. The reviewer traced surroundContentsextractinsert: for a range inside one text node, extract does replaceData(start, len, ""), then insert does splitText(start) — so the original node reference keeps the prefix. Because occurrences are wrapped descending by document position, any node shared with an earlier match is only ever truncated after that match's region, leaving its [from, to) offsets valid. All four adversarial cases were constructed and traced: multiple matches in one node, a match spanning A+B with a later match starting in B, adjacent zero-gap matches, and a match covering an entire node.

One precision note, not a defect: the comment's phrase "leaves the original node as the prefix" is imprecise when from == 0, where the original reference becomes an empty node rather than a prefix. The invariant still holds — a from == 0 match has no earlier content in that node to revisit, and the seg.start >= mEnd overlap check skips the emptied node.

refind() / findFocus lifecycle is safe. Every DOM rebuild routes through the "rendered" handler, which always calls applyMarks — even for a document with zero marks — so find re-applies either way. Closing the bar clears findQuery, and both the Swift (!lastFindQuery.isEmpty) and JS (if (findQuery)) guards prevent a stale refind(). No path leaves findFocus indexing dead matches.

FIND_EXCLUDE handles the null-parent case. A text node that is a direct child of contentEl returns closest(...) === null and is correctly accepted; the n.parentElement && guard covers it.

The createPDF race is genuinely fixed. createPDF re-lays-out the DOM to PDF rather than snapshotting a composited bitmap, so no separate layout flush is needed beyond the evaluateJavaScript completion ordering.

normalize() cannot corrupt nested find marks. It merges only adjacent text nodes and won't cross the surviving rmd-find element boundary.

Fixed in 2f0beaa

The topbar find button was gated on selectedFile != nil, but the menu command and its ⌘F shortcut were not — so ⌘F with no document open raised an empty find bar over an empty pane. Now consistent.

Known, accepted

wrapFindMatch is O(segments × occurrences). Bounded, fine for real documents; a single-character query in a very large file is the pathological case. Not worth optimizing until it bites.

The GUI checklist in the description remains unticked and still needs a human — in particular the two items guarding the subtle bugs: viewport must not move on an FSEvents re-render at match 3, and ⌘E must not bake highlights into the PDF.

jnahian added 2 commits July 9, 2026 18:17
Integration defect between footnotes and find. The footnote extension emits a
visually-hidden <h2 class="sr-only">Footnotes</h2>; it is invisible to the
reader but still a text node, so searching "footnotes" matched it, inflating the
count and wrapping a <mark> the user cannot see. Neither PR could catch this
alone -- the two features only meet after both land.
@jnahian jnahian merged commit 1f81978 into main Jul 9, 2026
@jnahian jnahian deleted the feat/find-in-page branch July 9, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant