From 8267b578b2891e9986b78f67e47be04fb321ec36 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 3 Oct 2026 23:17:52 -0700 Subject: [PATCH] Fix Markdown Find editing without moving the caret or viewport (#25144) * Fix Markdown Find editing without changing the caret or viewport * Preserve Markdown selections across search focus and stale updates --- .../editor/RichMarkdownEditorSurface.tsx | 2 + ...-markdown-search-focus.adversarial.test.ts | 137 ++++++++ .../editor/rich-markdown-search-focus.test.ts | 162 +++++++++ .../editor/rich-markdown-search-focus.ts | 44 +++ ...useRichMarkdownSearch.adversarial.test.tsx | 312 ++++++++++++++++++ .../useRichMarkdownSearch.editing.test.tsx | 139 ++++++++ .../editor/useRichMarkdownSearch.ts | 102 +++--- .../editor/useRichMarkdownSearchHighlights.ts | 104 ++++++ tests/e2e/markdown-find-adversarial.spec.ts | 307 +++++++++++++++++ tests/e2e/markdown-find-editing.spec.ts | 215 ++++++++++++ 10 files changed, 1459 insertions(+), 65 deletions(-) create mode 100644 src/renderer/src/components/editor/rich-markdown-search-focus.adversarial.test.ts create mode 100644 src/renderer/src/components/editor/rich-markdown-search-focus.test.ts create mode 100644 src/renderer/src/components/editor/rich-markdown-search-focus.ts create mode 100644 src/renderer/src/components/editor/useRichMarkdownSearch.adversarial.test.tsx create mode 100644 src/renderer/src/components/editor/useRichMarkdownSearch.editing.test.tsx create mode 100644 src/renderer/src/components/editor/useRichMarkdownSearchHighlights.ts create mode 100644 tests/e2e/markdown-find-adversarial.spec.ts create mode 100644 tests/e2e/markdown-find-editing.spec.ts diff --git a/src/renderer/src/components/editor/RichMarkdownEditorSurface.tsx b/src/renderer/src/components/editor/RichMarkdownEditorSurface.tsx index 8e2ca1e4abb..bf7fcc06936 100644 --- a/src/renderer/src/components/editor/RichMarkdownEditorSurface.tsx +++ b/src/renderer/src/components/editor/RichMarkdownEditorSurface.tsx @@ -20,6 +20,7 @@ import type { MarkdownReviewNote } from '@/lib/markdown-review-notes' import type { RichMarkdownAnnotationTarget } from './rich-markdown-review-annotations' import type { RichMarkdownReviewNotePosition } from './rich-markdown-review-note-layout' import type { DiffComment } from '../../../../shared/diff-comment-types' +import { focusRichMarkdownEditorFromSearch } from './rich-markdown-search-focus' function shouldFocusEmptyEditorFromSurfaceClick( event: React.MouseEvent, @@ -209,6 +210,7 @@ export function RichMarkdownEditorSurface({ // Image layout must not anchor-scroll over the restored tab position. className="relative h-full overflow-auto scrollbar-editor [overflow-anchor:none]" onMouseDown={(event) => { + focusRichMarkdownEditorFromSearch(event.nativeEvent, editor?.view ?? null) if (!shouldFocusEmptyEditorFromSurfaceClick(event, editor)) { return } diff --git a/src/renderer/src/components/editor/rich-markdown-search-focus.adversarial.test.ts b/src/renderer/src/components/editor/rich-markdown-search-focus.adversarial.test.ts new file mode 100644 index 00000000000..4265cacb0fb --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-search-focus.adversarial.test.ts @@ -0,0 +1,137 @@ +// @vitest-environment happy-dom +import { Editor } from '@tiptap/core' +import StarterKit from '@tiptap/starter-kit' +import { TableKit } from '@tiptap/extension-table' +import { CellSelection } from '@tiptap/pm/tables' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { focusRichMarkdownEditorFromSearch } from './rich-markdown-search-focus' + +function findTextPosition(editor: Editor, text: string): number { + let position: number | null = null + editor.state.doc.descendants((node, pos) => { + if (node.isText && node.text === text) { + position = pos + return false + } + return true + }) + if (position === null) { + throw new Error(`Expected editor text: ${text}`) + } + return position +} + +function createSurface(content: string) { + const root = document.createElement('div') + root.className = 'rich-markdown-editor-shell' + document.body.append(root) + const editor = new Editor({ extensions: [StarterKit, TableKit], content }) + root.append(editor.view.dom) + const search = document.createElement('div') + search.className = 'rich-markdown-search' + const input = document.createElement('input') + search.append(input) + root.append(search) + root.addEventListener('mousedown', (event) => { + if (event instanceof MouseEvent) { + focusRichMarkdownEditorFromSearch(event, editor.view) + } + }) + return { editor, input } +} + +afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() + document.body.replaceChildren() +}) + +describe('Find focus with editor-owned selection', () => { + it('restores the remembered caret before Shift click when Find owns the native selection', () => { + vi.useFakeTimers() + const { editor, input } = createSurface('

first

lower caret

') + const caret = findTextPosition(editor, 'lower caret') + 6 + const updates = vi.fn() + editor.on('update', updates) + try { + editor.commands.setTextSelection(caret) + input.focus() + document.getSelection()?.removeAllRanges() + const paragraph = editor.view.dom.querySelector('p:last-child') + const text = paragraph?.firstChild + if (!paragraph || !text) { + throw new Error('Expected lower paragraph text') + } + vi.spyOn(editor.view, 'posAtCoords').mockReturnValue({ pos: caret, inside: -1 }) + const originalDoc = editor.state.doc + const selection = editor.state.selection + const event = new MouseEvent('mousedown', { + button: 0, + shiftKey: true, + bubbles: true, + cancelable: true + }) + + paragraph.dispatchEvent(event) + + expect(document.activeElement).toBe(editor.view.dom) + expect(document.getSelection()?.anchorNode).toBe(text) + expect(document.getSelection()?.anchorOffset).toBe(6) + expect(document.getSelection()?.focusOffset).toBe(6) + expect(event.defaultPrevented).toBe(false) + + vi.advanceTimersByTime(20) + + expect(editor.state.selection).toBe(selection) + expect(editor.state.doc).toBe(originalDoc) + expect(updates).not.toHaveBeenCalled() + } finally { + editor.destroy() + } + }) + + it('focuses a handled cross-cell selection and preserves its cells without marking content dirty', () => { + vi.useFakeTimers() + const { editor, input } = createSurface( + '
firstsecond
' + ) + const updates = vi.fn() + try { + editor.commands.setTextSelection(findTextPosition(editor, 'first')) + editor.on('update', updates) + input.focus() + const secondPos = findTextPosition(editor, 'second') + vi.spyOn(editor.view, 'posAtCoords').mockReturnValue({ pos: secondPos, inside: secondPos }) + const second = editor.view.dom.querySelector('td:last-child p') + if (!second) { + throw new Error('Expected second table cell') + } + const originalDoc = editor.state.doc + const event = new MouseEvent('mousedown', { + button: 0, + shiftKey: true, + bubbles: true, + cancelable: true + }) + + second.dispatchEvent(event) + + const selection = editor.state.selection + expect(selection).toBeInstanceOf(CellSelection) + expect(event.defaultPrevented).toBe(true) + expect(document.activeElement).toBe(editor.view.dom) + expect(editor.view.dom.querySelectorAll('.selectedCell')).toHaveLength(2) + expect(editor.view.dom.classList.contains('ProseMirror-hideselection')).toBe(true) + expect(document.getSelection()?.rangeCount).toBe(1) + + vi.advanceTimersByTime(20) + + expect(editor.state.selection).toBe(selection) + expect(editor.state.doc).toBe(originalDoc) + expect(updates).not.toHaveBeenCalled() + expect(editor.view.dom.querySelectorAll('.selectedCell')).toHaveLength(2) + } finally { + editor.destroy() + } + }) +}) diff --git a/src/renderer/src/components/editor/rich-markdown-search-focus.test.ts b/src/renderer/src/components/editor/rich-markdown-search-focus.test.ts new file mode 100644 index 00000000000..d158f4466c9 --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-search-focus.test.ts @@ -0,0 +1,162 @@ +// @vitest-environment happy-dom +import { Schema } from '@tiptap/pm/model' +import { EditorState } from '@tiptap/pm/state' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { focusRichMarkdownEditorFromSearch } from './rich-markdown-search-focus' + +const schema = new Schema({ + nodes: { + doc: { content: 'paragraph+' }, + paragraph: { content: 'text*' }, + text: {} + } +}) + +function createSurface() { + const root = document.createElement('div') + root.className = 'rich-markdown-editor-shell' + const editorDom = document.createElement('div') + editorDom.contentEditable = 'true' + editorDom.tabIndex = -1 + const paragraph = document.createElement('p') + paragraph.textContent = 'First editable paragraph' + editorDom.append(paragraph) + const search = document.createElement('div') + search.className = 'rich-markdown-search' + const findInput = document.createElement('input') + const replaceInput = document.createElement('input') + search.append(findInput, replaceInput) + root.append(editorDom, search) + document.body.append(root) + const focus = vi.spyOn(editorDom, 'focus') + const viewFocus = vi.fn(() => editorDom.focus({ preventScroll: true })) + const view = { dom: editorDom, focus: viewFocus, state: EditorState.create({ schema }) } + root.addEventListener('mousedown', (event) => { + if (event instanceof MouseEvent) { + focusRichMarkdownEditorFromSearch(event, view) + } + }) + return { root, editorDom, paragraph, findInput, replaceInput, focus, viewFocus } +} + +afterEach(() => { + vi.restoreAllMocks() + document.body.replaceChildren() +}) + +describe('rich markdown search focus handoff', () => { + it.each(['find', 'replace'] as const)( + 'returns keyboard focus from %s without preventing native selection or requesting scroll', + (field) => { + const { paragraph, editorDom, findInput, replaceInput, focus, viewFocus } = createSurface() + const input = field === 'find' ? findInput : replaceInput + input.focus() + const event = new MouseEvent('mousedown', { bubbles: true, cancelable: true, button: 0 }) + + paragraph.dispatchEvent(event) + + expect(document.activeElement).toBe(editorDom) + expect(focus).toHaveBeenCalledExactlyOnceWith({ preventScroll: true }) + expect(viewFocus).not.toHaveBeenCalled() + expect(event.defaultPrevented).toBe(false) + } + ) + + it('restores the editor selection before native Shift click handling', () => { + const { paragraph, editorDom, findInput, viewFocus } = createSurface() + findInput.focus() + const event = new MouseEvent('mousedown', { + bubbles: true, + cancelable: true, + button: 0, + shiftKey: true + }) + + paragraph.dispatchEvent(event) + + expect(document.activeElement).toBe(editorDom) + expect(viewFocus).toHaveBeenCalledExactlyOnceWith() + expect(event.defaultPrevented).toBe(false) + }) + + it.each(['button', 'input', 'textarea', 'select', 'noneditable', 'task-label'])( + 'leaves embedded %s controls in charge of focus', + (kind) => { + const { editorDom, findInput, focus } = createSurface() + const control = document.createElement( + kind === 'noneditable' ? 'div' : kind === 'task-label' ? 'label' : kind + ) + if (kind === 'noneditable' || kind === 'task-label') { + control.setAttribute('contenteditable', 'false') + } + const child = document.createElement('span') + control.append(child) + editorDom.append(control) + findInput.focus() + + child.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, button: 0 })) + + expect(document.activeElement).toBe(findInput) + expect(focus).not.toHaveBeenCalled() + } + ) + + it('does not steal focus from a different editor find widget or unrelated field', () => { + const first = createSurface() + const second = createSurface() + const unrelated = document.createElement('input') + document.body.append(unrelated) + + for (const input of [second.findInput, unrelated]) { + input.focus() + first.paragraph.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, button: 0 })) + expect(document.activeElement).toBe(input) + } + + expect(first.focus).not.toHaveBeenCalled() + }) + + it('leaves ordinary editor clicks and blank surface clicks untouched', () => { + const { root, editorDom, paragraph, findInput, focus } = createSurface() + editorDom.focus() + focus.mockClear() + paragraph.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, button: 0 })) + findInput.focus() + root.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, button: 0 })) + + expect(focus).not.toHaveBeenCalled() + expect(document.activeElement).toBe(findInput) + }) + + it('leaves canceled and non-primary presses untouched', () => { + const { paragraph, findInput, focus } = createSurface() + findInput.focus() + const canceled = new MouseEvent('mousedown', { bubbles: true, cancelable: true, button: 0 }) + canceled.preventDefault() + + for (const event of [ + canceled, + new MouseEvent('mousedown', { bubbles: true, button: 1 }), + new MouseEvent('mousedown', { bubbles: true, button: 2 }) + ]) { + paragraph.dispatchEvent(event) + } + + expect(focus).not.toHaveBeenCalled() + expect(document.activeElement).toBe(findInput) + }) + + it('allows editable content nested inside an unrelated noneditable ancestor', () => { + const { root, paragraph, findInput, focus } = createSurface() + root.setAttribute('contenteditable', 'false') + findInput.focus() + + paragraph.dispatchEvent(new MouseEvent('mousedown', { bubbles: true, button: 0 })) + + expect(focus).toHaveBeenCalledExactlyOnceWith({ preventScroll: true }) + }) + + it('does nothing when the editor has not mounted', () => { + focusRichMarkdownEditorFromSearch(new MouseEvent('mousedown', { button: 0 }), null) + }) +}) diff --git a/src/renderer/src/components/editor/rich-markdown-search-focus.ts b/src/renderer/src/components/editor/rich-markdown-search-focus.ts new file mode 100644 index 00000000000..8ef1e0f50c8 --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-search-focus.ts @@ -0,0 +1,44 @@ +import type { EditorView } from '@tiptap/pm/view' +import { CellSelection } from '@tiptap/pm/tables' + +export function focusRichMarkdownEditorFromSearch( + event: MouseEvent, + view: Pick | null +): void { + if ( + !view || + event.button !== 0 || + (event.defaultPrevented && !(view.state.selection instanceof CellSelection)) + ) { + return + } + + const editorDom = view.dom + const target = event.target + if (!(target instanceof Element) || !editorDom.contains(target)) { + return + } + + const root = editorDom.closest('.rich-markdown-editor-shell') + const activeElement = editorDom.ownerDocument.activeElement + if ( + !root || + !activeElement?.closest('.rich-markdown-search') || + activeElement.closest('.rich-markdown-editor-shell') !== root + ) { + return + } + + const control = target.closest('button, input, textarea, select, [contenteditable="false"]') + if (control && editorDom.contains(control)) { + return + } + + if (event.shiftKey || event.defaultPrevented) { + // Shift extends the current selection; handled cell selection has no browser default. + view.focus() + } else { + // Native focus preserves the browser's upcoming click or drag selection. + editorDom.focus({ preventScroll: true }) + } +} diff --git a/src/renderer/src/components/editor/useRichMarkdownSearch.adversarial.test.tsx b/src/renderer/src/components/editor/useRichMarkdownSearch.adversarial.test.tsx new file mode 100644 index 00000000000..8eb7f376d47 --- /dev/null +++ b/src/renderer/src/components/editor/useRichMarkdownSearch.adversarial.test.tsx @@ -0,0 +1,312 @@ +// @vitest-environment happy-dom +import { StrictMode, useLayoutEffect, type ReactNode } from 'react' +import { act, renderHook } from '@testing-library/react' +import { Editor } from '@tiptap/react' +import StarterKit from '@tiptap/starter-kit' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { useRichMarkdownSearch } from './useRichMarkdownSearch' + +vi.mock('@/store', () => ({ + useAppStore: (select: (state: unknown) => unknown) => select({ keybindings: {} }) +})) + +const cleanups: (() => void)[] = [] + +function mountSearch(content = '

