fix(shortcuts): guard ev.reply() against destroyed senders + add roundtrip tests

+6 -1

backend/electron/entry.ts

··· 5 5 * All Electron-specific code lives here. 6 6 */ 7 7 8 - import { app } from 'electron'; 8 + import { app, globalShortcut } from 'electron'; 9 9 import fs from 'node:fs'; 10 10 import path from 'node:path'; 11 11 import unhandled from 'electron-unhandled'; ··· 40 40 // Shortcuts 41 41 registerLocalShortcut, 42 42 unregisterLocalShortcut, 43 + handleLocalShortcut, 43 44 // PubSub 44 45 scopes, 45 46 publish, ··· 95 96 const ROOT_DIR = app.getAppPath(); 96 97 97 98 const DEBUG = !!process.env.DEBUG; 99 + 100 + // Expose test utilities on global for Playwright evaluateMain access 101 + (globalThis as any).__peek_test = { handleLocalShortcut }; 102 + (globalThis as any).__peek_electron = { globalShortcut }; 98 103 99 104 // Parse --load-extension CLI arguments for dev workflow 100 105 // Usage: yarn start -- --load-extension=/path/to/extension

+24 -9

backend/electron/ipc.ts

··· 2758 2758 }); 2759 2759 2760 2760 // Register shortcut 2761 + // 2762 + // The callback closure captures the IpcMainEvent so ev.reply() can route 2763 + // the response back to the correct sender frame at callback time. 2764 + // 2765 + // ev.reply() internally uses sender.sendToFrame(frameId, ...) which correctly 2766 + // handles both: 2767 + // - Normal BrowserWindow senders (main frame) 2768 + // - Iframe sub-frame senders (consolidated extensions in extension-host.html) 2769 + // 2770 + // We guard against destroyed senders (extension reload, iframe removed from DOM) 2771 + // to prevent uncaught exceptions. The previous approaches that failed: 2772 + // - BrowserWindow.fromId(ev.sender.id): wrong ID space (WebContents vs Window) 2773 + // - BrowserWindow.fromWebContents(ev.sender): returns null for iframe senders 2774 + // - webContents.fromId(id): does not find sub-frame WebContents 2775 + // - sender.send(): sends to main frame only, not the specific iframe 2761 2776 ipcMain.on(IPC_CHANNELS.REGISTER_SHORTCUT, (ev, msg) => { 2762 2777 const isGlobal = msg.global === true; 2763 2778 const modeStr = msg.mode ? ` mode:${msg.mode}` : ''; 2764 2779 DEBUG && console.log('ipc register shortcut', msg.shortcut, isGlobal ? '(global)' : '(local)', modeStr); 2765 2780 2766 - // Capture BrowserWindow ID at registration time — ev.sender (WebContents) may be destroyed when shortcut fires later 2767 - const senderWindow = BrowserWindow.fromWebContents(ev.sender); 2768 - const windowId = senderWindow?.id; 2769 2781 const callback = () => { 2770 2782 DEBUG && console.log('on(registershortcut): shortcut executed', msg.shortcut, msg.replyTopic); 2771 - if (!windowId) return; 2772 - const win = BrowserWindow.fromId(windowId); 2773 - if (win && !win.isDestroyed()) { 2774 - win.webContents.send(msg.replyTopic, { foo: 'bar' }); 2775 - } else { 2776 - DEBUG && console.log('on(registershortcut): window', windowId, 'gone, skipping reply'); 2783 + try { 2784 + if (!ev.sender.isDestroyed()) { 2785 + ev.reply(msg.replyTopic, { foo: 'bar' }); 2786 + } else { 2787 + DEBUG && console.log('on(registershortcut): sender destroyed, skipping reply for', msg.shortcut); 2788 + } 2789 + } catch (err) { 2790 + // Guard against edge cases where sender state changes between check and send 2791 + DEBUG && console.log('on(registershortcut): reply failed for', msg.shortcut, err); 2777 2792 } 2778 2793 }; 2779 2794

+256

tests/desktop/smoke.spec.ts

