Arduino-pro-ide: Command + R shortcut not working in other apps while the IDE is running

[insidegui]

Describe the bug
On my Mac, while the IDE is running, the ⌘R keyboard shortcut stops working in other apps such as Safari and Xcode.

To Reproduce
Steps to reproduce the behavior:

1. Launch the IDE on the Mac
2. Try using the Command + R keyboard shortcut in another app
   Expected behavior
   The app should not take ownership of a standard keyboard shortcut such as Command + R when it's not the active app.

Desktop (please complete the following information):

• OS: macOS
• Version 10.15.4

[ubidefeo]

hi @insidegui
This is a weird behaviour, the app should not capture and hold any shortcut,
especially CMD+R which we don't use. Must be an Electron thing 😓
I'll make sure we check this behaviour next week.

ps: big fan :)

[kittaakos]

Thanks for logging it, yes I can confirm this; I was an idiot and overridden the OS level Ctrl/⌘+R.

Refs:

• Issue: https://github.com/electron/electron/issues/3725#issuecomment-540854069
• Better fix: https://electron.guide/final-polish/renderer/#prevent-browserwindow-refreshes

[kittaakos]

@insidegui, the fix is ready but I am afraid, all of us have to wait for the next release cycle. There is a workaround, however, one can replace the resources/app/src-gen/frontend/electron-main.js manually with the JS from below. 👇

The electron-main.js file can be found at:

• Windows: "C:\\path\\to\\Arduino Pro IDE-v0.0.x-win\\resources\\app\\src-gen\\frontend\\electron-main.js",
• macOS: /path/to/Arduino\ Pro\ IDE.app/Contents/Resources/app/src-gen/frontend/electron-main.js, and
• Linux: /path/to/Arduino Pro IDE-v0.0.x-linux-x64/resources/app/src-gen/frontend/electron-main.js.

Let me know if the problem persists. I am going to close this, please reopen if required.

// @ts-check

// Useful for Electron/NW.js apps as GUI apps on macOS doesn't inherit the `$PATH` define
// in your dotfiles (.bashrc/.bash_profile/.zshrc/etc).
// https://github.com/electron/electron/issues/550#issuecomment-162037357
// https://github.com/eclipse-theia/theia/pull/3534#issuecomment-439689082
require('fix-path')();

// Workaround for https://github.com/electron/electron/issues/9225. Chrome has an issue where
// in certain locales (e.g. PL), image metrics are wrongly computed. We explicitly set the
// LC_NUMERIC to prevent this from happening (selects the numeric formatting category of the
// C locale, http://en.cppreference.com/w/cpp/locale/LC_categories).
if (process.env.LC_ALL) {
    process.env.LC_ALL = 'C';
}
process.env.LC_NUMERIC = 'C';

const { v4 } = require('uuid');
const electron = require('electron');
const { join, resolve } = require('path');
const { fork } = require('child_process');
const { app, dialog, shell, BrowserWindow, ipcMain, Menu, globalShortcut } = electron;
const { ElectronSecurityToken } = require('@theia/core/lib/electron-common/electron-token');

const applicationName = `Theia Electron Example`;
const isSingleInstance = false;
const disallowReloadKeybinding = false;

if (isSingleInstance && !app.requestSingleInstanceLock()) {
    // There is another instance running, exit now. The other instance will request focus.
    app.quit();
    return;
}

const nativeKeymap = require('native-keymap');
const Storage = require('electron-store');
const electronStore = new Storage();

const electronSecurityToken = {
    value: v4(),
};

