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 --- .../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 +- 6 files changed, 117 insertions(+), 97 deletions(-) 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 (limited to 'packages/meshbay-hub/tests') diff --git a/packages/meshbay-hub/tests/harness/playlist_ui_probe.py b/packages/meshbay-hub/tests/harness/playlist_ui_probe.py index e6f99f7..c705244 100644 --- a/packages/meshbay-hub/tests/harness/playlist_ui_probe.py +++ b/packages/meshbay-hub/tests/harness/playlist_ui_probe.py @@ -180,11 +180,9 @@ const clickMenu = async (i) => { ['encrypt', 'decrypt']), v2hkdf: await crypto.subtle.importKey('raw', raw, 'HKDF', false, ['deriveKey']), }; - // Deleting a playlist asks. A modal dialog blocks the page and would wedge - // the probe, so the answer is stubbed rather than the question avoided — - // `confirm` is what the application really calls. + // Deleting a playlist asks, in the page (ask.js) — so the probe answers the + // dialog the way a person would, by clicking its OK button. let asked = null; - window.confirm = (q) => { asked = q; return true; }; render(html`<${Harness} />`, document.getElementById('root')); @@ -282,6 +280,13 @@ const clickMenu = async (i) => { await openToolbar(); await clickLabel(t('playlists.delete')); await clickLabel('Soirée'); + for (let i = 0; i < 40 && !asked; i++) { + const dlg = document.querySelector('[role=alertdialog]'); + if (dlg) { + asked = dlg.querySelector('.ask-message').textContent; + dlg.querySelector('button[type=submit]').click(); + } else await sleep(50); + } await sleep(300); steps.push({ step: 'deleted', asked: !!asked, lists: (await P.listPlaylists('u1')).map((p) => p.name) }); diff --git a/packages/meshbay-hub/tests/test_hook_ordering.py b/packages/meshbay-hub/tests/test_hook_ordering.py index d03cdf5..d5ffcfe 100644 --- a/packages/meshbay-hub/tests/test_hook_ordering.py +++ b/packages/meshbay-hub/tests/test_hook_ordering.py @@ -52,6 +52,8 @@ STATIC_FILES = [ # The shared pop-up menu (docs/playlists.md §10.1), reached from the media # views rather than imported by the shell. "menu.js", "playlist-menu.js", + # The page's own confirm/alert, mounted outside the app tree. + "ask.js", ] pytestmark = pytest.mark.skipif(not APP.exists(), reason="SPA sources unavailable") diff --git a/packages/meshbay-hub/tests/test_no_native_dialogs_in_the_spa.py b/packages/meshbay-hub/tests/test_no_native_dialogs_in_the_spa.py new file mode 100644 index 0000000..c868aa0 --- /dev/null +++ b/packages/meshbay-hub/tests/test_no_native_dialogs_in_the_spa.py @@ -0,0 +1,99 @@ +""" +No `prompt`, `confirm` or `alert` in the SPA — `ask.js` draws them instead. + +The same `static/` tree is the web page and the application (CLAUDE.md's "one +UI source"), and in the desktop client none of the three browser dialogs can be +used. Measured against this repo's own Electron 44: + + prompt('name?') -> Error: prompt() is not supported. + confirm('sure?') -> opens a real modal, and breaks typing once it closes + alert('hi') -> same as confirm + +`prompt` throws, which made the Files toolbar's New folder button do nothing +whatsoever: the call sat above its own try, so the click produced no folder, no +error, and nothing on screen to react to. + +`confirm` and `alert` fail later and more quietly. After one closes, +`document.hasFocus()` stays false: a click still moves `activeElement` into a +field, but 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 it was confirmed with real X input events under Xvfb (click and type +into an input and a textarea, before and after a `confirm()`): three runs out of +three, typing lands before the dialog and nothing lands after it. A test that +dispatches DOM events cannot see this, since the focus is lost below the page, +so the guard is on the source. +""" + +import re +from pathlib import Path + +import pytest + +STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" + +pytestmark = pytest.mark.skipif(not STATIC.exists(), + reason="SPA sources unavailable") + +# A call, not the word inside a comment or a longer identifier like +# `mkdir_prompt` / `promptForName` / `role="alert"`. +CALL = re.compile(r"(? str: + source = re.sub(r"/\*.*?\*/", "", source, flags=re.S) + return re.sub(r"^\s*//.*$", "", source, flags=re.M) + + +def test_nothing_calls_a_native_dialog(): + offenders = [] + for path in sorted(STATIC.glob("*.js")): + if path.name == "sw.js": + continue + for i, line in enumerate(_code_only( + path.read_text(encoding="utf-8")).splitlines(), 1): + if CALL.search(line): + offenders.append(f"{path.name}:{i}: {line.strip()}") + + assert not offenders, ( + "prompt() throws in the desktop client and confirm()/alert() leave it " + "unable to type; use ask()/tell() from ask.js:\n " + "\n ".join(offenders)) + + +def test_the_pattern_would_catch_a_real_call(): + """ + A guard that matches nothing passes over an empty set, which looks exactly + like success. Both spellings, and the near-misses it must not flag. + """ + assert CALL.search("const n = prompt('x');") + assert CALL.search("const n = window.prompt('x');") + assert CALL.search("if (!confirm(t('x'))) return;") + assert CALL.search("if (!window.confirm(q)) return;") + assert CALL.search("alert(err.message);") + assert not CALL.search("
") + assert not CALL.search("t('members.remove_confirm')") + assert not CALL.search("await ask(q)") + assert not CALL.search("t('group.mkdir_prompt')") + assert not CALL.search("promptForName();") + assert not CALL.search("this.prompt(1);") + + +def test_creating_a_folder_uses_a_field(): + """ + The control the ban is about. A typed name needs somewhere to type it, and + an inline field can show the node's refusal beside the input rather than + after a dialog has closed. + """ + source = (STATIC / "files-app.js").read_text(encoding="utf-8") + assert "tb-mkdir-input" in source + assert "newDirName" in source + assert "group.mkdir_prompt" in source, "the field has no label or placeholder" + + +def test_leaving_the_folder_drops_a_half_typed_name(): + """ + Otherwise the folder is created where the person is no longer looking — + they navigated away, the draft came along, and the name lands in a + directory they were not thinking about. + """ + source = (STATIC / "files-app.js").read_text(encoding="utf-8") + assert "useEffect(() => { setNewDirName(null); }, [currentPath]);" in source diff --git a/packages/meshbay-hub/tests/test_no_prompt_in_the_spa.py b/packages/meshbay-hub/tests/test_no_prompt_in_the_spa.py deleted file mode 100644 index d126a05..0000000 --- a/packages/meshbay-hub/tests/test_no_prompt_in_the_spa.py +++ /dev/null @@ -1,90 +0,0 @@ -""" -`window.prompt` does not exist in the desktop client. - -The same `static/` tree is the web page and the application (CLAUDE.md's "one -UI source"), and Electron does not implement `prompt` — Chromium leaves it to -the embedder and Electron declines. It does not return null: it **throws**, -`Error: prompt() is not supported.` - -That made the Files toolbar's New folder button do nothing whatsoever. The call -sat above its own try, so the click produced no folder, no error, and nothing -on screen to react to — the failure looks exactly like a dead button, which is -what it was reported as. - -Measured rather than assumed, against this repo's own Electron 44: - - prompt('name?') -> Error: prompt() is not supported. - confirm('sure?') -> opens a real modal - alert('hi') -> opens a real modal - -So `confirm` and `alert` stay allowed and are used in a dozen places; only -`prompt` is banned. Anything that needs typed input needs a field. -""" - -import re -from pathlib import Path - -import pytest - -STATIC = Path(__file__).resolve().parents[1] / "src" / "meshbay_hub" / "static" - -pytestmark = pytest.mark.skipif(not STATIC.exists(), - reason="SPA sources unavailable") - -# `prompt(` as a call, not `window.prompt` inside a comment or a longer -# identifier like `mkdir_prompt` / `promptForName`. -CALL = re.compile(r"(? str: - source = re.sub(r"/\*.*?\*/", "", source, flags=re.S) - return re.sub(r"^\s*//.*$", "", source, flags=re.M) - - -def test_nothing_calls_prompt(): - offenders = [] - for path in sorted(STATIC.glob("*.js")): - if path.name == "sw.js": - continue - for i, line in enumerate(_code_only( - path.read_text(encoding="utf-8")).splitlines(), 1): - if CALL.search(line): - offenders.append(f"{path.name}:{i}: {line.strip()}") - - assert not offenders, ( - "prompt() throws in the desktop client, and the click that reaches it " - "does nothing at all:\n " + "\n ".join(offenders)) - - -def test_the_pattern_would_catch_a_real_call(): - """ - A guard that matches nothing passes over an empty set, which looks exactly - like success. Both spellings, and the near-misses it must not flag. - """ - assert CALL.search("const n = prompt('x');") - assert CALL.search("const n = window.prompt('x');") - assert not CALL.search("t('group.mkdir_prompt')") - assert not CALL.search("promptForName();") - assert not CALL.search("this.prompt(1);") - - -def test_creating_a_folder_uses_a_field(): - """ - The control the ban is about. A typed name needs somewhere to type it, and - an inline field can show the node's refusal beside the input rather than - after a dialog has closed. - """ - source = (STATIC / "files-app.js").read_text(encoding="utf-8") - assert "tb-mkdir-input" in source - assert "newDirName" in source - assert "group.mkdir_prompt" in source, "the field has no label or placeholder" - - -def test_leaving_the_folder_drops_a_half_typed_name(): - """ - Otherwise the folder is created where the person is no longer looking — - they navigated away, the draft came along, and the name lands in a - directory they were not thinking about. - """ - source = (STATIC / "files-app.js").read_text(encoding="utf-8") - assert "useEffect(() => { setNewDirName(null); }, [currentPath]);" in source diff --git a/packages/meshbay-hub/tests/test_playlist_ui.py b/packages/meshbay-hub/tests/test_playlist_ui.py index c449dc7..8259088 100644 --- a/packages/meshbay-hub/tests/test_playlist_ui.py +++ b/packages/meshbay-hub/tests/test_playlist_ui.py @@ -101,8 +101,7 @@ def test_removing_a_track_removes_that_one(steps): def test_deleting_a_playlist_asks_first(steps): """A deletion is a tombstone: there is nothing in the interface that undoes - it. `confirm` and not a component — Electron implements it and a dozen - places in this SPA already use it.""" + it. Asked in the page, not by `confirm` (`test_no_native_dialogs_in_the_spa.py`).""" s = steps["deleted"] assert s["asked"] is True, "a playlist was deleted without asking" assert s["lists"] == [] diff --git a/packages/meshbay-hub/tests/test_zip_size_limit.py b/packages/meshbay-hub/tests/test_zip_size_limit.py index 26c5552..9243fd1 100644 --- a/packages/meshbay-hub/tests/test_zip_size_limit.py +++ b/packages/meshbay-hub/tests/test_zip_size_limit.py @@ -14,7 +14,7 @@ of allowance and nobody would ever notice. And that the two limits in play do not contradict each other: ZIP_MAX_BYTES (512 MB) bounds the archive, while MEMORY_CEILING (100 MB, test_memory_ceiling.py) bounds what may be built in the page — so a 400 MB zip is allowed when there is somewhere to stream it and -refused when the only route left is memory. The `confirm()` that offers the +refused when the only route left is memory. The `ask()` that offers the build-in-memory path therefore only ever appears below the ceiling. """ @@ -47,6 +47,11 @@ def _run(total_bytes, tmp_path, picker=False): (sandbox / src.name).write_text(src.read_text(encoding="utf-8"), encoding="utf-8") (tmp_path / "package.json").write_text('{"type":"module"}') + # ask.js draws a dialog in the DOM, which Node has none of; the question is + # answered here instead, exactly where `confirm` used to be stubbed. + (sandbox / "ask.js").write_text( + "export const ask = async (q) => globalThis.confirm(q);\n" + "export const tell = async () => {};\n", encoding="utf-8") script = tmp_path / "case.mjs" picker_js = "true" if picker else "false" -- cgit v1.2.3