mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
Fix browser shortcut frame regressions
This commit is contained in:
@@ -19,7 +19,9 @@ It validates the compositor behavior that unit tests cannot see:
|
||||
- the automation group loads the compiled production keyboard boundary and 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.
|
||||
boundary, handlers registered after preload still get first refusal, focused iframes
|
||||
share the same boundary, digit wildcard shortcuts cross, and background automation stays
|
||||
in the guest.
|
||||
|
||||
Run it with the repo Electron:
|
||||
|
||||
|
||||
@@ -260,8 +260,9 @@ describe("resident browser webviews", () => {
|
||||
});
|
||||
|
||||
it("finds the originating browser webview for focus restoration", () => {
|
||||
const webview = ensureResidentBrowserWebview({
|
||||
const webview = ensureTestBrowser({
|
||||
browserId: "browser-focus",
|
||||
workspaceId: "workspace-focus",
|
||||
url: "https://example.com",
|
||||
});
|
||||
|
||||
|
||||
@@ -25,6 +25,7 @@ export interface DesktopDialogOpenOptions {
|
||||
title?: string;
|
||||
defaultPath?: string;
|
||||
directory?: boolean;
|
||||
createDirectory?: boolean;
|
||||
multiple?: boolean;
|
||||
filters?: Array<{
|
||||
name: string;
|
||||
@@ -66,13 +67,15 @@ export interface DesktopEditorTargetDescriptor {
|
||||
id: string;
|
||||
label: string;
|
||||
kind: "editor" | "file-manager";
|
||||
icon: { kind: "image"; dataUrl: string } | { kind: "symbol"; name: "folder" | "terminal" };
|
||||
}
|
||||
|
||||
export interface DesktopEditorOpenTargetInput {
|
||||
editorId: string;
|
||||
path: string;
|
||||
cwd?: string;
|
||||
mode?: "open" | "reveal";
|
||||
workspacePath: string;
|
||||
filePath?: string;
|
||||
line?: number;
|
||||
column?: number;
|
||||
}
|
||||
|
||||
export interface DesktopEditorBridge {
|
||||
|
||||
@@ -494,7 +494,7 @@ function installHarnessWebviewGuards(win, options = {}) {
|
||||
webPreferences.nodeIntegration = false;
|
||||
webPreferences.contextIsolation = true;
|
||||
if (options.preloadPath) {
|
||||
webPreferences.nodeIntegrationInSubFrames = false;
|
||||
webPreferences.nodeIntegrationInSubFrames = true;
|
||||
webPreferences.nodeIntegrationInWorker = false;
|
||||
webPreferences.sandbox = true;
|
||||
webPreferences.webSecurity = true;
|
||||
@@ -1236,6 +1236,27 @@ function automationFixtureUrl() {
|
||||
<script>
|
||||
window.fixtureLog = [];
|
||||
window.preventBrowserShortcut = false;
|
||||
window.installLateBrowserShortcutHandler = () => {
|
||||
window.addEventListener(
|
||||
"keydown",
|
||||
(event) => {
|
||||
if (
|
||||
(event.metaKey || event.ctrlKey) &&
|
||||
!event.altKey &&
|
||||
!event.shiftKey &&
|
||||
event.key.toLowerCase() === "b"
|
||||
) {
|
||||
event.preventDefault();
|
||||
window.fixtureLog.push({
|
||||
event: "late-shortcut-b",
|
||||
defaultPrevented: event.defaultPrevented,
|
||||
trusted: event.isTrusted,
|
||||
});
|
||||
}
|
||||
},
|
||||
{ once: true },
|
||||
);
|
||||
};
|
||||
window.addEventListener("keydown", (event) => {
|
||||
if (
|
||||
(event.metaKey || event.ctrlKey) &&
|
||||
@@ -1774,6 +1795,57 @@ function installBrowserKeyboardSentinels() {
|
||||
};
|
||||
}
|
||||
|
||||
async function verifyLatePageShortcutFirstRefusal({ guest, sentinel, shortcutInputs }) {
|
||||
await guest.executeJavaScript(
|
||||
"window.preventBrowserShortcut = false; window.installLateBrowserShortcutHandler();",
|
||||
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 handlers get first refusal for browser shortcuts");
|
||||
return { group: "automation", check: "browser-shortcut-late-page-handler", pass: true };
|
||||
}
|
||||
|
||||
async function verifyFocusedIframeShortcut({ guest, shortcutInputs, expectedInput }) {
|
||||
const iframeFocused = await guest.executeJavaScript(
|
||||
`new Promise((resolve) => {
|
||||
const iframe = document.createElement("iframe");
|
||||
iframe.srcdoc = '<!doctype html><button id="iframe-target">Frame target</button>';
|
||||
iframe.addEventListener("load", () => {
|
||||
iframe.contentDocument.getElementById("iframe-target").focus();
|
||||
resolve(iframe.contentDocument.activeElement.id);
|
||||
}, { once: true });
|
||||
document.body.appendChild(iframe);
|
||||
})`,
|
||||
true,
|
||||
);
|
||||
if (iframeFocused !== "iframe-target") {
|
||||
fail(`automation iframe browser shortcut target was not focused: ${iframeFocused}`);
|
||||
}
|
||||
automationBrowserShortcut(guest);
|
||||
await waitForBrowserShortcutInput(shortcutInputs, 4);
|
||||
if (!isDeepStrictEqual(shortcutInputs[3], expectedInput)) {
|
||||
fail(
|
||||
`iframe browser shortcut crossed boundary incorrectly: inputs=${JSON.stringify(shortcutInputs)}`,
|
||||
);
|
||||
}
|
||||
pass("automation production guest preload forwards shortcuts from focused iframes");
|
||||
return { group: "automation", check: "browser-shortcut-focused-iframe", pass: true };
|
||||
}
|
||||
|
||||
async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta, sentinel }) {
|
||||
const checks = [];
|
||||
const { shortcutInputs } = sentinel;
|
||||
@@ -1812,6 +1884,8 @@ async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta,
|
||||
pass("automation page preventDefault keeps browser shortcut in the guest");
|
||||
checks.push({ group: "automation", check: "browser-shortcut-page-prevented", pass: true });
|
||||
|
||||
checks.push(await verifyLatePageShortcutFirstRefusal({ guest, sentinel, shortcutInputs }));
|
||||
|
||||
await guest.executeJavaScript("window.preventBrowserShortcut = false", true);
|
||||
const unhandledTarget = await guest.executeJavaScript(
|
||||
"document.getElementById('save').focus(); document.activeElement.id",
|
||||
@@ -1914,6 +1988,14 @@ async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta,
|
||||
pass("automation production guest preload forwards digit wildcard shortcuts");
|
||||
checks.push({ group: "automation", check: "browser-shortcut-digit-wildcard", pass: true });
|
||||
|
||||
checks.push(
|
||||
await verifyFocusedIframeShortcut({
|
||||
guest,
|
||||
shortcutInputs,
|
||||
expectedInput: expectedBrowserShortcutInput,
|
||||
}),
|
||||
);
|
||||
|
||||
const focusedGuestInput = await guest.executeJavaScript(
|
||||
"document.getElementById('name').focus(); document.activeElement.id",
|
||||
true,
|
||||
@@ -1986,6 +2068,7 @@ async function runAutomationGroup() {
|
||||
const { PaseoBrowserWebviewRegistry } = require(PRODUCTION_BROWSER_WEBVIEW_REGISTRY_PATH);
|
||||
const browserRegistry = new PaseoBrowserWebviewRegistry();
|
||||
const browserKeyboard = new BrowserKeyboard(browserRegistry);
|
||||
browserKeyboard.registerIpc();
|
||||
const browserKeyboardSentinels = installBrowserKeyboardSentinels();
|
||||
const handle = createInactiveHarnessWindow({
|
||||
width: 1000,
|
||||
|
||||
@@ -2,11 +2,11 @@ import { ipcRenderer } from "electron";
|
||||
import type { BrowserKeyboardPolicy, BrowserShortcutPrefix } from "./policy.js";
|
||||
|
||||
const POLICY_CHANNEL = "paseo:browser-keyboard-policy";
|
||||
const POLICY_REQUEST_CHANNEL = "paseo:browser-keyboard-policy-request";
|
||||
const SHORTCUT_INPUT_CHANNEL = "paseo:browser-shortcut-input";
|
||||
|
||||
let browserId: string | null = null;
|
||||
let policy: BrowserShortcutPrefix[] = [];
|
||||
let keydownListenerInstalled = false;
|
||||
|
||||
interface BrowserKeyboardPolicyPayload extends BrowserKeyboardPolicy {
|
||||
browserId: string;
|
||||
@@ -44,45 +44,42 @@ function matchesCode(prefixCode: string, eventCode: string): boolean {
|
||||
return /^(?:Digit|Numpad)[1-9]$/.test(eventCode);
|
||||
}
|
||||
|
||||
function installKeydownListener(): void {
|
||||
if (keydownListenerInstalled) {
|
||||
function stageShortcutForward(event: KeyboardEvent): void {
|
||||
if (!event.isTrusted || event.defaultPrevented || !browserId || !matchesPolicy(event)) {
|
||||
return;
|
||||
}
|
||||
keydownListenerInstalled = true;
|
||||
window.addEventListener("keydown", (event) => {
|
||||
if (!event.isTrusted || event.defaultPrevented || !browserId || !matchesPolicy(event)) {
|
||||
return;
|
||||
}
|
||||
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,
|
||||
});
|
||||
});
|
||||
|
||||
const shortcutBrowserId = browserId;
|
||||
window.addEventListener(
|
||||
"keydown",
|
||||
(completedEvent) => {
|
||||
if (completedEvent !== event || completedEvent.defaultPrevented) {
|
||||
return;
|
||||
}
|
||||
completedEvent.preventDefault();
|
||||
ipcRenderer.send(SHORTCUT_INPUT_CHANNEL, {
|
||||
alt: completedEvent.altKey,
|
||||
browserId: shortcutBrowserId,
|
||||
code: completedEvent.code,
|
||||
control: completedEvent.ctrlKey,
|
||||
key: completedEvent.key,
|
||||
meta: completedEvent.metaKey,
|
||||
repeat: completedEvent.repeat,
|
||||
shift: completedEvent.shiftKey,
|
||||
});
|
||||
},
|
||||
{ once: true },
|
||||
);
|
||||
}
|
||||
|
||||
function scheduleKeydownListener(): void {
|
||||
if (keydownListenerInstalled) {
|
||||
return;
|
||||
}
|
||||
const installAfterInitialPageHandlers = () => {
|
||||
setTimeout(installKeydownListener, 0);
|
||||
};
|
||||
if (document.readyState === "loading") {
|
||||
window.addEventListener("DOMContentLoaded", installAfterInitialPageHandlers, { once: true });
|
||||
return;
|
||||
}
|
||||
installAfterInitialPageHandlers();
|
||||
}
|
||||
window.addEventListener("keydown", stageShortcutForward, { capture: true });
|
||||
|
||||
ipcRenderer.on(POLICY_CHANNEL, (_event, value: BrowserKeyboardPolicyPayload) => {
|
||||
if (!value || typeof value.browserId !== "string" || !Array.isArray(value.prefixes)) {
|
||||
return;
|
||||
}
|
||||
browserId = value.browserId;
|
||||
policy = value.prefixes;
|
||||
scheduleKeydownListener();
|
||||
});
|
||||
|
||||
ipcRenderer.send(POLICY_REQUEST_CHANNEL);
|
||||
|
||||
@@ -8,6 +8,14 @@ interface SentMessage {
|
||||
}
|
||||
|
||||
class FakeBrowserContents {
|
||||
public readonly mainFrame = {
|
||||
framesInSubtree: [
|
||||
{
|
||||
detached: false,
|
||||
send: (channel: string, payload: unknown) => this.send(channel, payload),
|
||||
},
|
||||
],
|
||||
};
|
||||
public readonly ignoredMenuShortcuts: boolean[] = [];
|
||||
public readonly reloads: string[] = [];
|
||||
public readonly sent: SentMessage[] = [];
|
||||
|
||||
@@ -13,6 +13,7 @@ export type { BrowserKeyboardPolicy } from "./policy.js";
|
||||
|
||||
const POLICY_INPUT_CHANNEL = "paseo:browser:set-shortcut-policy";
|
||||
const POLICY_OUTPUT_CHANNEL = "paseo:browser-keyboard-policy";
|
||||
const POLICY_REQUEST_CHANNEL = "paseo:browser-keyboard-policy-request";
|
||||
const SHORTCUT_INPUT_CHANNEL = "paseo:browser-shortcut-input";
|
||||
const SHORTCUT_OUTPUT_CHANNEL = "paseo:event:browser-shortcut-input";
|
||||
const RESERVED_SHORTCUT_OUTPUT_CHANNEL = "paseo:event:browser-shortcut";
|
||||
@@ -26,6 +27,12 @@ interface BrowserKeyboardInputEvent {
|
||||
}
|
||||
|
||||
interface BrowserKeyboardGuestContents extends BrowserKeyboardContentsIdentity {
|
||||
readonly mainFrame: {
|
||||
readonly framesInSubtree: ReadonlyArray<{
|
||||
readonly detached: boolean;
|
||||
send(channel: string, ...args: unknown[]): void;
|
||||
}>;
|
||||
};
|
||||
isDestroyed(): boolean;
|
||||
isLoadingMainFrame(): boolean;
|
||||
on(event: "dom-ready", listener: () => void): void;
|
||||
@@ -36,7 +43,6 @@ interface BrowserKeyboardGuestContents extends BrowserKeyboardContentsIdentity {
|
||||
once(event: "destroyed", listener: () => void): void;
|
||||
reload(): void;
|
||||
reloadIgnoringCache(): void;
|
||||
send(channel: string, ...args: unknown[]): void;
|
||||
setIgnoreMenuShortcuts(ignore: boolean): void;
|
||||
stop(): void;
|
||||
}
|
||||
@@ -64,6 +70,12 @@ export class BrowserKeyboard {
|
||||
ipcMain.on(SHORTCUT_INPUT_CHANNEL, (event, rawInput: unknown) => {
|
||||
this.forwardShortcutInput(event.sender, rawInput);
|
||||
});
|
||||
ipcMain.on(POLICY_REQUEST_CHANNEL, (event) => {
|
||||
const payload = this.policyForGuest(event.sender);
|
||||
if (payload) {
|
||||
event.reply(POLICY_OUTPUT_CHANNEL, payload);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
public attach(input: {
|
||||
@@ -217,8 +229,29 @@ export class BrowserKeyboard {
|
||||
browserId: string,
|
||||
policy: BrowserKeyboardPolicy,
|
||||
): void {
|
||||
if (!guest.contents.isDestroyed()) {
|
||||
guest.contents.send(POLICY_OUTPUT_CHANNEL, { ...policy, browserId });
|
||||
if (guest.contents.isDestroyed()) {
|
||||
return;
|
||||
}
|
||||
const payload = { ...policy, browserId };
|
||||
for (const frame of guest.contents.mainFrame.framesInSubtree) {
|
||||
if (!frame.detached) {
|
||||
frame.send(POLICY_OUTPUT_CHANNEL, payload);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private policyForGuest(
|
||||
contents: BrowserKeyboardContentsIdentity,
|
||||
): (BrowserKeyboardPolicy & { browserId: string }) | null {
|
||||
const guest = this.attachedGuestsByWebContentsId.get(contents.id);
|
||||
if (!guest) {
|
||||
return null;
|
||||
}
|
||||
const registration = this.registrationForGuest(contents.id, guest);
|
||||
if (!registration) {
|
||||
return null;
|
||||
}
|
||||
const policy = this.policiesByHostWebContentsId.get(registration.hostWebContentsId);
|
||||
return policy ? { ...policy, browserId: registration.browserId } : null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -45,7 +45,7 @@ import {
|
||||
ensureNotificationCenterRegistration,
|
||||
} from "./features/notifications.js";
|
||||
import { registerOpenerHandlers } from "./features/opener.js";
|
||||
import { registerEditorTargetHandlers } from "./features/editor-targets.js";
|
||||
import { registerEditorTargetHandlers } from "./features/editor-targets/ipc.js";
|
||||
import { setupApplicationMenu } from "./features/menu.js";
|
||||
import {
|
||||
BROWSER_NEW_TAB_REQUEST_EVENT,
|
||||
|
||||
@@ -1,6 +1,13 @@
|
||||
import { contextBridge, ipcRenderer, webUtils } from "electron";
|
||||
import type { BrowserKeyboardPolicy } from "./features/browser-keyboard/index.js";
|
||||
import { PASEO_BROWSER_PROFILE_PARTITION } from "./features/browser-profile.js";
|
||||
|
||||
// This preload runs in Electron's sandbox and is tsc-compiled (not bundled), so it MUST
|
||||
// NOT emit any runtime module load other than "electron" — a require() of a local or
|
||||
// third-party module throws and aborts the preload before exposeInMainWorld runs, leaving
|
||||
// window.paseoDesktop undefined (the 0.1.108 regression, #2103). Keep this literal in sync
|
||||
// with PASEO_BROWSER_PROFILE_PARTITION in features/browser-profile.ts; preload-sandbox.test.ts
|
||||
// guards both the no-local-import rule and this drift. Type-only imports are fine (erased at emit).
|
||||
const PASEO_BROWSER_PROFILE_PARTITION = "persist:paseo-browser";
|
||||
|
||||
type EventHandler = (payload: unknown) => void;
|
||||
|
||||
@@ -72,9 +79,10 @@ contextBridge.exposeInMainWorld("paseoDesktop", {
|
||||
listTargets: () => ipcRenderer.invoke("paseo:editor:listTargets"),
|
||||
openTarget: (input: {
|
||||
editorId: string;
|
||||
path: string;
|
||||
cwd?: string;
|
||||
mode?: "open" | "reveal";
|
||||
workspacePath: string;
|
||||
filePath?: string;
|
||||
line?: number;
|
||||
column?: number;
|
||||
}) => ipcRenderer.invoke("paseo:editor:openTarget", input),
|
||||
},
|
||||
webUtils: {
|
||||
|
||||
Reference in New Issue
Block a user