app.on('ready', () => {

    // Explicitly set the app name to have better menu items on macOS. ("About", "Hide", and "Quit")
    // See: https://github.com/electron-userland/electron-builder/issues/2468
    app.setName(applicationName);

    const { screen } = electron;

    // Remove the default electron menus, waiting for the application to set its own.
    Menu.setApplicationMenu(Menu.buildFromTemplate([{
        role: 'help', submenu: [{ role: 'toggledevtools'}]
    }]));

    function createNewWindow(theUrl) {

        // We must center by hand because `browserWindow.center()` fails on multi-screen setups
        // See: https://github.com/electron/electron/issues/3490
        const { bounds } = screen.getDisplayNearestPoint(screen.getCursorScreenPoint());
        const height = Math.floor(bounds.height * (2/3));
        const width = Math.floor(bounds.width * (2/3));

        const y = Math.floor(bounds.y + (bounds.height - height) / 2);
        const x = Math.floor(bounds.x + (bounds.width - width) / 2);

        const WINDOW_STATE = 'windowstate';
        const windowState = electronStore.get(WINDOW_STATE, {
            width, height, x, y
        });

        let windowOptions = electronStore.get('windowOptions');

        if (!windowOptions) {
            windowOptions = {
                show: false,
                title: applicationName,
                width: windowState.width,
                height: windowState.height,
                minWidth: 200,
                minHeight: 120,
                x: windowState.x,
                y: windowState.y,
                isMaximized: windowState.isMaximized
            };
            electronStore.set('windowOptions', windowOptions);
        }

        // Always hide the window, we will show the window when it is ready to be shown in any case.
        const newWindow = new BrowserWindow(windowOptions);
        if (windowOptions.isMaximized) {
            newWindow.maximize();
        }
        newWindow.on('ready-to-show', () => newWindow.show());
        if (disallowReloadKeybinding) {
            newWindow.on('focus', event => {
                for (const accelerator of ['CmdOrCtrl+R','F5']) {
                    globalShortcut.register(accelerator, () => {});
                }
            });
            newWindow.on('blur', event => globalShortcut.unregisterAll());
        }

        // Prevent calls to "window.open" from opening an ElectronBrowser window,
        // and rather open in the OS default web browser.
        newWindow.webContents.on('new-window', (event, url) => {
            event.preventDefault();
            shell.openExternal(url);
        });

        // Save the window geometry state on every change
        const saveWindowState = () => {
            try {
                let bounds;
                if (newWindow.isMaximized()) {
                    bounds = electronStore.get(WINDOW_STATE, {});
                } else {
                    bounds = newWindow.getBounds();
                }
                electronStore.set(WINDOW_STATE, {
                    isMaximized: newWindow.isMaximized(),
                    width: bounds.width,
                    height: bounds.height,
                    x: bounds.x,
                    y: bounds.y
                });
            } catch (e) {
                console.error("Error while saving window state.", e);
            }
        };
        let delayedSaveTimeout;
        const saveWindowStateDelayed = () => {
            if (delayedSaveTimeout) {
                clearTimeout(delayedSaveTimeout);
            }
            delayedSaveTimeout = setTimeout(saveWindowState, 1000);
        };
        newWindow.on('close', saveWindowState);
        newWindow.on('resize', saveWindowStateDelayed);
        newWindow.on('move', saveWindowStateDelayed);

        // Fired when a beforeunload handler tries to prevent the page unloading
        newWindow.webContents.on('will-prevent-unload', event => {
            const preventStop = 0 !== dialog.showMessageBox(newWindow, {
                type: 'question',
                buttons: ['Yes', 'No'],
                title: 'Confirm',
                message: 'Are you sure you want to quit?',
                detail: 'Any unsaved changes will not be saved.'
            });

            if (!preventStop) {
                // This ignores the beforeunload callback, allowing the page to unload
                event.preventDefault();
            }
        });

        // Notify the renderer process on keyboard layout change
        nativeKeymap.onDidChangeKeyboardLayout(() => {
            if (!newWindow.isDestroyed()) {
                const newLayout = {
                    info: nativeKeymap.getCurrentKeyboardLayout(),
                    mapping: nativeKeymap.getKeyMap()
                };
                newWindow.webContents.send('keyboardLayoutChanged', newLayout);
            }
        });

        if (!!theUrl) {
            newWindow.loadURL(theUrl);
        }
        return newWindow;
    }

    app.on('window-all-closed', () => {
        app.quit();
    });
    ipcMain.on('create-new-window', (event, url) => {
        createNewWindow(url);
    });
    ipcMain.on('open-external', (event, url) => {
        shell.openExternal(url);
    });
    ipcMain.on('set-window-options', (event, options) => {
        electronStore.set('windowOptions', options);
    });
    ipcMain.on('get-window-options', event => {
        event.returnValue = electronStore.get('windowOptions');
    });

    // Check whether we are in bundled application or development mode.
    // @ts-ignore
    const devMode = process.defaultApp || /node_modules[/]electron[/]/.test(process.execPath);
    // Check if we should run everything as one process.
    const noBackendFork = process.argv.includes('--no-cluster');
    const mainWindow = createNewWindow();

    if (isSingleInstance) {
        app.on('second-instance', (event, commandLine, workingDirectory) => {
            // Someone tried to run a second instance, we should focus our window.
            if (mainWindow && !mainWindow.isDestroyed()) {
                if (mainWindow.isMinimized()) {
                    mainWindow.restore();
                }
                mainWindow.focus()
            }
        })
    }

    const loadMainWindow = (port) => {
        if (!mainWindow.isDestroyed()) {
            mainWindow.webContents.session.cookies.set({
                url: `http://localhost:${port}/`,
                name: ElectronSecurityToken,
                value: JSON.stringify(electronSecurityToken),
            }, error => {
                if (error) {
                    console.error(error);
                } else {
                    mainWindow.loadURL('file://' + join(__dirname, '../../lib/index.html') + '?port=' + port);
                }
            });
        }
    };

    // We cannot use the `process.cwd()` as the application project path (the location of the `package.json` in other words)
    // in a bundled electron application because it depends on the way we start it. For instance, on OS X, these are a differences:
    // https://github.com/eclipse-theia/theia/issues/3297#issuecomment-439172274
    process.env.THEIA_APP_PROJECT_PATH = resolve(__dirname, '..', '..');

    // Set the electron version for both the dev and the production mode. (https://github.com/eclipse-theia/theia/issues/3254)
    // Otherwise, the forked backend processes will not know that they're serving the electron frontend.
    // The forked backend should patch its `process.versions.electron` with this value if it is missing.
    process.env.THEIA_ELECTRON_VERSION = process.versions.electron;

    const mainPath = join(__dirname, '..', 'backend', 'main');
    // We spawn a separate process for the backend for Express to not run in the Electron main process.
    // See: https://github.com/eclipse-theia/theia/pull/7361#issuecomment-601272212
    // But when in debugging we want to run everything in the same process to make things easier.
    if (noBackendFork) {
        process.env[ElectronSecurityToken] = JSON.stringify(electronSecurityToken);
        require(mainPath).then(address => {
            loadMainWindow(address.port);
        }).catch((error) => {
            console.error(error);
            app.exit(1);
        });
    } else {
        // We want to pass flags passed to the Electron app to the backend process.
        // Quirk: When developing from sources, we execute Electron as `electron.exe electron-main.js ...args`, but when bundled,
        // the command looks like `bundled-application.exe ...args`.
        const cp = fork(mainPath, process.argv.slice(devMode ? 2 : 1), { env: Object.assign({
            [ElectronSecurityToken]: JSON.stringify(electronSecurityToken),
        }, process.env) });
        cp.on('message', (address) => {
            loadMainWindow(address.port);
        });
        cp.on('error', (error) => {
            console.error(error);
            app.exit(1);
        });
        app.on('quit', () => {
            // If we forked the process for the clusters, we need to manually terminate it.
            // See: https://github.com/eclipse-theia/theia/issues/835
            process.kill(cp.pid);
        });
    }
});

[ubidefeo]

@insidegui
if you need it right away try and patch according to @kittaakos 's directions, otherwise you'll have to wait for the next minor :)

[insidegui]

Awesome! Thanks