fix(desktop): keep find-in-page highlights consistent across React re-renders (#81726)

The scoped find walker wraps transcript text nodes in <mark> elements that React does not own. Assistant responses stream through markdown-text.tsx, which rebuilds the markdown DOM on every delta, and a new message is appended whenever the assistant answers — so a re-render of a changed region detaches the marks we inserted, dropping the user's highlights while the bar stays open. Watch the captured scope with a MutationObserver and re-wrap only when an unmarked occurrence of the active query actually reappears. The observer is gated behind a re-entrancy flag while the walker is mutating, coalesced to one re-apply per microtask, torn down when the bar closes or the query clears, and restores the active ordinal so a mid-stream re-render doesn't reset the user's place to match #1. An append that adds no matching text is a no-op; re-wrapping only fires when highlights genuinely went stale. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-12 11:22:25 +08:00
parent 4e86caa1e2
commit c856c5f6db
2 changed files with 274 additions and 2 deletions
@@ -319,4 +319,110 @@ describe('scope lifecycle', () => {
    const result = performScopedFind(foreground, 'needle', { forward: true, findNext: false })
    expect(result.count).toBe(1)
  })
+})
+
+describe('scoped find survives React re-render', () => {
+  // The transcript is React-owned: assistant answers stream, and a new message
+  // is appended whenever the assistant replies. React doesn't know about the
+  // `<mark>`s we insert, so a render that re-allocates a region detaches its
+  // marks. The scope watcher (MutationObserver) must re-wrap after the render
+  // commits. jsdom delivers observer callbacks + our queueMicrotask re-apply
+  // as microtasks, so awaiting a macrotask drains them all.
+  async function flushAll(): Promise<void> {
+    await new Promise(resolve => setTimeout(resolve, 0))
+  }
+
+  it('re-wraps content React re-allocated, keeping the active position', async () => {
+    const surface = plantSurface('surface', '<p>needle hay needle</p>')
+    captureFindScope()
+    performScopedFind(surface, 'needle', { forward: true, findNext: false })
+    expect(surface.querySelectorAll('mark.find-hit').length).toBe(2)
+
+    // Step onto match #2, then mock a React re-render that rebuilds the
+    // paragraph (destroying our marks) while the matching text remains.
+    performScopedFind(surface, 'needle', { forward: true, findNext: true })
+    expect(surface.querySelectorAll('mark.find-hit[data-find-active]')[0]).toBe(
+      surface.querySelectorAll('mark.find-hit')[1]
+    )
+
+    surface.querySelector('p')!.innerHTML = 'needle hay needle'
+    await flushAll()
+
+    // Marks restored, and the active mark is still the second match.
+    expect(surface.querySelectorAll('mark.find-hit').length).toBe(2)
+    expect(surface.querySelectorAll('mark.find-hit[data-find-active]').length).toBe(1)
+    expect(surface.querySelectorAll('mark.find-hit[data-find-active]')[0]).toBe(
+      surface.querySelectorAll('mark.find-hit')[1]
+    )
+    expect(surface.querySelector('p')!.textContent).toBe('needle hay needle')
+  })
+
+  it('keeps existing highlights and wraps a newly appended message', async () => {
+    // The task's target scenario: the bar is open with highlights, then the
+    // assistant appends a NEW message while the list stays mounted. Existing
+    // highlights must persist and the appended content must not be broken.
+    const surface = plantSurface('surface', '<p>needle in the first message</p>')
+    captureFindScope()
+    performScopedFind(surface, 'needle', { forward: true, findNext: false })
+    expect(surface.querySelectorAll('mark.find-hit').length).toBe(1)
+
+    // React appends a new message block.
+    const next = document.createElement('div')
+    next.innerHTML = 'second message needle'
+    surface.appendChild(next)
+    await flushAll()
+
+    // The first highlight survived the append AND the new match was wrapped
+    // too — nothing clobbered, nothing lost.
+    const marks = surface.querySelectorAll('mark.find-hit')
+    expect(marks.length).toBe(2)
+    expect(surface.querySelector('p')!.textContent).toBe('needle in the first message')
+    expect(next.textContent).toBe('second message needle')
+  })
+
+  it('does NOT re-wrap when React touched a region with no unmarked match', async () => {
+    const surface = plantSurface('surface', '<p>needle needle</p>')
+    captureFindScope()
+    performScopedFind(surface, 'needle', { forward: true, findNext: false })
+    expect(surface.querySelectorAll('mark.find-hit').length).toBe(2)
+
+    const before = [...surface.querySelectorAll('mark.find-hit')]
+    // React re-renders a paragraph that doesn't match the query — the marks
+    // are untouched and must not be rebuilt.
+    surface.insertAdjacentHTML('beforeend', '<p>irrelevant</p>')
+    await flushAll()
+
+    const after = [...surface.querySelectorAll('mark.find-hit')]
+    expect(after.length).toBe(before.length)
+    expect(after.every(mark => before.includes(mark))).toBe(true)
+  })
+
+  it('searches a typeahead that clears and re-runs still tears the watcher down', async () => {
+    const surface = plantSurface('surface', '<p>needle</p>')
+    captureFindScope()
+    performScopedFind(surface, 'needle', { forward: true, findNext: false })
+    expect(surface.querySelectorAll('mark.find-hit').length).toBe(1)
+
+    // Clearing the query tears down the watcher; a later React write that
+    // would normally re-wrap must NOT resurrect highlights (the bar closed).
+    performScopedFind(surface, '', { forward: true, findNext: false })
+    surface.querySelector('p')!.innerHTML = 'needle again'
+    await flushAll()
+
+    expect(surface.querySelectorAll('mark.find-hit').length).toBe(0)
+    expect(surface.querySelector('p')!.textContent).toBe('needle again')
+  })
+
+  it('releaseFindScope detaches the watcher so no write can re-highlight', async () => {
+    const surface = plantSurface('surface', '<p>needle</p>')
+    captureFindScope()
+    performScopedFind(surface, 'needle', { forward: true, findNext: false })
+    expect(surface.querySelectorAll('mark.find-hit').length).toBe(1)
+
+    releaseFindScope()
+    surface.querySelector('p')!.innerHTML = 'needle needle'
+    await flushAll()
+
+    expect(surface.querySelectorAll('mark.find-hit').length).toBe(0)
+  })
 })