beta beta

Edit here

', strict = false) { + vi.useFakeTimers() + const editor = new Editor({ extensions: [StarterKit], content }) + const root = document.createElement('div') + root.className = 'rich-markdown-editor-shell' + const searchBar = document.createElement('div') + searchBar.className = 'rich-markdown-search' + const input = document.createElement('input') + const otherControl = document.createElement('input') + searchBar.append(input) + root.append(editor.view.dom, searchBar) + document.body.append(root, otherControl) + const scrollContainer = document.createElement('div') + const rootRef = { current: root } + const scrollContainerRef = { current: scrollContainer } + const beforePassiveRef: { current: (() => void) | null } = { current: null } + const scrollTo = vi.spyOn(scrollContainer, 'scrollTo') + vi.spyOn(editor.view, 'coordsAtPos').mockReturnValue({ top: 20, bottom: 40, left: 0, right: 0 }) + const initialProps: { currentEditor: Editor | null } = { currentEditor: editor } + const hook = renderHook( + ({ currentEditor }: { currentEditor: Editor | null }) => { + const result = useRichMarkdownSearch({ editor: currentEditor, rootRef, scrollContainerRef }) + result.searchState.searchInputRef.current = input + useLayoutEffect(() => { + const callback = beforePassiveRef.current + beforePassiveRef.current = null + callback?.() + }) + return result + }, + { + initialProps, + wrapper: strict + ? ({ children }: { children: ReactNode }) => {children} + : undefined + } + ) + cleanups.push(() => { + hook.unmount() + editor.destroy() + root.remove() + otherControl.remove() + }) + act(() => hook.result.current.openSearch()) + const query = (value: string) => { + act(() => hook.result.current.searchActions.setSearchQuery(value)) + act(() => vi.advanceTimersByTime(150)) + } + const selectedText = () => + editor.state.doc.textBetween(editor.state.selection.from, editor.state.selection.to) + const documentCaret = () => { + act(() => { + editor.view.dom.focus({ preventScroll: true }) + editor.commands.setTextSelection(editor.state.doc.content.size - 1) + }) + } + return { + editor, + root, + hook, + input, + otherControl, + scrollTo, + query, + selectedText, + documentCaret, + beforePassiveRef + } +} + +afterEach(() => { + while (cleanups.length > 0) { + cleanups.pop()?.() + } + vi.useRealTimers() + vi.restoreAllMocks() +}) + +describe('rich Markdown search adversarial interactions', () => { + it('does not revive a delayed search after a document click and another control focus', () => { + const { editor, hook, otherControl, query, documentCaret, scrollTo } = mountSearch() + query('beta') + act(() => hook.result.current.searchActions.setSearchQuery('Edit')) + documentCaret() + const selection = editor.state.selection + act(() => otherControl.focus()) + scrollTo.mockClear() + act(() => vi.advanceTimersByTime(150)) + expect(editor.state.selection.eq(selection)).toBe(true) + expect(scrollTo).not.toHaveBeenCalled() + expect(document.activeElement).toBe(otherControl) + }) + + it('continues typing at the document caret through case and whole-word changes', () => { + const { editor, hook, query, documentCaret, scrollTo } = mountSearch() + query('beta') + documentCaret() + scrollTo.mockClear() + act(() => hook.result.current.searchActions.toggleMatchCase()) + act(() => hook.result.current.searchActions.toggleWholeWord()) + act(() => editor.commands.insertContent('XY')) + expect(editor.getText()).toBe('beta beta\n\nEdit hereXY') + expect(scrollTo).not.toHaveBeenCalled() + }) + + it('lets explicit search navigation select the only match after document editing', () => { + const { editor, hook, query, documentCaret, selectedText, scrollTo } = mountSearch( + '

