mirror of
https://github.com/getpaseo/paseo.git
synced 2026-07-29 12:01:31 +00:00
fix(desktop): respect browser shortcut ownership
This commit is contained in:
@@ -17,8 +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 a page-handled shortcut stays in the page while an unhandled
|
||||
shortcut crosses once.
|
||||
preload, then proves that a page window handler gets first refusal, an unhandled
|
||||
shortcut crosses once, digit wildcard shortcuts cross, and background automation
|
||||
stays in the guest.
|
||||
|
||||
Run it with the repo Electron:
|
||||
|
||||
|
||||
@@ -1228,7 +1228,7 @@ function automationFixtureUrl() {
|
||||
<script>
|
||||
window.fixtureLog = [];
|
||||
window.preventBrowserShortcut = false;
|
||||
document.addEventListener("keydown", (event) => {
|
||||
window.addEventListener("keydown", (event) => {
|
||||
if (
|
||||
(event.metaKey || event.ctrlKey) &&
|
||||
!event.altKey &&
|
||||
@@ -1559,16 +1559,16 @@ function sendContainedEnter(guest) {
|
||||
});
|
||||
}
|
||||
|
||||
function automationBrowserShortcut(guest) {
|
||||
function automationBrowserShortcut(guest, keyCode = "B") {
|
||||
const modifiers = [process.platform === "darwin" ? "meta" : "control"];
|
||||
guest.sendInputEvent({
|
||||
type: "keyDown",
|
||||
keyCode: "B",
|
||||
keyCode,
|
||||
modifiers,
|
||||
});
|
||||
guest.sendInputEvent({
|
||||
type: "keyUp",
|
||||
keyCode: "B",
|
||||
keyCode,
|
||||
modifiers,
|
||||
});
|
||||
}
|
||||
@@ -1847,6 +1847,26 @@ async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta,
|
||||
pass("automation production guest preload forwards one page-unhandled browser shortcut");
|
||||
checks.push({ group: "automation", check: "browser-shortcut-page-first-forward", pass: true });
|
||||
|
||||
automationBrowserShortcut(guest, "1");
|
||||
await waitForBrowserShortcutInput(shortcutInputs, 2);
|
||||
const expectedDigitShortcutInput = {
|
||||
alt: false,
|
||||
browserId,
|
||||
code: "Digit1",
|
||||
control: !usesMeta,
|
||||
key: "1",
|
||||
meta: usesMeta,
|
||||
repeat: false,
|
||||
shift: false,
|
||||
};
|
||||
if (!isDeepStrictEqual(shortcutInputs[1], expectedDigitShortcutInput)) {
|
||||
fail(
|
||||
`digit browser shortcut crossed boundary incorrectly: inputs=${JSON.stringify(shortcutInputs)}`,
|
||||
);
|
||||
}
|
||||
pass("automation production guest preload forwards digit wildcard shortcuts");
|
||||
checks.push({ group: "automation", check: "browser-shortcut-digit-wildcard", pass: true });
|
||||
|
||||
const focusedGuestInput = await guest.executeJavaScript(
|
||||
"document.getElementById('name').focus(); document.activeElement.id",
|
||||
true,
|
||||
@@ -1886,6 +1906,30 @@ async function verifyBrowserKeyboardIsolation({ guest, win, browserId, usesMeta,
|
||||
pass("automation guest Enter does not reach the active host composer");
|
||||
checks.push({ group: "automation", check: "guest-enter-host-isolation", pass: true });
|
||||
|
||||
win.hide();
|
||||
await delay(50);
|
||||
if (win.isFocused()) {
|
||||
fail("automation harness window stayed focused after hide");
|
||||
}
|
||||
await guest.executeJavaScript("document.getElementById('name').focus()", true);
|
||||
sendContainedEnter(guest);
|
||||
await waitForAutomationLog(
|
||||
guest,
|
||||
(entry) => entry.event === "keydown-name" && entry.key === "Enter" && entry.trusted === true,
|
||||
"trusted background guest Enter",
|
||||
);
|
||||
const hiddenHostComposerState = await win.webContents.executeJavaScript(
|
||||
"window.captureHarness.hostComposerState()",
|
||||
true,
|
||||
);
|
||||
if (!isDeepStrictEqual(hiddenHostComposerState, expectedHostComposerState)) {
|
||||
fail(
|
||||
`automation background guest Enter reached host composer: ${JSON.stringify(hiddenHostComposerState)}`,
|
||||
);
|
||||
}
|
||||
pass("automation background guest Enter stays in the guest");
|
||||
checks.push({ group: "automation", check: "background-guest-enter", pass: true });
|
||||
|
||||
return checks;
|
||||
}
|
||||
|
||||
@@ -1945,6 +1989,14 @@ async function runAutomationGroup() {
|
||||
repeat: false,
|
||||
shift: false,
|
||||
},
|
||||
{
|
||||
alt: false,
|
||||
code: "Digit",
|
||||
control: !usesMeta,
|
||||
meta: usesMeta,
|
||||
repeat: false,
|
||||
shift: false,
|
||||
},
|
||||
],
|
||||
});
|
||||
await delay(25);
|
||||
|
||||
@@ -6,6 +6,7 @@ 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;
|
||||
@@ -23,7 +24,7 @@ function matchesPolicy(event: KeyboardEvent): boolean {
|
||||
return false;
|
||||
}
|
||||
if (prefix.key === undefined) {
|
||||
return prefix.code === event.code;
|
||||
return matchesCode(prefix.code, event.code);
|
||||
}
|
||||
const eventKey = event.key.toLowerCase();
|
||||
if (eventKey === prefix.key) {
|
||||
@@ -32,10 +33,17 @@ function matchesPolicy(event: KeyboardEvent): boolean {
|
||||
if (prefix.shift && prefix.shiftedKey !== undefined && eventKey === prefix.shiftedKey) {
|
||||
return true;
|
||||
}
|
||||
return (prefix.alt || prefix.codeFallback === true) && prefix.code === event.code;
|
||||
return (prefix.alt || prefix.codeFallback === true) && matchesCode(prefix.code, event.code);
|
||||
});
|
||||
}
|
||||
|
||||
function matchesCode(prefixCode: string, eventCode: string): boolean {
|
||||
if (prefixCode !== "Digit") {
|
||||
return prefixCode === eventCode;
|
||||
}
|
||||
return /^(?:Digit|Numpad)[1-9]$/.test(eventCode);
|
||||
}
|
||||
|
||||
function isEditableTarget(target: EventTarget | null): boolean {
|
||||
if (!(target instanceof Element)) {
|
||||
return false;
|
||||
@@ -48,31 +56,52 @@ function isEditableTarget(target: EventTarget | null): boolean {
|
||||
return target.matches("input, textarea, select, [role=textbox]");
|
||||
}
|
||||
|
||||
function installKeydownListener(): void {
|
||||
if (keydownListenerInstalled) {
|
||||
return;
|
||||
}
|
||||
keydownListenerInstalled = true;
|
||||
window.addEventListener("keydown", (event) => {
|
||||
if (
|
||||
!event.isTrusted ||
|
||||
event.defaultPrevented ||
|
||||
!browserId ||
|
||||
isEditableTarget(event.target) ||
|
||||
!matchesPolicy(event)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
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,
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
function scheduleKeydownListener(): void {
|
||||
if (keydownListenerInstalled) {
|
||||
return;
|
||||
}
|
||||
const installAfterInitialPageHandlers = () => {
|
||||
setTimeout(installKeydownListener, 0);
|
||||
};
|
||||
if (document.readyState === "loading") {
|
||||
window.addEventListener("DOMContentLoaded", installAfterInitialPageHandlers, { once: true });
|
||||
return;
|
||||
}
|
||||
installAfterInitialPageHandlers();
|
||||
}
|
||||
|
||||
ipcRenderer.on(POLICY_CHANNEL, (_event, value: BrowserKeyboardPolicyPayload) => {
|
||||
browserId = value.browserId;
|
||||
policy = value.prefixes;
|
||||
});
|
||||
|
||||
window.addEventListener("keydown", (event) => {
|
||||
if (
|
||||
!event.isTrusted ||
|
||||
event.defaultPrevented ||
|
||||
!browserId ||
|
||||
isEditableTarget(event.target) ||
|
||||
!matchesPolicy(event)
|
||||
) {
|
||||
return;
|
||||
}
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
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,
|
||||
});
|
||||
scheduleKeydownListener();
|
||||
});
|
||||
|
||||
@@ -278,4 +278,21 @@ describe("BrowserKeyboard", () => {
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
test("keeps policy-owned shortcuts out of the application menu without preempting the page", () => {
|
||||
const keyboard = new BrowserKeyboard();
|
||||
const guest = new FakeBrowserContents(91);
|
||||
const host = new FakeBrowserContents(92);
|
||||
keyboard.publish(host.id, {
|
||||
prefixes: [
|
||||
{ alt: false, code: "KeyW", control: true, meta: false, repeat: false, shift: false },
|
||||
],
|
||||
});
|
||||
keyboard.attach({ browserId: "browser-a", contents: guest, hostContents: host });
|
||||
|
||||
const wasPrevented = guest.input(electronInput({ code: "KeyW", control: true, key: "w" }));
|
||||
|
||||
expect(wasPrevented).toBe(false);
|
||||
expect(guest.ignoredMenuShortcuts).toEqual([true]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@ import { ipcMain } from "electron";
|
||||
import {
|
||||
type BrowserKeyboardPolicy,
|
||||
classifyBrowserReservedShortcut,
|
||||
matchesBrowserShortcutPolicy,
|
||||
parseBrowserKeyboardPolicy,
|
||||
parseBrowserShortcutInput,
|
||||
} from "./policy.js";
|
||||
@@ -158,7 +159,21 @@ export class BrowserKeyboard {
|
||||
event: BrowserKeyboardInputEvent,
|
||||
input: Electron.Input,
|
||||
): void {
|
||||
guest.contents.setIgnoreMenuShortcuts(!input.control && !input.meta);
|
||||
const policy = this.policiesByHostWebContentsId.get(guest.hostWebContentsId);
|
||||
const belongsToBrowserPolicy =
|
||||
policy !== undefined &&
|
||||
matchesBrowserShortcutPolicy(policy, {
|
||||
alt: input.alt,
|
||||
code: input.code,
|
||||
control: input.control,
|
||||
key: input.key,
|
||||
meta: input.meta,
|
||||
repeat: input.isAutoRepeat,
|
||||
shift: input.shift,
|
||||
});
|
||||
guest.contents.setIgnoreMenuShortcuts(
|
||||
(!input.control && !input.meta) || belongsToBrowserPolicy,
|
||||
);
|
||||
const reservedShortcut = classifyBrowserReservedShortcut(input, {
|
||||
isMac: process.platform === "darwin",
|
||||
});
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { describe, expect, test } from "vitest";
|
||||
import {
|
||||
classifyBrowserReservedShortcut,
|
||||
matchesBrowserShortcutPolicy,
|
||||
parseBrowserKeyboardPolicy,
|
||||
parseBrowserShortcutInput,
|
||||
} from "./policy.js";
|
||||
@@ -106,4 +107,47 @@ describe("browser keyboard policy", () => {
|
||||
}),
|
||||
).toMatchObject({ browserId: " browser-1 " });
|
||||
});
|
||||
|
||||
test("matches digit shortcuts for the top row and numeric keypad", () => {
|
||||
const policy = parseBrowserKeyboardPolicy({
|
||||
prefixes: [
|
||||
{ alt: false, code: "Digit", control: true, meta: false, repeat: false, shift: false },
|
||||
],
|
||||
});
|
||||
expect(policy).not.toBeNull();
|
||||
|
||||
expect(
|
||||
matchesBrowserShortcutPolicy(policy!, {
|
||||
alt: false,
|
||||
code: "Digit3",
|
||||
control: true,
|
||||
key: "3",
|
||||
meta: false,
|
||||
repeat: false,
|
||||
shift: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
matchesBrowserShortcutPolicy(policy!, {
|
||||
alt: false,
|
||||
code: "Numpad3",
|
||||
control: true,
|
||||
key: "3",
|
||||
meta: false,
|
||||
repeat: false,
|
||||
shift: false,
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
matchesBrowserShortcutPolicy(policy!, {
|
||||
alt: false,
|
||||
code: "Digit0",
|
||||
control: true,
|
||||
key: "0",
|
||||
meta: false,
|
||||
repeat: false,
|
||||
shift: false,
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -25,6 +25,16 @@ export interface BrowserShortcutInput {
|
||||
shift: boolean;
|
||||
}
|
||||
|
||||
export interface BrowserShortcutMatchInput {
|
||||
alt: boolean;
|
||||
code: string;
|
||||
control: boolean;
|
||||
key: string;
|
||||
meta: boolean;
|
||||
repeat: boolean;
|
||||
shift: boolean;
|
||||
}
|
||||
|
||||
export type BrowserReservedShortcut = "new-tab" | "focus-url" | "reload" | "force-reload";
|
||||
|
||||
function isRecord(value: unknown): value is Record<string, unknown> {
|
||||
@@ -105,6 +115,43 @@ export function parseBrowserShortcutInput(value: unknown): BrowserShortcutInput
|
||||
};
|
||||
}
|
||||
|
||||
function matchesCode(prefixCode: string, inputCode: string): boolean {
|
||||
if (prefixCode !== "Digit") {
|
||||
return prefixCode === inputCode;
|
||||
}
|
||||
return /^(?:Digit|Numpad)[1-9]$/.test(inputCode);
|
||||
}
|
||||
|
||||
function matchesPrefix(prefix: BrowserShortcutPrefix, input: BrowserShortcutMatchInput): boolean {
|
||||
if (
|
||||
prefix.alt !== input.alt ||
|
||||
prefix.control !== input.control ||
|
||||
prefix.meta !== input.meta ||
|
||||
prefix.shift !== input.shift ||
|
||||
(prefix.repeat === false && input.repeat)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
if (prefix.key === undefined) {
|
||||
return matchesCode(prefix.code, input.code);
|
||||
}
|
||||
const key = input.key.toLowerCase();
|
||||
if (key === prefix.key) {
|
||||
return true;
|
||||
}
|
||||
if (prefix.shift && prefix.shiftedKey !== undefined && key === prefix.shiftedKey) {
|
||||
return true;
|
||||
}
|
||||
return (prefix.alt || prefix.codeFallback === true) && matchesCode(prefix.code, input.code);
|
||||
}
|
||||
|
||||
export function matchesBrowserShortcutPolicy(
|
||||
policy: BrowserKeyboardPolicy,
|
||||
input: BrowserShortcutMatchInput,
|
||||
): boolean {
|
||||
return policy.prefixes.some((prefix) => matchesPrefix(prefix, input));
|
||||
}
|
||||
|
||||
export function classifyBrowserReservedShortcut(
|
||||
input: {
|
||||
alt: boolean;
|
||||
|
||||
@@ -310,4 +310,39 @@ describe("PaseoBrowserWebviewRegistry", () => {
|
||||
|
||||
expect(registry.hasBrowserInOtherHostWindow(101, "browser-a")).toBe(false);
|
||||
});
|
||||
|
||||
it("keeps the same-window active selection made before the guest attaches", () => {
|
||||
const registry = new PaseoBrowserWebviewRegistry();
|
||||
|
||||
registry.setWorkspaceActiveBrowser({
|
||||
hostWebContentsId: 101,
|
||||
workspaceId: "workspace-a",
|
||||
browserId: "browser-a",
|
||||
});
|
||||
registry.registerWebContents({
|
||||
webContentsId: 11,
|
||||
browserId: "browser-a",
|
||||
hostWebContentsId: 101,
|
||||
});
|
||||
|
||||
expect(registry.getActiveBrowserIdForHostWindow(101)).toBe("browser-a");
|
||||
});
|
||||
|
||||
it("drops a pre-attach selection when the guest attaches to another host window", () => {
|
||||
const registry = new PaseoBrowserWebviewRegistry();
|
||||
|
||||
registry.setWorkspaceActiveBrowser({
|
||||
hostWebContentsId: 101,
|
||||
workspaceId: "workspace-a",
|
||||
browserId: "browser-a",
|
||||
});
|
||||
registry.registerWebContents({
|
||||
webContentsId: 11,
|
||||
browserId: "browser-a",
|
||||
hostWebContentsId: 202,
|
||||
});
|
||||
|
||||
expect(registry.getActiveBrowserIdForHostWindow(101)).toBeNull();
|
||||
expect(registry.getActiveBrowserIdForHostWindow(202)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user