From 81dd08ffd3acf0663ca2a0693e9dde14b7533904 Mon Sep 17 00:00:00 2001 From: Wolfie Date: Sat, 11 Jul 2026 20:19:50 -0700 Subject: [PATCH] fix(browser): make fill update rich text editors via execCommand (#6060) Rich editors reconcile browser editing transactions, while direct value/textContent writes can leave framework state stale. Classify explicit contenteditables through the requested agent-browser target, keep native fill and clear behavior for plain controls, and perform rich replacement or clear as one target-focused eval over stdin. Fail when the browser editing command is unavailable instead of presenting stale DOM state. Preserves input/change behavior and spinbutton handling for standard fields. Verified against Draft.js and ProseMirror model state plus focused-target clear regressions. Co-authored-by: Wolfgang Schoenberger <221313372+wolfiesch@users.noreply.github.com> Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com> --- src/main/browser/agent-browser-bridge.test.ts | 150 +++++++++++++++++- src/main/browser/agent-browser-bridge.ts | 130 ++++++++++++--- 2 files changed, 250 insertions(+), 30 deletions(-) diff --git a/src/main/browser/agent-browser-bridge.test.ts b/src/main/browser/agent-browser-bridge.test.ts index 55b650c73..f55f726f3 100644 --- a/src/main/browser/agent-browser-bridge.test.ts +++ b/src/main/browser/agent-browser-bridge.test.ts @@ -1,14 +1,14 @@ /* eslint-disable max-lines */ import { describe, it, expect, vi, beforeEach } from 'vitest' -const { execFileMock, webContentsFromIdMock, existsSyncMock, readFileSyncMock } = vi.hoisted( - () => ({ +const { execFileMock, webContentsFromIdMock, existsSyncMock, readFileSyncMock, stdinWrites } = + vi.hoisted(() => ({ execFileMock: vi.fn(), webContentsFromIdMock: vi.fn(), existsSyncMock: vi.fn(() => false), - readFileSyncMock: vi.fn(() => Buffer.from('')) - }) -) + readFileSyncMock: vi.fn(() => Buffer.from('')), + stdinWrites: [] as string[] + })) vi.mock('child_process', () => ({ execFile: execFileMock })) vi.mock('fs', () => ({ @@ -108,6 +108,22 @@ function mockWebContents(id: number, url = 'https://example.com', title = 'Examp function succeedWith(data: unknown): void { execFileMock.mockImplementation((_bin: string, _args: string[], _opts: unknown, cb: Function) => { cb(null, JSON.stringify({ success: true, data }), '') + return { + stdin: { on: vi.fn(), end: (text: string) => stdinWrites.push(text) } + } + }) +} + +function succeedForContentEditable(data: unknown = { ok: true }): void { + execFileMock.mockImplementation((_bin: string, args: string[], _opts: unknown, cb: Function) => { + const result = + args.includes('get') && args.includes('attr') && args.includes('contenteditable') + ? { value: 'true' } + : data + cb(null, JSON.stringify({ success: true, data: result }), '') + return { + stdin: { on: vi.fn(), end: (text: string) => stdinWrites.push(text) } + } }) } @@ -207,10 +223,57 @@ function createFillEvalNode(options: { function runFillEvalExpressions( expressions: string[], - document: { activeElement: unknown; getElementById: (id: string) => unknown } + document: { activeElement: unknown; getElementById: (id: string) => unknown }, + windowObject: Record = {} ): void { for (const expression of expressions) { - new Function('document', 'Event', `return (${expression})`)(document, TestEvent) + new Function('document', 'Event', 'window', `return (${expression})`)( + document, + TestEvent, + windowObject + ) + } +} + +function createContentEditableEvalEnvironment(initialText: string) { + const editor = { + tagName: 'DIV', + isContentEditable: true, + textContent: initialText, + matches: vi.fn(() => false), + getAttribute: vi.fn((name: string) => (name === 'contenteditable' ? 'true' : null)), + dispatchEvent: vi.fn() + } + let selected = false + const selection = { + selectAllChildren: vi.fn(() => { + selected = true + }) + } + const execCommand = vi.fn((command: string, _showUi: boolean, value: string) => { + // Chromium treats an empty insertText as a successful no-op; deletion is + // required to clear a selected contenteditable through the input pipeline. + if (command === 'delete') { + if (!selected) { + return false + } + editor.textContent = '' + } else if (value.length > 0) { + editor.textContent = selected ? value : editor.textContent + value + } + selected = false + return true + }) + return { + editor, + execCommand, + document: { + activeElement: editor, + body: {}, + getElementById: () => null, + execCommand + }, + windowObject: { getSelection: () => selection } } } @@ -222,6 +285,7 @@ describe('AgentBrowserBridge', () => { beforeEach(() => { vi.clearAllMocks() + stdinWrites.length = 0 CdpWsProxyMock.instances.length = 0 existsSyncMock.mockReturnValue(false) readFileSyncMock.mockReturnValue(Buffer.from('')) @@ -1529,6 +1593,78 @@ describe('AgentBrowserBridge', () => { expect(() => new Function(expression)).not.toThrow() }) + it('replaces contenteditable text through the browser editing pipeline', async () => { + succeedForContentEditable() + const environment = createContentEditableEvalEnvironment('existing text') + + await bridge.fill('@editor', 'replacement text') + + runFillEvalExpressions(stdinWrites, environment.document, environment.windowObject) + + expect(environment.editor.textContent).toBe('replacement text') + expect(environment.execCommand).toHaveBeenCalledWith('insertText', false, 'replacement text') + expect(environment.editor.dispatchEvent).not.toHaveBeenCalled() + }) + + it('clears selected contenteditable text with a browser delete command', async () => { + succeedForContentEditable() + const environment = createContentEditableEvalEnvironment('existing text') + + const result = await bridge.clear('@editor') + + runFillEvalExpressions(stdinWrites, environment.document, environment.windowObject) + + expect(environment.editor.textContent).toBe('') + expect(environment.execCommand).toHaveBeenCalledWith('delete', false, '') + expect(environment.editor.dispatchEvent).not.toHaveBeenCalled() + expect(result).toEqual({ cleared: '@editor' }) + }) + + it('fails contenteditable fill when the browser editing command is unavailable', async () => { + succeedForContentEditable() + const environment = createContentEditableEvalEnvironment('existing text') + environment.execCommand.mockReturnValue(false) + + await bridge.fill('@editor', 'replacement text') + + expect(() => + runFillEvalExpressions(stdinWrites, environment.document, environment.windowObject) + ).toThrow('Browser rich-text editing command failed') + expect(environment.editor.textContent).toBe('existing text') + expect(environment.editor.dispatchEvent).not.toHaveBeenCalled() + }) + + it('inserts paste-sized contenteditable text as one stdin editing transaction', async () => { + const firstChunk = 'x'.repeat(AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES) + succeedForContentEditable() + const environment = createContentEditableEvalEnvironment('existing text') + + await bridge.fill('@editor', `${firstChunk}tail`) + + const evalCalls = execFileMock.mock.calls.filter((call: unknown[]) => + (call[1] as string[]).includes('eval') + ) + runFillEvalExpressions(stdinWrites, environment.document, environment.windowObject) + + expect(evalCalls).toHaveLength(1) + expect(evalCalls[0][1]).toContain('--stdin') + expect(stdinWrites).toHaveLength(1) + expect(environment.editor.textContent).toBe(`${firstChunk}tail`) + }) + + it('uses target-aware agent-browser fill when clearing a non-rich target', async () => { + succeedWith({ filled: '@disabled' }) + + const result = await bridge.clear('@disabled') + + const commandArgs = execFileMock.mock.calls.map((call: unknown[]) => call[1] as string[]) + expect(commandArgs.some((args) => args.includes('eval'))).toBe(false) + expect(commandArgs.some((args) => args.includes('fill') && args.includes('@disabled'))).toBe( + true + ) + expect(result).toEqual({ cleared: '@disabled' }) + }) + it('routes focused spinbutton wrappers to editable descendants before filling', async () => { succeedWith({ ok: true }) diff --git a/src/main/browser/agent-browser-bridge.ts b/src/main/browser/agent-browser-bridge.ts index 4f460a56a..80817c81d 100644 --- a/src/main/browser/agent-browser-bridge.ts +++ b/src/main/browser/agent-browser-bridge.ts @@ -118,10 +118,49 @@ function focusedValueSetExpression( ].join('') } +// Why: rich editors reconcile only browser editing transactions; direct DOM +// fallback can look correct while leaving their model stale. +function focusedRichTextEditExpression( + valueExpression: string, + options?: { selectAll?: boolean } +): string { + const selectAll = options?.selectAll ? 'true' : 'false' + return [ + '(() => {', + ' const target = document.activeElement;', + ' const value = ', + valueExpression, + ';', + ` const selectAll = ${selectAll};`, + " const isEditable = target?.isContentEditable === true || /^(|true|plaintext-only)$/i.test(target?.getAttribute?.('contenteditable') ?? 'false');", + " if (!target || target === document.body || !isEditable) { throw new Error('Focused rich-text target is unavailable'); }", + ' if (selectAll) {', + " if (typeof window.getSelection !== 'function') { throw new Error('Rich-text selection is unavailable'); }", + ' const selection = window.getSelection();', + " if (!selection) { throw new Error('Rich-text selection is unavailable'); }", + ' selection.selectAllChildren(target);', + ' }', + " const editCommand = selectAll && value.length === 0 ? 'delete' : 'insertText';", + ' let edited = false;', + ' try {', + ' edited = document.execCommand(editCommand, false, value) === true;', + ' } catch { edited = false; }', + " if (!edited) { throw new Error('Browser rich-text editing command failed'); }", + ' })()' + ].join('') +} + +function isExplicitContentEditableResult(result: unknown): boolean { + const value = + result && typeof result === 'object' ? (result as { value?: unknown }).value : undefined + return typeof value === 'string' && /^(|true|plaintext-only)$/i.test(value) +} + type AgentBrowserExecOptions = { envOverrides?: NodeJS.ProcessEnv timeoutMs?: number timeoutError?: BrowserError + stdinText?: string } type EnqueueTargetedCommandOptions = { @@ -772,33 +811,35 @@ export class AgentBrowserBridge { browserPageId?: string ): Promise { await assertClipboardTextWriteWithinLimitWithYield(value) - // Why: Input.insertText via Electron's debugger API does not deliver text to - // focused inputs in webviews — this is a fundamental Electron limitation. - // Agent-browser's fill and click also fail for the same reason. - // Workaround: use agent-browser's focus to resolve the ref, then set the value - // directly via chunked JS and dispatch input/change events for React/framework compat. + // Why: agent-browser's CDP text insertion loses focus in Electron guests. + // Resolve the ref first, then edit through the browser's input pipeline. return this.enqueueTargetedCommand( worktreeId, browserPageId, async (sessionName) => { - await this.execAgentBrowser(sessionName, ['focus', element]) - await this.execAgentBrowser(sessionName, [ - 'eval', - focusedValueSetExpression(JSON.stringify('')) - ]) - for (const chunk of iterateBrowserTextInsertionChunks( - value, - AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES - )) { + if (!(await this.isExplicitContentEditableTarget(sessionName, element))) { + await this.execAgentBrowser(sessionName, ['focus', element]) await this.execAgentBrowser(sessionName, [ 'eval', - focusedValueSetExpression(JSON.stringify(chunk), { append: true }) + focusedValueSetExpression(JSON.stringify('')) ]) + for (const chunk of iterateBrowserTextInsertionChunks( + value, + AGENT_BROWSER_TEXT_ARGUMENT_MAX_BYTES + )) { + await this.execAgentBrowser(sessionName, [ + 'eval', + focusedValueSetExpression(JSON.stringify(chunk), { append: true }) + ]) + } + await this.execAgentBrowser(sessionName, [ + 'eval', + focusedValueSetExpression(JSON.stringify(''), { append: true, dispatchEvents: true }) + ]) + return { filled: element } as BrowserFillResult } - await this.execAgentBrowser(sessionName, [ - 'eval', - focusedValueSetExpression(JSON.stringify(''), { append: true, dispatchEvents: true }) - ]) + + await this.fillExplicitContentEditable(sessionName, element, value) return { filled: element } as BrowserFillResult }, { requireScopedTarget: true } @@ -1537,10 +1578,22 @@ export class AgentBrowserBridge { worktreeId?: string, browserPageId?: string ): Promise { - return this.enqueueTargetedCommand(worktreeId, browserPageId, async (sessionName) => { - // Why: agent-browser has no clear command — use fill with empty string - return (await this.execAgentBrowser(sessionName, ['fill', element, ''])) as BrowserClearResult - }) + return this.enqueueTargetedCommand( + worktreeId, + browserPageId, + async (sessionName) => { + if (!(await this.isExplicitContentEditableTarget(sessionName, element))) { + // Why: agent-browser resolves this ref directly, preserving iframe, + // shadow-root, and unfocusable-target semantics for ordinary fields. + await this.execAgentBrowser(sessionName, ['fill', element, '']) + return { cleared: element } + } + + await this.fillExplicitContentEditable(sessionName, element, '') + return { cleared: element } + }, + { requireScopedTarget: true } + ) } async selectAll( @@ -2387,6 +2440,32 @@ export class AgentBrowserBridge { return translated.result } + private async isExplicitContentEditableTarget( + sessionName: string, + element: string + ): Promise { + const result = await this.execAgentBrowser(sessionName, [ + 'get', + 'attr', + element, + 'contenteditable' + ]) + return isExplicitContentEditableResult(result) + } + + private async fillExplicitContentEditable( + sessionName: string, + element: string, + value: string + ): Promise { + await this.execAgentBrowser(sessionName, ['focus', element]) + // Why: stdin avoids argv limits while keeping replacement atomic; chunked + // editor transactions can move focus and split one fill across controls. + await this.execAgentBrowser(sessionName, ['eval', '--stdin'], { + stdinText: focusedRichTextEditExpression(JSON.stringify(value), { selectAll: true }) + }) + } + private createPageUnavailableError(sessionName: string): BrowserError { return new BrowserError('browser_tab_not_found', pageUnavailableMessageForSession(sessionName)) } @@ -2536,6 +2615,11 @@ export class AgentBrowserBridge { if (session) { session.activeProcess = child } + if (execOptions?.stdinText !== undefined && child?.stdin) { + // Why: eval --stdin keeps paste-sized scripts out of argv on every platform. + child.stdin.on('error', () => {}) + child.stdin.end(execOptions.stdinText) + } }) }