commit aabf83bdd9e21fda1e8e14cb2cd230d2fc8c88d2
parent 222b64411f78faef290ce905235fd6d2502993fe
Author: Christian Grothoff <christian@grothoff.org>
Date: Fri, 30 Oct 2026 10:31:25 +0100
A useEffect registration race in AccountAddressMenu.
The test opens the account-address dropdown, waits only until the menu item
is in the DOM, and then presses Escape on the trigger:
await accountAddressButton.click();
await copyAccountAddressButton.waitFor();
await browser.saveScreenshot("account-address-menu");
await accountAddressButton.press("Escape");
await copyAccountAddressButton.waitFor({ state: "hidden" }); // line 504
The component's Escape handler is installed from an effect gated on `open`,
so it does not exist until Preact flushes effects -- after the paint that
rendered the menu (requestAnimationFrame, with a 100 ms setTimeout
fallback). A key pressed inside that window reaches document with no
listener attached and is simply lost; nothing retries it, so the menu stays
open forever. That is exactly what the recorded run shows: the locator
resolved to a *visible* button 34 times over the full 15 s timeout.
Not reproduced here directly: re-running this test on an idle machine gets
past line 504 and fails much later (line 701, an unrelated MFA dialog in
this checkout's libeufin build), which already shows the step is
timing-dependent rather than deterministically broken. The mechanism is
proven in the merchant web UI though -- see
05-merchant-webui-simple-form-hydration-race.diff, where the same Preact
version in the same browser loses a Playwright fill() to an effect that had
not flushed yet, reproducibly, and a 300 ms delay before the interaction
makes it pass.
FIX
Install the listeners once, at mount, and test `open` inside them through a
ref. At mount the page has been on screen for a long time before anyone
can open the menu, so there is no window left in which the menu is
interactive but unhandled. Behaviour is otherwise identical: the handlers
still do nothing while the menu is closed.
Smaller alternative, if a document-level listener is unwanted: move
onKeyDown onto the container <div>. Preact attaches that during the render
that opens the menu, with no effect flush involved, so it is race-free
too -- but it only fires while focus is inside the menu or its trigger,
which is a (standard, but different) behaviour change.
Worth hardening the test as well, independently of this fix: asserting that
the menu *stays* reachable by pressing Escape only after some observable
signal that it is live would turn a silent 15 s hang into a clear failure.
Assisted by LLM.
Diffstat:
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/packages/libeufin-bank-webui/src/pages/AccountPage/views.tsx b/packages/libeufin-bank-webui/src/pages/AccountPage/views.tsx
@@ -41,20 +41,26 @@ function AccountAddressMenu({
const [open, setOpen] = useState(false);
const containerRef = useRef<HTMLDivElement>(null);
const triggerRef = useRef<HTMLButtonElement>(null);
+ // Read by the document listeners below instead of closing over `open`.
+ // They are installed once, at mount, so that the menu is never on screen
+ // for a frame without them: an effect keyed on `open` runs only after the
+ // paint that opened the menu, and a key pressed in that window is dropped
+ // with nothing left to retry it.
+ const openRef = useRef(open);
+ openRef.current = open;
const shortAddress = accountBankHost
? `${accountLoginName} @ ${accountBankHost}`
: accountLoginName;
useEffect(() => {
- if (!open) return;
-
const closeOnOutsideClick = (event: PointerEvent): void => {
+ if (!openRef.current) return;
if (!containerRef.current?.contains(event.target as Node)) {
setOpen(false);
}
};
const closeOnEscape = (event: KeyboardEvent): void => {
- if (event.key !== "Escape") return;
+ if (!openRef.current || event.key !== "Escape") return;
setOpen(false);
triggerRef.current?.focus();
};
@@ -65,7 +71,7 @@ function AccountAddressMenu({
document.removeEventListener("pointerdown", closeOnOutsideClick);
document.removeEventListener("keydown", closeOnEscape);
};
- }, [open]);
+ }, []);
return (
<div class="relative" ref={containerRef}>
