From 9f3445d03f106ee3ebd8b4b1bd546a08d9169af7 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Mon, 21 Sep 2026 19:53:33 +0200 Subject: fix: ask in the page instead of native confirm/alert A native confirm() or alert() leaves the desktop client unable to type until the window is refocused. ask.js draws both in the page; the SPA test now bans all three browser dialogs. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 9 ++ docs/MESHBAY_DESIGN.md | 1 + docs/playlists.md | 12 +-- .../src/meshbay_hub/static/admin-page.js | 7 +- packages/meshbay-hub/src/meshbay_hub/static/ask.js | 70 +++++++++++++++ .../meshbay-hub/src/meshbay_hub/static/chat-app.js | 3 +- .../src/meshbay_hub/static/explore-page.js | 3 +- .../src/meshbay_hub/static/file-utils.js | 3 +- .../src/meshbay_hub/static/files-app.js | 12 +-- .../src/meshbay_hub/static/group-settings.js | 15 ++-- .../src/meshbay_hub/static/locales/de.js | 4 + .../src/meshbay_hub/static/locales/en.js | 4 + .../src/meshbay_hub/static/locales/es.js | 4 + .../src/meshbay_hub/static/locales/fr.js | 4 + .../src/meshbay_hub/static/locales/it.js | 4 + .../src/meshbay_hub/static/locales/ja.js | 4 + .../src/meshbay_hub/static/locales/nl.js | 4 + .../src/meshbay_hub/static/locales/pl.js | 4 + .../src/meshbay_hub/static/locales/pt-BR.js | 4 + .../src/meshbay_hub/static/locales/zh-CN.js | 4 + .../src/meshbay_hub/static/node-page.js | 15 ++-- .../src/meshbay_hub/static/playlist-menu.js | 8 +- .../src/meshbay_hub/static/profile-page.js | 3 +- .../meshbay-hub/src/meshbay_hub/static/style.css | 3 + .../meshbay-hub/tests/harness/playlist_ui_probe.py | 13 ++- packages/meshbay-hub/tests/test_hook_ordering.py | 2 + .../tests/test_no_native_dialogs_in_the_spa.py | 99 ++++++++++++++++++++++ .../meshbay-hub/tests/test_no_prompt_in_the_spa.py | 90 -------------------- packages/meshbay-hub/tests/test_playlist_ui.py | 3 +- packages/meshbay-hub/tests/test_zip_size_limit.py | 7 +- 30 files changed, 285 insertions(+), 133 deletions(-) create mode 100644 packages/meshbay-hub/src/meshbay_hub/static/ask.js create mode 100644 packages/meshbay-hub/tests/test_no_native_dialogs_in_the_spa.py delete mode 100644 packages/meshbay-hub/tests/test_no_prompt_in_the_spa.py diff --git a/CLAUDE.md b/CLAUDE.md index 928d526..708d1ea 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -334,6 +334,15 @@ do. Read them before writing anything that touches the same mechanism. refused by the policy, which is T3's mitigation demonstrated rather than asserted +- **A native `confirm()` or `alert()` breaks typing in the desktop client.** + Once the dialog closes `document.hasFocus()` stays false: a click still moves + `activeElement` into a field, and every keystroke goes nowhere until the + window loses and regains focus. It was reported as "the Create group fields are + frozen" after removing a member, and reproduced with real X input under Xvfb. + Events dispatched in the DOM cannot see it, because the focus is lost below + the page. `ask.js` draws both in the page, and + `test_no_native_dialogs_in_the_spa.py` bans all three browser dialogs + - **A user unit cannot carry `User=`.** `meshbay-node.spec` installed the system template into `%{_userunitdir}`, where systemd refuses the file outright — the packaged unit could never have started, and nothing noticed because nobody had diff --git a/docs/MESHBAY_DESIGN.md b/docs/MESHBAY_DESIGN.md index 943aeef..a47cea0 100644 --- a/docs/MESHBAY_DESIGN.md +++ b/docs/MESHBAY_DESIGN.md @@ -3192,6 +3192,7 @@ process runs it — `systemctl --user` on Linux, Task Scheduler on Windows. | **Per-device revocation has no CLI** | A device is revoked over MNP (`roster.revoke_device`), from a device the node has already pinned. On a headless node the operator's only lever is `member unpin`, which removes **every** device of that account — so the per-device control the roster is built around is reachable from an interface and from nowhere else. §6.7 listed a `meshbay-node member device list\|revoke` verb that was never written, and that listing is how this was found: `USERGUIDE.md` was the first document written by reading the CLI rather than this specification, and the verb it copied out did not run | | **Migrations run on SQLite only** | The chain reaches head and agrees with the models there (§12), which is not where it ships. **The exposure is one revision deep, not the whole chain**: every revision behind the first packaged release was development that no installation ever ran, so nothing replays them on PostgreSQL. What is unguarded is the *next* migration — a default, an index type or a constraint PostgreSQL refuses reaches a deploy without the suite saying so | | **The loopback path removes access without writing an audit entry** | `ops.revoke_member` and `ops.unpin_member` log to the daemon's log and nothing to `audit.db`; the MNP admin handlers doing the same work audit `member_revoke` and `member_unpin`. So a removal made from the node page or the CLI — the two doors an operator sitting at their own machine actually uses — leaves the journal showing an admission and then, whenever that person next connects, a `join_refused` with nothing in between to explain it. §5.4's signed transcript is not what is missing: a loopback caller is authorized by being on localhost with the run token and signs nothing, so the gap is the record, not the authority. Found by reading a node's audit log for a refusal whose cause was six hours earlier and unrecorded | +| **The Create group wizard calls two hooks after an early return** | `CreateGroupWizard` (`create-group-page.js`) returns during node detection, before its `useRef`/`useEffect` for provisioning, so the hook count changes between renders. Preact tolerates a list that grows, and nothing is known to break; `test_hook_ordering.py` checks declaration order, not this. Found while tracing the frozen-fields report, which had another cause (`ask.js`) | | **A node key is read from the terminal or the desktop client, never a browser** | **Accepted.** `meshbay-node status` on the node's own machine and Node → Overview in the desktop client are the two places the key can be read; the Node page is Electron-only, because `platform.node` resolves to "not available" without the bridge, and no hub route exposes the key. The create-group wizard links it automatically over that same bridge, so the manual paste in **Profile → Link Node** exists for the operator who runs the node from a terminal and the hub from a browser — who has a terminal by definition. Anyone linking a node is already at a shell prompt, so a browser-reachable copy would buy nothing and widen what the hub knows about the node | --- diff --git a/docs/playlists.md b/docs/playlists.md index 232eae5..6378e63 100644 --- a/docs/playlists.md +++ b/docs/playlists.md @@ -69,7 +69,7 @@ were *wrong* are more useful than the ones that were right. | 1 | MNP MINOR bump to **2.1** | The wire version was already **3.0**; the bump is **3.1**, and `MNP_MIN_SUPPORTED` does not move because nothing here is required | | 2 | `kind` is `playlist:` | That refuses `playlist:favorites` — the one playlist every account has. The pattern is `[A-Za-z0-9_-]{1,64}` | | 3 | The blob is **msgpack**, "same as everything else on MNP" | The node never parses it, so the encoding is a private choice; MNP's codec is private to `transport.js`, a classic script. **JSON**, which is what keeps the merge runnable by `node` with nothing around it | -| 4 | Delete confirms inline, "not `window.confirm()`, which blocks the SPA" | Backwards. `prompt()` **throws** in Electron and is banned by a test; `confirm()` is measured to work and is used in twenty places. A *name* needs a field; a *confirmation* uses `confirm` | +| 4 | Delete confirms inline, "not `window.confirm()`, which blocks the SPA" | Right, for a reason nobody had yet. `prompt()` **throws** in Electron; `confirm()` opens, and leaves the window unable to type once it closes. A *name* needs a field; a *confirmation* uses `ask()` from `ask.js`, drawn by the page | | 5 | Submenus fly out, and are a sheet on a coarse pointer | They **expand in place**, which the account menu's language list already does. No flipping, no hover intent, no separate mobile design. The tracklist level loads when it is expanded | | 6 | Add all four new files to `STATIC_FILES` / `SPLIT_FILES` | Those lists check hook ordering. The four hookless modules do not belong in them, exactly as `source-merge.js` does not; only `menu.js` and `playlist-menu.js` do | | 7 | Four modules | **Five**: `playlist-crypto.js` is split out so seal/open can be executed standalone. A crypto layer that cannot be executed is one nobody has checked | @@ -921,15 +921,15 @@ Rules the list items carry: - **New** takes a name only, as asked, typed into **a field**: `window.prompt` does not exist in the desktop client — it throws, which is how the Files toolbar's New folder button came to do nothing at all, and - `test_no_prompt_in_the_spa.py` refuses a build that reintroduces it. The name + `test_no_native_dialogs_in_the_spa.py` refuses a build that reintroduces it. The name must be unique *for this user*, checked against the manifest and folded for case and accents, so "Soirée" and "soiree" are not two playlists. - **Delete** never lists Favourites (§5.1) — the store refuses it anyway, and offering an action that always fails is worse than not offering it. It asks - with **`window.confirm`**, which is the correction: `prompt()` throws in - Electron and is banned by a test, while `confirm()` is measured to work and is - used in twenty places in this SPA. A deletion is a tombstone and nothing in - the interface undoes it, so it is worth one question. + with **`ask()`** (`ask.js`), not `window.confirm`: in the desktop client a + native `confirm()` leaves the window unable to type after it closes, so the + whole SPA asks in the page. A deletion is a tombstone and nothing in the + interface undoes it, so it is worth one question. - **Remove a track** is the two-level one, expanded in place: pick the playlist, then its tracklist, one click per track. The second level is read from IndexedDB when it is opened (§10.1's `loadItems`). diff --git a/packages/meshbay-hub/src/meshbay_hub/static/admin-page.js b/packages/meshbay-hub/src/meshbay_hub/static/admin-page.js index d5cc7a8..b3d71dd 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/admin-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/admin-page.js @@ -2,6 +2,7 @@ import { html, useState, useEffect, useCallback, } from './vendor/htm-preact.js'; import { t } from './i18n.js'; +import { ask, tell } from './ask.js'; import { hubFetch } from './hub-client.js'; import { GroupName } from './group-name.js'; @@ -129,12 +130,12 @@ export function AdminPage({ token, role }) { const deleteUser = useCallback(async (u) => { // Suspension is the reversible tool and stays one click away; this one is // not, so it names the account and says what it cannot reach. - if (!confirm(t('admin.delete_confirm', { user: u.username }))) return; + if (!await ask(t('admin.delete_confirm', { user: u.username }))) return; try { await hubFetch(`/v1/admin/users/${u.id}`, { method: 'DELETE', token }); loadUsers(userSearch); } catch (err) { - alert(err.message); + tell(err.message); } }, [token, userSearch, loadUsers]); @@ -157,7 +158,7 @@ export function AdminPage({ token, role }) { // Suspending is the reversible tool and stays one click away; revoking // pushes a signed revocation to every node hosting the group and there is // no undo from here, so it names the group and asks first. - if (!confirm(t('admin.revoke_group_confirm', { group: g.name }))) return; + if (!await ask(t('admin.revoke_group_confirm', { group: g.name }))) return; try { await hubFetch('/v1/admin/revoke', { method: 'POST', body: { target: 'group', target_id: g.id }, token }); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/ask.js b/packages/meshbay-hub/src/meshbay_hub/static/ask.js new file mode 100644 index 0000000..8a8b895 --- /dev/null +++ b/packages/meshbay-hub/src/meshbay_hub/static/ask.js @@ -0,0 +1,70 @@ +import { html, render, useEffect, useRef } from './vendor/htm-preact.js'; +import { t } from './i18n.js'; + +/** + * `confirm` and `alert`, drawn by the page instead of the browser. + * + * In the desktop client a native `confirm()` leaves the window unable to type: + * once it closes, `document.hasFocus()` stays false, a click still moves + * `activeElement` to a field, and every keystroke after it goes nowhere — until + * the window loses and regains focus. Found as "the Create group fields are + * frozen" right after removing a member, and reproduced against Electron 44 by + * real X input events (`test_no_native_dialogs_in_the_spa.py`). `prompt` was + * already banned for throwing; the other two fail more quietly, and later. + * + * Both return a promise, so a call site reads as it did: `if (!(await + * ask(msg))) return;`. + */ + +function Dialog({ message, cancellable, onDone }) { + const okRef = useRef(null); + useEffect(() => { if (okRef.current) okRef.current.focus(); }, []); + + const onKeyDown = (e) => { + if (e.key === 'Escape') { e.preventDefault(); onDone(!cancellable); } + }; + + return html` +
{ + if (e.target.classList.contains('video-overlay')) onDone(!cancellable); + }}> +
{ e.preventDefault(); onDone(true); }}> +
+
${message}
+
+ ${cancellable && html` + `} + +
+
+
+
+ `; +} + +function open(message, cancellable) { + return new Promise((resolve) => { + const host = document.createElement('div'); + document.body.appendChild(host); + const previous = document.activeElement; + let settled = false; + const onDone = (value) => { + if (settled) return; + settled = true; + render(null, host); + host.remove(); + if (previous && previous.isConnected && typeof previous.focus === 'function') { + previous.focus(); + } + resolve(value); + }; + render(html`<${Dialog} message=${String(message)} cancellable=${cancellable} + onDone=${onDone} />`, host); + }); +} + +export const ask = (message) => open(message, true); +export const tell = (message) => open(message, false); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/chat-app.js b/packages/meshbay-hub/src/meshbay_hub/static/chat-app.js index 6e9de5f..0057218 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/chat-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/chat-app.js @@ -2,6 +2,7 @@ import { html, useState, useEffect, useLayoutEffect, useCallback, useRef, } from './vendor/htm-preact.js'; import { t, getLocale } from './i18n.js'; +import { tell } from './ask.js'; import { Icon } from './icon.js'; import { formatSize, CHUNK_SIZE, pipelinedDownload } from './file-utils.js'; @@ -516,7 +517,7 @@ function ChatPanel({ transportRef, username, userId, entries, gekRef, }]); jumpToBottom(); } catch (err) { - alert(err.message); + tell(err.message); } finally { setAttaching(false); } diff --git a/packages/meshbay-hub/src/meshbay_hub/static/explore-page.js b/packages/meshbay-hub/src/meshbay_hub/static/explore-page.js index bb47487..36b63a1 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/explore-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/explore-page.js @@ -2,6 +2,7 @@ import { html, useState, useEffect, useCallback, } from './vendor/htm-preact.js'; import { t } from './i18n.js'; +import { tell } from './ask.js'; import { hubFetch, navigate } from './hub-client.js'; import * as platform from './platform.js'; import { GroupName } from './group-name.js'; @@ -40,7 +41,7 @@ export function ExplorePage({ token, myGroupIds, allowPublicGroups = true }) { if (err.message.includes('Already a member')) { navigate(`/group/${gid}`); } else { - alert(err.message); + tell(err.message); } } finally { setJoining(null); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js index decf916..bdfba6e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js @@ -1,6 +1,7 @@ import * as downloads from './downloads.js'; import * as platform from './platform.js'; import { t } from './i18n.js'; +import { ask } from './ask.js'; import { ZipStream, entriesUnder } from './zipstream.js'; const FILE_ICONS = { @@ -556,7 +557,7 @@ async function downloadDirectory(transfers, transport, gek, entries, dir, { setE accept: { 'application/zip': ['.zip'] } }], }, 0); if (target === false) return false; - if (!target && !confirm(t('group.zip_no_stream', { + if (!target && !await ask(t('group.zip_no_stream', { size: formatSize(totalBytes), name: suggested, }))) { return false; diff --git a/packages/meshbay-hub/src/meshbay_hub/static/files-app.js b/packages/meshbay-hub/src/meshbay_hub/static/files-app.js index 3f4212e..6aabea3 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/files-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/files-app.js @@ -2,6 +2,7 @@ import { html, useState, useEffect, useRef, useCallback, } from './vendor/htm-preact.js'; import { t } from './i18n.js'; +import { ask } from './ask.js'; import { Icon } from './icon.js'; import { entriesUnder } from './zipstream.js'; import { transfers } from './transfers.js'; @@ -280,9 +281,10 @@ function FilesPanel({ * The name comes from a field in the toolbar rather than `window.prompt`, * which **throws** in Electron — "prompt() is not supported" — and threw * outside this function's try, so clicking the button did nothing at all: - * no folder, no error, nothing in the interface to react to. `confirm()` and - * `alert()` do work there and are used elsewhere; `prompt` is the one - * Chromium leaves to the embedder and Electron declines to implement. + * no folder, no error, nothing in the interface to react to. `prompt` is the + * one Chromium leaves to the embedder and Electron declines to implement; + * `confirm()` and `alert()` open but leave the window unable to type after + * them, which is why `ask.js` exists. * * An inline field is better anyway — it can show the refusal next to the * input instead of after the dialog has closed. @@ -636,10 +638,10 @@ function FilesPanel({ }), { disabled: selectedDirs.length === 0 })} ${mayEverDelete && !readOnly && action('trash', deletableCount ? t('group.delete_n', { n: deletableCount }) : t('group.delete'), - () => { + async () => { const names = [...deletableFiles.map(e => e.name), ...(operatorPaired ? selectedDirs : [])]; - if (!confirm(t('group.delete_n_confirm', { n: names.length, + if (!await ask(t('group.delete_n_confirm', { n: names.length, names: names.join(', ') }))) return; run(() => { for (const e of deletableFiles) deleteFile(e); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js b/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js index d7a1a66..5f811e8 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-settings.js @@ -2,6 +2,7 @@ import { html, useState, useEffect, useCallback, useMemo, useRef, } from './vendor/htm-preact.js'; import { t } from './i18n.js'; +import { ask } from './ask.js'; import { Icon } from './icon.js'; import { CollapsibleSection, ToggleSwitch } from './settings-ui.js'; import { hubFetch, navigate } from './hub-client.js'; @@ -189,7 +190,7 @@ function SharedDirectoriesTable({ roots, groupId, transport, signFn, } return; } - if (!confirm(t('node.root_remove_confirm', { name: rootName }))) return; + if (!await ask(t('node.root_remove_confirm', { name: rootName }))) return; const ok = await run(async () => { if (overMnp) await transport.removeRoot(groupId, rootName, signFn); else if (overLoopback) { @@ -531,7 +532,7 @@ function GroupSettingsPanel({ groupId, group, token, transportRef, gekRef, }, [approveCode, userId, transportRef, loadDevices]); const revokeDevice = useCallback(async (device) => { - if (!confirm(t('device.revoke_confirm'))) return; + if (!await ask(t('device.revoke_confirm'))) return; setDeviceMsg(''); try { await transportRef.current.revokeDevice( @@ -1046,7 +1047,7 @@ function GroupSettingsPanel({ groupId, group, token, transportRef, gekRef, ${isOwner ? html`