Fix two things live testing on 0.15.5 turned up

**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.
This commit is contained in:
2026-08-24 09:04:16 -07:00
parent c8fe822586
commit 0fdcbbc778
4 changed files with 119 additions and 4 deletions
+17 -1
View File
@@ -1,6 +1,6 @@
import { test } from "node:test"; import { test } from "node:test";
import assert from "node:assert/strict"; 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 * 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("C")]).locale, null);
assert.equal(interpretAccountInfo([settingsOk("POSIX")]).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);
});
+8 -1
View File
@@ -85,6 +85,8 @@ export interface AccountInfo {
const infoCache = new Map<string, { info: AccountInfo; fetchedAt: number }>(); const infoCache = new Map<string, { info: AccountInfo; fetchedAt: number }>();
const INFO_CACHE_MS = 30 * 60_000; const INFO_CACHE_MS = 30 * 60_000;
const EMPTY_INFO: AccountInfo = { locale: null, generation: null, edition: null }; 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: * 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. * tells us which generation we are talking to.
*/ */
async function fetchAccountInfo(authorization: string, session: UpstreamSession): Promise<AccountInfo> { async function fetchAccountInfo(authorization: string, session: UpstreamSession): Promise<AccountInfo> {
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 = const accountId =
session.primaryAccounts?.[STALWART_CAP] ?? session.primaryAccounts?.[STALWART_CAP] ??
session.primaryAccounts?.["urn:ietf:params:jmap:mail"] ?? session.primaryAccounts?.["urn:ietf:params:jmap:mail"] ??
@@ -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 (
<Dialog open onClose={() => undefined} title="Two fields">
<input id="first" value={first} onChange={(e) => setFirst(e.target.value)} />
<input id="second" value={second} onChange={(e) => setSecond(e.target.value)} />
</Dialog>
);
}
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(<TwoFieldDialog />));
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(<TwoFieldDialog />));
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");
});
});
+12 -2
View File
@@ -16,13 +16,23 @@ interface DialogProps {
export function Dialog({ open, onClose, title, children, footer, size = "md", closeOnBackdrop = true, className }: DialogProps) { export function Dialog({ open, onClose, title, children, footer, size = "md", closeOnBackdrop = true, className }: DialogProps) {
const ref = useRef<HTMLDivElement>(null); const ref = useRef<HTMLDivElement>(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(() => { useEffect(() => {
if (!open) return; if (!open) return;
const prev = document.activeElement as HTMLElement | null; const prev = document.activeElement as HTMLElement | null;
const onKey = (e: KeyboardEvent) => { const onKey = (e: KeyboardEvent) => {
if (e.key === "Escape") { if (e.key === "Escape") {
e.stopPropagation(); e.stopPropagation();
onClose(); onCloseRef.current();
} }
if (e.key === "Tab" && ref.current) { if (e.key === "Tab" && ref.current) {
const focusables = ref.current.querySelectorAll<HTMLElement>('button,[href],input,select,textarea,[tabindex]:not([tabindex="-1"]),[contenteditable="true"]'); const focusables = ref.current.querySelectorAll<HTMLElement>('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); document.removeEventListener("keydown", onKey, true);
prev?.focus?.(); prev?.focus?.();
}; };
}, [open, onClose]); }, [open]);
if (!open) return null; if (!open) return null;
return createPortal( return createPortal(
<div <div