beta

Edit here

' + ) + query('beta') + documentCaret() + act(() => editor.commands.insertContent('X')) + scrollTo.mockClear() + act(() => hook.result.current.searchActions.moveToMatch(1)) + expect(selectedText()).toBe('beta') + expect(scrollTo).toHaveBeenCalledOnce() + expect(editor.isFocused).toBe(true) + }) + + it('advances past replacement text which still contains the search query', () => { + const { editor, hook, query, selectedText } = mountSearch() + query('beta') + act(() => hook.result.current.searchActions.setReplaceQuery('betaX')) + act(() => hook.result.current.searchActions.replaceCurrentMatch()) + expect(editor.getText()).toBe('betaX beta\n\nEdit here') + expect(selectedText()).toBe('beta') + expect(editor.state.selection.from).toBe(7) + act(() => hook.result.current.searchActions.replaceCurrentMatch()) + expect(editor.getText()).toBe('betaX betaX\n\nEdit here') + }) + + it('does not move the document caret when Replace All retains matching text', () => { + const { editor, hook, query, documentCaret, scrollTo } = mountSearch() + query('beta') + act(() => hook.result.current.searchActions.setReplaceQuery('betaX')) + documentCaret() + scrollTo.mockClear() + act(() => hook.result.current.searchActions.replaceAllMatches()) + expect(editor.getText()).toBe('betaX betaX\n\nEdit here') + expect(editor.state.selection.empty).toBe(true) + expect(editor.state.selection.from).toBe(editor.state.doc.content.size - 1) + expect(scrollTo).not.toHaveBeenCalled() + }) + + it('clamps navigation after external edits remove the active match', () => { + const { editor, hook, query, selectedText } = mountSearch('

beta beta beta

') + query('beta') + act(() => hook.result.current.searchActions.moveToMatch(1)) + act(() => hook.result.current.searchActions.moveToMatch(1)) + act(() => editor.commands.setContent('

beta

')) + expect(hook.result.current.searchState.activeMatchIndex).toBe(0) + act(() => hook.result.current.searchActions.moveToMatch(1)) + expect(selectedText()).toBe('beta') + }) + + it('keeps decorations fresh without moving selection through batched document edits', () => { + const { editor, hook, query, documentCaret, scrollTo } = mountSearch() + query('beta') + documentCaret() + scrollTo.mockClear() + act(() => { + editor.commands.insertContent(' beta') + editor.commands.insertContent(' beta') + }) + expect(hook.result.current.searchState.matchCount).toBe(4) + expect(editor.state.selection.from).toBe(editor.state.doc.content.size - 1) + expect(editor.state.selection.empty).toBe(true) + expect(scrollTo).not.toHaveBeenCalled() + }) + + it('does not dispatch stale match positions when the document changes before passive effects', () => { + const { editor, hook, beforePassiveRef } = mountSearch('

beta

target

') + act(() => hook.result.current.searchActions.setSearchQuery('target')) + beforePassiveRef.current = () => editor.commands.setContent('

x

') + expect(() => act(() => vi.advanceTimersByTime(150))).not.toThrow() + expect(editor.getText()).toBe('x') + expect(hook.result.current.searchState.matchCount).toBe(0) + }) + + it('cancels a pending query when the input is cleared', () => { + const { editor, hook, query, scrollTo } = mountSearch() + query('beta') + const selection = editor.state.selection + scrollTo.mockClear() + act(() => hook.result.current.searchActions.setSearchQuery('Edit')) + act(() => hook.result.current.searchActions.setSearchQuery('')) + act(() => vi.advanceTimersByTime(150)) + expect(hook.result.current.searchState.matchCount).toBe(0) + expect(editor.state.selection.eq(selection)).toBe(true) + expect(scrollTo).not.toHaveBeenCalled() + }) + + it('uses refreshed positions when the document changes before search navigation commits', () => { + const { editor, hook, beforePassiveRef, selectedText, scrollTo } = mountSearch( + '

beta

target

' + ) + act(() => hook.result.current.searchActions.setSearchQuery('target')) + beforePassiveRef.current = () => editor.commands.setContent('

prefix prefix target

') + scrollTo.mockClear() + act(() => vi.advanceTimersByTime(150)) + expect(hook.result.current.searchState.matchCount).toBe(1) + expect(selectedText()).toBe('target') + expect(editor.state.selection.from).toBe(15) + expect(scrollTo).toHaveBeenCalledOnce() + }) + + it('moves Next relative to the clamped active match after several matches disappear', () => { + const { editor, hook, query } = mountSearch('

beta beta beta beta beta

') + query('beta') + for (let index = 0; index < 4; index++) { + act(() => hook.result.current.searchActions.moveToMatch(1)) + } + expect(hook.result.current.searchState.activeMatchIndex).toBe(4) + act(() => editor.commands.setContent('

beta beta beta

')) + expect(hook.result.current.searchState.activeMatchIndex).toBe(0) + act(() => hook.result.current.searchActions.moveToMatch(1)) + expect(hook.result.current.searchState.activeMatchIndex).toBe(1) + expect(editor.state.selection.from).toBe(6) + }) + + it('does not navigate to an old query when replacing during the debounce window', () => { + const { editor, hook, query, scrollTo } = mountSearch() + query('beta') + act(() => hook.result.current.searchActions.setSearchQuery('Edit')) + act(() => hook.result.current.searchActions.setReplaceQuery('changed')) + act(() => editor.commands.setTextSelection(editor.state.doc.content.size - 1)) + scrollTo.mockClear() + act(() => hook.result.current.searchActions.replaceCurrentMatch()) + expect(editor.getText()).toBe('beta beta\n\nchanged here') + expect(editor.state.selection.empty).toBe(true) + expect(editor.state.selection.from).toBe(editor.state.doc.content.size - 1) + expect(scrollTo).not.toHaveBeenCalled() + }) + + it('survives strict effect replay, null editor, and a fresh editor instance', () => { + const { editor, hook, input, query, documentCaret, scrollTo } = mountSearch(undefined, true) + query('beta') + documentCaret() + act(() => editor.commands.insertContent('X')) + act(() => input.focus()) + act(() => hook.rerender({ currentEditor: null })) + const replacement = new Editor({ extensions: [StarterKit], content: '

