Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 117 additions & 6 deletions apps/desktop/src/preview/Manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,70 @@ describe("isPreviewRefreshShortcut", () => {
});
});

describe("isPreviewEditingShortcut", () => {
const input = (platform: NodeJS.Platform, key: string, overrides: Partial<Electron.Input> = {}) =>
({
type: "keyDown",
key,
meta: platform === "darwin",
control: platform !== "darwin",
shift: false,
alt: false,
...overrides,
}) as Electron.Input;

it.each(["darwin", "linux", "win32"] as const)(
"allows native editing chords on %s without allowing host shortcuts",
(platform) => {
for (const key of ["a", "c", "v", "x", "z", "V"]) {
expect(PreviewManager.isPreviewEditingShortcut(input(platform, key), platform)).toBe(true);
}
const redo =
platform === "win32" ? input(platform, "y") : input(platform, "z", { shift: true });
expect(PreviewManager.isPreviewEditingShortcut(redo, platform)).toBe(true);
expect(
PreviewManager.isPreviewEditingShortcut(
input(platform, "v", { shift: true, alt: platform === "darwin" }),
platform,
),
).toBe(true);

for (const key of ["k", ",", "w", "j", "q", "+", "=", "-", "0", "r", "F12"]) {
expect(PreviewManager.isPreviewEditingShortcut(input(platform, key), platform)).toBe(false);
}
for (const modifiers of [
{ meta: false, control: false },
{ meta: true, control: true },
{ meta: platform !== "darwin", control: platform === "darwin" },
{ alt: true },
{ shift: true, alt: platform !== "darwin" },
]) {
expect(
PreviewManager.isPreviewEditingShortcut(input(platform, "v", modifiers), platform),
).toBe(false);
}
expect(
PreviewManager.isPreviewEditingShortcut(input(platform, "a", { shift: true }), platform),
).toBe(false);
},
);

it("recognizes macOS Paste and Match Style when Option changes the key to a symbol", () => {
const pasteAndMatchStyle = input("darwin", "◊", { code: "KeyV", alt: true, shift: true });
expect(PreviewManager.isPreviewEditingShortcut(pasteAndMatchStyle, "darwin")).toBe(true);
for (const modifiers of [
{ code: "KeyC" },
{ alt: false },
{ shift: false },
{ control: true },
]) {
expect(
PreviewManager.isPreviewEditingShortcut({ ...pasteAndMatchStyle, ...modifiers }, "darwin"),
).toBe(false);
}
});
});