··· 3592 3592 } 3593 3593 }); 3594 3594 }); 3595 + 3596 + // ============================================================================ 3597 + // Shortcut Roundtrip Tests 3598 + // 3599 + // Tests the full IPC roundtrip for shortcut registration and callback firing. 3600 + // The flow: renderer registers shortcut via IPC -> main stores callback with ev.reply -> 3601 + // shortcut fires -> callback calls ev.reply(replyTopic) -> renderer ipcRenderer.on fires cb. 3602 + // 3603 + // Since Playwright keyboard.press does NOT reliably trigger Electron's before-input-event, 3604 + // we trigger the shortcut callback by calling handleLocalShortcut from the main process 3605 + // via evaluateMain with a synthetic input event. 3606 + // ============================================================================ 3607 + 3608 + test.describe('Shortcut Roundtrip @desktop', () => { 3609 + test('local shortcut from background window roundtrip', async () => { 3610 + // Register a local shortcut from bgWindow, trigger it via handleLocalShortcut 3611 + // in the main process, verify callback fires in the renderer. 3612 + // This tests the basic ev.reply roundtrip for a normal BrowserWindow WebContents. 3613 + const bgWindow = sharedBgWindow; 3614 + 3615 + // Register a local shortcut from the bgWindow 3616 + await bgWindow.evaluate(() => { 3617 + (window as any).__shortcutFired = false; 3618 + (window as any).app.shortcuts.register('Alt+F7', () => { 3619 + (window as any).__shortcutFired = true; 3620 + }); 3621 + }); 3622 + 3623 + // Wait for IPC registration to propagate to main process 3624 + await sleep(300); 3625 + 3626 + // Trigger the shortcut from the main process by calling handleLocalShortcut 3627 + // with a synthetic input event matching Alt+F7 3628 + const handled = await sharedApp.evaluateMain!(({ app }) => { 3629 + try { 3630 + const { handleLocalShortcut } = (globalThis as any).__peek_test; 3631 + return handleLocalShortcut({ 3632 + type: 'keyDown', 3633 + alt: true, 3634 + shift: false, 3635 + meta: false, 3636 + control: false, 3637 + code: 'F7' 3638 + }); 3639 + } catch (e: any) { 3640 + return 'peek_test-failed: ' + e.message; 3641 + } 3642 + }); 3643 + 3644 + // handleLocalShortcut should return true (shortcut was found and callback invoked) 3645 + expect(handled).toBe(true); 3646 + 3647 + // Wait for the reply to reach the renderer and trigger the callback 3648 + await bgWindow.waitForFunction( 3649 + () => (window as any).__shortcutFired === true, 3650 + { timeout: 5000 } 3651 + ); 3652 + 3653 + const fired = await bgWindow.evaluate(() => (window as any).__shortcutFired); 3654 + expect(fired).toBe(true); 3655 + 3656 + // Clean up 3657 + await bgWindow.evaluate(() => { 3658 + (window as any).app.shortcuts.unregister('Alt+F7'); 3659 + delete (window as any).__shortcutFired; 3660 + }); 3661 + }); 3662 + 3663 + test('local shortcut from extension-host iframe roundtrip', async () => { 3664 + // Register a local shortcut from a consolidated extension iframe (in extension-host.html), 3665 + // trigger it via handleLocalShortcut in the main process, verify callback fires 3666 + // in the iframe. This tests ev.reply for iframe WebContents where 3667 + // BrowserWindow.fromWebContents() returns null. 3668 + const bgWindow = sharedBgWindow; 3669 + 3670 + // Find the extension host window 3671 + const hostWindow = sharedApp.windows().find(w => w.url().includes('extension-host.html')); 3672 + expect(hostWindow).toBeDefined(); 3673 + 3674 + // Wait for iframes to load 3675 + await hostWindow!.waitForFunction( 3676 + () => { 3677 + const container = document.getElementById('extensions'); 3678 + const iframes = container ? container.querySelectorAll('iframe') : []; 3679 + return iframes.length >= 1; 3680 + }, 3681 + { timeout: 10000 } 3682 + ); 3683 + 3684 + // Find a frame for a consolidated extension (e.g., slides or groups) 3685 + const frames = hostWindow!.frames(); 3686 + const extFrame = frames.find(f => 3687 + f.url().includes('peek://slides/') || f.url().includes('peek://groups/') 3688 + ); 3689 + expect(extFrame).toBeDefined(); 3690 + 3691 + // Register a local shortcut from the iframe context 3692 + await extFrame!.evaluate(() => { 3693 + (window as any).__iframeShortcutFired = false; 3694 + (window as any).app.shortcuts.register('Alt+F8', () => { 3695 + (window as any).__iframeShortcutFired = true; 3696 + }); 3697 + }); 3698 + 3699 + // Wait for IPC registration to reach main process 3700 + await sleep(300); 3701 + 3702 + // Trigger the shortcut from the main process 3703 + const handled = await sharedApp.evaluateMain!(({ app }) => { 3704 + try { 3705 + const { handleLocalShortcut } = (globalThis as any).__peek_test; 3706 + return handleLocalShortcut({ 3707 + type: 'keyDown', 3708 + alt: true, 3709 + shift: false, 3710 + meta: false, 3711 + control: false, 3712 + code: 'F8' 3713 + }); 3714 + } catch (e: any) { 3715 + return 'peek_test-failed: ' + e.message; 3716 + } 3717 + }); 3718 + 3719 + expect(handled).toBe(true); 3720 + 3721 + // Wait for the reply to reach the iframe and trigger the callback 3722 + await extFrame!.waitForFunction( 3723 + () => (window as any).__iframeShortcutFired === true, 3724 + { timeout: 5000 } 3725 + ); 3726 + 3727 + const fired = await extFrame!.evaluate(() => (window as any).__iframeShortcutFired); 3728 + expect(fired).toBe(true); 3729 + 3730 + // Clean up 3731 + await extFrame!.evaluate(() => { 3732 + (window as any).app.shortcuts.unregister('Alt+F8'); 3733 + delete (window as any).__iframeShortcutFired; 3734 + }); 3735 + }); 3736 + 3737 + test('global shortcut from extension-host iframe roundtrip', async () => { 3738 + // Register a global shortcut from a consolidated extension iframe, 3739 + // trigger it from the main process, verify callback fires in the iframe. 3740 + // This is the exact scenario that breaks slides extension hotkeys. 3741 + 3742 + // Find the extension host window 3743 + const hostWindow = sharedApp.windows().find(w => w.url().includes('extension-host.html')); 3744 + expect(hostWindow).toBeDefined(); 3745 + 3746 + // Find a frame for a consolidated extension 3747 + const frames = hostWindow!.frames(); 3748 + const extFrame = frames.find(f => 3749 + f.url().includes('peek://slides/') || f.url().includes('peek://groups/') 3750 + ); 3751 + expect(extFrame).toBeDefined(); 3752 + 3753 + // Register a GLOBAL shortcut from the iframe context 3754 + await extFrame!.evaluate(() => { 3755 + (window as any).__globalIframeShortcutFired = false; 3756 + (window as any).app.shortcuts.register('CommandOrControl+Shift+F11', () => { 3757 + (window as any).__globalIframeShortcutFired = true; 3758 + }, { global: true }); 3759 + }); 3760 + 3761 + // Wait for IPC registration to propagate 3762 + await sleep(300); 3763 + 3764 + // Verify the shortcut was registered in the main process 3765 + const isRegistered = await sharedApp.evaluateMain!(({ app: electronApp }) => { 3766 + // Access globalShortcut from the electron app module context 3767 + const electron = (globalThis as any).__peek_electron || {}; 3768 + if (electron.globalShortcut) { 3769 + return electron.globalShortcut.isRegistered('CommandOrControl+Shift+F11'); 3770 + } 3771 + // Fallback: check via the imported shortcuts module 3772 + return 'no-globalShortcut-access'; 3773 + }); 3774 + expect(isRegistered).toBe(true); 3775 + 3776 + // Trigger the global shortcut callback. Electron's globalShortcut.register stores 3777 + // a callback. We simulate the OS-level trigger by emitting the accelerator. 3778 + // Since we can't trigger OS-level shortcuts from Playwright, we use a workaround: 3779 + // call the internal shortcut callback via the electron module. 3780 + // Unfortunately globalShortcut has no programmatic trigger API. 3781 + // The real test is that the global shortcut was registered (above) and that the 3782 + // ev.reply mechanism works (tested in the local shortcut tests with same sender). 3783 + // Here we verify the registration path works for iframe senders. 3784 + 3785 + // Clean up 3786 + await extFrame!.evaluate(() => { 3787 + (window as any).app.shortcuts.unregister('CommandOrControl+Shift+F11', { global: true }); 3788 + delete (window as any).__globalIframeShortcutFired; 3789 + }); 3790 + }); 3791 + 3792 + test('shortcut from external extension window roundtrip', async () => { 3793 + // Register a local shortcut from the example extension (separate BrowserWindow), 3794 + // trigger it via handleLocalShortcut, verify callback fires. 3795 + // This tests the standard BrowserWindow path for comparison with the iframe path. 3796 + 3797 + // Find the example extension window 3798 + const exampleWindow = await waitForWindow( 3799 + () => sharedApp.windows(), 3800 + 'peek://ext/example/background.html', 3801 + 15000 3802 + ); 3803 + expect(exampleWindow).toBeDefined(); 3804 + 3805 + // Register a local shortcut from the example extension window 3806 + await exampleWindow.evaluate(() => { 3807 + (window as any).__extShortcutFired = false; 3808 + (window as any).app.shortcuts.register('Alt+F6', () => { 3809 + (window as any).__extShortcutFired = true; 3810 + }); 3811 + }); 3812 + 3813 + // Wait for IPC to propagate 3814 + await sleep(300); 3815 + 3816 + // Trigger the shortcut from the main process 3817 + const handled = await sharedApp.evaluateMain!(({ app }) => { 3818 + try { 3819 + const { handleLocalShortcut } = (globalThis as any).__peek_test; 3820 + return handleLocalShortcut({ 3821 + type: 'keyDown', 3822 + alt: true, 3823 + shift: false, 3824 + meta: false, 3825 + control: false, 3826 + code: 'F6' 3827 + }); 3828 + } catch (e: any) { 3829 + return 'peek_test-failed: ' + e.message; 3830 + } 3831 + }); 3832 + 3833 + expect(handled).toBe(true); 3834 + 3835 + // Wait for the callback to fire in the example extension window 3836 + await exampleWindow.waitForFunction( 3837 + () => (window as any).__extShortcutFired === true, 3838 + { timeout: 5000 } 3839 + ); 3840 + 3841 + const fired = await exampleWindow.evaluate(() => (window as any).__extShortcutFired); 3842 + expect(fired).toBe(true); 3843 + 3844 + // Clean up 3845 + await exampleWindow.evaluate(() => { 3846 + (window as any).app.shortcuts.unregister('Alt+F6'); 3847 + delete (window as any).__extShortcutFired; 3848 + }); 3849 + }); 3850 + });

+5

tests/fixtures/desktop-app.ts

··· 50 50 /** Get extension background windows */ 51 51 getExtensionWindows(): Page[]; 52 52 53 + /** Evaluate code in the main (Node) process (Electron only) */ 54 + evaluateMain?<R>(fn: (ctx: { app: Electron.App; require: NodeRequire }) => R): Promise<R>; 55 + 53 56 /** Close the app */ 54 57 close(): Promise<void>; 55 58 } ··· 278 281 getBackgroundWindow: async () => { 279 282 return waitForWindowHelper(() => electronApp.windows(), 'app/background.html'); 280 283 }, 284 + 285 + evaluateMain: (fn: any) => electronApp.evaluate(fn as any), 281 286 282 287 getExtensionWindows: () => { 283 288 return electronApp.windows().filter(w =>

Configure Feed

Configure Feed