From c384878aa0d6ef7a33bda23887dc264b94386725 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 18 Aug 2026 14:07:16 +0200 Subject: fix(client): a film can go full-screen, and automatic saving is automatic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Full-screen was denied, and the denial was invisible.** The permission handler was written from a true sentence — nothing here needs a camera, a microphone or a location — and implemented as `callback(false)` for everything. Chromium's own video controls ask for the `fullscreen` permission, so a film could not be watched full-screen. What made it hard to find, and what the test now pins: **a denied `fullscreen` does not reject.** `requestFullscreen()` returns a promise that never settles. No exception, no console message, nothing in the renderer that mentions a permission — the button just does nothing. Measured rather than reasoned: the probe reported `NEVER SETTLED` while the main process, instrumented for one run, logged `PERMISSION ASKED: fullscreen`. After the fix the same probe reports `granted` with `document.fullscreenElement` set. The handler now enumerates what is *granted* — `fullscreen`, and nothing else — so a camera, a microphone, a location, notifications and MIDI are still refused and whatever Chromium adds next arrives refused rather than quietly allowed. `Permissions.query` takes the other handler, so both now answer from the one list instead of eventually disagreeing. The old test asserted `callback(false)`, which is to say it locked in the bug. It is replaced by three: what must stay denied, that `fullscreen` is granted, and that both handlers read the same list. **"Save automatically" opened a dialog.** The automatic path required a folder to have been chosen first, and on a new profile nobody has chosen one — so the very first download fell through to Save As, which is the one thing the setting promises not to do. A browser does not make you pick a folder before it will save a file; the system Downloads folder is the answer when there is no other. Verified on a fresh profile with a home of its own: no dialog, 1024 bytes on disk, destination reported as the default (`/home/…/Téléchargements` on this machine, via the localized XDG directory). A folder that *was* chosen and has since gone still asks. Silently redirecting those files is worse than a dialog: someone who picked an external drive wants to be told it is not there, not to find the film in their home directory a week later. Settings shows the effective destination either way, and offers "forget" only for a folder somebody actually chose. 813 tests pass. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 8 +++ docs/desktop-client-v1.md | 13 ++++- packages/meshbay-client/src/main.js | 65 ++++++++++++++++++---- packages/meshbay-hub/src/meshbay_hub/static/app.js | 2 +- packages/meshbay-hub/tests/test_desktop_shell.py | 65 ++++++++++++++++++++-- 5 files changed, 136 insertions(+), 17 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 161aac4..fcddaf5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -402,6 +402,14 @@ anything that assumes one key per person. for it would have failed these tests for no visible reason. Use `Object.defineProperty`; the suite is now green on 18 and 24 +- **A refusal that never rejects.** Denying Chromium's `fullscreen` permission + does not make `requestFullscreen()` throw — the promise never settles. The + deny-everything handler was written from a true sentence ("nothing here needs + a camera") and quietly broke watching a film full-screen, with no error + anywhere to lead back to it. Prefer enumerating what is *granted*: the list + is short, and the next thing Chromium invents arrives refused rather than + silently allowed + - **A fallback chain reaches its floor silently.** `_openDownloadTarget` tries a granted folder, then a service worker, then "collect it in memory and hand the browser a blob". In the desktop application the first two do not exist — diff --git a/docs/desktop-client-v1.md b/docs/desktop-client-v1.md index 23287dd..c0bbb90 100644 --- a/docs/desktop-client-v1.md +++ b/docs/desktop-client-v1.md @@ -181,7 +181,18 @@ Electron it is nearly all of it. ### 3.1 What running it changed -Three of the statements above were wrong, and only launching the application found them. +Four of the statements above were wrong, and only launching the application found them. + +**"Nothing here needs a camera, a microphone or a location" was true, and the handler +written from it was still wrong.** Denying every permission also denied `fullscreen`, and +Chromium's own video controls ask for it — so a film could not be watched full-screen. +What makes this worth recording rather than just fixing: **a denied `fullscreen` does not +reject.** `requestFullscreen()` returns a promise that never settles. No error, no console +message, nothing in the renderer that names a permission; the button simply does nothing, +and the operator reported it as "impossible to go full-screen" with no lead to follow. The +probe reported `NEVER SETTLED` while the main process logged `PERMISSION ASKED: +fullscreen`, which is what tied the two ends together. The handler now enumerates what is +*granted* — one entry — so anything Chromium adds later still arrives refused. **The CSP cannot live in a `` tag.** `frame-ancestors` is ignored there — Chromium says so in the console — so a policy carrying it has one directive that silently does diff --git a/packages/meshbay-client/src/main.js b/packages/meshbay-client/src/main.js index f97be8e..8e11f1f 100644 --- a/packages/meshbay-client/src/main.js +++ b/packages/meshbay-client/src/main.js @@ -25,6 +25,7 @@ const { app, BrowserWindow, dialog, ipcMain, protocol, safeStorage, shell } = const crypto = require('node:crypto'); const fs = require('node:fs'); const fsp = require('node:fs/promises'); +const os = require('node:os'); const path = require('node:path'); const { pathToFileURL } = require('node:url'); @@ -247,6 +248,10 @@ function publicKeyB64(privateKey) { return der.subarray(der.length - 32).toString('base64'); } +// Everything the interface is allowed to ask Chromium for. Watching a film +// full-screen is the whole list. +const GRANTED_PERMISSIONS = new Set(['fullscreen']); + // ── Window ────────────────────────────────────────────────────────────────── let mainWindow = null; @@ -297,9 +302,23 @@ function createWindow() { if (/^https?:$/.test(new URL(url).protocol)) shell.openExternal(url); return { action: 'deny' }; }); - // Nothing in this application needs a camera, a microphone or a location. + // Deny by default, with one exception, and the exception is the point of the + // comment. A blanket `callback(false)` here is what stopped a film going + // fullscreen: Chromium's own video controls ask for `fullscreen`, and a + // **denial does not reject** — `requestFullscreen()` returns a promise that + // never settles, so the button simply does nothing and there is no error + // anywhere to find. Measured, not deduced: the probe reported `NEVER SETTLED` + // and the main process logged `PERMISSION ASKED: fullscreen`. + // + // So: enumerate what is granted rather than what is refused. A camera, a + // microphone, a location, notifications and MIDI are all still refused, and + // anything Chromium adds later arrives refused rather than quietly allowed. win.webContents.session.setPermissionRequestHandler( - (_wc, _permission, callback) => callback(false)); + (_wc, permission, callback) => callback(GRANTED_PERMISSIONS.has(permission))); + // `Permissions.query` takes the other handler; same answer, or the two can + // disagree about what the page is allowed to do. + win.webContents.session.setPermissionCheckHandler( + (_wc, permission) => GRANTED_PERMISSIONS.has(permission)); win.on('close', () => { if (!win.isMinimized() && !win.isFullScreen()) { @@ -496,12 +515,27 @@ function registerBridge() { return config.downloadDir; }); - ipcMain.handle('folder:get', () => { + // Where downloads land when nobody has chosen anywhere. A browser does not + // make you pick a folder before it will save a file, and neither should this + // — "save automatically" that opens a dialog is not automatic. + function defaultDownloadDir() { + try { return app.getPath('downloads'); } catch { return os.homedir(); } + } + + function chosenDownloadDir() { const dir = config.downloadDir; // A folder that has been removed or unmounted is not a folder any more, // and saying so beats failing on the first chunk of a download. if (!dir) return null; try { return fs.statSync(dir).isDirectory() ? dir : null; } catch { return null; } + } + + ipcMain.handle('folder:get', () => { + const chosen = chosenDownloadDir(); + // `name` is what the settings row already renders, for the browser's + // directory handle as much as for this. `isDefault` is how it knows not to + // offer "forget" for a folder nobody chose. + return { name: chosen || defaultDownloadDir(), isDefault: !chosen }; }); ipcMain.handle('folder:forget', () => { @@ -512,22 +546,31 @@ function registerBridge() { ipcMain.handle('save:begin', async (_e, suggestedName, opts) => { const wanted = path.basename(String(suggestedName || 'download')); - const remembered = config.downloadDir; + const chosen = chosenDownloadDir(); let target = null; - // "Save automatically" means exactly that: no dialog, into the folder that - // was chosen once. - if (opts && opts.auto && remembered) { + // "Save automatically" means exactly that: no dialog. Into the chosen + // folder if there is one, otherwise the system's Downloads folder — the + // first version required a folder to have been picked first, so the very + // first automatic download opened a dialog, which is the one thing the + // setting says it will not do. + // + // The one case that still asks: a folder *was* chosen and has since gone. + // Redirecting those files somewhere else without saying so is worse than a + // dialog — someone who picked an external drive wants to know it is not + // there, not to find the film in their home directory a week later. + if (opts && opts.auto && !(config.downloadDir && !chosen)) { + const dir = chosen || defaultDownloadDir(); try { - if (fs.statSync(remembered).isDirectory()) { - target = path.join(remembered, freeName(remembered, wanted)); - } + fs.mkdirSync(dir, { recursive: true }); + target = path.join(dir, freeName(dir, wanted)); } catch { target = null; } } if (!target) { + const dir = chosen || defaultDownloadDir(); const result = await dialog.showSaveDialog(mainWindow, { - defaultPath: remembered ? path.join(remembered, wanted) : wanted, + defaultPath: path.join(dir, wanted), }); if (result.canceled || !result.filePath) return null; target = result.filePath; diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index f49d412..07fc781 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -4096,7 +4096,7 @@ function SettingsPage({ user, theme, onThemeChange, groups }) { - ${dlDir && html` + ${dlDir && !dlDir.isDefault && html`