@@ -30,6 +30,19 @@
 *   `<mark>` carries `data-active`. We re-derive it on every step instead of
 *   keeping a counter, so a stale count after a DOM mutation self-heals on
 *   the next search.
+ * - The transcript is React-owned and re-renders while the bar is open —
+ *   assistant responses stream through `markdown-text.tsx`, which rebuilds
+ *   the markdown DOM on every delta, and a new message is appended whenever
+ *   the assistant answers. React does not know about the `<mark>`s we insert,
+ *   so a re-render of a region whose text or structure changed detaches our
+ *   marks (React re-allocates those nodes) while leaving marks in untouched
+ *   regions intact. To keep highlights consistent across those renders we
+ *   watch the captured scope with a MutationObserver and re-apply the wrap
+ *   whenever an unmarked occurrence of the query reappears — but ONLY then,
+ *   so a responsive render that left every mark in place costs nothing and an
+ *   append that adds no matching text is a no-op. The observer is gated off
+ *   while WE are mutating (a re-entrancy flag), coalesced to one re-apply per
+ *   microtask, and torn down when the bar closes (#81726).
 * - We do NOT call `webContents.findInPage` from here. That bridge still
 *   exists for multi-window secondary sessions (each window searches its
 *   own webContents), but for the primary window it cannot be scoped, so
@@ -43,6 +56,25 @@ const HIGHLIGHT_CLASS = 'find-hit'
 const ACTIVE_ATTR = 'data-find-active'
 const ROOT_ATTR = 'data-find-root'

+// ── Re-render watch ─────────────────────────────────────────────────────────
+// The transcript is React-owned and re-renders under us (streaming markdown,
+// message append). React doesn't know about our `<mark>`s, so a render that
+// re-allocates a region detaches the marks in that region. We watch the scope
+// and re-wrap when an unmarked occurrence of the active query appears.
+//
+// `applying` is the re-entrancy guard: it's set for the whole synchronous
+// re-apply, and any observer callback that fires during it flips `pending`
+// so we take one more look afterwards instead of recursing infinitely. All
+// observer work is coalesced to a single microtask so a burst of mutations
+// (one streaming delta) lands as one re-apply.
+let observer: MutationObserver | null = null
+let scopeRoot: HTMLElement | null = null
+let activeQuery = ''
+let lastActiveOrdinal = 0
+let applying = false
+let pending = false
+let scheduled = false
+
 /**
 * The element the open FindBar should search. Captured at bar-open time and
 * reused for every query / step until the bar closes. Reading the visible
@@ -63,9 +95,25 @@ export function captureFindScope(): HTMLElement | null {
    root.setAttribute(ROOT_ATTR, '')
  }

+  resetScopeState()
+  scopeRoot = root
+
  return root
 }

+/** Forget a past scope: stop watching it and drop any queued re-apply. Called
+ *  whenever the bar re-opens or closes so state never leaks across searches. */
+function resetScopeState(): void {
+  observer?.disconnect()
+  observer = null
+  scopeRoot = null
+  activeQuery = ''
+  lastActiveOrdinal = 0
+  applying = false
+  pending = false
+  scheduled = false
+}
+
 /** The scope captured by the open bar, or null if none is active. */
 export function currentFindScope(): HTMLElement | null {
  const roots = document.querySelectorAll<HTMLElement>(`[${ROOT_ATTR}]`)
@@ -348,6 +396,10 @@ export function performScopedFind(
  if (!query) {
    clearHighlights(root)
    setActiveMark(null)
+    // No query, no marks to maintain — stop watching and forget the query.
+    activeQuery = ''
+    lastActiveOrdinal = 0
+    stopObserver()

    return DEFAULT_RESULT
  }
@@ -375,12 +427,24 @@ export function performScopedFind(
  let marks = existingMarks

  if (!sameQuery) {
-    clearHighlights(root)
-    marks = highlightMatches(root, query)
+    // Mutate under the re-entrancy guard so the re-render watcher doesn't
+    // fire on the marks WE are replacing (it has nothing to repair yet).
+    applying = true
+
+    try {
+      clearHighlights(root)
+      marks = highlightMatches(root, query)
+    } finally {
+      applying = false
+    }
  }

  if (marks.length === 0) {
    setActiveMark(null)
+    // A query that matches nothing has nothing to maintain.
+    activeQuery = ''
+    lastActiveOrdinal = 0
+    stopObserver()

    return DEFAULT_RESULT
  }
@@ -404,9 +468,110 @@ export function performScopedFind(

  setActiveMark(marks[nextIndex] ?? null)

+  // A live search must survive React re-rendering the transcript under us.
+  // Remember the query + active position and start watching the scope; the
+  // observer re-wraps when a render detaches our marks and re-activates the
+  // same ordinal so the user's place doesn't reset to match #1 on the next
+  // streamed token.
+  activeQuery = query
+  lastActiveOrdinal = nextIndex + 1
+  ensureObserver(root)
+
  return { count: marks.length, activeOrdinal: nextIndex + 1 }
 }

+// ── Re-apply on React re-render ─────────────────────────────────────────────
+/**
+ * Attach the scope watcher, if it isn't already attached. Only observes the
+ * current scope; a fresh `captureFindScope` (or close) detaches it.
+ */
+function ensureObserver(root: Element): void {
+  if (observer && scopeRoot === root) {
+    return
+  }
+
+  stopObserver()
+  observer = new MutationObserver(() => scheduleReapply())
+  observer.observe(root, { childList: true, subtree: true })
+}
+
+function stopObserver(): void {
+  observer?.disconnect()
+  observer = null
+}
+
+/**
+ * Coalesce a mutation burst (one streaming delta) into a single re-apply that
+ * runs on the next microtask. A no-op when the marks still cover the query —
+ * React reusing an untouched region doesn't need any work.
+ */
+function scheduleReapply(): void {
+  if (applying) {
+    // A mutation landed while we were mutating the tree. We'll look again
+    // once this apply finishes rather than recursing now.
+    pending = true
+
+    return
+  }
+
+  if (scheduled || !scopeRoot || !activeQuery) {
+    return
+  }
+
+  scheduled = true
+
+  queueMicrotask(() => {
+    scheduled = false
+
+    if (applying || !scopeRoot || !activeQuery) {
+      return
+    }
+
+    // Nothing to repair: every occurrence is still wrapped (React only
+    // touched a region that doesn't match the query).
+    if (!hasUnmarkedMatch(scopeRoot, activeQuery)) {
+      return
+    }
+
+    reapplying(scopeRoot)
+  })
+}
+
+/**
+ * Re-wrap every occurrence in the scope after React detached our previous
+ * marks, re-activating the position the user was on. Runs with `applying` set
+ * so the observer ignores the mutations WE make, then drains any `pending`
+ * mutation that arrived during the apply.
+ */
+function reapplying(root: HTMLElement): void {
+  const restoreOrdinal = lastActiveOrdinal
+
+  applying = true
+
+  try {
+    clearHighlights(root)
+    const marks = highlightMatches(root, activeQuery)
+
+    if (marks.length === 0) {
+      return
+    }
+
+    // Re-activate the same ordinal the user last stood on, clamped to the
+    // re-wrapped set, so a mid-stream re-render doesn't bounce their place
+    // back to match #1.
+    const nextIndex = Math.min(restoreOrdinal - 1, marks.length - 1)
+    setActiveMark(marks[nextIndex] ?? null)
+    lastActiveOrdinal = nextIndex + 1
+  } finally {
+    applying = false
+  }
+
+  if (pending) {
+    pending = false
+    scheduleReapply()
+  }
+}
+
 /** Tear down highlights and the scope marker — called when the bar closes. */
 export function releaseFindScope(): void {
  const roots = document.querySelectorAll<HTMLElement>(`[${ROOT_ATTR}]`)
@@ -417,4 +582,5 @@ export function releaseFindScope(): void {
  }

  setActiveMark(null)
+  resetScopeState()
 }