From ad0b913efb6663de1c8fd1f1659520c50604b229 Mon Sep 17 00:00:00 2001 From: John Ellis Date: Mon, 24 Aug 2026 09:04:16 -0700 Subject: [PATCH] Fix two things live testing on 0.15.5 turned up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **About said "not detected".** Generation was only worked out from the reply to a registry method, which we never send to a server that does not advertise urn:stalwart:jmap — every 0.16 build does, and nothing older knows the capability at all, so its absence is already the answer. Say so, instead of shrugging. A session with no capabilities at all stays unknown, which is a different thing from old. **The caret jumped out of the OTP field after one digit.** Dialog's autofocus effect listed onClose in its dependencies, and every caller passes an inline arrow, so each keystroke in a dialog holding state tore the effect down, set it up again, and refocused the first field — which in the disable-2FA dialog is the password. Keep the handler in a ref so the effect depends only on `open`. This was a bug in the shared dialog rather than in one screen; every dialog with more than one field had it. The test for it fails against the old dependency array, not just passes against the new one. --- server/src/accountinfo.test.ts | 18 ++++- server/src/upstream.ts | 9 ++- web/src/ui/__tests__/dialog-focus.test.tsx | 82 ++++++++++++++++++++++ web/src/ui/dialog.tsx | 14 +++- 4 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 web/src/ui/__tests__/dialog-focus.test.tsx diff --git a/server/src/accountinfo.test.ts b/server/src/accountinfo.test.ts index c9e1f34..e018e3d 100644 --- a/server/src/accountinfo.test.ts +++ b/server/src/accountinfo.test.ts @@ -1,6 +1,6 @@ import { test } from "node:test"; import assert from "node:assert/strict"; -import { interpretAccountInfo } from "./upstream.js"; +import { getAccountInfo, interpretAccountInfo } from "./upstream.js"; /** * The account locale used to be read only from `x:Account/get`, which needs @@ -49,3 +49,19 @@ test("locales that carry no language are dropped, not passed through", () => { assert.equal(interpretAccountInfo([settingsOk("C")]).locale, null); assert.equal(interpretAccountInfo([settingsOk("POSIX")]).locale, null); }); + +test("a server that never heard of the Stalwart capability is reported as pre-0.16", async () => { + // 0.16 always advertises urn:stalwart:jmap and nothing older knows it at all, + // so its absence is the answer - and asking anyway would fail the whole + // request on those servers. This is what the live 0.15.5 box hits. + const session = { capabilities: { "urn:ietf:params:jmap:core": {}, "urn:ietf:params:jmap:mail": {} }, accounts: {}, primaryAccounts: {} }; + const info = await getAccountInfo("session-pre-016", "Basic x", session as never); + assert.equal(info.generation, "pre-0.16"); + assert.equal(info.locale, null); + assert.equal(info.edition, null); +}); + +test("no capabilities at all leaves the generation unknown", async () => { + const info = await getAccountInfo("session-no-caps", "Basic x", { accounts: {}, primaryAccounts: {} } as never); + assert.equal(info.generation, null); +}); diff --git a/server/src/upstream.ts b/server/src/upstream.ts index 36f14ee..ea5036c 100644 --- a/server/src/upstream.ts +++ b/server/src/upstream.ts @@ -85,6 +85,8 @@ export interface AccountInfo { const infoCache = new Map(); const INFO_CACHE_MS = 30 * 60_000; const EMPTY_INFO: AccountInfo = { locale: null, generation: null, edition: null }; +/** A server that has never heard of the registry: nothing to read, but dated. */ +const PRE_REGISTRY_INFO: AccountInfo = { locale: null, generation: "pre-0.16", edition: null }; /** * glibc modifiers that name a script rather than a dialect or a currency: @@ -138,7 +140,12 @@ export function normalizeLocale(raw: unknown): string | null { * tells us which generation we are talking to. */ async function fetchAccountInfo(authorization: string, session: UpstreamSession): Promise { - if (!session.capabilities || !(STALWART_CAP in session.capabilities)) return EMPTY_INFO; + // Every 0.16 build advertises urn:stalwart:jmap, and no earlier one knows it + // at all, so its absence already answers the question — and asking anyway + // would fail the whole request, since those servers reject a `using` naming + // a capability they cannot parse. + if (!session.capabilities) return EMPTY_INFO; + if (!(STALWART_CAP in session.capabilities)) return PRE_REGISTRY_INFO; const accountId = session.primaryAccounts?.[STALWART_CAP] ?? session.primaryAccounts?.["urn:ietf:params:jmap:mail"] ?? diff --git a/web/src/ui/__tests__/dialog-focus.test.tsx b/web/src/ui/__tests__/dialog-focus.test.tsx new file mode 100644 index 0000000..7f8e924 --- /dev/null +++ b/web/src/ui/__tests__/dialog-focus.test.tsx @@ -0,0 +1,82 @@ +import { act, useState } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { Dialog } from "../dialog"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +/** + * Dialogs are almost always given an inline arrow for onClose, so its identity + * changes on every render of the parent. While that was in the effect's + * dependencies, any dialog holding state tore the effect down and set it up + * again on each keystroke — and its autofocus dragged the caret back to the + * first field. Typing a digit into the second field jumped you to the first. + */ + +/** A dialog with two fields, whose parent re-renders as either is typed in. */ +function TwoFieldDialog() { + const [first, setFirst] = useState(""); + const [second, setSecond] = useState(""); + return ( + undefined} title="Two fields"> + setFirst(e.target.value)} /> + setSecond(e.target.value)} /> + + ); +} + +describe("Dialog focus handling", () => { + let host: HTMLDivElement; + let root: Root; + + beforeEach(() => { + host = document.createElement("div"); + document.body.appendChild(host); + root = createRoot(host); + }); + + afterEach(() => { + act(() => root.unmount()); + host.remove(); + }); + + const type = (el: HTMLInputElement, value: string) => { + act(() => { + el.focus(); + // What React's onChange sees when a character is typed. + const setter = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, "value")!.set!; + setter.call(el, value); + el.dispatchEvent(new Event("input", { bubbles: true })); + }); + }; + + it("autofocuses the first field when it opens", async () => { + act(() => root.render()); + await act(async () => { + await new Promise((r) => setTimeout(r, 30)); + }); + expect(document.activeElement?.id).toBe("first"); + }); + + it("leaves the caret alone while a later field is typed in", async () => { + act(() => root.render()); + await act(async () => { + await new Promise((r) => setTimeout(r, 30)); + }); + + const second = document.getElementById("second") as HTMLInputElement; + type(second, "1"); + // The old effect re-ran here and pulled focus back to the first field. + await act(async () => { + await new Promise((r) => setTimeout(r, 30)); + }); + expect(document.activeElement?.id).toBe("second"); + + type(second, "12"); + await act(async () => { + await new Promise((r) => setTimeout(r, 30)); + }); + expect(document.activeElement?.id).toBe("second"); + expect(second.value).toBe("12"); + }); +}); diff --git a/web/src/ui/dialog.tsx b/web/src/ui/dialog.tsx index cc0e46b..b746d77 100644 --- a/web/src/ui/dialog.tsx +++ b/web/src/ui/dialog.tsx @@ -16,13 +16,23 @@ interface DialogProps { export function Dialog({ open, onClose, title, children, footer, size = "md", closeOnBackdrop = true, className }: DialogProps) { const ref = useRef(null); + /* + * Callers almost always pass an inline arrow for onClose, so its identity + * changes on every render of the parent. Depending on it here would tear the + * effect down and set it up again on every keystroke in a dialog that holds + * state, and the autofocus below would drag the caret back to the first + * field mid-typing. Keep the latest handler in a ref instead, so the effect + * depends only on `open`. + */ + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose; useEffect(() => { if (!open) return; const prev = document.activeElement as HTMLElement | null; const onKey = (e: KeyboardEvent) => { if (e.key === "Escape") { e.stopPropagation(); - onClose(); + onCloseRef.current(); } if (e.key === "Tab" && ref.current) { const focusables = ref.current.querySelectorAll('button,[href],input,select,textarea,[tabindex]:not([tabindex="-1"]),[contenteditable="true"]'); @@ -48,7 +58,7 @@ export function Dialog({ open, onClose, title, children, footer, size = "md", cl document.removeEventListener("keydown", onKey, true); prev?.focus?.(); }; - }, [open, onClose]); + }, [open]); if (!open) return null; return createPortal(