From 5530b7ffad8d548c9d777c2c1bd7e273bdaced38 Mon Sep 17 00:00:00 2001 From: Yaowei Zheng Date: Wed, 29 Jul 2026 16:13:13 +0800 Subject: [PATCH] fix(web): keep browser autofill out of config forms; drop the closed panel's stray divider (#114) Co-authored-by: Claude Opus 5 (1M context) --- packages/web/e2e/layout.spec.mjs | 34 +++++++++++++++- packages/web/e2e/subagent.spec.mjs | 15 +++++++ packages/web/src/components/ui/input.tsx | 40 ++++++++++++++++++- packages/web/src/features/chat/chat-input.tsx | 4 ++ packages/web/src/features/chat/draft-view.tsx | 2 + .../web/src/features/chat/files-panel.tsx | 12 ++++-- .../web/src/features/chat/subagents-panel.tsx | 10 +++-- packages/web/test/autofill.test.ts | 38 ++++++++++++++++++ 8 files changed, 146 insertions(+), 9 deletions(-) create mode 100644 packages/web/test/autofill.test.ts diff --git a/packages/web/e2e/layout.spec.mjs b/packages/web/e2e/layout.spec.mjs index a6c67f9..0ca0df8 100644 --- a/packages/web/e2e/layout.spec.mjs +++ b/packages/web/e2e/layout.spec.mjs @@ -7,7 +7,9 @@ * once inside a session; * - the models page at 390x844 must not overflow horizontally, and text must not overlap * (the group header's provider name used to get pushed out of the button box and overlap - * the group-level actions); + * the group-level actions), and its model dialog opts every field out of browser autofill + * (the API-key box used to receive the account's saved password, the box above it the + * username); * - every chat-page dropdown menu, opened at phone widths (375/390), must keep its panel * inside the viewport and must not shove the page sideways (the model menu used to run * ~34px off-screen left, the skills menu ~92px off-screen right — with its autofocused @@ -212,6 +214,36 @@ test("layout: en draft + context gauge + mobile models", async ({ page }) => { expect(d.scrollWidth, "models @390 no horizontal overflow").toBeLessThanOrEqual(d.clientWidth); expect(await textOverlapCount(page), "models @390 no overlapping text").toBe(0); + // --- Model dialog: no field may invite the browser's saved login. The dialog's fields are + // unowned (no
), so the browser groups them with the rest of the page and picks a + // "username" box on its own — it used to fill the account credentials into the API key and + // the field above it. A password box additionally has to say "new-password": Chrome and + // Safari ignore autocomplete="off" there. --- + await page.getByText("claude-4-8").first().click(); + const dialogFields = await page.evaluate(() => + [...document.querySelectorAll("input")] + .filter((i) => i.type !== "checkbox" && i.type !== "file") + .map((i) => ({ + type: i.type, + autocomplete: i.getAttribute("autocomplete"), + ignored: i.hasAttribute("data-1p-ignore") && i.getAttribute("data-lpignore") === "true", + })), + ); + expect(dialogFields.length, "model dialog fields found").toBeGreaterThan(3); + expect( + dialogFields.filter((f) => f.type === "password"), + "the API-key box opts out as new-password", + ).toEqual([{ type: "password", autocomplete: "new-password", ignored: true }]); + expect( + dialogFields.filter((f) => f.autocomplete !== "off" && f.type !== "password"), + "no other field declares an autofill role", + ).toEqual([]); + expect( + dialogFields.filter((f) => !f.ignored), + "every field carries the password-manager opt-out", + ).toEqual([]); + await page.keyboard.press("Escape"); + // --- Sidebar "New chat" button: no background fill (its resting state outside the draft page should have a transparent background) --- await page.setViewportSize({ width: 1280, height: 720 }); const newChat = page.locator("nav").getByRole("button", { name: "New chat" }); diff --git a/packages/web/e2e/subagent.spec.mjs b/packages/web/e2e/subagent.spec.mjs index ae1c916..d4f71cf 100644 --- a/packages/web/e2e/subagent.spec.mjs +++ b/packages/web/e2e/subagent.spec.mjs @@ -180,6 +180,21 @@ test("subagent renders as a chip; the panel shows the call graph and child conve await expect(filesToggle).toHaveAttribute("aria-expanded", "false"); await expect(agentsHeading).toBeInViewport(); await expect(filesHeading).not.toBeInViewport(); + // The closed panel must leave NOTHING behind: its clipping window is zero-width, and with + // border-box sizing a divider there would still paint its 1px right beside the open panel — + // a hairline, the resize gutter, then the real divider, which reads as a second, empty panel. + // Polled: the width transition is still running right after the toggle click. + const shellWidth = (heading) => + heading.evaluate((el) => el.closest(".overflow-hidden").getBoundingClientRect().width); + await expect + .poll(() => shellWidth(filesHeading), { + message: "closed panel occupies no width, divider included", + }) + .toBe(0); + expect( + await shellWidth(agentsHeading), + "open panel is the only one taking width", + ).toBeGreaterThan(0); // --- Historical topology: a plain follow-up Task makes the first turn's graph historical // (the boundary itself — the panel closing on a new Task — is covered by the reload-free diff --git a/packages/web/src/components/ui/input.tsx b/packages/web/src/components/ui/input.tsx index 3e2a822..21fa6da 100644 --- a/packages/web/src/components/ui/input.tsx +++ b/packages/web/src/components/ui/input.tsx @@ -48,6 +48,39 @@ export const errorClass = "!border-red-400 !bg-red-50 hover:!border-red-500 focus:!border-red-500 focus:!ring-red-400/30 " + "dark:!border-red-800 dark:!bg-red-950/40 dark:hover:!border-red-700 dark:focus:!border-red-600"; +/** + * Autofill policy: a control opts OUT unless its caller declares a real credential role. + * Almost every field in this app holds an API key, a model id, a URL, a directory or a + * price, and the browser's saved-login heuristics kept dropping the account's username and + * password into them (a dialog's fields are unowned — no element — so the browser + * groups them with everything else on the page and picks a "username" box on its own). + * Only login.tsx and the password dialogs pass a role ("username" / "current-password" / + * "new-password"), and those keep the browser's help. + * + * `autocomplete="off"` alone does NOT cover a password box: Chrome and Safari ignore it + * there and offer the saved login anyway. An opted-out SECRET field therefore goes out as + * `new-password` — the one value password managers read as "not the account password" — + * and both cases carry the manager-extension opt-outs (1Password / LastPass / Bitwarden / + * Dashlane), which read their own attributes rather than `autocomplete`. + */ +export function autofillProps(autoComplete: string | undefined, secret: boolean) { + // A declared role (anything but the opt-out) is the caller's decision: pass it through untouched. + if (autoComplete !== undefined && autoComplete !== "off") return { autoComplete }; + return { + autoComplete: secret ? "new-password" : "off", + "data-1p-ignore": "", + "data-lpignore": "true", + "data-bwignore": "", + "data-form-type": "other", + }; +} + +/** + * The same opt-out as a spreadable constant, for the handful of raw ``s that don't go + * through Input (menu search boxes, the Workspace path editor): `{...noAutofill}`. + */ +export const noAutofill = autofillProps(undefined, false); + export function Input({ label, hint, @@ -56,6 +89,7 @@ export function Input({ required, size = "base", className, + autoComplete, ...rest }: InputProps) { const bad = Boolean(error) || Boolean(invalid); @@ -70,6 +104,9 @@ export function Input({ aria-invalid={bad ? true : undefined} aria-required={required || undefined} aria-describedby={error ? errorId : undefined} + // Secret while masked; PasswordInput's reveal toggle flips the type to text, and the + // opt-out that matters was already read from the password state. + {...autofillProps(autoComplete, rest.type === "password")} {...rest} /> @@ -89,7 +126,7 @@ export interface TextareaProps extends TextareaHTMLAttributes(function Textarea( - { label, hint, error, invalid, required, mono, size = "base", className, ...rest }, + { label, hint, error, invalid, required, mono, size = "base", className, autoComplete, ...rest }, ref, ) { const bad = Boolean(error) || Boolean(invalid); @@ -102,6 +139,7 @@ export const Textarea = forwardRef(function aria-invalid={bad ? true : undefined} aria-required={required || undefined} aria-describedby={error ? errorId : undefined} + {...autofillProps(autoComplete, false)} {...rest} /> diff --git a/packages/web/src/features/chat/chat-input.tsx b/packages/web/src/features/chat/chat-input.tsx index 2027e70..0135a79 100644 --- a/packages/web/src/features/chat/chat-input.tsx +++ b/packages/web/src/features/chat/chat-input.tsx @@ -65,6 +65,7 @@ import { resolveContextWindow } from "../../lib/context"; import { useLocale } from "../../state/locale"; import { Dropdown } from "../../components/ui/dropdown"; import { GlyphIcon } from "../../components/ui/glyph-icon"; +import { noAutofill } from "../../components/ui/input"; import { SkillIcon } from "../skills/skill-icon-view"; import { ZoomableImage } from "../../components/ui/image-zoom"; import { ProviderLogo } from "../../components/ui/provider-logo"; @@ -280,6 +281,7 @@ function ModelMenuList({ onChange={(e) => setQuery(e.target.value)} placeholder={S.models.searchPlaceholder} aria-label={S.models.searchPlaceholder} + {...noAutofill} className="w-full rounded border border-transparent bg-transparent px-1 py-0.5 text-xs text-gray-700 placeholder:text-gray-400 focus:outline-none dark:text-gray-200 dark:placeholder:text-gray-500" /> @@ -680,6 +682,7 @@ function SkillSelect({ onChange={(e) => setQuery(e.target.value)} placeholder={S.chat.skillsSearchPlaceholder} aria-label={S.chat.skillsSearchPlaceholder} + {...noAutofill} className="w-full rounded border border-transparent bg-transparent px-1 py-0.5 text-xs text-gray-700 placeholder:text-gray-400 focus:outline-none dark:text-gray-200 dark:placeholder:text-gray-500" /> @@ -2022,6 +2025,7 @@ export function ChatInput({ placeholder={S.chat.goalBudgetPlaceholder} aria-invalid={goalBudgetDraftInvalid} aria-describedby="goal-budget-hint" + {...noAutofill} title={ goalBudgetDraftInvalid ? S.chat.goalBudgetInvalid : S.chat.goalBudgetHint } diff --git a/packages/web/src/features/chat/draft-view.tsx b/packages/web/src/features/chat/draft-view.tsx index 011550a..62be940 100644 --- a/packages/web/src/features/chat/draft-view.tsx +++ b/packages/web/src/features/chat/draft-view.tsx @@ -49,6 +49,7 @@ import { useSessions } from "../../state/sessions"; import { AgentAvatar } from "../../components/ui/agent-avatar"; import { Chevron } from "../../components/ui/chevron"; import { Dropdown } from "../../components/ui/dropdown"; +import { noAutofill } from "../../components/ui/input"; import { PenguinLogo } from "../../components/ui/penguin-logo"; import { toastError } from "../../components/ui/toast"; import { useVersionInfo } from "../../lib/use-version-info"; @@ -967,6 +968,7 @@ function WorkspaceSelect({ value={pathDraft} placeholder="…" aria-label={S.chat.workspace} + {...noAutofill} onChange={(e) => setPathDraft(e.target.value)} onBlur={() => void commitPathEdit()} onKeyDown={(e) => { diff --git a/packages/web/src/features/chat/files-panel.tsx b/packages/web/src/features/chat/files-panel.tsx index aa05487..6609aa7 100644 --- a/packages/web/src/features/chat/files-panel.tsx +++ b/packages/web/src/features/chat/files-panel.tsx @@ -74,9 +74,15 @@ export function FilesPanel({ session, panel }: { session: SessionInfo; panel: Fi // input) anchored to the nearest initial containing block instead, it would bypass this // overflow-hidden and stretch the **document** wide, making a horizontal scrollbar // appear out of nowhere. - className={`relative flex min-h-0 shrink-0 flex-col overflow-hidden border-l border-gray-200 dark:border-gray-800 ${ - panel.resizing ? "pointer-events-none" : "transition-[width] duration-200" - }`} + // + // The divider belongs to the OPEN state only: with border-box sizing a closed panel's + // border still paints its 1px even at width 0, and since both docked panels stay + // mounted, the closed one left a stray line beside the open one — a hairline, a gap + // (the open panel's resize handle) and then the real divider, which reads as an extra + // empty panel wedged in. Both closed, the two leftover lines stacked at the window edge. + className={`relative flex min-h-0 shrink-0 flex-col overflow-hidden ${ + panel.open ? "border-l border-gray-200 dark:border-gray-800" : "" + } ${panel.resizing ? "pointer-events-none" : "transition-[width] duration-200"}`} > {/* Content is fixed at the target width; the outer element is only a clipping window: during the open/close animation the outer element passes through intermediate diff --git a/packages/web/src/features/chat/subagents-panel.tsx b/packages/web/src/features/chat/subagents-panel.tsx index fc490dd..11b0369 100644 --- a/packages/web/src/features/chat/subagents-panel.tsx +++ b/packages/web/src/features/chat/subagents-panel.tsx @@ -80,11 +80,13 @@ export function SubagentsPanel({
diff --git a/packages/web/test/autofill.test.ts b/packages/web/test/autofill.test.ts new file mode 100644 index 0000000..8f19887 --- /dev/null +++ b/packages/web/test/autofill.test.ts @@ -0,0 +1,38 @@ +/** + * Autofill policy of the shared form controls (autofillProps in components/ui/input.tsx): + * fields opt out by default, a declared credential role passes through, and an opted-out + * password box goes out as "new-password" — plain "off" is ignored by Chrome/Safari there, + * which is how the account's saved login used to land in an API-key field. + */ +import { describe, it, expect } from "vitest"; +import { autofillProps } from "../src/components/ui/input"; + +const IGNORES = { + "data-1p-ignore": "", + "data-lpignore": "true", + "data-bwignore": "", + "data-form-type": "other", +}; + +describe("autofillProps", () => { + it("opts an undeclared field out, extension attributes included", () => { + expect(autofillProps(undefined, false)).toEqual({ autoComplete: "off", ...IGNORES }); + }); + + it("emits new-password for a secret field, never a bare off", () => { + // Chrome and Safari ignore autocomplete="off" on a password box and offer the saved + // login anyway; "new-password" is the only value they read as "not the account password". + expect(autofillProps(undefined, true)).toEqual({ autoComplete: "new-password", ...IGNORES }); + expect(autofillProps("off", true)).toEqual({ autoComplete: "new-password", ...IGNORES }); + }); + + it("treats an explicit off as the same opt-out as no value at all", () => { + expect(autofillProps("off", false)).toEqual(autofillProps(undefined, false)); + }); + + it("passes a declared credential role through untouched (login, password dialogs)", () => { + for (const role of ["username", "current-password", "new-password"]) { + expect(autofillProps(role, role.endsWith("password"))).toEqual({ autoComplete: role }); + } + }); +});