diff --git a/src/actions.ts b/src/actions.ts index c7a626c9..033062a3 100644 --- a/src/actions.ts +++ b/src/actions.ts @@ -1,4 +1,4 @@ -import type { Page, Frame } from 'playwright-core'; +import type { Page, Frame, Locator } from 'playwright-core'; import { mkdirSync } from 'node:fs'; import path from 'node:path'; import type { BrowserManager, ScreencastFrame } from './browser.js'; @@ -144,6 +144,43 @@ interface SnapshotData { refs?: Record; } +/** + * Safely perform check/uncheck on a locator. + * If the element is hidden (common with UI frameworks like Element UI, Ant Design, Vuetify + * which hide native checkbox inputs), falls back to using { force: true }. + * This prevents Playwright from hanging indefinitely waiting for the element to become visible. + * @see https://github.com/vercel-labs/agent-browser/issues/335 + */ +export async function safeCheckAction(locator: Locator, action: 'check' | 'uncheck'): Promise { + try { + // First, try with a short timeout to detect hidden elements quickly + if (action === 'check') { + await locator.check({ timeout: 5000 }); + } else { + await locator.uncheck({ timeout: 5000 }); + } + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + // If the element is not visible or the action timed out, retry with force: true + // This handles hidden native checkboxes in UI component libraries + if ( + message.includes('not visible') || + message.includes('hidden') || + message.includes('Timeout') || + message.includes('waiting for') || + message.includes('Element is not visible') + ) { + if (action === 'check') { + await locator.check({ force: true }); + } else { + await locator.uncheck({ force: true }); + } + } else { + throw error; + } + } +} + /** * Convert Playwright errors to AI-friendly messages * @internal Exported for testing @@ -808,7 +845,7 @@ async function handleFill(command: FillCommand, browser: BrowserManager): Promis async function handleCheck(command: CheckCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); try { - await locator.check(); + await safeCheckAction(locator, 'check'); } catch (error) { throw toAIFriendlyError(error, command.selector); } @@ -818,7 +855,7 @@ async function handleCheck(command: CheckCommand, browser: BrowserManager): Prom async function handleUncheck(command: UncheckCommand, browser: BrowserManager): Promise { const locator = browser.getLocator(command.selector); try { - await locator.uncheck(); + await safeCheckAction(locator, 'uncheck'); } catch (error) { throw toAIFriendlyError(error, command.selector); } @@ -897,7 +934,7 @@ async function handleGetByRole( await locator.fill(command.value ?? ''); return successResponse(command.id, { filled: true }); case 'check': - await locator.check(); + await safeCheckAction(locator, 'check'); return successResponse(command.id, { checked: true }); case 'hover': await locator.hover(); @@ -937,7 +974,7 @@ async function handleGetByLabel( await locator.fill(command.value ?? ''); return successResponse(command.id, { filled: true }); case 'check': - await locator.check(); + await safeCheckAction(locator, 'check'); return successResponse(command.id, { checked: true }); } } @@ -1704,7 +1741,7 @@ async function handleGetByTestId( await locator.fill(command.value ?? ''); return successResponse(command.id, { filled: true }); case 'check': - await locator.check(); + await safeCheckAction(locator, 'check'); return successResponse(command.id, { checked: true }); case 'hover': await locator.hover(); @@ -1725,7 +1762,7 @@ async function handleNth(command: NthCommand, browser: BrowserManager): Promise< await locator.fill(command.value ?? ''); return successResponse(command.id, { filled: true }); case 'check': - await locator.check(); + await safeCheckAction(locator, 'check'); return successResponse(command.id, { checked: true }); case 'hover': await locator.hover(); diff --git a/src/safe-check.test.ts b/src/safe-check.test.ts new file mode 100644 index 00000000..96bcce67 --- /dev/null +++ b/src/safe-check.test.ts @@ -0,0 +1,119 @@ +import { describe, it, expect, vi } from 'vitest'; +import { safeCheckAction } from './actions.js'; + +/** + * Tests for safeCheckAction - fix for issue #335 + * Ensures check/uncheck does not hang on hidden checkbox elements + * (common in Element UI, Ant Design, Vuetify, etc.) + */ +describe('safeCheckAction', () => { + it('should call check() normally when element is visible', async () => { + const locator = { + check: vi.fn().mockResolvedValue(undefined), + uncheck: vi.fn().mockResolvedValue(undefined), + } as any; + + await safeCheckAction(locator, 'check'); + expect(locator.check).toHaveBeenCalledWith({ timeout: 5000 }); + }); + + it('should call uncheck() normally when element is visible', async () => { + const locator = { + check: vi.fn().mockResolvedValue(undefined), + uncheck: vi.fn().mockResolvedValue(undefined), + } as any; + + await safeCheckAction(locator, 'uncheck'); + expect(locator.uncheck).toHaveBeenCalledWith({ timeout: 5000 }); + }); + + it('should retry with force:true when element is not visible (check)', async () => { + const locator = { + check: vi + .fn() + .mockRejectedValueOnce(new Error('Element is not visible')) + .mockResolvedValueOnce(undefined), + uncheck: vi.fn(), + } as any; + + await safeCheckAction(locator, 'check'); + expect(locator.check).toHaveBeenCalledTimes(2); + expect(locator.check).toHaveBeenLastCalledWith({ force: true }); + }); + + it('should retry with force:true when element is not visible (uncheck)', async () => { + const locator = { + check: vi.fn(), + uncheck: vi + .fn() + .mockRejectedValueOnce(new Error('Element is not visible')) + .mockResolvedValueOnce(undefined), + } as any; + + await safeCheckAction(locator, 'uncheck'); + expect(locator.uncheck).toHaveBeenCalledTimes(2); + expect(locator.uncheck).toHaveBeenLastCalledWith({ force: true }); + }); + + it('should retry with force:true on timeout (hidden checkbox)', async () => { + const locator = { + check: vi + .fn() + .mockRejectedValueOnce(new Error('Timeout 5000ms exceeded waiting for element')) + .mockResolvedValueOnce(undefined), + uncheck: vi.fn(), + } as any; + + await safeCheckAction(locator, 'check'); + expect(locator.check).toHaveBeenCalledTimes(2); + expect(locator.check).toHaveBeenLastCalledWith({ force: true }); + }); + + it('should retry with force:true when element is hidden', async () => { + const locator = { + check: vi + .fn() + .mockRejectedValueOnce(new Error('element is hidden')) + .mockResolvedValueOnce(undefined), + uncheck: vi.fn(), + } as any; + + await safeCheckAction(locator, 'check'); + expect(locator.check).toHaveBeenCalledTimes(2); + expect(locator.check).toHaveBeenLastCalledWith({ force: true }); + }); + + it('should retry with force:true on "waiting for" error', async () => { + const locator = { + check: vi + .fn() + .mockRejectedValueOnce(new Error('waiting for locator to be visible')) + .mockResolvedValueOnce(undefined), + uncheck: vi.fn(), + } as any; + + await safeCheckAction(locator, 'check'); + expect(locator.check).toHaveBeenCalledTimes(2); + expect(locator.check).toHaveBeenLastCalledWith({ force: true }); + }); + + it('should rethrow non-visibility errors', async () => { + const locator = { + check: vi.fn().mockRejectedValue(new Error('strict mode violation')), + uncheck: vi.fn(), + } as any; + + await expect(safeCheckAction(locator, 'check')).rejects.toThrow('strict mode violation'); + expect(locator.check).toHaveBeenCalledTimes(1); + }); + + it('should rethrow unknown errors for uncheck', async () => { + const locator = { + check: vi.fn(), + uncheck: vi.fn().mockRejectedValue(new Error('some other error')), + } as any; + + await expect(safeCheckAction(locator, 'uncheck')).rejects.toThrow('some other error'); + expect(locator.uncheck).toHaveBeenCalledTimes(1); + }); +});