diff --git a/src/renderer/src/components/browser-pane/assemble-chrome/BrowserFind.test.tsx b/src/renderer/src/components/browser-pane/assemble-chrome/BrowserFind.test.tsx index 0f188046377..3c1dd78d356 100644 --- a/src/renderer/src/components/browser-pane/assemble-chrome/BrowserFind.test.tsx +++ b/src/renderer/src/components/browser-pane/assemble-chrome/BrowserFind.test.tsx @@ -10,12 +10,26 @@ function createWebviewRef(): { stopFindInPage: ReturnType addEventListener: ReturnType removeEventListener: ReturnType + 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 { @@ -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( + + ) + 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() + + 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( + + ) + const second = createWebviewRef() + first.ref.current = second.ref.current + rerender() + 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( + + ) + fireEvent.change(screen.getByPlaceholderText('Find in page...'), { + target: { value: 'needle' } + }) + act(() => vi.advanceTimersByTime(250)) + act(() => first.found(1, 1, 2)) + rerender( + + ) + const second = createWebviewRef() + first.ref.current = second.ref.current + rerender( + + ) + act(() => vi.advanceTimersByTime(250)) + expect(second.findInPage).not.toHaveBeenCalled() + rerender() + expect(second.findInPage).toHaveBeenCalledExactlyOnceWith('needle', { findNext: true }) + expect(screen.queryByText('1 of 2')).toBeNull() + }) +}) diff --git a/src/renderer/src/components/browser-pane/assemble-chrome/BrowserFind.tsx b/src/renderer/src/components/browser-pane/assemble-chrome/BrowserFind.tsx index 1800d8d041b..dab11e02fff 100644 --- a/src/renderer/src/components/browser-pane/assemble-chrome/BrowserFind.tsx +++ b/src/renderer/src/components/browser-pane/assemble-chrome/BrowserFind.tsx @@ -21,11 +21,24 @@ export default function BrowserFind({ }: BrowserFindProps): React.JSX.Element | null { const inputRef = useRef(null) const wasOpenRef = useRef(isOpen) - const activeFindQueryRef = useRef(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) ? ( {totalMatches > 0 ? translate(