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
130 changes: 130 additions & 0 deletions src/__tests__/main/app-lifecycle/window-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const mockGuestWebContents = {
guestWebContentsEventHandlers.set(event, handler);
}),
executeJavaScript: vi.fn().mockResolvedValue(undefined),
paste: vi.fn(),
};

// Mock BrowserWindow instance methods
Expand Down Expand Up @@ -1263,6 +1264,135 @@ describe('app-lifecycle/window-manager', () => {
);
});

it('pastes into browser-tab form fields via guest.paste() on Cmd/Ctrl+V (#1063)', async () => {
// The permission handler denies `clipboard-read` to webviews, so
// Chromium's native Cmd/Ctrl+V silently fails inside browser-tab form
// fields. The before-input-event handler must intercept the paste chord
// and drive the privileged guest.paste() instead.
const { createWindowManager } = await import('../../../main/app-lifecycle/window-manager');

const windowManager = createWindowManager({
windowStateStore: mockWindowStateStore as unknown as Parameters<
typeof createWindowManager
>[0]['windowStateStore'],
isDevelopment: false,
preloadPath: '/path/to/preload.js',
rendererProductionUrl: 'app://app/index.html',
devServerUrl: 'http://localhost:5173',
useNativeTitleBar: false,
autoHideMenuBar: false,
});

windowManager.createWindow();

const attachHandler = webContentsEventHandlers.get('did-attach-webview');
attachHandler?.({} as any, mockGuestWebContents as any);

const beforeInputHandler = guestWebContentsEventHandlers.get('before-input-event');
expect(beforeInputHandler).toBeDefined();

// Cmd+V (macOS)
const metaEvent = { preventDefault: vi.fn() };
beforeInputHandler?.(metaEvent, {
type: 'keyDown',
key: 'v',
code: 'KeyV',
meta: true,
control: false,
alt: false,
shift: false,
});
expect(metaEvent.preventDefault).toHaveBeenCalled();
expect(mockGuestWebContents.paste).toHaveBeenCalledTimes(1);

// Ctrl+V (Windows/Linux)
const ctrlEvent = { preventDefault: vi.fn() };
beforeInputHandler?.(ctrlEvent, {
type: 'keyDown',
key: 'v',
code: 'KeyV',
meta: false,
control: true,
alt: false,
shift: false,
});
expect(ctrlEvent.preventDefault).toHaveBeenCalled();
expect(mockGuestWebContents.paste).toHaveBeenCalledTimes(2);

// The paste chord must NOT also be forwarded to the renderer as an app
// shortcut - it is fully consumed here.
expect(mockWebContents.send).not.toHaveBeenCalledWith(
'browser-tab:shortcutKey',
expect.objectContaining({ key: 'v' })
);

// The page-level fallback must also exclude V from its passthrough
// list. Otherwise it can race the privileged paste path above.
guestWebContentsEventHandlers.get('dom-ready')?.();
const injectedScript = mockGuestWebContents.executeJavaScript.mock.calls.at(-1)?.[0];
expect(injectedScript).toContain("'acxz'.indexOf(k)");
expect(injectedScript).not.toContain("'acvxz'.indexOf(k)");
});

it('does not hijack non-paste edit chords or plain "v" on browser-tab guests', async () => {
const { createWindowManager } = await import('../../../main/app-lifecycle/window-manager');

const windowManager = createWindowManager({
windowStateStore: mockWindowStateStore as unknown as Parameters<
typeof createWindowManager
>[0]['windowStateStore'],
isDevelopment: false,
preloadPath: '/path/to/preload.js',
rendererProductionUrl: 'app://app/index.html',
devServerUrl: 'http://localhost:5173',
useNativeTitleBar: false,
autoHideMenuBar: false,
});

windowManager.createWindow();

const attachHandler = webContentsEventHandlers.get('did-attach-webview');
attachHandler?.({} as any, mockGuestWebContents as any);

const beforeInputHandler = guestWebContentsEventHandlers.get('before-input-event');

// Plain "v" (no modifier) is normal typing - must pass through untouched.
const plainEvent = { preventDefault: vi.fn() };
beforeInputHandler?.(plainEvent, {
type: 'keyDown',
key: 'v',
code: 'KeyV',
meta: false,
control: false,
alt: false,
shift: false,
});
expect(plainEvent.preventDefault).not.toHaveBeenCalled();

// Cmd+Shift+V (paste-and-match-style etc.) is not the plain paste chord.
const shiftEvent = { preventDefault: vi.fn() };
beforeInputHandler?.(shiftEvent, {
type: 'keyDown',
key: 'v',
code: 'KeyV',
meta: true,
control: false,
alt: false,
shift: true,
});

// Cmd+Shift+V is not the plain paste chord, so the handler must NOT
// drive the privileged paste path. It is also not a text-editing
// passthrough, so it is consumed (preventDefault) and forwarded to the
// renderer as an app shortcut rather than reaching the page.
expect(mockGuestWebContents.paste).not.toHaveBeenCalled();
Comment on lines +1372 to +1388

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Ctrl+Alt+V (AltGr+V) and Cmd+Alt+V edge cases untested

The isPaste guard explicitly requires !input.alt, so Ctrl+Alt+V (which on Windows/Linux AltGr keyboards produces @ or other characters, not a paste) correctly passes through. There is no test for this combination, however, so a future change to the modifier logic that accidentally drops the !alt guard would not be caught. Adding a Ctrl+Alt+V case to the "does not hijack" test would make this invariant explicit.

expect(shiftEvent.preventDefault).toHaveBeenCalled();
expect(mockWebContents.send).toHaveBeenCalledWith(
'browser-tab:shortcutKey',
expect.objectContaining({ key: 'v', shift: true })
);
});

