diff --git a/src/renderer/src/components/editor/PdfViewer.tsx b/src/renderer/src/components/editor/PdfViewer.tsx index 1243f2664a2..19c821b20cd 100644 --- a/src/renderer/src/components/editor/PdfViewer.tsx +++ b/src/renderer/src/components/editor/PdfViewer.tsx @@ -9,10 +9,9 @@ import type { } from 'pdfjs-dist/web/pdf_viewer.mjs' import 'pdfjs-dist/web/pdf_viewer.css' import PdfFind from './PdfFind' -import { getShortcutPlatform } from '@/lib/shortcut-platform' import { useShortcutLabel } from '@/hooks/useShortcutLabel' import { useAppStore } from '@/store' -import { keybindingMatchesAction } from '../../../../shared/keybindings' +import { usePdfViewerFindShortcut } from './use-pdf-viewer-find-shortcut' import workerUrl from 'pdfjs-dist/build/pdf.worker.min.mjs?url' import { translate } from '@/i18n/i18n' @@ -60,6 +59,7 @@ function PdfDocumentViewer({ preferenceKey = null, scrollCacheKey = null }: PdfViewerProps): JSX.Element { + const rootRef = useRef(null) const containerRef = useRef(null) const viewerDivRef = useRef(null) const loaderRef = useRef(null) @@ -184,23 +184,13 @@ function PdfDocumentViewer({ ) }, [isCommandOwner, stepZoom, zoomReset]) - useEffect(() => { - const handleKeyDown = (e: KeyboardEvent): void => { - const platform = getShortcutPlatform() - if (keybindingMatchesAction('editor.find', e, platform, keybindings)) { - e.preventDefault() - e.stopPropagation() - setFindOpen(true) - } - } - window.addEventListener('keydown', handleKeyDown, true) - return () => window.removeEventListener('keydown', handleKeyDown, true) - }, [keybindings]) + const openFind = useCallback(() => setFindOpen(true), []) + usePdfViewerFindShortcut({ rootRef, ownsCommands: isCommandOwner, keybindings, openFind }) const zoomPercent = Math.round(scale * 100) return ( -
+
{/* Why: PDFViewer requires its container to be position:absolute. diff --git a/src/renderer/src/components/editor/markdown-preview-search.ts b/src/renderer/src/components/editor/markdown-preview-search.ts index 715cc5b166b..62dab78650c 100644 --- a/src/renderer/src/components/editor/markdown-preview-search.ts +++ b/src/renderer/src/components/editor/markdown-preview-search.ts @@ -1,11 +1,16 @@ import { keybindingMatchesAction, type KeybindingOverrides } from '../../../../shared/keybindings' -import { findTextMatchRanges, isMarkdownPreviewSearchQueryTooLarge } from './markdown-text-matches' +import { + createDomTextSearchHighlights, + findDomTextSearchRanges, + type DomTextSearchInstance, + type DomTextSearchScope +} from '../../lib/dom-text-search-highlights' export { findTextMatchRanges, - isMarkdownPreviewSearchQueryTooLarge, - MARKDOWN_PREVIEW_SEARCH_QUERY_MAX_BYTES, + isTextSearchQueryTooLarge as isMarkdownPreviewSearchQueryTooLarge, + TEXT_SEARCH_QUERY_MAX_BYTES as MARKDOWN_PREVIEW_SEARCH_QUERY_MAX_BYTES, type TextMatchOptions -} from './markdown-text-matches' +} from '../../lib/text-match-ranges' export function isMarkdownPreviewFindShortcut( event: Pick, @@ -23,128 +28,29 @@ export function isMarkdownPreviewReplaceShortcut( return keybindingMatchesAction('editor.replace', event, platform, keybindings) } -// Why: react-markdown owns the preview DOM. Injecting 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 +// Why: react-markdown owns the preview DOM; matches are painted as CSS highlights +// (see dom-text-search-highlights.ts). 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. -declare const markdownPreviewSearchInstanceBrand: unique symbol +const previewHighlights = createDomTextSearchHighlights({ + match: 'markdown-preview-search-match', + active: 'markdown-preview-search-active-match' +}) /** Per-preview identity for the highlight maps; only compared by reference. */ -export type MarkdownPreviewSearchInstance = { - readonly [markdownPreviewSearchInstanceBrand]?: never -} +export type MarkdownPreviewSearchInstance = DomTextSearchInstance -const searchRangesByInstance = new Map() -const activeRangeByInstance = new Map() - -// 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>): 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>): 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) - } +const DOCUMENT_ONLY_SCOPE: DomTextSearchScope = { + rejectElement: (element) => + element.closest( + '.markdown-annotation-controls,[data-orca-export-hide],.code-block-copy-btn,.mermaid-block' + ) !== null, + joinedTextSelector: 'code' } export function clearMarkdownPreviewSearchHighlights( instanceId: MarkdownPreviewSearchInstance ): void { - searchRangesByInstance.delete(instanceId) - activeRangeByInstance.delete(instanceId) - const api = getHighlightApi() - if (api) { - paintMatchHighlight(api) - paintActiveHighlight(api) - } -} - -function appendTextSearchRanges(nodes: Text[], query: string, ranges: Range[]): void { - const text = nodes.map((node) => node.data).join('') - if (!text.trim()) { - return - } - let nodeIndex = 0 - let offset = 0 - for (const { start, end } of findTextMatchRanges(text, query)) { - while (nodeIndex < nodes.length - 1 && offset + nodes[nodeIndex].length <= start) { - offset += nodes[nodeIndex++].length - } - const range = document.createRange() - range.setStart(nodes[nodeIndex], start - offset) - while (nodeIndex < nodes.length - 1 && offset + nodes[nodeIndex].length < end) { - offset += nodes[nodeIndex++].length - } - range.setEnd(nodes[nodeIndex], end - offset) - ranges.push(range) - } + previewHighlights.clear(instanceId) } export function applyMarkdownPreviewSearchHighlights( @@ -153,65 +59,12 @@ export function applyMarkdownPreviewSearchHighlights( query: string, options: { documentOnly?: boolean } = {} ): Range[] { - const ranges: Range[] = [] - - if (query && !isMarkdownPreviewSearchQueryTooLarge(query)) { - const walker = document.createTreeWalker(root, NodeFilter.SHOW_TEXT, { - acceptNode(node) { - if (!(node.parentElement instanceof HTMLElement)) { - return NodeFilter.FILTER_REJECT - } - if ( - options.documentOnly && - node.parentElement.closest( - '.markdown-annotation-controls,[data-orca-export-hide],.code-block-copy-btn,.mermaid-block' - ) - ) { - return NodeFilter.FILTER_REJECT - } - if ( - !node.textContent?.trim() && - !(options.documentOnly && node.parentElement.closest('code')) - ) { - return NodeFilter.FILTER_REJECT - } - return NodeFilter.FILTER_ACCEPT - } - }) - - let currentNode = walker.nextNode() - while (currentNode) { - if (!(currentNode instanceof Text)) { - currentNode = walker.nextNode() - continue - } - let code = options.documentOnly ? currentNode.parentElement?.closest('code') : null - while (code) { - const parentCode = code.parentElement?.closest('code') - if (!parentCode || !root.contains(parentCode)) { - break - } - code = parentCode - } - const nodes = [currentNode] - let next = walker.nextNode() - while (code && next instanceof Text && code.contains(next)) { - nodes.push(next) - next = walker.nextNode() - } - appendTextSearchRanges(nodes, query, ranges) - currentNode = next - } - } - - searchRangesByInstance.set(instanceId, ranges) - activeRangeByInstance.delete(instanceId) - const api = getHighlightApi() - if (api) { - paintMatchHighlight(api) - paintActiveHighlight(api) - } - + const ranges = findDomTextSearchRanges( + root, + query, + options.documentOnly ? DOCUMENT_ONLY_SCOPE : {} + ) + previewHighlights.setMatches(instanceId, ranges) return ranges } @@ -222,19 +75,7 @@ export function setActiveMarkdownPreviewSearchMatch( options: { scrollIntoView?: boolean } = {} ): void { 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) - } + previewHighlights.setActive(instanceId, active) if (active && options.scrollIntoView !== false) { // The Range's start container is a Text node; scroll its element into view. diff --git a/src/renderer/src/components/editor/use-pdf-viewer-find-shortcut.test.tsx b/src/renderer/src/components/editor/use-pdf-viewer-find-shortcut.test.tsx new file mode 100644 index 00000000000..2007593327b --- /dev/null +++ b/src/renderer/src/components/editor/use-pdf-viewer-find-shortcut.test.tsx @@ -0,0 +1,183 @@ +// @vitest-environment happy-dom +import { act, cleanup, render } from '@testing-library/react' +import { useContext, useRef } from 'react' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { EditorCommandOwnerContext } from './editor-command-owner-context' +import { usePdfViewerFindShortcut } from './use-pdf-viewer-find-shortcut' +import { useNativeChatFind } from '../native-chat/use-native-chat-find' + +vi.mock('@/store', () => ({ useAppStore: { getState: () => ({ keybindings: undefined }) } })) +vi.mock('../../store', () => ({ useAppStore: { getState: () => ({ keybindings: undefined }) } })) +vi.mock('@/lib/web-client-location', () => ({ isWebClientLocation: () => false })) + +afterEach(() => { + cleanup() + document.body.replaceChildren() +}) + +function pressFind(target: EventTarget): KeyboardEvent { + const isMac = navigator.userAgent.includes('Mac') + const event = new KeyboardEvent('keydown', { + key: 'f', + code: 'KeyF', + bubbles: true, + cancelable: true, + metaKey: isMac, + ctrlKey: !isMac + }) + target.dispatchEvent(event) + return event +} + +function PdfViewerStandIn({ + name, + openFind +}: { + name: string + openFind: () => void +}): React.JSX.Element { + const rootRef = useRef(null) + const ownsCommands = useContext(EditorCommandOwnerContext) + usePdfViewerFindShortcut({ rootRef, ownsCommands, keybindings: undefined, openFind }) + return ( +
+ +
+ ) +} + +/** The real split shape: each group's strip and body (where editors render), and the retained + * overlay host for terminals, chats and browsers as a sibling of the layout, outside every body. */ +function Workspace({ + focusedGroup, + openFind +}: { + focusedGroup: 'pdf' | 'chat' | 'second-pdf' | null + openFind: Record<'pdf' | 'second-pdf', () => void> +}): React.JSX.Element { + return ( +
+
+
+
+
+ +
+
+ + + +
+
+
+
+
+
+