fix(markdown-preview): paint Find matches without mutating react DOM (crash 237acef1) (#8678)

Co-authored-by: Orca <help@stably.ai>

src/renderer/src/assets/main.css

@@ -204,6 +204,8 @@
   --git-graph-lane-4: #40b0a6;
   --git-graph-lane-5: #b66dff;
   --annotation-highlight: #f59e0b;
+  --markdown-search-match: #facc15;
+  --markdown-search-match-active: #fb923c;
   /* Why: tab-group splits use a dedicated divider — `--border` is too faint on
      card/editor surfaces when panes are side-by-side or stacked. Default must
      meet >=3:1 against `--card`, not only the hover/drag strong state. */
@@ -292,6 +294,8 @@
   --git-graph-lane-4: #40b0a6;
   --git-graph-lane-5: #b66dff;
   --annotation-highlight: #fbbf24;
+  --markdown-search-match: #facc15;
+  --markdown-search-match-active: #fb923c;
   /* Brighter than terminal defaults so the always-visible line clears 3:1 on `--card`. */
   --tab-group-split-divider: #71717a;
   --tab-group-split-divider-strong: #a1a1aa;

src/renderer/src/assets/markdown-preview.css

@@ -237,15 +237,17 @@
   }
 }
 
-.markdown-preview-search-match {
-  padding: 0;
-  border-radius: 2px;
-  background: color-mix(in srgb, #facc15 50%, transparent);
+/* Why: search hits are painted with the CSS Custom Highlight API (Ranges, no
+   DOM mutation) instead of injected <mark> elements — see markdown-preview-search.ts.
+   ::highlight() supports color/background only, not border-radius/padding. */
+::highlight(markdown-preview-search-match) {
+  background-color: color-mix(in srgb, var(--markdown-search-match) 50%, transparent);
   color: inherit;
 }
 
-.markdown-preview-search-match[data-active] {
-  background: color-mix(in srgb, #fb923c 60%, transparent);
+::highlight(markdown-preview-search-active-match) {
+  background-color: color-mix(in srgb, var(--markdown-search-match-active) 60%, transparent);
+  color: inherit;
 }
 
 .markdown-review-toolbar {

src/renderer/src/components/editor/MarkdownPreview.tsx

@@ -484,12 +484,18 @@ export default function MarkdownPreview({
     input.focus()
     input.select()
   }, [])
-  const matchesRef = useRef<HTMLElement[]>([])
+  const matchesRef = useRef<Range[]>([])
+  // Stable token identifying this preview in the document-global highlight
+  // registry, so split/floating previews don't clobber each other's Find paint.
+  const searchInstanceRef = useRef<object>({})
   const lastAppliedInitialAnchorRef = useRef<string | null>(null)
   const pendingEditorRevealFrameIdsRef = useRef<number[]>([])
   const [isSearchOpen, setIsSearchOpen] = useState(false)
   const [query, setQuery] = useState('')
   const [matchCount, setMatchCount] = useState(0)
+  // Bumps whenever the match ranges are recomputed, so the active-highlight
+  // effect re-runs even when a streamed rerender yields the same count/index.
+  const [searchRevision, setSearchRevision] = useState(0)
   const [activeMatchIndex, setActiveMatchIndex] = useState(-1)
   const isMac = navigator.userAgent.includes('Mac')
   const openFile = useAppStore((s) => s.openFile)
@@ -835,29 +841,36 @@ export default function MarkdownPreview({
       return
     }
 
+    const instanceId = searchInstanceRef.current
+
     if (!isSearchOpen) {
       matchesRef.current = []
       setMatchCount(0)
-      clearMarkdownPreviewSearchHighlights(body)
+      clearMarkdownPreviewSearchHighlights(instanceId)
       return
     }
 
-    // Search decorations are applied imperatively because the rendered preview is
-    // already owned by react-markdown. Rewriting the markdown AST for transient
-    // find state would make navigation and link rendering much harder to reason about.
-    const matches = applyMarkdownPreviewSearchHighlights(body, query)
+    // Search decorations are painted via the CSS Custom Highlight API (Ranges,
+    // no DOM mutation) because the rendered preview is owned by react-markdown;
+    // splitting its nodes to inject <mark> corrupted react's tree (crash 237acef1).
+    const matches = applyMarkdownPreviewSearchHighlights(instanceId, body, query)
     matchesRef.current = matches
     setMatchCount(matches.length)
+    setSearchRevision((v) => v + 1)
     setActiveMatchIndex((cur) =>
       matches.length === 0 ? -1 : cur >= 0 && cur < matches.length ? cur : 0
     )
 
-    return () => clearMarkdownPreviewSearchHighlights(body)
+    return () => clearMarkdownPreviewSearchHighlights(instanceId)
   }, [renderedContent, isSearchOpen, query])
 
   useEffect(() => {
-    setActiveMarkdownPreviewSearchMatch(matchesRef.current, activeMatchIndex)
-  }, [activeMatchIndex, matchCount])
+    setActiveMarkdownPreviewSearchMatch(
+      searchInstanceRef.current,
+      matchesRef.current,
+      activeMatchIndex
+    )
+  }, [activeMatchIndex, matchCount, searchRevision])
 
   useLayoutEffect(() => {
     if (!initialAnchor || initialAnchor === lastAppliedInitialAnchorRef.current) {
@@ -1919,7 +1932,10 @@ export default function MarkdownPreview({
             ) : null}
           </div>
         ) : null}
-        <div ref={bodyRef} className="markdown-body">
+        {/* Why: translate="no" keeps browser/OS page-translation from swapping
+            text nodes react owns, which otherwise triggers the same
+            insertBefore/removeChild reconciliation crash (237acef1). */}
+        <div ref={bodyRef} className="markdown-body" translate="no">
           {/* Why: remarkFrontmatter strips front matter from normal markdown
         output. When the user opts in from the preview actions menu, render the
         raw metadata as a compact read-only block above the document body. */}

src/renderer/src/components/editor/markdown-preview-search-crash.test.tsx

@@ -0,0 +1,193 @@
+// @vitest-environment happy-dom
+//
+// Regression guards for crash 237acef1: search highlighting must not mutate the
+// DOM react-markdown owns. Injecting <mark> by splitting react's text nodes (and
+// normalize()-merging them on clear) left react with stale child pointers, so
+// the next streamed-content commit threw
+//   NotFoundError: Failed to execute 'insertBefore' on 'Node': The node before
+//   which the new node is to be inserted is not a child of this node.
+import { act } from 'react'
+import { createRoot, type Root } from 'react-dom/client'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+import {
+  applyMarkdownPreviewSearchHighlights,
+  clearMarkdownPreviewSearchHighlights,
+  setActiveMarkdownPreviewSearchMatch
+} from './markdown-preview-search'
+
+const SEARCH_HIGHLIGHT_NAME = 'markdown-preview-search-match'
+const ACTIVE_SEARCH_HIGHLIGHT_NAME = 'markdown-preview-search-active-match'
+
+function MarkdownBody({ parts }: { parts: readonly string[] }): React.JSX.Element {
+  // Mirror react-markdown: a <p> whose children are text nodes react owns.
+  return (
+    <div className="markdown-body">
+      <p>{parts.map((part) => part)}</p>
+    </div>
+  )
+}
+
+describe('markdown preview search highlighting keeps react-owned DOM intact (crash 237acef1)', () => {
+  let container: HTMLDivElement
+  let root: Root
+  const instance = {}
+
+  beforeEach(() => {
+    container = document.createElement('div')
+    document.body.append(container)
+    root = createRoot(container)
+  })
+
+  afterEach(() => {
+    act(() => root.unmount())
+    container.remove()
+    clearMarkdownPreviewSearchHighlights(instance)
+  })
+
+  function render(parts: readonly string[]): HTMLElement {
+    act(() => root.render(<MarkdownBody parts={parts} />))
+    const body = container.querySelector<HTMLElement>('.markdown-body')
+    if (!body) {
+      throw new Error('missing markdown body')
+    }
+    return body
+  }
+
+  it('applies highlights without mutating the react-owned subtree', () => {
+    const body = render(['hello world'])
+    const matches = applyMarkdownPreviewSearchHighlights(instance, body, 'lo')
+    expect(matches.length).toBe(1)
+    // No <mark> is injected; the text react owns is byte-for-byte intact.
+    expect(body.querySelector('mark')).toBeNull()
+    expect(body.textContent).toBe('hello world')
+  })
+
+  it('survives a streamed re-render while highlights are active', () => {
+    const body = render(['alpha ', 'beta'])
+    applyMarkdownPreviewSearchHighlights(instance, body, 'beta')
+    // Highlights stay registered while streamed content rewrites the paragraph.
+    // When they mutated react's text nodes this commit threw NotFoundError; with
+    // no DOM mutation it is clean. (afterEach clears the instance.)
+    expect(() => render(['alpha ', 'gamma', ' delta'])).not.toThrow()
+  })
+})
+
+// The paint path only runs where the CSS Custom Highlight API exists (prod
+// Electron), which jsdom/happy-dom lack — so stub a faithful, Electron-shaped
+// registry + Highlight to exercise it. The stub Highlight throws if handed any
+// constructor argument, catching any accidental `new Highlight(...ranges)`
+// spread (which overflows V8's arg stack on large docs).
+class StubHighlight {
+  readonly ranges = new Set<Range>()
+  constructor(...args: Range[]) {
+    if (args.length > 0) {
+      throw new RangeError('Maximum call stack size exceeded')
+    }
+  }
+  add(range: Range): void {
+    this.ranges.add(range)
+  }
+}
+
+class StubHighlightRegistry {
+  readonly entries = new Map<string, StubHighlight>()
+  set(name: string, highlight: StubHighlight): void {
+    this.entries.set(name, highlight)
+  }
+  delete(name: string): void {
+    this.entries.delete(name)
+  }
+}
+
+describe('markdown preview search painting with the CSS Custom Highlight API', () => {
+  let registry: StubHighlightRegistry
+
+  beforeEach(() => {
+    registry = new StubHighlightRegistry()
+    vi.stubGlobal('Highlight', StubHighlight)
+    vi.stubGlobal('CSS', { highlights: registry })
+  })
+
+  afterEach(() => {
+    vi.unstubAllGlobals()
+  })
+
+  function bodyWithText(text: string): HTMLElement {
+    const div = document.createElement('div')
+    div.className = 'markdown-body'
+    div.textContent = text
+    document.body.append(div)
+    return div
+  }
+
+  it('paints many matches without a constructor-arg spread', () => {
+    const instance = {}
+    const body = bodyWithText('a a a a a a a a')
+    // A spread paint (new Highlight(...ranges)) would throw via the stub.
+    expect(() => applyMarkdownPreviewSearchHighlights(instance, body, 'a')).not.toThrow()
+    expect(registry.entries.get(SEARCH_HIGHLIGHT_NAME)?.ranges.size).toBe(8)
+    clearMarkdownPreviewSearchHighlights(instance)
+    body.remove()
+  })
+
+  it('unions highlights across previews so a second Find does not clobber the first', () => {
+    const a = {}
+    const b = {}
+    const bodyA = bodyWithText('alpha alpha')
+    const bodyB = bodyWithText('alpha')
+
+    const matchesA = applyMarkdownPreviewSearchHighlights(a, bodyA, 'alpha')
+    const matchesB = applyMarkdownPreviewSearchHighlights(b, bodyB, 'alpha')
+    expect(matchesA.length).toBe(2)
+    expect(matchesB.length).toBe(1)
+    // The registry paints the union (A's 2 + B's 1), not just the last writer's.
+    expect(registry.entries.get(SEARCH_HIGHLIGHT_NAME)?.ranges.size).toBe(3)
+
+    // Closing B keeps A's highlights.
+    clearMarkdownPreviewSearchHighlights(b)
+    expect(registry.entries.get(SEARCH_HIGHLIGHT_NAME)?.ranges.size).toBe(2)
+
+    clearMarkdownPreviewSearchHighlights(a)
+    expect(registry.entries.has(SEARCH_HIGHLIGHT_NAME)).toBe(false)
+    setActiveMarkdownPreviewSearchMatch(a, matchesA, -1)
+    bodyA.remove()
+    bodyB.remove()
+  })
+
+  it('navigation repaints only the active highlight, not the full match set', () => {
+    const instance = {}
+    const body = bodyWithText('a a a a')
+    const matches = applyMarkdownPreviewSearchHighlights(instance, body, 'a')
+    const paintedAfterApply = registry.entries.get(SEARCH_HIGHLIGHT_NAME)
+    setActiveMarkdownPreviewSearchMatch(instance, matches, 0)
+    setActiveMarkdownPreviewSearchMatch(instance, matches, 1)
+    // The SEARCH highlight object is untouched by navigation (not rebuilt each Next/Prev)...
+    expect(registry.entries.get(SEARCH_HIGHLIGHT_NAME)).toBe(paintedAfterApply)
+    // ...while the ACTIVE highlight tracks the current match.
+    expect(registry.entries.get(ACTIVE_SEARCH_HIGHLIGHT_NAME)?.ranges.has(matches[1])).toBe(true)
+    clearMarkdownPreviewSearchHighlights(instance)
+    body.remove()
+  })
+
+  it('re-apply drops the active highlight until the caller repaints it (same-count rerender)', () => {
+    const instance = {}
+    const body = bodyWithText('one two one two')
+    const first = applyMarkdownPreviewSearchHighlights(instance, body, 'one')
+    expect(first.length).toBe(2)
+    setActiveMarkdownPreviewSearchMatch(instance, first, 0)
+    expect(registry.entries.has(ACTIVE_SEARCH_HIGHLIGHT_NAME)).toBe(true)
+
+    // A streamed rerender / new query re-applies and clears the active range even
+    // when the match count is unchanged (regression for the vanishing active mark).
+    const second = applyMarkdownPreviewSearchHighlights(instance, body, 'two')
+    expect(second.length).toBe(first.length)
+    expect(registry.entries.has(ACTIVE_SEARCH_HIGHLIGHT_NAME)).toBe(false)
+
+    // MarkdownPreview repaints via its searchRevision effect; the module restores it.
+    setActiveMarkdownPreviewSearchMatch(instance, second, 0)
+    expect(registry.entries.get(ACTIVE_SEARCH_HIGHLIGHT_NAME)?.ranges.has(second[0])).toBe(true)
+    clearMarkdownPreviewSearchHighlights(instance)
+    body.remove()
+  })
+})

src/renderer/src/components/editor/markdown-preview-search.ts

@@ -166,92 +166,170 @@ function buildLocaleLowercaseIndex(text: string): {
   return { text: normalized, originalStartByNormalizedOffset, originalEndByNormalizedOffset }
 }
 
-export function clearMarkdownPreviewSearchHighlights(root: HTMLElement): void {
-  const highlights = root.querySelectorAll<HTMLElement>('[data-markdown-preview-search-match]')
-  for (const highlight of highlights) {
-    const textNode = document.createTextNode(highlight.textContent ?? '')
-    highlight.replaceWith(textNode)
+// Why: react-markdown owns the preview DOM. Injecting <mark> by splitting its
+// text nodes (and normalize()-merging them on clear) left react holding stale
+// child pointers, so the next streamed-content commit threw NotFoundError
+// ("insertBefore ... not a child of this node"; crash 237acef1). Paint matches
+// with the CSS Custom Highlight API instead — it highlights Ranges without
+// mutating the DOM react manages. The static names below must match the
+// ::highlight() selectors in markdown-preview.css.
+const SEARCH_HIGHLIGHT_NAME = 'markdown-preview-search-match'
+const ACTIVE_SEARCH_HIGHLIGHT_NAME = 'markdown-preview-search-active-match'
+
+type HighlightLike = { add(range: Range): void }
+type HighlightRegistryLike = {
+  set(name: string, highlight: HighlightLike): void
+  delete(name: string): void
+}
+
+// Accessed via globalThis so the code degrades to a no-op where the API is
+// absent (older Chromium, jsdom/happy-dom in tests) — match counting and
+// navigation still work off the returned Ranges; only the paint is skipped.
+function getHighlightApi(): {
+  registry: HighlightRegistryLike
+  create: (ranges: readonly Range[]) => HighlightLike
+} | null {
+  const scope = globalThis as {
+    CSS?: { highlights?: HighlightRegistryLike }
+    Highlight?: new () => HighlightLike
+  }
+  const registry = scope.CSS?.highlights
+  const HighlightCtor = scope.Highlight
+  if (!registry || typeof HighlightCtor !== 'function') {
+    return null
+  }
+  return {
+    registry,
+    // Why: build with .add() rather than new Highlight(...ranges). A big doc +
+    // short query yields 100k+ ranges, and spreading that many constructor
+    // args overflows V8's argument stack (RangeError) — the same large-content
+    // regime as the bug this file fixes.
+    create: (ranges) => {
+      const highlight = new HighlightCtor()
+      for (const range of ranges) {
+        highlight.add(range)
+      }
+      return highlight
+    }
+  }
+}
+
+// Why: CSS.highlights is a document-global registry keyed by a static name, but
+// several MarkdownPreview instances can be open at once (split panes, floating
+// window). Track each instance's ranges by its own token and paint the UNION,
+// so a second preview's Find does not clobber the first's highlights. Ranges
+// live in each instance's own subtree, so the union paints every pane correctly.
+const searchRangesByInstance = new Map<object, readonly Range[]>()
+const activeRangeByInstance = new Map<object, Range>()
+
+// Avoid array spread when collecting union ranges — a large doc can produce
+// 100k+ ranges and create()/registry writes must not build variadic arg lists.
+function paintMatchHighlight(api: NonNullable<ReturnType<typeof getHighlightApi>>): void {
+  const matchRanges: Range[] = []
+  for (const ranges of searchRangesByInstance.values()) {
+    for (const range of ranges) {
+      matchRanges.push(range)
+    }
+  }
+  if (matchRanges.length > 0) {
+    api.registry.set(SEARCH_HIGHLIGHT_NAME, api.create(matchRanges))
+  } else {
+    api.registry.delete(SEARCH_HIGHLIGHT_NAME)
+  }
+}
+
+function paintActiveHighlight(api: NonNullable<ReturnType<typeof getHighlightApi>>): void {
+  const activeRanges: Range[] = []
+  for (const range of activeRangeByInstance.values()) {
+    activeRanges.push(range)
+  }
+  if (activeRanges.length > 0) {
+    api.registry.set(ACTIVE_SEARCH_HIGHLIGHT_NAME, api.create(activeRanges))
+  } else {
+    api.registry.delete(ACTIVE_SEARCH_HIGHLIGHT_NAME)
+  }
+}
+
+export function clearMarkdownPreviewSearchHighlights(instanceId: object): void {
+  searchRangesByInstance.delete(instanceId)
+  activeRangeByInstance.delete(instanceId)
+  const api = getHighlightApi()
+  if (api) {
+    paintMatchHighlight(api)
+    paintActiveHighlight(api)
   }
-  root.normalize()
 }
 
 export function applyMarkdownPreviewSearchHighlights(
+  instanceId: object,
   root: HTMLElement,
   query: string
-): HTMLElement[] {
-  clearMarkdownPreviewSearchHighlights(root)
+): Range[] {
+  const ranges: Range[] = []
 
-  if (!query || isMarkdownPreviewSearchQueryTooLarge(query)) {
-    return []
+  if (query && !isMarkdownPreviewSearchQueryTooLarge(query)) {
+    const walker = document.createTreeWalker(root, NodeFilter.SHOW_TEXT, {
+      acceptNode(node) {
+        if (!(node.parentElement instanceof HTMLElement)) {
+          return NodeFilter.FILTER_REJECT
+        }
+        if (!node.textContent?.trim()) {
+          return NodeFilter.FILTER_REJECT
+        }
+        return NodeFilter.FILTER_ACCEPT
+      }
+    })
+
+    let currentNode = walker.nextNode()
+    while (currentNode) {
+      if (currentNode instanceof Text) {
+        const text = currentNode.textContent ?? ''
+        // findTextMatchRanges returns offsets into the original text, so they
+        // map straight onto this Text node without any DOM rewrite.
+        for (const { start, end } of findTextMatchRanges(text, query)) {
+          const range = document.createRange()
+          range.setStart(currentNode, start)
+          range.setEnd(currentNode, end)
+          ranges.push(range)
+        }
+      }
+      currentNode = walker.nextNode()
+    }
   }
 
-  const walker = document.createTreeWalker(root, NodeFilter.SHOW_TEXT, {
-    acceptNode(node) {
-      if (!(node.parentElement instanceof HTMLElement)) {
-        return NodeFilter.FILTER_REJECT
-      }
-      if (node.parentElement.closest('[data-markdown-preview-search-match]')) {
-        return NodeFilter.FILTER_REJECT
-      }
-      if (!node.textContent?.trim()) {
-        return NodeFilter.FILTER_REJECT
-      }
-      return NodeFilter.FILTER_ACCEPT
-    }
-  })
-
-  const textNodes: Text[] = []
-  let currentNode = walker.nextNode()
-  while (currentNode) {
-    if (currentNode instanceof Text) {
-      textNodes.push(currentNode)
-    }
-    currentNode = walker.nextNode()
+  searchRangesByInstance.set(instanceId, ranges)
+  activeRangeByInstance.delete(instanceId)
+  const api = getHighlightApi()
+  if (api) {
+    paintMatchHighlight(api)
+    paintActiveHighlight(api)
   }
 
-  const matches: HTMLElement[] = []
-  for (const textNode of textNodes) {
-    const text = textNode.textContent ?? ''
-    const ranges = findTextMatchRanges(text, query)
-    if (ranges.length === 0) {
-      continue
-    }
-
-    const fragment = document.createDocumentFragment()
-    let cursor = 0
-    for (const range of ranges) {
-      if (range.start > cursor) {
-        fragment.append(document.createTextNode(text.slice(cursor, range.start)))
-      }
-
-      const highlight = document.createElement('mark')
-      highlight.dataset.markdownPreviewSearchMatch = 'true'
-      highlight.className = 'markdown-preview-search-match'
-      highlight.textContent = text.slice(range.start, range.end)
-      fragment.append(highlight)
-      matches.push(highlight)
-      cursor = range.end
-    }
-
-    if (cursor < text.length) {
-      fragment.append(document.createTextNode(text.slice(cursor)))
-    }
-
-    textNode.replaceWith(fragment)
-  }
-
-  return matches
+  return ranges
 }
 
 export function setActiveMarkdownPreviewSearchMatch(
-  matches: readonly HTMLElement[],
+  instanceId: object,
+  matches: readonly Range[],
   activeIndex: number
 ): void {
-  for (const [index, match] of matches.entries()) {
-    const isActive = index === activeIndex
-    match.toggleAttribute('data-active', isActive)
-    if (isActive) {
-      match.scrollIntoView({ block: 'center', inline: 'nearest' })
-    }
+  const active = activeIndex >= 0 ? matches[activeIndex] : undefined
+
+  if (active) {
+    activeRangeByInstance.set(instanceId, active)
+  } else {
+    activeRangeByInstance.delete(instanceId)
+  }
+
+  const api = getHighlightApi()
+  if (api) {
+    // Only the active range changed — don't rebuild the (potentially 100k-range)
+    // match highlight on every Next/Prev navigation.
+    paintActiveHighlight(api)
+  }
+
+  if (active) {
+    // The Range's start container is a Text node; scroll its element into view.
+    active.startContainer.parentElement?.scrollIntoView({ block: 'center', inline: 'nearest' })
   }
 }