beta

' }) + vi.spyOn(replacement.view, 'coordsAtPos').mockReturnValue({ + top: 20, + bottom: 40, + left: 0, + right: 0 + }) + scrollTo.mockClear() + act(() => hook.rerender({ currentEditor: replacement })) + expect(hook.result.current.searchState.matchCount).toBe(1) + expect( + replacement.state.doc.textBetween( + replacement.state.selection.from, + replacement.state.selection.to + ) + ).toBe('beta') + expect(scrollTo).toHaveBeenCalledOnce() + act(() => hook.rerender({ currentEditor: null })) + replacement.destroy() + }) + + it('survives a debounce completing after its editor has been destroyed', () => { + const { editor, hook } = mountSearch() + act(() => hook.result.current.searchActions.setSearchQuery('beta')) + act(() => editor.destroy()) + expect(() => act(() => vi.advanceTimersByTime(150))).not.toThrow() + }) + + it('does not replay an old Next request into a replacement editor after focus leaves Find', () => { + const { editor, hook, root, query, documentCaret, scrollTo } = mountSearch() + query('beta') + act(() => hook.result.current.searchActions.moveToMatch(1)) + documentCaret() + act(() => hook.rerender({ currentEditor: null })) + editor.view.dom.remove() + const replacement = new Editor({ extensions: [StarterKit], content: '

beta

' }) + root.prepend(replacement.view.dom) + vi.spyOn(replacement.view, 'coordsAtPos').mockReturnValue({ + top: 20, + bottom: 40, + left: 0, + right: 0 + }) + scrollTo.mockClear() + act(() => hook.rerender({ currentEditor: replacement })) + expect(replacement.state.selection.empty).toBe(true) + expect(scrollTo).not.toHaveBeenCalled() + act(() => hook.rerender({ currentEditor: null })) + replacement.destroy() + }) + + it('survives the editor being destroyed by a selection-update listener', () => { + const { editor, query } = mountSearch() + editor.on('selectionUpdate', () => editor.destroy()) + expect(() => query('beta')).not.toThrow() + }) +}) diff --git a/src/renderer/src/components/editor/useRichMarkdownSearch.editing.test.tsx b/src/renderer/src/components/editor/useRichMarkdownSearch.editing.test.tsx new file mode 100644 index 00000000000..b9a8df6e89f --- /dev/null +++ b/src/renderer/src/components/editor/useRichMarkdownSearch.editing.test.tsx @@ -0,0 +1,139 @@ +// @vitest-environment happy-dom +import { act, renderHook } from '@testing-library/react' +import { Editor } from '@tiptap/react' +import StarterKit from '@tiptap/starter-kit' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { useRichMarkdownSearch } from './useRichMarkdownSearch' + +vi.mock('@/store', () => ({ + useAppStore: (select: (state: unknown) => unknown) => select({ keybindings: {} }) +})) + +function mountSearch() { + const editor = new Editor({ + extensions: [StarterKit], + content: '

beta beta

Edit here

' + }) + const scrollContainer = document.createElement('div') + const scrollTo = vi.spyOn(scrollContainer, 'scrollTo') + vi.spyOn(editor.view, 'coordsAtPos').mockReturnValue({ top: 20, bottom: 40, left: 0, right: 0 }) + const root = document.createElement('div') + root.className = 'rich-markdown-editor-shell' + const search = document.createElement('div') + search.className = 'rich-markdown-search' + const input = document.createElement('input') + search.append(input) + root.append(editor.view.dom) + root.append(search) + document.body.append(root) + const hook = renderHook(() => { + const result = useRichMarkdownSearch({ + editor, + rootRef: { current: root }, + scrollContainerRef: { current: scrollContainer } + }) + result.searchState.searchInputRef.current = input + return result + }) + act(() => hook.result.current.openSearch()) + act(() => hook.result.current.searchActions.setSearchQuery('beta')) + act(() => vi.advanceTimersByTime(150)) + const dispose = () => { + hook.unmount() + editor.destroy() + root.remove() + } + return { editor, hook, scrollTo, dispose } +} + +afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() +}) + +describe('rich markdown editing with Find open', () => { + it('updates highlights without moving the typing caret or scrolling to a match', () => { + vi.useFakeTimers() + const { editor, hook, scrollTo, dispose } = mountSearch() + try { + const insertion = editor.state.doc.content.size - 1 + act(() => editor.commands.setTextSelection(insertion)) + scrollTo.mockClear() + act(() => editor.commands.insertContent('X')) + act(() => editor.commands.insertContent('Y')) + expect(editor.getText()).toBe('beta beta\n\nEdit hereXY') + expect(editor.state.selection.from).toBe(insertion + 2) + expect(editor.state.selection.empty).toBe(true) + expect(hook.result.current.searchState.matchCount).toBe(2) + expect(hook.result.current.searchState.isSearchOpen).toBe(true) + expect(scrollTo).not.toHaveBeenCalled() + act(() => editor.commands.undo()) + expect(editor.getText()).toBe('beta beta\n\nEdit here') + expect(scrollTo).not.toHaveBeenCalled() + } finally { + dispose() + } + }) + + it('preserves selection when edits remove every match and later restore one', () => { + vi.useFakeTimers() + const { editor, hook, scrollTo, dispose } = mountSearch() + try { + scrollTo.mockClear() + act(() => editor.commands.setContent('

No results

End

