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>
This commit is contained in:
parent
93497ac689
commit
81dd08ffd3
|
|
@ -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<string, unknown> = {}
|
||||
): 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 })
|
||||
|
||||
|
|
|
|||
|
|
@ -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<BrowserFillResult> {
|
||||
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<BrowserClearResult> {
|
||||
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<boolean> {
|
||||
const result = await this.execAgentBrowser(sessionName, [
|
||||
'get',
|
||||
'attr',
|
||||
element,
|
||||
'contenteditable'
|
||||
])
|
||||
return isExplicitContentEditableResult(result)
|
||||
}
|
||||
|
||||
private async fillExplicitContentEditable(
|
||||
sessionName: string,
|
||||
element: string,
|
||||
value: string
|
||||
): Promise<void> {
|
||||
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)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue