Repository: manusa/electronim
Pull request #220 (2022-11-21 17:39:55)

🐛 global shortcuts are app-specific (don't hijack OS-wide keyboard events)

sonarcloud[bot]: SonarCloud Quality Gate failed.&nbsp; &nbsp; [![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')](https://sonarcloud.io/dashboard?id=manusa_electronim&pullRequest=220)

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=BUG)  
[![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=VULNERABILITY)  
[![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=220&resolved=false&types=SECURITY_HOTSPOT) [![E](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E-16px.png 'E')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=220&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=220&resolved=false&types=SECURITY_HOTSPOT)  
[![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=220&resolved=false&types=CODE_SMELL)

[![100.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/100-16px.png '100.0%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=220&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=220&metric=new_coverage&view=list)  
[![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=220&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=220&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/src/browser-window/index.js b/src/browser-window/index.js
index b04e243..5d18cc1 100644
--- a/src/browser-window/index.js
+++ b/src/browser-window/index.js
@@ -13,11 +13,14 @@
    See the License for the specific language governing permissions and
    limitations under the License.
  */
+const {registerAppShortcuts} = require('./keyboard-shortcuts');
+
 const showDialog = (window, browserView) => {
   window.setBrowserView(browserView);
   const {width, height} = window.getContentBounds();
   browserView.setBounds({x: 0, y: 0, width, height});
   browserView.setAutoResize({width: false, horizontal: false, height: false, vertical: false});
+  browserView.webContents.focus();
 };
 
-module.exports = {showDialog};
+module.exports = {registerAppShortcuts, showDialog};
diff --git a/src/browser-window/keyboard-shortcuts.js b/src/browser-window/keyboard-shortcuts.js
new file mode 100644
index 0000000..9234761
--- /dev/null
+++ b/src/browser-window/keyboard-shortcuts.js
@@ -0,0 +1,43 @@
+/*
+   Copyright 2022 Marc Nuri San Felix
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+ */
+const {ipcMain: eventBus} = require('electron');
+const {APP_EVENTS} = require('../constants');
+
+const eventKey = ({key, shift = false, control = false, alt = false, meta = false}) =>
+  `${key}-${shift}-${control}-${alt}-${meta}`;
+
+const EVENTS = new Map();
+
+EVENTS.set(eventKey({key: 'Escape'}), () => {
+  eventBus.emit(APP_EVENTS.appMenuClose);
+  eventBus.emit(APP_EVENTS.closeDialog);
+});
+
+EVENTS.set(eventKey({key: 'F11'}), () => {
+  eventBus.emit(APP_EVENTS.fullscreenToggle);
+});
+
+const registerAppShortcuts = (_, webContents) => {
+  webContents.on('before-input-event', (event, {key, shift, control, alt, meta}) => {
+    const func = EVENTS.get(eventKey({key, shift, control, alt, meta}));
+    if (func) {
+      event.preventDefault();
+      func();
+    }
+  });
+};
+
+module.exports = {registerAppShortcuts};
diff --git a/src/index.js b/src/index.js
index 0404a57..20fe442 100644
--- a/src/index.js
+++ b/src/index.js
@@ -14,8 +14,10 @@
    limitations under the License.
  */
 const {app} = require('electron');
+const {registerAppShortcuts} = require('./browser-window');
 const main = require('./main');
 
 app.name = 'ElectronIM';
 
 app.on('ready', main.init);
+app.on('web-contents-created', registerAppShortcuts);
diff --git a/src/main/index.js b/src/main/index.js
index 679358e..9f57d83 100644
--- a/src/main/index.js
+++ b/src/main/index.js
@@ -17,7 +17,6 @@ const path = require('path');
 const {
   BrowserWindow, Notification, app, desktopCapturer, ipcMain: eventBus, nativeTheme
 } = require('electron');
-const {registerGlobalShortcuts} = require('./keyboard-shortcuts');
 const {APP_EVENTS} = require('../constants');
 const {openAboutDialog} = require('../about');
 const {newAppMenu, isNotAppMenu} = require('../app-menu');
@@ -222,7 +221,6 @@ const browserVersionsReady = () => {
 };
 
 const init = () => {
-  registerGlobalShortcuts();
   fixUserDataLocation();
   loadDictionaries();
   const {width = 800, height = 600, theme} = loadSettings();
diff --git a/src/main/keyboard-shortcuts.js b/src/main/keyboard-shortcuts.js
deleted file mode 100644
index be5b224..0000000
--- a/src/main/keyboard-shortcuts.js
+++ /dev/null
@@ -1,27 +0,0 @@
-/*
-   Copyright 2022 Marc Nuri San Felix
-
-   Licensed under the Apache License, Version 2.0 (the "License");
-   you may not use this file except in compliance with the License.
-   You may obtain a copy of the License at
-
-       http://www.apache.org/licenses/LICENSE-2.0
-
-   Unless required by applicable law or agreed to in writing, software
-   distributed under the License is distributed on an "AS IS" BASIS,
-   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-   See the License for the specific language governing permissions and
-   limitations under the License.
- */
-const {globalShortcut, ipcMain: eventBus} = require('electron');
-const {APP_EVENTS} = require('../constants');
-
-const registerGlobalShortcuts = () => {
-  globalShortcut.register('Escape', () => {
-    eventBus.emit(APP_EVENTS.appMenuClose);
-    eventBus.emit(APP_EVENTS.closeDialog);
-  });
-  globalShortcut.register('F11', () => eventBus.emit(APP_EVENTS.fullscreenToggle));
-};
-
-module.exports = {registerGlobalShortcuts};
diff --git a/src/settings/settings.browser.css b/src/settings/settings.browser.css
index b4be249..2edb6a4 100644
--- a/src/settings/settings.browser.css
+++ b/src/settings/settings.browser.css
@@ -56,14 +56,15 @@
     margin: 0;
     transition:
         margin-top 0.3s  ease-out,
-        height 0.3s  ease-out,
         transform 0.3s ease-out;
+    overflow-y: hidden;
 }
 
 .settings__tab--expanded .settings__tab-advanced {
     transform: scaleY(1);
     height: 100%;
     margin-top: var(--material3-card-spacing) ;
+    overflow-y: clip;
 }
 
 .settings__tab .expand-button {


Test patch:
diff --git a/src/__tests__/electron.js b/src/__tests__/electron.js
index b42fd81..5dd6871 100644
--- a/src/__tests__/electron.js
+++ b/src/__tests__/electron.js
@@ -39,6 +39,7 @@ const mockBrowserWindowInstance = () => {
       cut: jest.fn(),
       destroy: jest.fn(),
       executeJavaScript: jest.fn(async () => {}),
+      focus: jest.fn(),
       goBack: jest.fn(),
       loadURL: jest.fn(url => {
         instance.webContents.loadedUrl = url;
@@ -67,6 +68,7 @@ const mockElectronInstance = ({...overriddenProps} = {}) => {
     MenuItem: jest.fn(),
     app: {
       getPath: jest.fn(),
+      on: jest.fn(),
       setPath: jest.fn()
     },
     contextBridge: {
diff --git a/src/__tests__/index.test.js b/src/__tests__/index.test.js
index ae8dc40..fbfd78a 100644
--- a/src/__tests__/index.test.js
+++ b/src/__tests__/index.test.js
@@ -18,11 +18,7 @@ describe('Entrypoint test suite', () => {
   let main;
   beforeEach(() => {
     jest.resetModules();
-    jest.mock('electron', () => ({
-      app: {
-        on: jest.fn()
-      }
-    }));
+    jest.mock('electron', () => require('../__tests__').mockElectronInstance());
     jest.mock('../main', () => ({
       init: jest.fn()
     }));
@@ -35,7 +31,16 @@ describe('Entrypoint test suite', () => {
     require('../index');
     // Then
     expect(app.name).toBe('ElectronIM');
-    expect(app.on).toHaveBeenCalledTimes(1);
+    expect(app.on).toHaveBeenCalledTimes(2);
     expect(app.on). toHaveBeenCalledWith('ready', main.init);
   });
+  test('Adds event listener to register app keyboard shortcuts on every webContents', () => {
+    // Given
+    // When
+    require('../index');
+    // Then
+    expect(app.on).toHaveBeenCalledTimes(2);
+    expect(app.on)
+      .toHaveBeenCalledWith('web-contents-created', require('../browser-window').registerAppShortcuts);
+  });
 });
diff --git a/src/browser-window/__tests__/index.test.js b/src/browser-window/__tests__/index.test.js
index 87f91b8..e4031be 100644
--- a/src/browser-window/__tests__/index.test.js
+++ b/src/browser-window/__tests__/index.test.js
@@ -20,14 +20,9 @@ describe('browser-window util module test suite', () => {
   });
   test('showDialog, should fill provided window with provided BrowserView', () => {
     // Given
-    const window = {
-      getContentBounds: jest.fn(() => ({width: 13, height: 37})),
-      setBrowserView: jest.fn()
-    };
-    const dialog = {
-      setBounds: jest.fn(),
-      setAutoResize: jest.fn()
-    };
+    const window = require('../../__tests__/electron').mockBrowserWindowInstance();
+    window.getContentBounds = jest.fn(() => ({width: 13, height: 37}));
+    const dialog = require('../../__tests__/electron').mockBrowserWindowInstance();
     // When
     browserWindow.showDialog(window, dialog);
     // Then
@@ -38,5 +33,6 @@ describe('browser-window util module test suite', () => {
     expect(dialog.setAutoResize).toHaveBeenCalledTimes(1);
     expect(dialog.setAutoResize)
       .toHaveBeenCalledWith({width: false, horizontal: false, height: false, vertical: false});
+    expect(dialog.webContents.focus).toHaveBeenCalledTimes(1);
   });
 });
diff --git a/src/browser-window/__tests__/keyboard-shortcuts.test.js b/src/browser-window/__tests__/keyboard-shortcuts.test.js
new file mode 100644
index 0000000..d7d17e4
--- /dev/null
+++ b/src/browser-window/__tests__/keyboard-shortcuts.test.js
@@ -0,0 +1,47 @@
+/*
+   Copyright 2022 Marc Nuri San Felix
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+ */
+describe('Main :: Global Keyboard Shortcuts module test suite', () => {
+  let electron;
+  let browserWindow;
+  let inputEvent;
+  beforeEach(() => {
+    jest.resetModules();
+    jest.mock('electron', () => require('../../__tests__').mockElectronInstance());
+    electron = require('electron');
+    browserWindow = require('../../__tests__').mockBrowserWindowInstance();
+    require('../').registerAppShortcuts({}, browserWindow.webContents);
+    inputEvent = {
+      preventDefault: jest.fn()
+    };
+  });
+  test.each([
+    [{key: 'Escape', appEvent: 'appMenuClose'}], [{key: 'Escape', appEvent: 'closeDialog'}],
+    [{key: 'F11', appEvent: 'fullscreenToggle'}]
+  ])('Key "$key" triggers "$appEvent" app event', ({key, appEvent}) => {
+    browserWindow.listeners['before-input-event'](inputEvent, {key});
+    expect(electron.ipcMain.emit).toHaveBeenCalledWith(appEvent);
+  });
+  describe('preventDefault', () => {
+    test('calls preventDefault if key is registered', () => {
+      browserWindow.listeners['before-input-event'](inputEvent, {key: 'Escape'});
+      expect(inputEvent.preventDefault).toHaveBeenCalled();
+    });
+    test('doesn\'t call preventDefault if key is not registered', () => {
+      browserWindow.listeners['before-input-event'](inputEvent, {});
+      expect(inputEvent.preventDefault).not.toHaveBeenCalled();
+    });
+  });
+});
diff --git a/src/main/__tests__/keyboard-shortcuts.test.js b/src/main/__tests__/keyboard-shortcuts.test.js
deleted file mode 100644
index b09b274..0000000
--- a/src/main/__tests__/keyboard-shortcuts.test.js
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
-   Copyright 2022 Marc Nuri San Felix
-
-   Licensed under the Apache License, Version 2.0 (the "License");
-   you may not use this file except in compliance with the License.
-   You may obtain a copy of the License at
-
-       http://www.apache.org/licenses/LICENSE-2.0
-
-   Unless required by applicable law or agreed to in writing, software
-   distributed under the License is distributed on an "AS IS" BASIS,
-   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-   See the License for the specific language governing permissions and
-   limitations under the License.
- */
-describe('Main :: Global Keyboard Shortcuts module test suite', () => {
-  let electron;
-  beforeEach(() => {
-    jest.resetModules();
-    jest.mock('electron', () => require('../../__tests__').mockElectronInstance());
-    require('../').init();
-    electron = require('electron');
-  });
-  test.each([
-    ['Escape', 'appMenuClose'], ['Escape', 'closeDialog'],
-    ['F11', 'fullscreenToggle']
-  ])('Accelerator "%s" triggers "%s" app event', (accelerator, appEvent) => {
-    electron.globalShortcut.listeners[accelerator]();
-    expect(electron.ipcMain.emit).toHaveBeenCalledWith(appEvent);
-  });
-});
diff --git a/src/settings/__tests__/index.test.js b/src/settings/__tests__/index.test.js
index d6e1755..358582d 100644
--- a/src/settings/__tests__/index.test.js
+++ b/src/settings/__tests__/index.test.js
@@ -15,16 +15,15 @@
  */
 /* eslint-disable no-use-before-define */
 describe('Settings module test suite', () => {
-  let mockBrowserView;
+  let electron;
   let fs;
   let path;
   let settings;
   beforeEach(() => {
     jest.resetModules();
-    mockBrowserView = require('../../__tests__').mockBrowserWindowInstance();
-    jest.mock('electron', () => ({
-      BrowserView: jest.fn(() => mockBrowserView)
-    }));
+    jest.mock('../../constants', () => ({}));
+    jest.mock('electron', () => require('../../__tests__').mockElectronInstance());
+    electron = require('electron');
     jest.mock('fs');
     jest.mock('os', () => ({homedir: () => '$HOME'}));
     fs = require('fs');
@@ -125,10 +124,8 @@ describe('Settings module test suite', () => {
     let mainWindow;
     let openSettings;
     beforeEach(() => {
-      mainWindow = {
-        getContentBounds: jest.fn(() => ({width: 13, height: 37})),
-        setBrowserView: jest.fn()
-      };
+      mainWindow = electron.browserWindowInstance;
+      mainWindow.getContentBounds = jest.fn(() => ({width: 13, height: 37}));
       openSettings = settings.openSettingsDialog(mainWindow);
     });
     test('webPreferences is sandboxed and has no node integration', () => {
@@ -145,8 +142,9 @@ describe('Settings module test suite', () => {
       // When
       openSettings();
       // Then
-      expect(mockBrowserView.webContents.loadURL).toHaveBeenCalledTimes(1);
-      expect(mockBrowserView.webContents.loadURL).toHaveBeenCalledWith(expect.stringMatching(/.+?\/index.html$/));
+      expect(electron.browserViewInstance.webContents.loadURL).toHaveBeenCalledTimes(1);
+      expect(electron.browserViewInstance.webContents.loadURL)
+        .toHaveBeenCalledWith(expect.stringMatching(/.+?\/index.html$/));
     });
   });
   const expectHomeDirectoryCreated = () => {