fix(browser): restart Find when its native guest changes (#34738)

* fix(browser): restart Find when its native guest changes

* fix(browser): keep Find counter blank while the current search is pending

---------

Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com>
This commit is contained in:
OrcaWin
2026-10-28 01:21:25 -07:00
committed by GitHub
co-authored by OrcaWin
parent 96cd21bbc4
commit 43ab2dba08
2 changed files with 168 additions and 26 deletions
@@ -10,12 +10,26 @@ function createWebviewRef(): {
stopFindInPage: ReturnType<typeof vi.fn>
addEventListener: ReturnType<typeof vi.fn>
removeEventListener: ReturnType<typeof vi.fn>
found: (requestId: number, activeMatchOrdinal: number, matches: number) => void
} {
const findInPage = vi.fn()
let requestSequence = 0
const findInPage = vi.fn(() => ++requestSequence)
const stopFindInPage = vi.fn()
// Track listeners independently of ref.current to verify cleanup after a guest swap.
const addEventListener = vi.fn()
const removeEventListener = vi.fn()
const events = new EventTarget()
const addEventListener = vi.fn((type: string, listener: EventListener) =>
events.addEventListener(type, listener)
)
const removeEventListener = vi.fn((type: string, listener: EventListener) =>
events.removeEventListener(type, listener)
)
const found = (requestId: number, activeMatchOrdinal: number, matches: number): void => {
events.dispatchEvent(
Object.assign(new Event('found-in-page'), {
result: { requestId, activeMatchOrdinal, matches, finalUpdate: true }
})
)
}
const ref = {
current: {
findInPage,
@@ -24,7 +38,7 @@ function createWebviewRef(): {
removeEventListener
} as unknown as Electron.WebviewTag
}
return { ref, findInPage, stopFindInPage, addEventListener, removeEventListener }
return { ref, findInPage, stopFindInPage, addEventListener, removeEventListener, found }
}
function openFindWithQuery(query: string): ReturnType<typeof createWebviewRef> {
@@ -137,3 +151,99 @@ describe('BrowserFind listener rebinding', () => {
expect(first.removeEventListener).toHaveBeenCalledWith('found-in-page', staleListener)
})
})
describe('BrowserFind result ownership', () => {
it('searches a replacement guest and discards the previous guest counts', () => {
vi.useFakeTimers()
const first = createWebviewRef()
const { rerender } = render(
<BrowserFind isOpen onClose={vi.fn()} webviewRef={first.ref} guestGeneration={1} />
)
fireEvent.change(screen.getByPlaceholderText('Find in page...'), {
target: { value: 'needle' }
})
act(() => vi.advanceTimersByTime(250))
act(() => first.found(1, 2, 7))
expect(screen.getByText('2 of 7')).not.toBeNull()
const second = createWebviewRef()
first.ref.current = second.ref.current
rerender(<BrowserFind isOpen onClose={vi.fn()} webviewRef={first.ref} guestGeneration={2} />)
expect(screen.getByPlaceholderText('Find in page...').getAttribute('value')).toBe('needle')
expect(screen.queryByText('2 of 7')).toBeNull()
expect(screen.queryByText('No matches')).toBeNull()
act(() => vi.advanceTimersByTime(250))
expect(second.findInPage).toHaveBeenCalledExactlyOnceWith('needle', { findNext: true })
act(() => first.found(1, 6, 7))
expect(screen.queryByText('6 of 7')).toBeNull()
act(() => second.found(1, 1, 3))
expect(screen.getByText('1 of 3')).not.toBeNull()
fireEvent.click(screen.getByTitle('Next match'))
expect(second.findInPage).toHaveBeenLastCalledWith('needle', { forward: true, findNext: false })
})
it('ignores replies from an older request in the same guest', () => {
const webview = openFindWithQuery('needle')
act(() => webview.found(1, 1, 2))
expect(screen.getByText('1 of 2')).not.toBeNull()
fireEvent.change(screen.getByPlaceholderText('Find in page...'), {
target: { value: 'replacement' }
})
expect(screen.queryByText('1 of 2')).toBeNull()
expect(screen.queryByText('No matches')).toBeNull()
act(() => vi.advanceTimersByTime(250))
act(() => webview.found(2, 1, 4))
expect(screen.getByText('1 of 4')).not.toBeNull()
act(() => webview.found(1, 2, 2))
expect(screen.getByText('1 of 4')).not.toBeNull()
})
it('ignores a delayed reply after advancing the current query', () => {
const webview = openFindWithQuery('needle')
fireEvent.click(screen.getByTitle('Next match'))
act(() => webview.found(2, 2, 7))
expect(screen.getByText('2 of 7')).not.toBeNull()
act(() => webview.found(1, 1, 7))
expect(screen.getByText('2 of 7')).not.toBeNull()
})
it('does not search an empty query after replacing the guest', () => {
vi.useFakeTimers()
const first = createWebviewRef()
const { rerender } = render(
<BrowserFind isOpen onClose={vi.fn()} webviewRef={first.ref} guestGeneration={1} />
)
const second = createWebviewRef()
first.ref.current = second.ref.current
rerender(<BrowserFind isOpen onClose={vi.fn()} webviewRef={first.ref} guestGeneration={2} />)
act(() => vi.advanceTimersByTime(250))
expect(second.findInPage).not.toHaveBeenCalled()
})
it('waits to search a replacement guest until a closed Find bar reopens', () => {
vi.useFakeTimers()
const first = createWebviewRef()
const { rerender } = render(
<BrowserFind isOpen onClose={vi.fn()} webviewRef={first.ref} guestGeneration={1} />
)
fireEvent.change(screen.getByPlaceholderText('Find in page...'), {
target: { value: 'needle' }
})
act(() => vi.advanceTimersByTime(250))
act(() => first.found(1, 1, 2))
rerender(
<BrowserFind isOpen={false} onClose={vi.fn()} webviewRef={first.ref} guestGeneration={1} />
)
const second = createWebviewRef()
first.ref.current = second.ref.current
rerender(
<BrowserFind isOpen={false} onClose={vi.fn()} webviewRef={first.ref} guestGeneration={2} />
)
act(() => vi.advanceTimersByTime(250))
expect(second.findInPage).not.toHaveBeenCalled()
rerender(<BrowserFind isOpen onClose={vi.fn()} webviewRef={first.ref} guestGeneration={2} />)
expect(second.findInPage).toHaveBeenCalledExactlyOnceWith('needle', { findNext: true })
expect(screen.queryByText('1 of 2')).toBeNull()
})
})
@@ -21,11 +21,24 @@ export default function BrowserFind({
}: BrowserFindProps): React.JSX.Element | null {
const inputRef = useRef<HTMLInputElement>(null)
const wasOpenRef = useRef(isOpen)
const activeFindQueryRef = useRef<string | null>(null)
const activeFindRef = useRef<{
query: string
guestGeneration: number | null
webview: Electron.WebviewTag
requestId: number
} | null>(null)
const [query, setQuery] = useState('')
const [activeMatch, setActiveMatch] = useState(0)
const [totalMatches, setTotalMatches] = useState(0)
const [result, setResult] = useState<{
query: string
guestGeneration: number | null
activeMatch: number
totalMatches: number
} | null>(null)
const requestQuery = getFindRequestQuery(query)
const currentResult =
result?.query === requestQuery && result.guestGeneration === guestGeneration ? result : null
const activeMatch = currentResult?.activeMatch ?? 0
const totalMatches = currentResult?.totalMatches ?? 0
const safeFindInPage = useCallback(
(text: string, opts?: Electron.FindInPageOptions): void => {
@@ -34,12 +47,13 @@ export default function BrowserFind({
return
}
try {
webview.findInPage(text, opts)
const requestId = webview.findInPage(text, opts)
activeFindRef.current = { query: text, guestGeneration, webview, requestId }
} catch {
// Ignore guest teardown races during navigation or tab close.
}
},
[webviewRef]
[webviewRef, guestGeneration]
)
const safeStopFindInPage = useCallback((): void => {
@@ -55,21 +69,27 @@ export default function BrowserFind({
}, [webviewRef])
// Electron findNext:true starts a new session; advancing the current selection requires false.
const isCurrentFind = useCallback(
(): boolean =>
activeFindRef.current?.query === requestQuery &&
activeFindRef.current.guestGeneration === guestGeneration &&
activeFindRef.current.webview === webviewRef.current,
[requestQuery, guestGeneration, webviewRef]
)
const findNext = useCallback(() => {
if (requestQuery) {
const findNext = activeFindQueryRef.current !== requestQuery
const findNext = !isCurrentFind()
safeFindInPage(requestQuery, { forward: true, findNext })
activeFindQueryRef.current = requestQuery
}
}, [requestQuery, safeFindInPage])
}, [requestQuery, safeFindInPage, isCurrentFind])
const findPrevious = useCallback(() => {
if (requestQuery) {
const findNext = activeFindQueryRef.current !== requestQuery
const findNext = !isCurrentFind()
safeFindInPage(requestQuery, { forward: false, findNext })
activeFindQueryRef.current = requestQuery
}
}, [requestQuery, safeFindInPage])
}, [requestQuery, safeFindInPage, isCurrentFind])
useEffect(() => {
if (isOpen) {
@@ -84,22 +104,21 @@ export default function BrowserFind({
const wasOpen = wasOpenRef.current
wasOpenRef.current = isOpen
if (!isOpen) {
activeFindQueryRef.current = null
activeFindRef.current = null
return
}
if (!requestQuery) {
activeFindQueryRef.current = null
activeFindRef.current = null
safeStopFindInPage()
return
}
// A reopen or a changed query starts a fresh session, so findNext must be true here.
const runFind = (): void => {
if (activeFindQueryRef.current === requestQuery) {
if (isCurrentFind()) {
return
}
safeFindInPage(requestQuery, { findNext: true })
activeFindQueryRef.current = requestQuery
}
if (!wasOpen) {
runFind()
@@ -108,7 +127,7 @@ export default function BrowserFind({
// Debounce typing to avoid highlight flashes; reopening and Enter still use the live query.
const id = window.setTimeout(runFind, 200)
return () => window.clearTimeout(id)
}, [isOpen, requestQuery, safeFindInPage, safeStopFindInPage])
}, [isOpen, requestQuery, safeFindInPage, safeStopFindInPage, isCurrentFind])
// Rebind on generation changes: client-hosted restart swaps the guest without remounting or closing Find.
useEffect(() => {
@@ -117,9 +136,22 @@ export default function BrowserFind({
return
}
const handleFoundInPage = (event: Electron.FoundInPageEvent): void => {
const activeFind = activeFindRef.current
if (
!activeFind ||
activeFind.webview !== webview ||
activeFind.guestGeneration !== guestGeneration ||
activeFind.requestId !== event.result.requestId
) {
return
}
const { activeMatchOrdinal, matches } = event.result
setActiveMatch(activeMatchOrdinal)
setTotalMatches(matches)
setResult({
query: activeFind.query,
guestGeneration,
activeMatch: activeMatchOrdinal,
totalMatches: matches
})
}
webview.addEventListener('found-in-page', handleFoundInPage)
return () => {
@@ -131,9 +163,8 @@ export default function BrowserFind({
}
}, [webviewRef, isOpen, guestGeneration])
if ((!isOpen || !requestQuery) && (activeMatch !== 0 || totalMatches !== 0)) {
setActiveMatch(0)
setTotalMatches(0)
if ((!isOpen || !requestQuery) && result !== null) {
setResult(null)
}
const handleKeyDown = useCallback(
@@ -173,7 +204,8 @@ export default function BrowserFind({
className="min-w-0 flex-1 border-none bg-transparent text-sm text-white outline-none placeholder:text-zinc-500"
/>
{query ? (
{/* Why: a pending request has no count yet; blank avoids flashing "No matches". */}
{query && (currentResult !== null || !requestQuery) ? (
<span className="shrink-0 text-xs text-zinc-400">
{totalMatches > 0
? translate(