From e064f765cbff5151a40af0b2b619e40c0d33d76b Mon Sep 17 00:00:00 2001
From: Dominic Hopton <1811056+grork@users.noreply.github.com>
Date: Tue, 1 Jan 2019 20:38:45 -0800
Subject: [PATCH] Fix CSS & JavaScript injection (Fixes #703, Fixes #731, PR
 #732)

* Fix for CSS Injection not working (#703)

Issue:
When using `onHeadersReceived`, the code was passing `null` for the filters.
This appears to trigger behaviour that matches _no_ urls at all.
This results in it never being called to inject the CSS.

Fix:
Pass an empty array instead. Now it's called for all URLs.

Tests pass & linting is clean

* Fix JavaScript injection (#731)

Issue:
It appears that on low endd evices (Core m3 MacBook), the attachment to
`DOMContentLoaded` happens _after_ the event has been raised, so does
not have a chance to inject the script.

Fix:
Move the attachment to the top of the file -- before the imports. This
triggers a bunch of linting erros, so also added disablement inplace.

Additional:
Clarified when the injected JS gets loaded, and what it can assume about
the DOM.
---
 app/src/components/mainWindow/mainWindow.js |  6 +++---
 app/src/static/preload.js                   | 23 ++++++++++++++-------
 docs/api.md                                 |  2 ++
 3 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/app/src/components/mainWindow/mainWindow.js b/app/src/components/mainWindow/mainWindow.js
index cd9a4ef..7ebd15e 100644
--- a/app/src/components/mainWindow/mainWindow.js
+++ b/app/src/components/mainWindow/mainWindow.js
@@ -55,10 +55,10 @@ function maybeInjectCss(browserWindow) {
 
   // on every page navigation inject the css
   browserWindow.webContents.on('did-navigate', () => {
-    // we have to inject the css in onHeadersReceived to prevent the fouc
-    // will run multiple times
+    // we have to inject the css in onHeadersReceived so they're early enough
+    // will run multiple times, so did-finish-load will remove this handler
     browserWindow.webContents.session.webRequest.onHeadersReceived(
-      null,
+      [], // Pass an empty filter list; null will not match _any_ urls
       onHeadersReceived,
     );
   });
diff --git a/app/src/static/preload.js b/app/src/static/preload.js
index f479193..c409a4a 100644
--- a/app/src/static/preload.js
+++ b/app/src/static/preload.js
@@ -1,9 +1,22 @@
 /**
  Preload file that will be executed in the renderer process
  */
-import { ipcRenderer } from 'electron';
-import path from 'path';
-import fs from 'fs';
+
+/**
+ * Note: This needs to be attached prior to the imports, as the they will delay
+ * the attachment till after the event has been raised.
+ */
+document.addEventListener('DOMContentLoaded', () => {
+  // Due to the early attachment, this triggers a linter error
+  // because it's not yet been defined.
+  // eslint-disable-next-line no-use-before-define
+  injectScripts();
+});
+
+// Disable imports being first due to the above event attachment
+import { ipcRenderer } from 'electron'; // eslint-disable-line import/first
+import path from 'path'; // eslint-disable-line import/first
+import fs from 'fs'; // eslint-disable-line import/first
 
 const INJECT_JS_PATH = path.join(__dirname, '../../', 'inject/inject.js');
 const log = require('loglevel');
@@ -49,10 +62,6 @@ function notifyNotificationClick() {
 
 setNotificationCallback(notifyNotificationCreate, notifyNotificationClick);
 
-document.addEventListener('DOMContentLoaded', () => {
-  injectScripts();
-});
-
 ipcRenderer.on('params', (event, message) => {
   const appArgs = JSON.parse(message);
   log.info('nativefier.json', appArgs);
diff --git a/docs/api.md b/docs/api.md
index 66e39d6..a0cb020 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -396,6 +396,8 @@ Forces the maximum disk space to be used by the disk cache. Value is given in by
 
 Allows you to inject a javascript or css file. This command can be run multiple times to inject the files.
 
+_Note:_ The javascript file is loaded _after_ `DOMContentLoaded`, so you can assume the DOM is complete & available.
+
 Example:
 
 ```bash