')) + expect(hook.result.current.searchState.matchCount).toBe(0) + act(() => editor.commands.setTextSelection(editor.state.doc.content.size - 1)) + act(() => editor.commands.insertContent(' beta')) + expect(hook.result.current.searchState.matchCount).toBe(1) + expect(editor.state.selection.empty).toBe(true) + expect(editor.state.selection.from).toBe(editor.state.doc.content.size - 1) + expect(scrollTo).not.toHaveBeenCalled() + } finally { + dispose() + } + }) + + it('still navigates on Next with only one match, and advances after Replace', () => { + vi.useFakeTimers() + const { editor, hook, scrollTo, dispose } = mountSearch() + try { + act(() => hook.result.current.searchActions.setReplaceQuery('changed')) + act(() => hook.result.current.searchActions.replaceCurrentMatch()) + expect(editor.getText()).toBe('changed beta\n\nEdit here') + expect(hook.result.current.searchState.matchCount).toBe(1) + expect( + editor.state.doc.textBetween(editor.state.selection.from, editor.state.selection.to) + ).toBe('beta') + act(() => editor.commands.setTextSelection(editor.state.doc.content.size - 1)) + scrollTo.mockClear() + act(() => hook.result.current.searchActions.moveToMatch(1)) + expect( + editor.state.doc.textBetween(editor.state.selection.from, editor.state.selection.to) + ).toBe('beta') + expect(scrollTo).toHaveBeenCalledOnce() + } finally { + dispose() + } + }) + + it('does not apply delayed search navigation after focus returns to the document', () => { + vi.useFakeTimers() + const { editor, hook, scrollTo, dispose } = mountSearch() + try { + act(() => hook.result.current.searchActions.setSearchQuery('Edit')) + act(() => { + editor.view.dom.focus({ preventScroll: true }) + editor.commands.setTextSelection(editor.state.doc.content.size - 1) + }) + expect(editor.isFocused).toBe(true) + const selection = editor.state.selection + scrollTo.mockClear() + act(() => vi.advanceTimersByTime(150)) + expect(hook.result.current.searchState.matchCount).toBe(1) + expect(editor.state.selection.eq(selection)).toBe(true) + expect(scrollTo).not.toHaveBeenCalled() + } finally { + dispose() + } + }) +}) diff --git a/src/renderer/src/components/editor/useRichMarkdownSearch.ts b/src/renderer/src/components/editor/useRichMarkdownSearch.ts index 1a0f132e67c..bcc304c3463 100644 --- a/src/renderer/src/components/editor/useRichMarkdownSearch.ts +++ b/src/renderer/src/components/editor/useRichMarkdownSearch.ts @@ -1,6 +1,5 @@ import { useCallback, useEffect, useMemo, useRef, useState, type RefObject } from 'react' import type { Editor } from '@tiptap/react' -import { TextSelection } from '@tiptap/pm/state' import { getShortcutPlatform } from '@/lib/shortcut-platform' import { useAppStore } from '@/store' import { @@ -14,6 +13,7 @@ import { richMarkdownSearchPluginKey } from './rich-markdown-search' import { createRichMarkdownSearchMatchesCache } from './rich-markdown-search-matches-cache' +import { useRichMarkdownSearchHighlights } from './useRichMarkdownSearchHighlights' export function useRichMarkdownSearch({ editor, @@ -42,7 +42,8 @@ export function useRichMarkdownSearch({ const [matchCase, setMatchCase] = useState(false) const [wholeWord, setWholeWord] = useState(false) const [rawActiveMatchIndex, setRawActiveMatchIndex] = useState(-1) - const [searchRevision, setSearchRevision] = useState(0) + const [navigationRequest, setNavigationRequest] = useState({ revision: 0, selectMatch: false }) + const [, setSearchRevision] = useState(0) // Why: debouncing the query that drives match computation prevents the // expensive full-doc walk from running on every keystroke — the old // un-debounced path froze the main thread on large documents. @@ -59,24 +60,24 @@ export function useRichMarkdownSearch({ const searchRequestQuery = isMarkdownPreviewSearchQueryTooLarge(debouncedQuery) ? '' : debouncedQuery + const searchDocument = editor && !editor.isDestroyed ? editor.state.doc : null const matches = useMemo(() => { - if (!editor || !isSearchOpen || !searchRequestQuery) { + if (!searchDocument || !isSearchOpen || !searchRequestQuery) { return [] } - return findMatches(editor.state.doc, searchRequestQuery, { + return findMatches(searchDocument, searchRequestQuery, { matchCase, wholeWord }) - // searchRevision is bumped on ProseMirror doc edits to trigger recomputation - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [editor, findMatches, isSearchOpen, searchRequestQuery, searchRevision, matchCase, wholeWord]) + }, [findMatches, isSearchOpen, searchRequestQuery, searchDocument, matchCase, wholeWord]) const matchCount = matches.length const getLiveMatches = useCallback(() => { if ( !editor || + editor.isDestroyed || !isSearchOpen || !searchQuery || isMarkdownPreviewSearchQueryTooLarge(searchQuery) @@ -175,10 +176,14 @@ export function useRichMarkdownSearch({ if (!match || liveMatches.some((candidate) => candidate.touchesReadOnlyAtom)) { return } - // Why: removing the active match shifts the next match into the same index, - // so leaving rawActiveMatchIndex untouched advances to it after recompute. replaceRange(match.from, match.to) - }, [activeMatchIndex, getLiveMatches, replaceRange]) + // Skip matches inside the replacement, including when it still contains the query. + const replacementEnd = match.from + replaceQuery.length + const nextIndex = getLiveMatches().findIndex((candidate) => candidate.from >= replacementEnd) + setRawActiveMatchIndex(Math.max(0, nextIndex)) + setDebouncedQuery(searchQuery) + setNavigationRequest((request) => ({ revision: request.revision + 1, selectMatch: true })) + }, [activeMatchIndex, getLiveMatches, replaceQuery, replaceRange, searchQuery]) const replaceAllMatches = useCallback(() => { if (!editor) { @@ -203,24 +208,25 @@ export function useRichMarkdownSearch({ } } editor.view.dispatch(tr) - }, [editor, getLiveMatches, replaceQuery]) + setDebouncedQuery(searchQuery) + setNavigationRequest((request) => ({ revision: request.revision + 1, selectMatch: false })) + }, [editor, getLiveMatches, replaceQuery, searchQuery]) const moveToMatch = useCallback( (direction: 1 | -1) => { - if (matchCount === 0) { + const liveMatchCount = getLiveMatches().length + if (liveMatchCount === 0) { return } - // Why: rawActiveMatchIndex starts at -1 before the user navigates, but the - // derived activeMatchIndex is already 0 (first match shown). Using 0 as the - // base when raw is -1 ensures the first Enter press advances to match 1 - // instead of computing (-1+1)%N = 0 and leaving the effect unchanged. setRawActiveMatchIndex((currentIndex) => { - const baseIndex = Math.max(currentIndex, 0) - return (baseIndex + direction + matchCount) % matchCount + const baseIndex = currentIndex >= 0 && currentIndex < liveMatchCount ? currentIndex : 0 + return (baseIndex + direction + liveMatchCount) % liveMatchCount }) + setDebouncedQuery(searchQuery) + setNavigationRequest((request) => ({ revision: request.revision + 1, selectMatch: true })) }, - [matchCount] + [getLiveMatches, searchQuery] ) const handleEditorUpdate = useCallback(() => { @@ -259,52 +265,18 @@ export function useRichMarkdownSearch({ searchInputRef.current?.select() }, [isSearchOpen]) - // Why: single effect to sync search state to ProseMirror. The old two-effect - // chain (compute matches → set state → dispatch) caused an extra render cycle - // and called findRichMarkdownSearchMatches twice per change. - useEffect(() => { - if (!editor) { - return - } - - const query = isSearchOpen ? searchRequestQuery : '' - - // Why: combining decoration meta and selection+scrollIntoView into one - // transaction avoids a split-dispatch where the first dispatch updates - // editor.state and the second dispatch's scrollIntoView can be lost - // when ProseMirror coalesces view updates. - // Why: passing pre-computed matches avoids the plugin re-walking the - // entire document — the old double-walk froze the UI on large files. - const tr = editor.state.tr - tr.setMeta(richMarkdownSearchPluginKey, { - activeIndex: activeMatchIndex, - matches, - query - }) - - const activeMatch = query && activeMatchIndex >= 0 ? matches[activeMatchIndex] : null - if (activeMatch) { - tr.setSelection(TextSelection.create(tr.doc, activeMatch.from, activeMatch.to)) - } - - editor.view.dispatch(tr) - - // Why: ProseMirror's tr.scrollIntoView() delegates to the view's - // scrollDOMIntoView which may fail to reach the outer flex scroll container - // (the editor element itself has min-height: 100% and no overflow). - // Reading coordsAtPos *after* the dispatch and manually scrolling the - // container mirrors the approach used by MarkdownPreview search. - if (activeMatch) { - const container = scrollContainerRef.current - if (container) { - const coords = editor.view.coordsAtPos(activeMatch.from) - const containerRect = container.getBoundingClientRect() - const relativeTop = coords.top - containerRect.top - const targetScroll = container.scrollTop + relativeTop - containerRect.height / 2 - container.scrollTo({ top: targetScroll, behavior: 'instant' }) - } - } - }, [activeMatchIndex, searchRequestQuery, editor, isSearchOpen, matches, scrollContainerRef]) + useRichMarkdownSearchHighlights({ + activeMatchIndex, + editor, + matchCase, + matches, + navigationRequest, + query: isSearchOpen ? searchRequestQuery : '', + scrollContainerRef, + wholeWord, + rootRef, + searchDocument + }) useEffect(() => { const handleKeyDown = (event: KeyboardEvent): void => { diff --git a/src/renderer/src/components/editor/useRichMarkdownSearchHighlights.ts b/src/renderer/src/components/editor/useRichMarkdownSearchHighlights.ts new file mode 100644 index 00000000000..c57c2a4e973 --- /dev/null +++ b/src/renderer/src/components/editor/useRichMarkdownSearchHighlights.ts @@ -0,0 +1,104 @@ +import { useEffect, useRef, type RefObject } from 'react' +import type { Editor } from '@tiptap/react' +import type { Node as ProseMirrorNode } from '@tiptap/pm/model' +import { TextSelection } from '@tiptap/pm/state' +import { richMarkdownSearchPluginKey, type RichMarkdownSearchMatch } from './rich-markdown-search' + +type SearchNavigation = { + editor: Editor + query: string + matchCase: boolean + wholeWord: boolean + navigationRequest: { revision: number; selectMatch: boolean } +} + +export function useRichMarkdownSearchHighlights({ + activeMatchIndex, + editor, + matchCase, + matches, + navigationRequest, + query, + rootRef, + searchDocument, + scrollContainerRef, + wholeWord +}: { + activeMatchIndex: number + editor: Editor | null + matchCase: boolean + matches: RichMarkdownSearchMatch[] + navigationRequest: SearchNavigation['navigationRequest'] + query: string + rootRef: RefObject + searchDocument: ProseMirrorNode | null + scrollContainerRef: RefObject + wholeWord: boolean +}): void { + const lastNavigationRef = useRef(null) + + useEffect(() => { + if (!editor || editor.isDestroyed) { + lastNavigationRef.current = null + return + } + // A newer editor transaction can arrive between render and this effect. + if (editor.state.doc !== searchDocument) { + return + } + const previous = lastNavigationRef.current + const searchChanged = + previous?.editor !== editor || + previous.query !== query || + previous.matchCase !== matchCase || + previous.wholeWord !== wholeWord + const navigationRequested = + previous?.editor === editor && previous.navigationRequest !== navigationRequest + lastNavigationRef.current = { editor, query, matchCase, wholeWord, navigationRequest } + + // Refreshing matches after an edit must preserve the user's caret and viewport. + const activeElement = rootRef.current?.ownerDocument.activeElement + const searchOwnsFocus = + activeElement && + rootRef.current?.contains(activeElement) && + activeElement.closest('.rich-markdown-search') + const shouldNavigate = navigationRequested + ? navigationRequest.selectMatch + : searchChanged && searchOwnsFocus + const activeMatch = + shouldNavigate && query && activeMatchIndex >= 0 ? matches[activeMatchIndex] : null + const tr = editor.state.tr.setMeta(richMarkdownSearchPluginKey, { + activeIndex: activeMatchIndex, + matches, + query + }) + if (activeMatch) { + tr.setSelection(TextSelection.create(tr.doc, activeMatch.from, activeMatch.to)) + } + editor.view.dispatch(tr) + if (editor.isDestroyed || editor.state.doc !== tr.doc) { + return + } + + // The editor's scrollIntoView does not reliably reach the outer flex viewport. + const container = scrollContainerRef.current + if (activeMatch && container) { + const coords = editor.view.coordsAtPos(activeMatch.from) + const containerRect = container.getBoundingClientRect() + const relativeTop = coords.top - containerRect.top + const targetScroll = container.scrollTop + relativeTop - containerRect.height / 2 + container.scrollTo({ top: targetScroll, behavior: 'instant' }) + } + }, [ + activeMatchIndex, + editor, + matchCase, + matches, + navigationRequest, + query, + rootRef, + searchDocument, + scrollContainerRef, + wholeWord + ]) +} diff --git a/tests/e2e/markdown-find-adversarial.spec.ts b/tests/e2e/markdown-find-adversarial.spec.ts new file mode 100644 index 00000000000..99ebe2282cc --- /dev/null +++ b/tests/e2e/markdown-find-adversarial.spec.ts @@ -0,0 +1,307 @@ +import type { Locator, Page } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { pressShortcut } from './helpers/shortcuts' +import { + cleanupMarkdownFixture, + createMarkdownFixture, + getActiveWorktreeContext, + openMarkdownFixture, + waitForRichMarkdownEditor +} from './helpers/markdown-editor-fixture' + +const FIRST = 'First alpha paragraph for pointer selection.' +const SECOND = 'Second beta paragraph for pointer selection.' +const SOURCE = [ + '# Adversarial Find interactions', + 'Needle first match.', + ...Array.from({ length: 65 }, (_, index) => `Spacer ${index} keeps the search match away.`), + FIRST, + SECOND, + '| Left | Right |\n| --- | --- |\n| Cell alpha | Cell beta |\n| Cell gamma | Cell delta |', + '- [ ] Task marker', + '
\nDetails marker\n\nDetails body marker\n\n
', + '```javascript\nconst codeMarker = "clean";\n```', + 'Late target paragraph for a pending query.', + ...Array.from({ length: 30 }, (_, index) => `Tail spacer ${index} keeps the second match away.`), + 'Needle final match.' +].join('\n\n') + +async function textPoint(target: Locator, offset: number) { + return target.evaluate((element, offset) => { + const walker = document.createTreeWalker(element, NodeFilter.SHOW_TEXT) + let remaining = offset + let node = walker.nextNode() + while (node) { + if (node instanceof Text && remaining <= node.length) { + const range = document.createRange() + range.setStart(node, remaining) + range.collapse(true) + const bounds = range.getBoundingClientRect() + return { x: bounds.left, y: bounds.top + bounds.height / 2 } + } + remaining -= node.textContent?.length ?? 0 + node = walker.nextNode() + } + throw new Error('Text offset was not found') + }, offset) +} + +async function selectedText(page: Page) { + return page.evaluate(() => window.getSelection()?.toString() ?? '') +} + +async function centered(target: Locator) { + await target.evaluate((element) => element.scrollIntoView({ block: 'center' })) +} + +async function find(page: Page, query = 'Needle', status = '1/2') { + await pressShortcut(page, 'f') + const input = page.getByRole('textbox', { name: 'Find in rich markdown editor' }) + await expect(input).toBeFocused() + await input.fill(query) + await expect(page.locator('.rich-markdown-search-status')).toHaveText(status) + return input +} + +async function drag(page: Page, start: Locator, end: Locator, from: number, to: number) { + const firstPoint = await textPoint(start, from) + const lastPoint = await textPoint(end, to) + await page.mouse.move(firstPoint.x, firstPoint.y) + await page.mouse.down() + await page.mouse.move(lastPoint.x, lastPoint.y, { steps: 15 }) + await page.mouse.up() +} + +test.beforeEach(async ({ orcaPage, registerPostElectronShutdownCleanup }, testInfo) => { + const context = await getActiveWorktreeContext(orcaPage) + const filePath = await createMarkdownFixture( + context, + '.orca-e2e-markdown-adversarial', + 'find-interactions', + testInfo.workerIndex, + SOURCE + ) + registerPostElectronShutdownCleanup(() => cleanupMarkdownFixture(filePath)) + await openMarkdownFixture(orcaPage, context, filePath) + const editor = await waitForRichMarkdownEditor(orcaPage) + await editor.locator('p').first().click() +}) + +test('Find preserves Shift-click, double-click, and multi-paragraph copy selection', async ({ + orcaPage +}, testInfo) => { + const editor = orcaPage.locator('.rich-markdown-editor') + const first = editor.getByText(FIRST, { exact: true }) + const second = editor.getByText(SECOND, { exact: true }) + const search = await find(orcaPage) + await centered(first) + const viewport = orcaPage.locator('.rich-markdown-editor-shell .overflow-auto') + const scroll = await viewport.evaluate((element) => element.scrollTop) + const start = await textPoint(first, 6) + const end = await textPoint(second, 11) + await orcaPage.mouse.click(start.x, start.y) + await orcaPage.keyboard.down('Shift') + await orcaPage.mouse.click(end.x, end.y) + await orcaPage.keyboard.up('Shift') + expect(await selectedText(orcaPage)).toBe(`${FIRST.slice(6)}\n\n${SECOND.slice(0, 11)}`) + await expect(editor).toBeFocused() + await expect( + orcaPage + .locator('[data-tab-id]') + .filter({ hasText: 'find-interactions' }) + .last() + .locator('span.rounded-full') + ).toHaveCount(0) + await orcaPage.mouse.click(start.x, start.y) + await search.focus() + await orcaPage.keyboard.down('Shift') + await orcaPage.mouse.click(end.x, end.y) + await orcaPage.keyboard.up('Shift') + expect(await selectedText(orcaPage)).toBe(`${FIRST.slice(6)}\n\n${SECOND.slice(0, 11)}`) + const copied = await editor.evaluate((element) => { + const clipboard = new DataTransfer() + element.dispatchEvent( + new ClipboardEvent('copy', { bubbles: true, cancelable: true, clipboardData: clipboard }) + ) + return { plain: clipboard.getData('text/plain'), html: clipboard.getData('text/html') } + }) + expect(copied.plain).toContain(FIRST.slice(6)) + expect(copied.plain).toContain(SECOND.slice(0, 11)) + expect(copied.html).toContain('

