From 6383c807cbdf9324011ea73a36587ccb64dc7381 Mon Sep 17 00:00:00 2001 From: Jony <619963502@qq.com> Date: Wed, 29 Jul 2026 00:22:48 +0800 Subject: [PATCH] fix(desktop): keep find bar out of its results --- .../find-in-page-native-fixture/package.json | 6 + .../electron/find-in-page-native.test.mjs | 104 ++++++++++++++++++ apps/desktop/electron/find-in-page.test.ts | 48 +++++++- apps/desktop/electron/find-in-page.ts | 40 +++++++ apps/desktop/electron/main.ts | 11 +- apps/desktop/package.json | 1 + apps/desktop/src/components/find-bar.test.tsx | 43 +++++++- apps/desktop/src/components/find-bar.tsx | 46 +++++++- apps/desktop/src/store/find-in-page.ts | 4 +- 9 files changed, 285 insertions(+), 18 deletions(-) create mode 100644 apps/desktop/electron/find-in-page-native-fixture/package.json create mode 100644 apps/desktop/electron/find-in-page-native.test.mjs diff --git a/apps/desktop/electron/find-in-page-native-fixture/package.json b/apps/desktop/electron/find-in-page-native-fixture/package.json new file mode 100644 index 0000000000000..733769cd92e14 --- /dev/null +++ b/apps/desktop/electron/find-in-page-native-fixture/package.json @@ -0,0 +1,6 @@ +{ + "name": "find-in-page-native-fixture", + "private": true, + "type": "module", + "main": "../find-in-page-native.test.mjs" +} diff --git a/apps/desktop/electron/find-in-page-native.test.mjs b/apps/desktop/electron/find-in-page-native.test.mjs new file mode 100644 index 0000000000000..25d5f9c6df489 --- /dev/null +++ b/apps/desktop/electron/find-in-page-native.test.mjs @@ -0,0 +1,104 @@ +import assert from 'node:assert/strict' +import { mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { app, BrowserWindow } from 'electron' + +const runtimeDir = mkdtempSync(join(tmpdir(), 'hermes-find-in-page-')) +app.setPath('userData', runtimeDir) +app.setPath('sessionData', runtimeDir) + +async function findCount(window, query, afterFirstResult) { + return await new Promise((resolve, reject) => { + const timeout = setTimeout(() => reject(new Error(`findInPage timed out for ${query}`)), 5000) + let requestId + let firstResult = true + + const onResult = (_event, result) => { + if (result.requestId !== requestId) return + if (firstResult) { + firstResult = false + afterFirstResult?.() + } + if (!result.finalUpdate) return + clearTimeout(timeout) + window.webContents.off('found-in-page', onResult) + resolve(result.matches) + } + + window.webContents.on('found-in-page', onResult) + requestId = window.webContents.findInPage(query) + }) +} + +async function run() { + const window = new BrowserWindow({ + show: true, + x: 0, + y: 0, + width: 320, + height: 240, + skipTaskbar: true, + opacity: 0, + webPreferences: { backgroundThrottling: false } + }) + + try { + const html = ['', '

needle

'].join('') + const fixturePath = join(runtimeDir, 'fixture.html') + writeFileSync(fixturePath, html) + await window.loadFile(fixturePath) + await window.webContents.executeJavaScript('document.body.innerText') + + assert.equal(await findCount(window, 'needle'), 2, 'control: search input is indexed') + window.webContents.stopFindInPage('clearSelection') + + await window.webContents.executeJavaScript('query.focus(); query.setSelectionRange(6, 6); query.inert = true') + const count = await findCount(window, 'needle', () => { + void window.webContents.executeJavaScript('query.inert = false; query.focus(); query.setSelectionRange(6, 6)') + }) + + assert.equal(count, 1, 'transient inert excludes the visible query from Chromium indexing') + + const state = await window.webContents.executeJavaScript(`JSON.stringify({ + type: query.type, + explicitRole: query.getAttribute('role'), + inert: query.inert, + focused: document.activeElement === query, + selectionStart: query.selectionStart, + value: query.value + })`) + assert.deepEqual(JSON.parse(state), { + type: 'search', + explicitRole: null, + inert: false, + focused: true, + selectionStart: 6, + value: 'needle' + }) + + window.webContents.debugger.attach('1.3') + try { + await window.webContents.debugger.sendCommand('Accessibility.enable') + const { nodes } = await window.webContents.debugger.sendCommand('Accessibility.getFullAXTree') + assert.ok( + nodes.some(node => node.role?.value === 'searchbox' && node.name?.value === 'Find in page'), + 'Chromium accessibility tree exposes a truthful searchbox' + ) + } finally { + window.webContents.debugger.detach() + } + } finally { + window.destroy() + } +} + +app + .whenReady() + .then(run) + .then(() => app.exit(0)) + .catch(error => { + console.error(error) + app.exit(1) + }) diff --git a/apps/desktop/electron/find-in-page.test.ts b/apps/desktop/electron/find-in-page.test.ts index 8e7352e96470f..c238847cdba55 100644 --- a/apps/desktop/electron/find-in-page.test.ts +++ b/apps/desktop/electron/find-in-page.test.ts @@ -10,7 +10,14 @@ import { EventEmitter } from 'node:events' import type { BrowserWindow } from 'electron' import { describe, test } from 'vitest' -import { formatFoundInPage, installFindShortcut, installFoundInPageForwarder, performFind, stopFind } from './find-in-page' +import { + formatFoundInPage, + installFindShortcut, + installFoundInPageForwarder, + performFind, + performFindAfterIndexingStarted, + stopFind +} from './find-in-page' // Minimal webContents stub. The Electron.WebContents type is huge, so we // model just the slice the helpers touch (`isDestroyed`, `findInPage`, @@ -24,10 +31,11 @@ interface FakeWebContents { } isDestroyed: () => boolean destroy: () => void - findInPage: (query: string, options: { forward: boolean; findNext: boolean }) => void + findInPage: (query: string, options: { forward: boolean; findNext: boolean }) => number stopFindInPage: (action: 'clearSelection' | 'keepSelection' | 'activateSelection') => void send: (channel: string, payload: unknown) => void on: typeof EventEmitter.prototype.on + once: typeof EventEmitter.prototype.once off: typeof EventEmitter.prototype.off emit: (event: string | symbol, ...args: unknown[]) => boolean } @@ -52,6 +60,8 @@ function makeFakeWebContents(): FakeWebContents { }, findInPage(query: string, options: { forward: boolean; findNext: boolean }) { calls.find.push({ query, options }) + + return 17 }, stopFindInPage(action: 'clearSelection' | 'keepSelection' | 'activateSelection') { calls.stop.push(action) @@ -60,6 +70,7 @@ function makeFakeWebContents(): FakeWebContents { calls.send.push({ channel, payload }) }, on: emitter.on.bind(emitter), + once: emitter.once.bind(emitter), off: emitter.off.bind(emitter), emit: emitter.emit.bind(emitter) } @@ -137,6 +148,39 @@ describe('performFind', () => { }) }) +describe('performFindAfterIndexingStarted', () => { + test('resolves only after the matching request emits its first result', async () => { + const wc = makeFakeWebContents() + let resolved = false + + const pending = performFindAfterIndexingStarted(asWC(wc), 'needle', { + forward: true, + findNext: false + }).then(() => { + resolved = true + }) + + await Promise.resolve() + assert.equal(resolved, false) + + wc.emit('found-in-page', {}, { requestId: 9, matches: 1 }) + await Promise.resolve() + assert.equal(resolved, false) + + wc.emit('found-in-page', {}, { requestId: 17, matches: 1 }) + await pending + assert.equal(resolved, true) + }) + + test('resolves safely if the webContents is destroyed before a result', async () => { + const wc = makeFakeWebContents() + const pending = performFindAfterIndexingStarted(asWC(wc), 'needle', null) + + wc.destroy() + await pending + }) +}) + describe('stopFind', () => { test('calls stopFindInPage with the default action (clearSelection)', () => { const wc = makeFakeWebContents() diff --git a/apps/desktop/electron/find-in-page.ts b/apps/desktop/electron/find-in-page.ts index fc30a34a01b79..57f3f901bf9e5 100644 --- a/apps/desktop/electron/find-in-page.ts +++ b/apps/desktop/electron/find-in-page.ts @@ -71,6 +71,46 @@ export function performFind( }) } +/** + * Start a find request and resolve after Chromium emits its first matching + * result. This acknowledgment lets the renderer remove a temporary `inert` + * boundary only after the query field has been excluded from the index. + */ +export function performFindAfterIndexingStarted( + webContents: Electron.WebContents | null | undefined, + query: string, + options: FindInPageOptions | null | undefined +): Promise { + if (!webContents || webContents.isDestroyed()) { + return Promise.resolve() + } + + return new Promise(resolve => { + let requestId: number | undefined + + const finish = () => { + webContents.off('found-in-page', onFound) + webContents.off('destroyed', finish) + resolve() + } + + const onFound = (_event: Electron.Event, result: { requestId?: number }) => { + if (requestId !== undefined && result?.requestId === requestId) { + finish() + } + } + + webContents.on('found-in-page', onFound) + webContents.once('destroyed', finish) + + const opts = options && typeof options === 'object' ? options : {} + requestId = webContents.findInPage(String(query ?? ''), { + forward: opts.forward !== false, + findNext: Boolean(opts.findNext) + }) + }) +} + /** * Stop the current find and clear highlights. The default `action` matches * what the renderer sends on Escape / close. diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 46e29d95c9ba8..2aeb23c6cc07d 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -117,7 +117,7 @@ import { tuiResumeArgs } from './external-terminal' import { findGitBash as _findGitBash } from './find-git-bash' -import { installFindShortcut, installFoundInPageForwarder, performFind, stopFind } from './find-in-page' +import { installFindShortcut, installFoundInPageForwarder, performFindAfterIndexingStarted, stopFind } from './find-in-page' import { createFirstRunSetupGate } from './first-run-setup-gate' import { readDirForIpc } from './fs-read-dir' import { @@ -12908,7 +12908,7 @@ function ensureFoundInPageForwarder(sender: Electron.WebContents): void { }) } -ipcMain.handle('hermes:find-in-page', (event, query, options) => { +ipcMain.handle('hermes:find-in-page', async (event, query, options) => { const win = BrowserWindow.fromWebContents(event.sender) if (!win || win.isDestroyed()) { @@ -12916,11 +12916,10 @@ ipcMain.handle('hermes:find-in-page', (event, query, options) => { } ensureFoundInPageForwarder(event.sender) - performFind(win.webContents, query, options) + await performFindAfterIndexingStarted(win.webContents, query, options) - // The match count arrives asynchronously via `found-in-page`; the - // synchronous return value is intentionally `{ count: 0 }` to mirror - // Electron's own `findInPage` return semantics (an opaque request id). + // The match count still arrives asynchronously via `found-in-page`; this + // reply only acknowledges that Chromium has begun returning this request. return { count: 0 } }) diff --git a/apps/desktop/package.json b/apps/desktop/package.json index caed49330ad47..072a173595d88 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -59,6 +59,7 @@ "fix": "npm run lint:fix && npm run fmt", "test:ui": "vitest run --project ui", "test:desktop:platforms": "vitest run --project electron", + "test:find-in-page-native": "electron electron/find-in-page-native-fixture", "test": "vitest run", "preview": "node scripts/assert-root-install.mjs && vite preview --host 127.0.0.1 --port 4174", "check:test:desktop:platforms": "npm run test:desktop:platforms", diff --git a/apps/desktop/src/components/find-bar.test.tsx b/apps/desktop/src/components/find-bar.test.tsx index fcc107eb66d71..ad607b087b84d 100644 --- a/apps/desktop/src/components/find-bar.test.tsx +++ b/apps/desktop/src/components/find-bar.test.tsx @@ -406,8 +406,11 @@ describe('FindBar', () => { openFindBar() renderFindBar() - const input = await screen.findByRole('textbox', { name: /find in page/i }) + const input = await screen.findByRole('searchbox', { name: /find in page/i }) expect(input).toBeTruthy() + expect(input.getAttribute('type')).toBe('search') + expect(input.getAttribute('role')).toBeNull() + expect(screen.getByRole('search').className).toContain('top-[calc(var(--titlebar-height,34px)+0.5rem)]') expect(screen.getByRole('button', { name: /close/i })).toBeTruthy() expect(screen.getByRole('button', { name: /next match/i })).toBeTruthy() expect(screen.getByRole('button', { name: /previous match/i })).toBeTruthy() @@ -422,7 +425,7 @@ describe('FindBar', () => { openFindBar() renderFindBar() - const input = await screen.findByRole('textbox', { name: /find in page/i }) + const input = await screen.findByRole('searchbox', { name: /find in page/i }) // eslint-disable-next-line no-restricted-globals -- asserting real focus requires the live document await waitFor(() => expect(document.activeElement).toBe(input)) }) @@ -434,7 +437,7 @@ describe('FindBar', () => { openFindBar() renderFindBar() - const input = screen.getByRole('textbox', { name: /find in page/i }) + const input = screen.getByRole('searchbox', { name: /find in page/i }) fireEvent.change(input, { target: { value: 'n' } }) fireEvent.change(input, { target: { value: 'ne' } }) fireEvent.change(input, { target: { value: 'nee' } }) @@ -453,6 +456,36 @@ describe('FindBar', () => { } }) + it('excludes the semantic searchbox only while Chromium indexes the page', async () => { + vi.useFakeTimers() + let resolveFind: (() => void) | undefined + bridge.findInPage.mockReturnValueOnce(new Promise(resolve => (resolveFind = () => resolve({ count: 0 })))) + + try { + openFindBar() + renderFindBar() + + const input = screen.getByRole('searchbox', { name: /find in page/i }) as HTMLInputElement + input.focus() + fireEvent.change(input, { target: { value: 'needle' } }) + input.setSelectionRange(6, 6) + + act(() => vi.advanceTimersByTime(200)) + + expect(input.inert).toBe(true) + expect(input.type).toBe('search') + expect(input.getAttribute('role')).toBeNull() + + await act(async () => resolveFind?.()) + expect(input.inert).toBe(false) + // eslint-disable-next-line no-restricted-globals -- the exclusion cycle must restore real focus + expect(document.activeElement).toBe(input) + expect(input.selectionStart).toBe(6) + } finally { + vi.useRealTimers() + } + }) + it('does not fire a pending search after the bar closes', async () => { vi.useFakeTimers() @@ -460,7 +493,7 @@ describe('FindBar', () => { openFindBar() renderFindBar() - fireEvent.change(screen.getByRole('textbox', { name: /find in page/i }), { + fireEvent.change(screen.getByRole('searchbox', { name: /find in page/i }), { target: { value: 'needle' } }) @@ -477,7 +510,7 @@ describe('FindBar', () => { $findInPage.set({ active: true, query: 'needle', matchOrdinal: 1, matchCount: 4 }) renderFindBar() - const input = screen.getByRole('textbox', { name: /find in page/i }) + const input = screen.getByRole('searchbox', { name: /find in page/i }) fireEvent.keyDown(input, { key: 'Enter' }) expect(bridge.findInPage).toHaveBeenLastCalledWith('needle', { forward: true, findNext: true }) diff --git a/apps/desktop/src/components/find-bar.tsx b/apps/desktop/src/components/find-bar.tsx index bffc6c0d485f8..b39901fc639aa 100644 --- a/apps/desktop/src/components/find-bar.tsx +++ b/apps/desktop/src/components/find-bar.tsx @@ -37,6 +37,7 @@ export function FindBar() { const { t } = useI18n() const { active, query, matchOrdinal, matchCount } = useStore($findInPage) const inputRef = useRef(null) + const nativeSearchRequestRef = useRef(0) const [localQuery, setLocalQuery] = useState('') const [filesPaneRight, setFilesPaneRight] = useState(null) const { pathname } = useLocation() @@ -157,11 +158,49 @@ export function FindBar() { // Debounce search — fire findInPage 200ms after the user stops typing. useEffect(() => { + const requestId = ++nativeSearchRequestRef.current + const input = inputRef.current + if (!active || !localQuery) { + if (input?.inert) { + input.inert = false + } + return undefined } - const id = setTimeout(() => setFindQuery(localQuery), 200) + const id = setTimeout(() => { + if (!input) { + void setFindQuery(localQuery) + + return + } + + const hadFocus = document.activeElement === input + const selectionStart = input.selectionStart + const selectionEnd = input.selectionEnd + + // The HTML inert contract excludes this truthful search control from + // find-in-page. Keep it inert until the IPC reply confirms Electron has + // started the request, then restore focus and selection. + input.inert = true + + void setFindQuery(localQuery).finally(() => { + if (nativeSearchRequestRef.current !== requestId) { + return + } + + input.inert = false + + if (hadFocus && input.isConnected) { + input.focus({ preventScroll: true }) + + if (selectionStart !== null && selectionEnd !== null) { + input.setSelectionRange(selectionStart, selectionEnd) + } + } + }) + }, 200) // Cleanup covers every exit: another keystroke, the bar closing, and // unmount. Nothing can fire a find after the bar is gone. @@ -214,7 +253,7 @@ export function FindBar() { // Empty query: clear highlights immediately rather than after the debounce. if (!value) { - setFindQuery('') + void setFindQuery('') } } @@ -261,12 +300,13 @@ export function FindBar() { > diff --git a/apps/desktop/src/store/find-in-page.ts b/apps/desktop/src/store/find-in-page.ts index 9d26bc8b4075c..d106a0818810b 100644 --- a/apps/desktop/src/store/find-in-page.ts +++ b/apps/desktop/src/store/find-in-page.ts @@ -28,7 +28,7 @@ export function closeFindBar(): void { void window.hermesDesktop?.stopFindInPage() } -export function setFindQuery(query: string): void { +export async function setFindQuery(query: string): Promise { const prev = $findInPage.get() // Never search for a closed bar. The component clears its debounce on @@ -46,7 +46,7 @@ export function setFindQuery(query: string): void { } $findInPage.set({ ...prev, query }) - void window.hermesDesktop?.findInPage(query, { forward: true, findNext: false }) + await window.hermesDesktop?.findInPage(query, { forward: true, findNext: false }) } export function findNext(): void {