Repository: manusa/electronim
Pull request #164 (2022-11-01 06:27:21)

♻️ help module is isolated and uses exposeInMainWorld

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

[![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=164&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=164&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=164&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=164&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=164&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=164&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=164&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=164&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=manusa_electronim&pullRequest=164&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=164&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=164&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=manusa_electronim&pullRequest=164&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=164&metric=new_coverage&view=list) [100.0% Coverage](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=164&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=164&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=manusa_electronim&pullRequest=164&metric=new_duplicated_lines_density&view=list)



Patch:
diff --git a/src/help/help.browser.mjs b/src/help/help.browser.mjs
index 8424991..0e73334 100644
--- a/src/help/help.browser.mjs
+++ b/src/help/help.browser.mjs
@@ -13,12 +13,11 @@
    See the License for the specific language governing permissions and
    limitations under the License.
  */
-const {docs, ipcRenderer} = window;
+const {docs, close} = window.electron;
 
-import {APP_EVENTS, ELECTRONIM_VERSION, TopBar, html, render} from '../components/index.mjs';
+import {ELECTRONIM_VERSION, TopBar, html, render} from '../components/index.mjs';
 
 const helpRoot = () => document.querySelector('.help-root');
-const close = () => ipcRenderer.send(APP_EVENTS.closeDialog);
 
 const Document = ({id}) => html`
   <a id=${id} />
diff --git a/src/help/index.html b/src/help/index.html
index e914992..b34bc70 100644
--- a/src/help/index.html
+++ b/src/help/index.html
@@ -20,12 +20,13 @@ limitations under the License.
     <meta http-equiv="Content-Security-Policy" content="script-src 'self';">
     <meta name="viewport" content="width=device-width, initial-scale=1">
     <title>ElectronIM Help</title>
+    <link rel="stylesheet" type="text/css" href="./help.browser.css" />
   </head>
   <body>
     <div class="help-root"></div>
+    <script src="./help.browser.mjs" type="module" ></script>
     <script src="../../bundles/lib/bulma/css/bulma.css.js"></script>
     <script src="../../bundles/lib/bulma/bulma.dark.scss.js"></script>
     <script src="../../bundles/lib/@fortawesome/fontawesome-free/css/all.css.js"></script>
-    <script src="./help.browser.mjs" type="module"></script>
   </body>
 </html>
diff --git a/src/help/index.js b/src/help/index.js
index 04e3c16..43fe7c1 100644
--- a/src/help/index.js
+++ b/src/help/index.js
@@ -19,7 +19,7 @@ const {handleRedirect} = require('../tab-manager/redirect');
 const {showDialog} = require('../browser-window');
 
 const webPreferences = {
-  contextIsolation: false,
+  contextIsolation: true,
   nodeIntegration: false,
   sandbox: true,
   preload: path.resolve(__dirname, '..', '..', 'bundles', 'help.preload.js')
diff --git a/src/help/preload.js b/src/help/preload.js
index 2d59fb5..f9cbe66 100644
--- a/src/help/preload.js
+++ b/src/help/preload.js
@@ -14,11 +14,10 @@
    limitations under the License.
  */
 /* eslint-disable no-undef */
-const {ipcRenderer} = require('electron');
+const {contextBridge, ipcRenderer} = require('electron');
 const {docs} = require('!val-loader!./docs.browser.val-loader');
 
-require('./help.browser.css');
-
-window.ipcRenderer = ipcRenderer;
-
-window.docs = docs;
+contextBridge.exposeInMainWorld('electron', {
+  close: () => ipcRenderer.send(APP_EVENTS.closeDialog),
+  docs
+});


Test patch:
diff --git a/src/help/__tests__/help.browser.test.mjs b/src/help/__tests__/help.browser.test.mjs
index 5121b31..bfa044f 100644
--- a/src/help/__tests__/help.browser.test.mjs
+++ b/src/help/__tests__/help.browser.test.mjs
@@ -24,6 +24,14 @@ describe('Help in Browser test suite', () => {
     mockIpcRenderer = {
       send: jest.fn()
     };
+    jest.mock('electron', () => ({
+      ipcRenderer: mockIpcRenderer,
+      contextBridge: {
+        exposeInMainWorld: jest.fn((api, object) => {
+          window[api] = object;
+        })
+      }
+    }));
     await import('../../../bundles/help.preload');
     window.ipcRenderer = mockIpcRenderer;
     await loadDOM({meta: import.meta, path: ['..', 'index.html']});
diff --git a/src/help/__tests__/index.html.test.mjs b/src/help/__tests__/index.html.test.mjs
new file mode 100644
index 0000000..b016da4
--- /dev/null
+++ b/src/help/__tests__/index.html.test.mjs
@@ -0,0 +1,35 @@
+/*
+   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.
+ */
+import {jest} from '@jest/globals';
+import {loadDOM} from '../../__tests__/index.mjs';
+
+describe('Help index.html test suite', () => {
+  beforeEach(async () => {
+    jest.resetModules();
+    window.electron = {
+      close: jest.fn(),
+      docs: {
+        doc: 'This is a doc'
+      }
+    };
+    await loadDOM({meta: import.meta, path: ['..', 'index.html']});
+  });
+  test('loads required styles', () => {
+    expect(Array.from(document.querySelectorAll('link[rel=stylesheet]'))
+      .map(link => link.getAttribute('href')))
+      .toEqual(['./help.browser.css']);
+  });
+});
diff --git a/src/help/__tests__/index.test.js b/src/help/__tests__/index.test.js
index 4c96c15..f3a83f0 100644
--- a/src/help/__tests__/index.test.js
+++ b/src/help/__tests__/index.test.js
@@ -32,14 +32,32 @@ describe('Help module test suite', () => {
         setBrowserView: jest.fn()
       };
     });
-    test('webPreferences is sandboxed and has no node integration', () => {
-      // When
-      help.openHelpDialog(mainWindow)();
-      // Then
-      const BrowserView = require('electron').BrowserView;
-      expect(BrowserView).toHaveBeenCalledTimes(1);
-      expect(BrowserView).toHaveBeenCalledWith({
-        webPreferences: expect.objectContaining({sandbox: true, nodeIntegration: false})
+    describe('webPreferences', () => {
+      test('is sandboxed', () => {
+        // When
+        help.openHelpDialog(mainWindow)();
+        // Then
+        const BrowserView = require('electron').BrowserView;
+        expect(BrowserView).toHaveBeenCalledTimes(1);
+        expect(BrowserView).toHaveBeenCalledWith({
+          webPreferences: expect.objectContaining({sandbox: true, nodeIntegration: false})
+        });
+      });
+      test('has no node integration', () => {
+        // When
+        help.openHelpDialog(mainWindow)();
+        // Then
+        expect(require('electron').BrowserView).toHaveBeenCalledWith({
+          webPreferences: expect.objectContaining({nodeIntegration: false})
+        });
+      });
+      test('has context isolation', () => {
+        // When
+        help.openHelpDialog(mainWindow)();
+        // Then
+        expect(require('electron').BrowserView).toHaveBeenCalledWith({
+          webPreferences: expect.objectContaining({contextIsolation: true})
+        });
       });
     });
     test('should open dialog and add event listeners', () => {
diff --git a/src/help/__tests__/preload.test.js b/src/help/__tests__/preload.test.js
index b2c7599..9df5115 100644
--- a/src/help/__tests__/preload.test.js
+++ b/src/help/__tests__/preload.test.js
@@ -13,11 +13,16 @@
    See the License for the specific language governing permissions and
    limitations under the License.
  */
-const {waitFor} = require('@testing-library/dom');
-
 describe('Help Module preload test suite', () => {
+  let electron;
   beforeEach(() => {
     jest.resetModules();
+    jest.mock('electron', () => ({
+      contextBridge: {exposeInMainWorld: jest.fn()},
+      ipcRenderer: {send: jest.fn()}
+    }));
+    electron = require('electron');
+    window.APP_EVENTS = require('../../constants').APP_EVENTS;
   });
   describe('preload (just for coverage and sanity, see bundle tests)', () => {
     beforeEach(() => {
@@ -29,30 +34,27 @@ describe('Help Module preload test suite', () => {
       }), {virtual: true});
       require('../preload');
     });
-    test('adds required variables', () => {
-      expect(window.docs).toEqual({one: 'this is a doc'});
+    describe('creates an API', () => {
+      test('with doc entries', () => {
+        expect(electron.contextBridge.exposeInMainWorld).toHaveBeenCalledWith('electron', {
+          close: expect.toBeFunction(),
+          docs: {one: 'this is a doc'}
+        });
+      });
+      test('with close function', () => {
+        const electronApi = electron.contextBridge.exposeInMainWorld.mock.calls[0][1];
+        electronApi.close();
+        expect(electron.ipcRenderer.send).toHaveBeenCalledWith('closeDialog');
+      });
     });
   });
   describe('preload.bundle', () => {
     beforeEach(() => {
       require('../../../bundles/help.preload');
     });
-    test('loads styles in order', async () => {
-      // When
-      document.body.append(document.createElement('div'));
-      // Then
-      await waitFor(() => expect(document.head.children.length).toBeGreaterThan(0));
-      const styles = Array.from(document.querySelectorAll('style'));
-      expect(styles).toHaveLength(10);
-      expect(styles[1].innerHTML).toMatch(/:root \{.+--color-accent-fg:/s); // Variables
-      expect(styles[2].innerHTML).toContain('html.electronim,'); // Base
-      expect(styles[3].innerHTML).toContain('.electronim h1,'); // Typography
-      expect(styles[5].innerHTML).toContain('.electronim .control .checkbox {'); // CheckBox
-      expect(styles[6].innerHTML).toContain('.electronim .top-bar.navbar {'); // NavBar
-      expect(styles[9].innerHTML).toContain('.help-root {'); // Help-specific
-    });
     test('loads document contents with valid asset URLs', () => {
-      expect(window.docs).toEqual(expect.objectContaining({
+      const electronApi = electron.contextBridge.exposeInMainWorld.mock.calls[0][1];
+      expect(electronApi.docs).toEqual(expect.objectContaining({
         'Keyboard-shortcuts.md': expect.stringMatching(/<h1>Keyboard Shortcuts/i),
         'Roadmap.md': expect.any(String),
         'Screenshots.md': expect.stringContaining('<img src="../../docs/screenshots/main.png" alt="Main" />'),