describe("previewWindowOpenAction", () => {
const details = (overrides: {
readonly url?: string;
Expand Down Expand Up @@ -484,6 +548,7 @@ describe("PreviewManager", () => {
const hostWebContents = { sendInputEvent };
Object.assign(preview.webContents, { hostWebContents });
fromId.mockReturnValue(preview.webContents);
getFocusedWebContents.mockReturnValue(preview.webContents as never);
yield* manager.setMainWindow({
isDestroyed: () => false,
once: vi.fn(),
Expand All @@ -497,7 +562,7 @@ describe("PreviewManager", () => {
).toHaveBeenCalledWith(true);
const beforeInput = preview.listeners.get("before-input-event")!;
for (const control of [false, true]) {
for (const key of ["k", ",", "w", "j", "q", "+", "a", "c", "v", "x"]) {
for (const key of ["k", ",", "w", "j", "q", "+"]) {
for (const type of ["keyDown", "keyUp"]) {
const preventDefault = vi.fn();
beforeInput(
Expand All @@ -506,6 +571,9 @@ describe("PreviewManager", () => {
);
yield* Effect.yieldNow;
expect(preventDefault).not.toHaveBeenCalled();
expect(
(preview.webContents as Electron.WebContents).setIgnoreMenuShortcuts,
).toHaveBeenLastCalledWith(true);
}
}
}
Expand All @@ -527,12 +595,55 @@ describe("PreviewManager", () => {
expect(preventDefault).toHaveBeenCalledOnce();
expect(preview.reload).toHaveBeenCalledOnce();
expect(sendInputEvent).not.toHaveBeenCalled();
}),
),
);

const setIgnoreMenuShortcuts = vi.fn();
preview.listeners.get("did-create-window")!({
webContents: { setIgnoreMenuShortcuts, setWindowOpenHandler: vi.fn() },
} as never);
expect(setIgnoreMenuShortcuts).toHaveBeenCalledWith(true);
effectIt.effect("preserves focused browser editing in tabs and sign-in popups", () =>
withManager((manager) =>
Effect.gen(function* () {
const preview = makeFaviconWebContents();
fromId.mockReturnValue(preview.webContents);
yield* manager.createTab("tab_editing");
yield* manager.registerWebview("tab_editing", 42);

const popup = makeFaviconWebContents({ id: 43 });
preview.listeners.get("did-create-window")!({ webContents: popup.webContents } as never);
expect(
(popup.webContents as Electron.WebContents).setIgnoreMenuShortcuts,
).toHaveBeenCalledWith(true);

for (const browser of [preview, popup]) {
const contents = browser.webContents as Electron.WebContents;
getFocusedWebContents.mockReturnValue(browser.webContents as never);
const beforeInput = browser.listeners.get("before-input-event")!;
const preventDefault = vi.fn();
const input = {
type: "keyDown",
key: "v",
meta: true,
control: false,
shift: false,
alt: false,
};
beforeInput({ preventDefault } as never, input as never);
expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(false);
// Releasing Command must not disable native fallback for the pending paste.
beforeInput(
{ preventDefault } as never,
{ ...input, type: "keyUp", key: "Meta", meta: false } as never,
);
expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(false);

beforeInput({ preventDefault } as never, { ...input, key: "w" } as never);
expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(true);

// An injected paste in an unfocused guest cannot edit the active renderer.
getFocusedWebContents.mockReturnValue(null);
beforeInput({ preventDefault } as never, input as never);
expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(true);
expect(preventDefault).not.toHaveBeenCalled();
}
}),
),
);
Expand Down
40 changes: 38 additions & 2 deletions apps/desktop/src/preview/Manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,29 @@ export const isPreviewRefreshShortcut = (input: Electron.Input): boolean =>
!input.shift &&
!input.alt;

export const isPreviewEditingShortcut = (
input: Electron.Input,
platform: NodeJS.Platform,
): boolean => {
const isMac = platform === "darwin";
if (isMac ? !input.meta || input.control : !input.control || input.meta) return false;

const key = input.key.toLowerCase();
// Option changes the DOM key for macOS Paste and Match Style (for example, to ◊).
if (isMac && input.alt && input.shift && input.code === "KeyV") return true;
if (key === "v" && input.shift) return input.alt === isMac;
if (input.alt) return false;
if (key === "z") return !input.shift || platform !== "win32";
if (input.shift) return false;
return (
key === "a" ||
key === "c" ||
key === "v" ||
key === "x" ||
(key === "y" && platform === "win32")
);
};

const isPreviewInputSignal = (value: unknown): value is PreviewInputSignal => {
if (typeof value !== "object" || value === null || !("kind" in value)) return false;
if (value.kind === "pointer") {
Expand Down Expand Up @@ -1837,14 +1860,27 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
}).pipe(Effect.ignore),
);
};
const syncMenuShortcuts = (contents: Electron.WebContents, input: Electron.Input): void => {
if (input.type !== "keyDown") return;
// Native editing roles must remain available after the page handles the key.
// Background automation must not edit whichever other renderer has focus.
contents.setIgnoreMenuShortcuts(
!isPreviewEditingShortcut(input, hostPlatform) ||
webContents.getFocusedWebContents() !== contents,
);
};
// A popup opens with Electron's default handler, so the page inside it could
// otherwise spawn native windows without limit. Nothing in an OAuth flow
// opens a second popup, so the chain stops at the first one.
const windowCreated = (window: Electron.BrowserWindow): void => {
window.webContents.setIgnoreMenuShortcuts(true);
window.webContents.setWindowOpenHandler(() => ({ action: "deny" }));
window.webContents.on("before-input-event", (_event, input) => {
syncMenuShortcuts(window.webContents, input);
});
};
const beforeInput = (event: Electron.Event, input: Electron.Input): void => {
syncMenuShortcuts(wc, input);
if (isPreviewRefreshShortcut(input)) {
event.preventDefault();
runFork(
Expand Down Expand Up @@ -1876,8 +1912,8 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
);
const install = Effect.fn("PreviewManager.installWebContentsListeners")(function* () {
yield* attempt({ operation: "attachListeners", tabId, webContentsId: wc.id }, () => {
// Preview input belongs to the page, including keys injected through CDP.
// Never let it invoke the host application's menu accelerators.
// Only focused native editing shortcuts may reach the application menu.
// Other preview input, including CDP keys, belongs to the page.
wc.setIgnoreMenuShortcuts(true);
wc.on("did-start-navigation", navigationStarted);
wc.on("did-navigate", syncNavigation);
Expand Down
Loading