From 16a0deaa4ddc23202ea0c9baa5ded5287c89b8d3 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 14 Jul 2026 15:16:14 +0000 Subject: [PATCH] Fix desktop browser review regressions --- docs/browser-capture-harness.md | 7 +- packages/desktop/capture-harness/main.js | 77 +++++++------------ .../browser-keyboard/guest-preload.ts | 28 +++---- 3 files changed, 42 insertions(+), 70 deletions(-) diff --git a/docs/browser-capture-harness.md b/docs/browser-capture-harness.md index 69e019442..038968b6a 100644 --- a/docs/browser-capture-harness.md +++ b/docs/browser-capture-harness.md @@ -17,10 +17,9 @@ It validates the compositor behavior that unit tests cannot see: - the real-Electron host-composer sentinel proves guest Enter cannot submit a focused host composer; - the automation group loads the compiled production keyboard boundary and guest - preload, then proves that page window handlers get first refusal even when they - register after preload initialization, unhandled shortcuts cross from both ordinary - and editable page targets, digit wildcard shortcuts cross, and background automation - stays in the guest. + preload, then proves that initial page window handlers get first refusal, unhandled + shortcuts synchronously suppress editable browser defaults before crossing the host + boundary, digit wildcard shortcuts cross, and background automation stays in the guest. Run it with the repo Electron: diff --git a/packages/desktop/capture-harness/main.js b/packages/desktop/capture-harness/main.js index 22efc27e4..b7cc9a303 100644 --- a/packages/desktop/capture-harness/main.js +++ b/packages/desktop/capture-harness/main.js @@ -1774,46 +1774,6 @@ function installBrowserKeyboardSentinels() { }; } -async function verifyLateBrowserShortcutFirstRefusal({ guest, sentinel }) { - const { shortcutInputs } = sentinel; - await guest.executeJavaScript( - `window.addEventListener("keydown", function preventLateBrowserShortcut(event) { - if ( - (event.metaKey || event.ctrlKey) && - !event.altKey && - !event.shiftKey && - event.key.toLowerCase() === "b" - ) { - window.removeEventListener("keydown", preventLateBrowserShortcut); - event.preventDefault(); - window.fixtureLog.push({ - event: "late-shortcut-b", - defaultPrevented: event.defaultPrevented, - trusted: event.isTrusted, - }); - } - });`, - true, - ); - automationBrowserShortcut(guest); - await waitForAutomationLog( - guest, - (entry) => - entry.event === "late-shortcut-b" && - entry.defaultPrevented === true && - entry.trusted === true, - "late page-prevented trusted browser shortcut", - ); - await delay(100); - if (shortcutInputs.length !== 0 || sentinel.applicationMenuShortcutHits !== 0) { - fail( - `late page-prevented browser shortcut escaped guest: inputs=${JSON.stringify(shortcutInputs)} menu=${sentinel.applicationMenuShortcutHits}`, - ); - } - pass("automation late page preventDefault keeps browser shortcut in the guest"); - return { group: "automation", check: "browser-shortcut-late-page-prevented", pass: true }; -} - async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta, sentinel }) { const checks = []; const { shortcutInputs } = sentinel; @@ -1853,8 +1813,6 @@ async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta, checks.push({ group: "automation", check: "browser-shortcut-page-prevented", pass: true }); await guest.executeJavaScript("window.preventBrowserShortcut = false", true); - checks.push(await verifyLateBrowserShortcutFirstRefusal({ guest, sentinel })); - const unhandledTarget = await guest.executeJavaScript( "document.getElementById('save').focus(); document.activeElement.id", true, @@ -1899,21 +1857,42 @@ async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta, checks.push({ group: "automation", check: "browser-shortcut-page-first-forward", pass: true }); const editableTarget = await guest.executeJavaScript( - "document.getElementById('name').focus(); document.activeElement.id", + `(() => { + const editable = document.querySelector("p"); + editable.contentEditable = "true"; + editable.focus(); + const range = document.createRange(); + range.selectNodeContents(editable); + const selection = window.getSelection(); + selection.removeAllRanges(); + selection.addRange(range); + return document.activeElement === editable; + })()`, true, ); - if (editableTarget !== "name") { - fail(`automation editable browser shortcut target was not focused: ${editableTarget}`); + if (!editableTarget) { + fail("automation editable browser shortcut target was not focused"); } automationBrowserShortcut(guest); await waitForBrowserShortcutInput(shortcutInputs, 2); - if (!isDeepStrictEqual(shortcutInputs[1], expectedBrowserShortcutInput)) { + await delay(100); + const editableState = await guest.executeJavaScript( + `(() => { + const editable = document.querySelector("p"); + return { bold: document.queryCommandState("bold"), html: editable.innerHTML }; + })()`, + true, + ); + if ( + !isDeepStrictEqual(shortcutInputs[1], expectedBrowserShortcutInput) || + !isDeepStrictEqual(editableState, { bold: false, html: "Connected as Maya" }) + ) { fail( - `editable browser shortcut crossed boundary incorrectly: inputs=${JSON.stringify(shortcutInputs)}`, + `editable browser shortcut crossed boundary incorrectly: inputs=${JSON.stringify(shortcutInputs)} editable=${JSON.stringify(editableState)}`, ); } - pass("automation production guest preload forwards an unhandled editable browser shortcut"); - checks.push({ group: "automation", check: "browser-shortcut-editable-forward", pass: true }); + pass("automation guest preload owns an unhandled editable browser shortcut"); + checks.push({ group: "automation", check: "browser-shortcut-editable-owned", pass: true }); automationBrowserShortcut(guest, "1"); await waitForBrowserShortcutInput(shortcutInputs, 3); diff --git a/packages/desktop/src/features/browser-keyboard/guest-preload.ts b/packages/desktop/src/features/browser-keyboard/guest-preload.ts index 6faa70c83..a543f6afb 100644 --- a/packages/desktop/src/features/browser-keyboard/guest-preload.ts +++ b/packages/desktop/src/features/browser-keyboard/guest-preload.ts @@ -53,23 +53,17 @@ function installKeydownListener(): void { if (!event.isTrusted || event.defaultPrevented || !browserId || !matchesPolicy(event)) { return; } - const shortcutBrowserId = browserId; - setTimeout(() => { - if (event.defaultPrevented) { - return; - } - event.preventDefault(); - ipcRenderer.send(SHORTCUT_INPUT_CHANNEL, { - alt: event.altKey, - browserId: shortcutBrowserId, - code: event.code, - control: event.ctrlKey, - key: event.key, - meta: event.metaKey, - repeat: event.repeat, - shift: event.shiftKey, - }); - }, 0); + event.preventDefault(); + ipcRenderer.send(SHORTCUT_INPUT_CHANNEL, { + alt: event.altKey, + browserId, + code: event.code, + control: event.ctrlKey, + key: event.key, + meta: event.metaKey, + repeat: event.repeat, + shift: event.shiftKey, + }); }); }