') + await expect(editor.getByText('Needle first match.', { exact: true })).toBeVisible() + await expect.poll(() => viewport.evaluate((element) => element.scrollTop)).toBeCloseTo(scroll, 0) + await orcaPage.screenshot({ path: testInfo.outputPath('shift-click-copy-selection.png') }) + await search.focus() + const word = await textPoint(first, 9) + await orcaPage.mouse.dblclick(word.x, word.y) + expect(await selectedText(orcaPage)).toBe('alpha') + await orcaPage.keyboard.type('xy', { delay: 100 }) + await expect( + editor.getByText('First xy paragraph for pointer selection.', { exact: true }) + ).toBeVisible() + await expect(search).toHaveValue('Needle') + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('1/2') +}) + +test('Replace input returns to a multi-paragraph drag and explicit next-match navigation', async ({ + orcaPage +}, testInfo) => { + const editor = orcaPage.locator('.rich-markdown-editor') + const search = await find(orcaPage) + await orcaPage.getByRole('button', { name: 'Toggle replace', exact: true }).click() + const replace = orcaPage.getByRole('textbox', { name: 'Replace in rich markdown editor' }) + await replace.fill('Thread') + const first = editor.getByText(FIRST, { exact: true }) + const second = editor.getByText(SECOND, { exact: true }) + await centered(first) + await drag(orcaPage, first, second, 6, 11) + expect(await selectedText(orcaPage)).toBe(`${FIRST.slice(6)}\n\n${SECOND.slice(0, 11)}`) + await orcaPage.keyboard.type('xy', { delay: 100 }) + await expect( + editor.getByText(`${FIRST.slice(0, 6)}xy${SECOND.slice(11)}`, { exact: true }) + ).toBeVisible() + await orcaPage.getByRole('button', { name: 'Next match', exact: true }).click() + await expect.poll(() => selectedText(orcaPage)).toBe('Needle') + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('2/2') + await replace.focus() + await orcaPage.getByRole('button', { name: 'Replace', exact: true }).click() + await expect(editor.getByText('Thread final match.', { exact: true })).toBeVisible() + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('1/1') + await expect(replace).toBeFocused() + await orcaPage.keyboard.press('Escape') + await expect(search).toHaveCount(0) + await pressShortcut(orcaPage, 'f') + await expect(search).toBeFocused() + await expect(search).toHaveValue('') + await orcaPage.screenshot({ path: testInfo.outputPath('replace-next-reopen.png') }) +}) + +test('Find returns focus and copies the intended cells after Shift-clicking a table', async ({ + orcaPage +}, testInfo) => { + const editor = orcaPage.locator('.rich-markdown-editor') + const search = await find(orcaPage, 'Cell alpha', '1/1') + const firstCell = editor.getByText('Cell alpha', { exact: true }) + const lastCell = editor.getByText('Cell delta', { exact: true }) + await centered(firstCell) + const viewport = orcaPage.locator('.rich-markdown-editor-shell .overflow-auto') + const scroll = await viewport.evaluate((element) => element.scrollTop) + const point = await textPoint(lastCell, 6) + await orcaPage.keyboard.down('Shift') + await orcaPage.mouse.move(point.x, point.y) + await orcaPage.mouse.down() + await orcaPage.evaluate( + () => + new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + }) + ) + await orcaPage.mouse.up() + await orcaPage.keyboard.up('Shift') + await expect(editor.locator('td.selectedCell')).toHaveCount(4) + await orcaPage.screenshot({ path: testInfo.outputPath('shift-cell-selection.png') }) + await expect(editor).toBeFocused() + await expect(search).toHaveValue('Cell alpha') + await expect.poll(() => viewport.evaluate((element) => element.scrollTop)).toBeCloseTo(scroll, 0) + const tab = orcaPage.locator('[data-tab-id]').filter({ hasText: 'find-interactions' }).last() + await expect(tab.locator('span.rounded-full')).toHaveCount(0) + const copied = await editor.evaluate((element) => { + const clipboard = new DataTransfer() + element.dispatchEvent( + new ClipboardEvent('copy', { + bubbles: true, + cancelable: true, + clipboardData: clipboard + }) + ) + return clipboard.getData('text/plain') + }) + expect(copied).toBe('Cell alpha\n\nCell beta\n\nCell gamma\n\nCell delta') +}) + +test('Find preserves embedded task, details, and code editing', async ({ orcaPage }, testInfo) => { + const editor = orcaPage.locator('.rich-markdown-editor') + const search = await find(orcaPage) + await centered(editor.getByRole('checkbox')) + await search.focus() + const checkbox = editor.getByRole('checkbox') + await checkbox.check() + await expect(checkbox).toBeChecked() + await expect(search).toHaveValue('Needle') + await search.focus() + const details = editor.locator('[data-type="details"]') + await details.getByRole('button').click() + await expect(details.locator('[data-type="detailsContent"]')).toBeHidden() + await search.focus() + await details.getByRole('button').click() + await expect(details.locator('[data-type="detailsContent"]')).toBeVisible() + const body = editor.getByText('Details body marker', { exact: true }) + const bodyPoint = await textPoint(body, 7) + await search.focus() + await orcaPage.mouse.click(bodyPoint.x, bodyPoint.y) + await orcaPage.keyboard.type('xy', { delay: 100 }) + await expect(editor.getByText('Detailsxy body marker', { exact: true })).toBeVisible() + const code = editor.getByText('const codeMarker = "clean";', { exact: true }) + await centered(code) + const codePoint = await textPoint(code, 6) + await search.focus() + await orcaPage.mouse.click(codePoint.x, codePoint.y) + await orcaPage.keyboard.type('xy', { delay: 100 }) + await expect(editor.getByText('const xycodeMarker = "clean";', { exact: true })).toBeVisible() + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('1/2') + await orcaPage.screenshot({ path: testInfo.outputPath('embedded-control-editing.png') }) +}) + +test('a pending Find query cannot claim focus after document and checkbox interaction', async ({ + orcaPage +}, testInfo) => { + const editor = orcaPage.locator('.rich-markdown-editor') + const search = await find(orcaPage) + const target = editor.getByText('Late target paragraph for a pending query.', { exact: true }) + await centered(target) + const point = await textPoint(target, 5) + const viewport = orcaPage.locator('.rich-markdown-editor-shell .overflow-auto') + const scroll = await viewport.evaluate((element) => element.scrollTop) + const checkbox = editor.getByRole('checkbox') + const checkboxPoint = await checkbox.evaluate((element) => { + const bounds = element.getBoundingClientRect() + return { x: bounds.left + bounds.width / 2, y: bounds.top + bounds.height / 2 } + }) + await search.fill('Cell alpha') + await orcaPage.mouse.click(point.x, point.y) + await expect(editor).toBeFocused() + await orcaPage.mouse.click(checkboxPoint.x, checkboxPoint.y) + await expect(checkbox).toBeChecked() + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('1/1') + await expect.poll(() => viewport.evaluate((element) => element.scrollTop)).toBeCloseTo(scroll, 0) + await expect(target).toHaveText('Late target paragraph for a pending query.') + await orcaPage.mouse.click(point.x, point.y) + await orcaPage.keyboard.type('xy', { delay: 100 }) + await expect( + editor.getByText('Late xytarget paragraph for a pending query.', { exact: true }) + ).toBeVisible() + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('1/1') + await expect.poll(() => viewport.evaluate((element) => element.scrollTop)).toBeCloseTo(scroll, 0) + await expect(search).toHaveValue('Cell alpha') + await orcaPage.screenshot({ path: testInfo.outputPath('pending-query-editor-caret.png') }) +}) + +test('Replace advances when its replacement still contains the search query', async ({ + orcaPage +}, testInfo) => { + const editor = orcaPage.locator('.rich-markdown-editor') + await find(orcaPage) + await orcaPage.getByRole('button', { name: 'Toggle replace', exact: true }).click() + await orcaPage.getByRole('textbox', { name: 'Replace in rich markdown editor' }).fill('NeedleX') + await orcaPage.getByRole('button', { name: 'Replace', exact: true }).click() + await expect(editor.getByText('NeedleX first match.', { exact: true })).toBeVisible() + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('2/2') + await expect( + editor.getByText('Needle final match.', { exact: true }).locator('[data-active="true"]') + ).toHaveText('Needle') + await orcaPage.getByRole('button', { name: 'Replace', exact: true }).click() + await expect(editor.getByText('NeedleX final match.', { exact: true })).toBeVisible() + await expect(editor.getByText('NeedleX first match.', { exact: true })).toBeVisible() + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('1/2') + await orcaPage.screenshot({ path: testInfo.outputPath('replacement-retains-query.png') }) +}) diff --git a/tests/e2e/markdown-find-editing.spec.ts b/tests/e2e/markdown-find-editing.spec.ts new file mode 100644 index 00000000000..b227ba81390 --- /dev/null +++ b/tests/e2e/markdown-find-editing.spec.ts @@ -0,0 +1,215 @@ +import type { Locator, Page } from '@stablyai/playwright-test' +import path from 'node:path' +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { pressShortcut } from './helpers/shortcuts' +import { + cleanupMarkdownFixture, + createMarkdownFixture, + getActiveWorktreeContext, + openMarkdownFixture, + waitForRichMarkdownEditor +} from './helpers/markdown-editor-fixture' + +const TARGET_TEXT = 'Editing target paragraph keeps the pointer position and viewport.' +const MARKDOWN = [ + '# Find and pointer editing', + 'Needle remains at the beginning.', + ...Array.from({ length: 100 }, (_, index) => + index === 70 ? TARGET_TEXT : `Paragraph ${index} provides room to scroll through the document.` + ), + 'Needle remains at the end.' +].join('\n\n') +const TABLE_MARKDOWN = MARKDOWN.replace( + TARGET_TEXT, + `| Name | Value |\n| --- | --- |\n| Target | ${TARGET_TEXT} |` +) + +async function textPoint(paragraph: Locator, offset: number) { + return paragraph.evaluate((element, offset) => { + const text = element.firstChild + if (!(text instanceof Text)) { + throw new Error('Expected a plain-text paragraph') + } + const range = document.createRange() + range.setStart(text, offset) + range.collapse(true) + const bounds = range.getBoundingClientRect() + return { x: bounds.left, y: bounds.top + bounds.height / 2 } + }, offset) +} + +async function readParagraphSelection(paragraph: Locator) { + return paragraph.evaluate((element) => { + const selection = window.getSelection() + if (!selection?.anchorNode || !selection.focusNode) { + return null + } + if (!element.contains(selection.anchorNode) || !element.contains(selection.focusNode)) { + return null + } + const start = document.createRange() + start.selectNodeContents(element) + start.setEnd(selection.anchorNode, selection.anchorOffset) + const end = document.createRange() + end.selectNodeContents(element) + end.setEnd(selection.focusNode, selection.focusOffset) + return { + from: Math.min(start.toString().length, end.toString().length), + to: Math.max(start.toString().length, end.toString().length), + text: selection.toString() + } + }) +} + +async function centerParagraph(paragraph: Locator): Promise { + await paragraph.evaluate((element) => element.scrollIntoView({ block: 'center' })) +} + +async function pointAtParagraph(page: Page, paragraph: Locator, drag: boolean) { + const start = await textPoint(paragraph, 8) + await page.mouse.move(start.x, start.y) + await page.mouse.down() + if (drag) { + const end = await textPoint(paragraph, 24) + await page.mouse.move(end.x, end.y, { steps: 12 }) + } + // Tiptap focus commands can schedule a stale-selection scroll for the next frame. + await page.evaluate( + () => + new Promise((resolve) => { + requestAnimationFrame(() => requestAnimationFrame(() => resolve())) + }) + ) + await page.mouse.up() + const selection = await readParagraphSelection(paragraph) + expect(selection).not.toBeNull() + if (!selection) { + throw new Error('Pointer selection left the intended paragraph') + } + if (drag) { + expect(selection.text).toBe('target paragraph') + } else { + expect(selection.from).toBe(selection.to) + } + return selection +} + +test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) +}) + +for (const interaction of ['click', 'drag'] as const) { + test(`keeps Find open while a document ${interaction} edits at the pointer position`, async ({ + orcaPage, + registerPostElectronShutdownCleanup + }, testInfo) => { + const context = await getActiveWorktreeContext(orcaPage) + const filePath = await createMarkdownFixture( + context, + '.orca-e2e-markdown-find-editing', + `find-${interaction}`, + testInfo.workerIndex, + MARKDOWN + ) + registerPostElectronShutdownCleanup(() => cleanupMarkdownFixture(filePath)) + await openMarkdownFixture(orcaPage, context, filePath) + const editor = await waitForRichMarkdownEditor(orcaPage) + const paragraph = editor.locator('p').filter({ hasText: 'Editing' }) + const viewport = orcaPage.locator('.rich-markdown-editor-shell .overflow-auto') + await editor.locator('p').first().click() + await pressShortcut(orcaPage, 'f') + const search = orcaPage.getByRole('textbox', { name: 'Find in rich markdown editor' }) + await expect(search).toBeFocused() + await search.fill('Needle') + await orcaPage.getByRole('button', { name: 'Match case', exact: true }).click() + await orcaPage.getByRole('button', { name: 'Match whole word', exact: true }).click() + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('1/2') + await centerParagraph(paragraph) + const originalScroll = await viewport.evaluate((element) => element.scrollTop) + const selection = await pointAtParagraph(orcaPage, paragraph, interaction === 'drag') + + // Separate key events allow document updates to expose a caret reset between characters. + await orcaPage.keyboard.type('xy', { delay: 100 }) + await orcaPage.screenshot({ path: testInfo.outputPath('find-document-edit.png') }) + await expect(paragraph).toHaveText( + `${TARGET_TEXT.slice(0, selection.from)}xy${TARGET_TEXT.slice(selection.to)}` + ) + await expect(editor.locator('p').first()).toHaveText('Needle remains at the beginning.') + await expect + .poll(() => readParagraphSelection(paragraph)) + .toEqual({ + from: selection.from + 2, + to: selection.from + 2, + text: '' + }) + await expect + .poll(() => viewport.evaluate((element) => element.scrollTop)) + .toBeCloseTo(originalScroll, 0) + await expect(search).toBeVisible() + await expect(search).toHaveValue('Needle') + await expect(orcaPage.getByRole('button', { name: 'Match case', exact: true })).toHaveAttribute( + 'aria-pressed', + 'true' + ) + await expect( + orcaPage.getByRole('button', { name: 'Match whole word', exact: true }) + ).toHaveAttribute('aria-pressed', 'true') + await expect(orcaPage.locator('.rich-markdown-search-status')).toHaveText('1/2') + }) + + test(`keeps the scrolled table and ${interaction} selection after switching Source to Rich`, async ({ + orcaPage, + registerPostElectronShutdownCleanup + }, testInfo) => { + const context = await getActiveWorktreeContext(orcaPage) + const filePath = await createMarkdownFixture( + context, + '.orca-e2e-markdown-find-editing', + `source-${interaction}`, + testInfo.workerIndex, + TABLE_MARKDOWN + ) + registerPostElectronShutdownCleanup(() => cleanupMarkdownFixture(filePath)) + await openMarkdownFixture(orcaPage, context, filePath) + await waitForRichMarkdownEditor(orcaPage) + await orcaPage.getByRole('radio', { name: 'Source', exact: true }).click() + await expect(orcaPage.locator('.monaco-editor')).toBeVisible() + await orcaPage.getByRole('radio', { name: 'Rich Editor', exact: true }).click() + const editor = await waitForRichMarkdownEditor(orcaPage) + await expect(editor).not.toBeFocused() + const paragraph = editor.locator('td p').filter({ hasText: 'Editing' }) + const viewport = orcaPage.locator('.rich-markdown-editor-shell .overflow-auto') + await centerParagraph(paragraph) + const originalScroll = await viewport.evaluate((element) => element.scrollTop) + const selection = await pointAtParagraph(orcaPage, paragraph, interaction === 'drag') + await expect(editor).toBeFocused() + await expect(paragraph).toHaveText(TARGET_TEXT) + await expect + .poll(() => viewport.evaluate((element) => element.scrollTop)) + .toBeCloseTo(originalScroll, 0) + const tab = orcaPage + .locator('[data-tab-id]') + .filter({ hasText: path.basename(filePath) }) + .last() + await expect(tab.locator('span.rounded-full')).toHaveCount(0) + await expect(tab.getByRole('button', { name: 'Close tab' })).toBeVisible() + await orcaPage.screenshot({ path: testInfo.outputPath('source-rich-table-selection.png') }) + await orcaPage.keyboard.type('xy', { delay: 100 }) + await orcaPage.screenshot({ path: testInfo.outputPath('source-rich-pointer-edit.png') }) + await expect(paragraph).toHaveText( + `${TARGET_TEXT.slice(0, selection.from)}xy${TARGET_TEXT.slice(selection.to)}` + ) + await expect + .poll(() => readParagraphSelection(paragraph)) + .toEqual({ + from: selection.from + 2, + to: selection.from + 2, + text: '' + }) + await expect + .poll(() => viewport.evaluate((element) => element.scrollTop)) + .toBeCloseTo(originalScroll, 0) + }) +}