diff --git a/src/browser/form-fields.ts b/src/browser/form-fields.ts new file mode 100644 index 00000000000..9e0dac4ddd6 --- /dev/null +++ b/src/browser/form-fields.ts @@ -0,0 +1,32 @@ +import type { BrowserFormField } from "./client-actions-core.js"; + +export const DEFAULT_FILL_FIELD_TYPE = "text"; + +type BrowserFormFieldValue = NonNullable; + +export function normalizeBrowserFormFieldRef(value: unknown): string { + return typeof value === "string" ? value.trim() : ""; +} + +export function normalizeBrowserFormFieldType(value: unknown): string { + const type = typeof value === "string" ? value.trim() : ""; + return type || DEFAULT_FILL_FIELD_TYPE; +} + +export function normalizeBrowserFormFieldValue(value: unknown): BrowserFormFieldValue | undefined { + return typeof value === "string" || typeof value === "number" || typeof value === "boolean" + ? value + : undefined; +} + +export function normalizeBrowserFormField( + record: Record, +): BrowserFormField | null { + const ref = normalizeBrowserFormFieldRef(record.ref); + if (!ref) { + return null; + } + const type = normalizeBrowserFormFieldType(record.type); + const value = normalizeBrowserFormFieldValue(record.value); + return value === undefined ? { ref, type } : { ref, type, value }; +} diff --git a/src/browser/pw-tools-core.interactions.ts b/src/browser/pw-tools-core.interactions.ts index cd6ad0e165c..f3eec30c1d1 100644 --- a/src/browser/pw-tools-core.interactions.ts +++ b/src/browser/pw-tools-core.interactions.ts @@ -1,4 +1,5 @@ import type { BrowserFormField } from "./client-actions-core.js"; +import { DEFAULT_FILL_FIELD_TYPE } from "./form-fields.js"; import { DEFAULT_UPLOAD_DIR, resolveStrictExistingPathsWithinRoot } from "./paths.js"; import { ensurePageState, @@ -188,7 +189,7 @@ export async function fillFormViaPlaywright(opts: { const timeout = Math.max(500, Math.min(60_000, opts.timeoutMs ?? 8000)); for (const field of opts.fields) { const ref = field.ref.trim(); - const type = field.type.trim(); + const type = (field.type || DEFAULT_FILL_FIELD_TYPE).trim() || DEFAULT_FILL_FIELD_TYPE; const rawValue = field.value; const value = typeof rawValue === "string" @@ -196,7 +197,7 @@ export async function fillFormViaPlaywright(opts: { : typeof rawValue === "number" || typeof rawValue === "boolean" ? String(rawValue) : ""; - if (!ref || !type) { + if (!ref) { continue; } const locator = refLocator(page, ref); diff --git a/src/browser/routes/agent.act.ts b/src/browser/routes/agent.act.ts index 16470da7ca2..2ae6073c7cf 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -1,4 +1,5 @@ import type { BrowserFormField } from "../client-actions-core.js"; +import { normalizeBrowserFormField } from "../form-fields.js"; import type { BrowserRouteContext } from "../server-context.js"; import { registerBrowserAgentActDownloadRoutes } from "./agent.act.download.js"; import { registerBrowserAgentActHookRoutes } from "./agent.act.hooks.js"; @@ -190,21 +191,7 @@ export function registerBrowserAgentActRoutes( if (!field || typeof field !== "object") { return null; } - const rec = field as Record; - const ref = toStringOrEmpty(rec.ref); - const type = toStringOrEmpty(rec.type) || "text"; - if (!ref) { - return null; - } - const value = - typeof rec.value === "string" || - typeof rec.value === "number" || - typeof rec.value === "boolean" - ? rec.value - : undefined; - const parsed: BrowserFormField = - value === undefined ? { ref, type } : { ref, type, value }; - return parsed; + return normalizeBrowserFormField(field as Record); }) .filter((field): field is BrowserFormField => field !== null); if (!fields.length) { diff --git a/src/browser/server.agent-contract-form-layout-act-commands.test.ts b/src/browser/server.agent-contract-form-layout-act-commands.test.ts index 484a4d8faa9..738bf8b7e2d 100644 --- a/src/browser/server.agent-contract-form-layout-act-commands.test.ts +++ b/src/browser/server.agent-contract-form-layout-act-commands.test.ts @@ -58,27 +58,35 @@ describe("browser control server", () => { values: ["a", "b"], }); - const fill = await postJson<{ ok: boolean }>(`${base}/act`, { - kind: "fill", - fields: [{ ref: "6", type: "textbox", value: "hello" }], - }); - expect(fill.ok).toBe(true); - expect(pwMocks.fillFormViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: state.cdpBaseUrl, - targetId: "abcd1234", - fields: [{ ref: "6", type: "textbox", value: "hello" }], - }); - - const fillWithoutType = await postJson<{ ok: boolean }>(`${base}/act`, { - kind: "fill", - fields: [{ ref: "7", value: "world" }], - }); - expect(fillWithoutType.ok).toBe(true); - expect(pwMocks.fillFormViaPlaywright).toHaveBeenCalledWith({ - cdpUrl: state.cdpBaseUrl, - targetId: "abcd1234", - fields: [{ ref: "7", type: "text", value: "world" }], - }); + const fillCases: Array<{ + input: Record; + expected: Record; + }> = [ + { + input: { ref: "6", type: "textbox", value: "hello" }, + expected: { ref: "6", type: "textbox", value: "hello" }, + }, + { + input: { ref: "7", value: "world" }, + expected: { ref: "7", type: "text", value: "world" }, + }, + { + input: { ref: "8", type: " ", value: "trimmed-default" }, + expected: { ref: "8", type: "text", value: "trimmed-default" }, + }, + ]; + for (const { input, expected } of fillCases) { + const fill = await postJson<{ ok: boolean }>(`${base}/act`, { + kind: "fill", + fields: [input], + }); + expect(fill.ok).toBe(true); + expect(pwMocks.fillFormViaPlaywright).toHaveBeenCalledWith({ + cdpUrl: state.cdpBaseUrl, + targetId: "abcd1234", + fields: [expected], + }); + } const resize = await postJson<{ ok: boolean }>(`${base}/act`, { kind: "resize", diff --git a/src/cli/browser-cli-actions-input/shared.test.ts b/src/cli/browser-cli-actions-input/shared.test.ts index 553fd90fbef..f3b4e73b0d3 100644 --- a/src/cli/browser-cli-actions-input/shared.test.ts +++ b/src/cli/browser-cli-actions-input/shared.test.ts @@ -2,10 +2,24 @@ import { describe, expect, it } from "vitest"; import { readFields } from "./shared.js"; describe("readFields", () => { - it("defaults missing type to text", async () => { - await expect(readFields({ fields: '[{"ref":"7","value":"world"}]' })).resolves.toEqual([ - { ref: "7", type: "text", value: "world" }, - ]); + it.each([ + { + name: "keeps explicit type", + fields: '[{"ref":"6","type":"textbox","value":"hello"}]', + expected: [{ ref: "6", type: "textbox", value: "hello" }], + }, + { + name: "defaults missing type to text", + fields: '[{"ref":"7","value":"world"}]', + expected: [{ ref: "7", type: "text", value: "world" }], + }, + { + name: "defaults blank type to text", + fields: '[{"ref":"8","type":" ","value":"blank"}]', + expected: [{ ref: "8", type: "text", value: "blank" }], + }, + ])("$name", async ({ fields, expected }) => { + await expect(readFields({ fields })).resolves.toEqual(expected); }); it("requires ref", async () => { diff --git a/src/cli/browser-cli-actions-input/shared.ts b/src/cli/browser-cli-actions-input/shared.ts index 69e1ddd8b57..4d426e82304 100644 --- a/src/cli/browser-cli-actions-input/shared.ts +++ b/src/cli/browser-cli-actions-input/shared.ts @@ -1,5 +1,9 @@ import type { Command } from "commander"; import type { BrowserFormField } from "../../browser/client-actions-core.js"; +import { + normalizeBrowserFormField, + normalizeBrowserFormFieldValue, +} from "../../browser/form-fields.js"; import { danger } from "../../globals.js"; import { defaultRuntime } from "../../runtime.js"; import { callBrowserRequest, type BrowserParentOpts } from "../browser-cli-shared.js"; @@ -68,21 +72,16 @@ export async function readFields(opts: { throw new Error(`fields[${index}] must be an object`); } const rec = entry as Record; - const ref = typeof rec.ref === "string" ? rec.ref.trim() : ""; - const type = typeof rec.type === "string" ? rec.type.trim() : ""; - if (!ref) { + const parsedField = normalizeBrowserFormField(rec); + if (!parsedField) { throw new Error(`fields[${index}] must include ref`); } - const resolvedType = type || "text"; if ( - typeof rec.value === "string" || - typeof rec.value === "number" || - typeof rec.value === "boolean" + rec.value === undefined || + rec.value === null || + normalizeBrowserFormFieldValue(rec.value) !== undefined ) { - return { ref, type: resolvedType, value: rec.value }; - } - if (rec.value === undefined || rec.value === null) { - return { ref, type: resolvedType }; + return parsedField; } throw new Error(`fields[${index}].value must be string, number, boolean, or null`); });