// Electron 41 removed the legacy `'crashed'` event in favor of
// `'render-process-gone'`. These tests pin the wiring so a future
// revert can't silently drop renderer-crash reporting.
Expand Down
26 changes: 21 additions & 5 deletions src/main/app-lifecycle/window-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ interface BrowserTabGuestContents {
): void;
on(event: string, handler: (...args: any[]) => void): void;
executeJavaScript(code: string): Promise<unknown>;
// Privileged Electron paste: bypasses the web-facing `clipboard-read`
// permission that the permission handler denies to webviews (issue #1063).
paste(): void;
}

function isAllowedBrowserTabUrl(rawUrl: string): boolean {
Expand Down Expand Up @@ -354,11 +357,24 @@ export function createWindowManager(deps: WindowManagerDependencies): WindowMana
if (!input.meta && !input.control && !input.alt) return;
if (input.type !== 'keyDown') return;
const k = input.key.toLowerCase();
// Let standard text-editing shortcuts pass through to the page.
// `f` is intentionally NOT in this list: Cmd+F must reach the
// renderer so the in-page find bar can open.
// Cmd/Ctrl+V: drive paste through the trusted guest webContents API.
// Chromium's native paste needs the `clipboard-read` permission, which
// the permission handler denies to webviews as a security boundary, so
// native paste silently fails inside browser-tab form fields (issue
// #1063). guest.paste() is a privileged Electron call that bypasses
// that web-facing permission, mirroring the right-click Paste menu
// item (issue #1065).
const isPaste = (input.meta || input.control) && !input.alt && !input.shift && k === 'v';
if (isPaste) {
event.preventDefault();
guest.paste();
return;
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
// Let the remaining standard text-editing shortcuts pass through to
// the page. `f` is intentionally NOT in this list: Cmd+F must reach
// the renderer so the in-page find bar can open.
const isTextEditing =
(input.meta || input.control) && !input.alt && !input.shift && 'acvxz'.includes(k);
(input.meta || input.control) && !input.alt && !input.shift && 'acxz'.includes(k);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const isRedo = (input.meta || input.control) && !input.alt && input.shift && k === 'z';
if (isTextEditing || isRedo) return;
event.preventDefault();
Expand All @@ -383,7 +399,7 @@ export function createWindowManager(deps: WindowManagerDependencies): WindowMana
var hasAlt=e.altKey;
if(!hasMod&&!hasAlt)return;
var k=e.key.toLowerCase();
var te=hasMod&&!hasAlt&&!e.shiftKey&&'acvxz'.indexOf(k)!==-1;
var te=hasMod&&!hasAlt&&!e.shiftKey&&'acxz'.indexOf(k)!==-1;
var re=hasMod&&!hasAlt&&e.shiftKey&&k==='z';
if(te||re)return;
e.preventDefault();
Expand Down
10 changes: 6 additions & 4 deletions src/renderer/components/MainPanel/BrowserTabView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,11 @@ export const BrowserTabView = React.memo(
// Uses stopImmediatePropagation to prevent any other listener
// (including the main-process-injected bubble-phase one) from
// double-firing.
// `f` is intentionally NOT in the text-editing pass-through list: Cmd+F
// must reach the app so the in-page find bar can open. The remaining
// letters (a/c/v/x/z) keep their native text-editing behavior inside
// `f` and `v` are intentionally NOT in the text-editing pass-through
// list. Cmd+F must reach the app so the in-page find bar can open, and
// Cmd/Ctrl+V is handled separately via the privileged paste path
// (guest.paste()) so paste works inside the webview. The remaining
// letters (a/c/x/z) keep their native text-editing behavior inside
// page inputs.
const keyboardInjection = `(function(){
if(window.__maestroShortcutCaptureInstalled)return;
Expand All @@ -390,7 +392,7 @@ export const BrowserTabView = React.memo(
var hasAlt=e.altKey;
if(!hasMod&&!hasAlt)return;
var k=e.key.toLowerCase();
var te=hasMod&&!hasAlt&&!e.shiftKey&&'acvxz'.indexOf(k)!==-1;
var te=hasMod&&!hasAlt&&!e.shiftKey&&'acxz'.indexOf(k)!==-1;
var re=hasMod&&!hasAlt&&e.shiftKey&&k==='z';
if(te||re)return;
e.preventDefault();
Expand Down