diff --git a/src/__tests__/main/app-lifecycle/window-manager.test.ts b/src/__tests__/main/app-lifecycle/window-manager.test.ts index 02fc055c80..9e27868960 100644 --- a/src/__tests__/main/app-lifecycle/window-manager.test.ts +++ b/src/__tests__/main/app-lifecycle/window-manager.test.ts @@ -15,6 +15,13 @@ let windowCloseHandler: (() => void) | null = null; const webContentsEventHandlers = new Map void>(); const guestWebContentsEventHandlers = new Map void>(); +const mockGuestNavigationHistory = { + canGoBack: vi.fn(() => false), + goBack: vi.fn(), + canGoForward: vi.fn(() => false), + goForward: vi.fn(), +}; + const mockGuestWebContents = { getType: vi.fn(() => 'webview'), setWindowOpenHandler: vi.fn(), @@ -22,6 +29,18 @@ const mockGuestWebContents = { guestWebContentsEventHandlers.set(event, handler); }), executeJavaScript: vi.fn().mockResolvedValue(undefined), + // Edit + navigation surface used by the browser-tab context menu (#1065) + cut: vi.fn(), + copy: vi.fn(), + paste: vi.fn(), + selectAll: vi.fn(), + copyImageAt: vi.fn(), + reload: vi.fn(), + replaceMisspelling: vi.fn(), + navigationHistory: mockGuestNavigationHistory, + session: { + addWordToSpellCheckerDictionary: vi.fn(), + }, }; // Mock BrowserWindow instance methods @@ -77,13 +96,25 @@ class MockBrowserWindow { // Mock ipcMain const mockHandle = vi.fn(); +// Mock Menu / shell / clipboard for the browser-tab context menu (#1065) +const mockMenuPopup = vi.fn(); +const mockBuildFromTemplate = vi.fn(() => ({ popup: mockMenuPopup })); +const mockShellOpenExternal = vi.fn((..._args: unknown[]) => Promise.resolve()); +const mockClipboardWriteText = vi.fn(); + vi.mock('electron', () => ({ BrowserWindow: MockBrowserWindow, ipcMain: { handle: (...args: unknown[]) => mockHandle(...args), }, Menu: { - buildFromTemplate: vi.fn(() => ({ popup: vi.fn() })), + buildFromTemplate: (...args: unknown[]) => mockBuildFromTemplate(...args), + }, + shell: { + openExternal: (...args: unknown[]) => mockShellOpenExternal(...args), + }, + clipboard: { + writeText: (...args: unknown[]) => mockClipboardWriteText(...args), }, screen: { getAllDisplays: () => [{ workArea: { x: 0, y: 0, width: 1920, height: 1080 } }], @@ -154,12 +185,221 @@ describe('app-lifecycle/window-manager', () => { mockWindowInstance.getBounds.mockReturnValue({ x: 100, y: 100, width: 1200, height: 800 }); mockWebContents.getType.mockReturnValue('window'); mockGuestWebContents.getType.mockReturnValue('webview'); + mockGuestNavigationHistory.canGoBack.mockReturnValue(false); + mockGuestNavigationHistory.canGoForward.mockReturnValue(false); + mockBuildFromTemplate.mockReturnValue({ popup: mockMenuPopup }); }); afterEach(() => { vi.restoreAllMocks(); }); + describe('browser-tab context menu (#1065)', () => { + const baseDeps = { + isDevelopment: false, + preloadPath: '/path/to/preload.js', + rendererProductionUrl: 'app://app/index.html', + devServerUrl: 'http://localhost:5173', + useNativeTitleBar: false, + autoHideMenuBar: false, + }; + + // Attaches a browser-tab guest and returns its `context-menu` handler. + async function attachGuestAndGetContextMenuHandler() { + const { createWindowManager } = await import('../../../main/app-lifecycle/window-manager'); + const windowManager = createWindowManager({ + windowStateStore: mockWindowStateStore as unknown as Parameters< + typeof createWindowManager + >[0]['windowStateStore'], + ...baseDeps, + }); + windowManager.createWindow(); + const attachHandler = webContentsEventHandlers.get('did-attach-webview'); + attachHandler?.({} as any, mockGuestWebContents as any); + return guestWebContentsEventHandlers.get('context-menu'); + } + + function makeParams(overrides: Record = {}): any { + return { + isEditable: false, + editFlags: { + canCut: true, + canCopy: true, + canPaste: true, + canSelectAll: true, + canUndo: false, + canRedo: false, + canDelete: true, + canEditRichly: true, + }, + misspelledWord: '', + dictionarySuggestions: [], + linkURL: '', + srcURL: '', + mediaType: 'none', + selectionText: '', + x: 0, + y: 0, + ...overrides, + }; + } + + // Pull the template handed to Menu.buildFromTemplate on the latest popup. + function lastTemplate(): any[] { + const calls = mockBuildFromTemplate.mock.calls; + return calls[calls.length - 1][0] as unknown as any[]; + } + + function findItem(template: any[], label: string): any { + return template.find((item) => item.label === label); + } + + it('registers a context-menu handler on the attached guest', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + expect(handler).toBeTruthy(); + }); + + it('builds Cut/Copy/Paste/Select All for editable fields and acts on the guest', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.({} as any, makeParams({ isEditable: true })); + + const template = lastTemplate(); + expect(findItem(template, 'Cut')).toBeTruthy(); + expect(findItem(template, 'Copy')).toBeTruthy(); + expect(findItem(template, 'Paste')).toBeTruthy(); + expect(findItem(template, 'Select All')).toBeTruthy(); + + // Actions target the guest webContents, not the host window. + findItem(template, 'Paste').click(); + expect(mockGuestWebContents.paste).toHaveBeenCalledTimes(1); + + // The menu is popped over the host window. + expect(mockMenuPopup).toHaveBeenCalledWith( + expect.objectContaining({ window: expect.any(MockBrowserWindow) }) + ); + }); + + it('disables edit items when editFlags forbid the action', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.( + {} as any, + makeParams({ + isEditable: true, + editFlags: { + canCut: false, + canCopy: false, + canPaste: false, + canSelectAll: false, + }, + }) + ); + + const template = lastTemplate(); + expect(findItem(template, 'Cut').enabled).toBe(false); + expect(findItem(template, 'Paste').enabled).toBe(false); + }); + + it('offers spellcheck suggestions for a misspelled word in an editable field', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.( + {} as any, + makeParams({ + isEditable: true, + misspelledWord: 'teh', + dictionarySuggestions: ['the', 'tech'], + }) + ); + + const template = lastTemplate(); + const suggestion = findItem(template, 'the'); + expect(suggestion).toBeTruthy(); + suggestion.click(); + expect(mockGuestWebContents.replaceMisspelling).toHaveBeenCalledWith('the'); + + findItem(template, 'Add to Dictionary').click(); + expect(mockGuestWebContents.session.addWordToSpellCheckerDictionary).toHaveBeenCalledWith( + 'teh' + ); + }); + + it('copies links and opens http(s) links externally', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.({} as any, makeParams({ linkURL: 'https://example.com/login' })); + + const template = lastTemplate(); + findItem(template, 'Copy Link').click(); + expect(mockClipboardWriteText).toHaveBeenCalledWith('https://example.com/login'); + + findItem(template, 'Open Link in Browser').click(); + expect(mockShellOpenExternal).toHaveBeenCalledWith('https://example.com/login'); + }); + + it('does not offer external open for non-http(s) link schemes', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.({} as any, makeParams({ linkURL: 'javascript:alert(1)' })); + + const template = lastTemplate(); + // Copy Link is still offered, but a dangerous scheme is never handed to + // shell.openExternal. + expect(findItem(template, 'Copy Link')).toBeTruthy(); + expect(findItem(template, 'Open Link in Browser')).toBeUndefined(); + }); + + it('copies images and image addresses', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.( + {} as any, + makeParams({ mediaType: 'image', srcURL: 'https://cdn.example.com/a.png', x: 12, y: 34 }) + ); + + const template = lastTemplate(); + findItem(template, 'Copy Image').click(); + expect(mockGuestWebContents.copyImageAt).toHaveBeenCalledWith(12, 34); + + findItem(template, 'Copy Image Address').click(); + expect(mockClipboardWriteText).toHaveBeenCalledWith('https://cdn.example.com/a.png'); + }); + + it('offers Copy for selected non-editable text', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.({} as any, makeParams({ selectionText: 'hello world' })); + + const template = lastTemplate(); + findItem(template, 'Copy').click(); + expect(mockGuestWebContents.copy).toHaveBeenCalledTimes(1); + }); + + it('always offers Back/Forward/Reload and reflects navigation state', async () => { + mockGuestNavigationHistory.canGoBack.mockReturnValue(true); + mockGuestNavigationHistory.canGoForward.mockReturnValue(false); + + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.({} as any, makeParams()); + + const template = lastTemplate(); + const back = findItem(template, 'Back'); + const forward = findItem(template, 'Forward'); + expect(back.enabled).toBe(true); + expect(forward.enabled).toBe(false); + + back.click(); + expect(mockGuestNavigationHistory.goBack).toHaveBeenCalledTimes(1); + + findItem(template, 'Reload').click(); + expect(mockGuestWebContents.reload).toHaveBeenCalledTimes(1); + }); + + it('does not emit a leading or doubled separator for the navigation-only menu', async () => { + const handler = await attachGuestAndGetContextMenuHandler(); + handler?.({} as any, makeParams()); + + const template = lastTemplate(); + expect(template[0].type).not.toBe('separator'); + // Navigation-only menu has exactly Back/Forward/Reload, no separators. + expect(template.filter((item) => item.type === 'separator')).toHaveLength(0); + }); + }); + describe('createWindowManager', () => { it('should create a window manager with createWindow method', async () => { const { createWindowManager } = await import('../../../main/app-lifecycle/window-manager'); diff --git a/src/main/app-lifecycle/window-manager.ts b/src/main/app-lifecycle/window-manager.ts index fb8b59d086..30b2278b3d 100644 --- a/src/main/app-lifecycle/window-manager.ts +++ b/src/main/app-lifecycle/window-manager.ts @@ -3,7 +3,7 @@ * Handles window state persistence, DevTools, crash detection, and auto-updater initialization. */ -import { BrowserWindow, Menu, ipcMain, screen } from 'electron'; +import { BrowserWindow, Menu, ipcMain, shell, clipboard, screen } from 'electron'; import type Store from 'electron-store'; import type { WindowState } from '../stores/types'; import { logger } from '../utils/logger'; @@ -114,6 +114,144 @@ function attachBrowserTabGuestSecurity(guestContents: BrowserTabGuestContents): }); } +// Protocols we're willing to hand to the system browser from a browser-tab +// link's "Open Link in Browser" action. Anything else (file:, javascript:, +// custom schemes) is dropped so a malicious page can't smuggle a dangerous URL +// into shell.openExternal via the context menu. +const EXTERNAL_LINK_PROTOCOLS = new Set(['http:', 'https:', 'mailto:']); + +function isExternallyOpenableLink(rawUrl: string): boolean { + try { + return EXTERNAL_LINK_PROTOCOLS.has(new URL(rawUrl).protocol); + } catch { + return false; + } +} + +/** + * Builds the right-click context-menu template for embedded browser-tab + * (``) content. Electron does not render any default context menu for + * guest webContents, so without this the right-click does nothing inside a + * browser tab (issue #1065). + * + * All actions operate on the *guest* webContents, not the host app window: + * edit roles can't be used here because a menu popped over the main window + * would target the wrong webContents, so each item calls the guest's edit / + * navigation methods directly. + */ +function buildBrowserTabContextMenuTemplate( + guest: Electron.WebContents, + params: Electron.ContextMenuParams +): Electron.MenuItemConstructorOptions[] { + const flags = params.editFlags; + + // Editable fields get the full text-editing menu (plus spellcheck + // suggestions when Chromium flags a misspelled word). This is the path that + // matters most for login/form workflows inside browser tabs. + if (params.isEditable) { + const template: Electron.MenuItemConstructorOptions[] = []; + + const suggestions = params.dictionarySuggestions ?? []; + if (params.misspelledWord) { + if (suggestions.length === 0) { + template.push({ label: 'No suggestions', enabled: false }); + } else { + for (const suggestion of suggestions) { + template.push({ + label: suggestion, + click: () => guest.replaceMisspelling(suggestion), + }); + } + } + template.push( + { + label: 'Add to Dictionary', + click: () => guest.session.addWordToSpellCheckerDictionary(params.misspelledWord), + }, + { type: 'separator' } + ); + } + + template.push( + { label: 'Cut', enabled: flags.canCut, click: () => guest.cut() }, + { label: 'Copy', enabled: flags.canCopy, click: () => guest.copy() }, + { label: 'Paste', enabled: flags.canPaste, click: () => guest.paste() }, + { type: 'separator' }, + { label: 'Select All', enabled: flags.canSelectAll, click: () => guest.selectAll() } + ); + return template; + } + + // Non-editable content: assemble independent sections (link, image, + // selection, navigation) and join them with separators so we never emit a + // leading, trailing, or doubled divider. + const sections: Electron.MenuItemConstructorOptions[][] = []; + + if (params.linkURL) { + const linkURL = params.linkURL; + const linkSection: Electron.MenuItemConstructorOptions[] = [ + { label: 'Copy Link', click: () => clipboard.writeText(linkURL) }, + ]; + if (isExternallyOpenableLink(linkURL)) { + linkSection.push({ + label: 'Open Link in Browser', + click: () => { + void shell.openExternal(linkURL).catch((err) => { + logger.warn(`Failed to open link in browser: ${linkURL}`, 'Window', { + error: err instanceof Error ? err.message : String(err), + }); + }); + }, + }); + } + sections.push(linkSection); + } + + if (params.mediaType === 'image' && params.srcURL) { + const srcURL = params.srcURL; + sections.push([ + { label: 'Copy Image', click: () => guest.copyImageAt(params.x, params.y) }, + { label: 'Copy Image Address', click: () => clipboard.writeText(srcURL) }, + ]); + } + + if (params.selectionText.trim().length > 0) { + sections.push([{ label: 'Copy', enabled: flags.canCopy, click: () => guest.copy() }]); + } + + // Navigation actions are always offered so right-clicking blank page + // background still produces a useful menu. canGoBack/goBack live on + // navigationHistory (the WebContents-level methods are deprecated). + const nav = guest.navigationHistory; + sections.push([ + { label: 'Back', enabled: nav.canGoBack(), click: () => nav.goBack() }, + { label: 'Forward', enabled: nav.canGoForward(), click: () => nav.goForward() }, + { label: 'Reload', click: () => guest.reload() }, + ]); + + const template: Electron.MenuItemConstructorOptions[] = []; + sections.forEach((section, index) => { + if (index > 0) template.push({ type: 'separator' }); + template.push(...section); + }); + return template; +} + +/** + * Wires a context-menu handler onto an attached browser-tab guest so + * right-clicking inside the embedded page shows a native menu whose actions + * target the guest webContents. + */ +function attachBrowserTabContextMenu(guest: Electron.WebContents, mainWindow: BrowserWindow): void { + guest.on('context-menu', (_event, params) => { + const template = buildBrowserTabContextMenuTemplate(guest, params); + // Defensive: the builder always emits a navigation section today, but guard + // against a future code path returning [] so we never popup() an empty menu. + if (template.length === 0) return; + Menu.buildFromTemplate(template).popup({ window: mainWindow }); + }); +} + /** * Reports a crash event to Sentry from the main process. * Lazily loads Sentry to avoid module initialization issues. @@ -336,6 +474,11 @@ export function createWindowManager(deps: WindowManagerDependencies): WindowMana mainWindow.webContents.on('did-attach-webview', (_event, guestContents) => { attachBrowserTabGuestSecurity(guestContents as BrowserTabGuestContents); + // Electron renders no default context menu for guest + // content, so right-click is dead inside browser tabs until we wire + // one up against the guest webContents (issue #1065). + attachBrowserTabContextMenu(guestContents as unknown as Electron.WebContents, mainWindow); + // Forward app shortcuts from the webview guest process to the renderer. // When a has focus, keyboard events are trapped in its guest // Chromium process and never reach the renderer's window keydown handler.