Expose Chrome version to todos webview (fix #1211) (#1478)

* Expose Chrome version to todos webview (fix #1211)

The TickTick todo service fails to load if the Chrome version number
does not appear in the User-Agent string. However, login to Google Tasks
is prevented by the same.

We adopt the "chromeless" User-Agent logic from the service webview,
which selectively exposes the Chrome version everywhere except the
Google login screen. The common logic was moved into the
userAgent-helpers module.

* Refactor user agent switching

* "Chromeless" user agent switching is extracted into a separate model
* Both the service and the todos webview uses the same model

8 files changed, 120 insertions, 51 deletions

diff --git a/src/features/todos/components/TodosWebview.js b/src/features/todos/components/TodosWebview.js
index 18a900e8a..634ec4caa 100644
--- a/src/features/todos/components/TodosWebview.js
+++ b/src/features/todos/components/TodosWebview.js
@@ -14,8 +14,6 @@ import Appear from '../../../components/ui/effects/Appear';
 import UpgradeButton from '../../../components/ui/UpgradeButton';
 import { TODOS_PARTITION_ID } from '..';
 
-import userAgent from '../../../helpers/userAgent-helpers';
-
 // NOTE: https://stackoverflow.com/questions/5717093/check-if-a-javascript-string-is-a-url
 function validURL(str) {
   let url;
@@ -112,6 +110,7 @@ class TodosWebview extends Component {
     resize: PropTypes.func.isRequired,
     width: PropTypes.number.isRequired,
     minWidth: PropTypes.number.isRequired,
+    userAgent: PropTypes.string.isRequired,
     isTodosIncludedInCurrentPlan: PropTypes.bool.isRequired,
     stores: PropTypes.shape({
       settings: PropTypes.instanceOf(SettingsStore).isRequired,
@@ -139,11 +138,6 @@ class TodosWebview extends Component {
     this.node.addEventListener('mousemove', this.resizePanel.bind(this));
     this.node.addEventListener('mouseup', this.stopResize.bind(this));
     this.node.addEventListener('mouseleave', this.stopResize.bind(this));
-
-    const webViewInstance = this;
-    this.webview.addEventListener('dom-ready', () => {
-      webViewInstance.webview.setUserAgent(userAgent(true));
-    });
   }
 
   startResize = (event) => {
@@ -214,6 +208,7 @@ class TodosWebview extends Component {
       classes,
       isTodosServiceActive,
       isVisible,
+      userAgent,
       isTodosIncludedInCurrentPlan,
       stores,
     } = this.props;
@@ -275,6 +270,7 @@ class TodosWebview extends Component {
             partition={TODOS_PARTITION_ID}
             preload="./features/todos/preload.js"
             ref={(webview) => { this.webview = webview ? webview.view : null; }}
+            useragent={userAgent}
             src={todoUrl}
           />
           )
diff --git a/src/features/todos/containers/TodosScreen.js b/src/features/todos/containers/TodosScreen.js
index 884925be6..631893f93 100644
--- a/src/features/todos/containers/TodosScreen.js
+++ b/src/features/todos/containers/TodosScreen.js
@@ -27,6 +27,7 @@ class TodosScreen extends Component {
           width={todosStore.width}
           minWidth={TODOS_MIN_WIDTH}
           resize={width => todoActions.resize({ width })}
+          userAgent={todosStore.userAgent}
           isTodosIncludedInCurrentPlan
         />
       </ErrorBoundary>
diff --git a/src/features/todos/store.js b/src/features/todos/store.js
index c1d6a9049..af8519d9c 100644
--- a/src/features/todos/store.js
+++ b/src/features/todos/store.js
@@ -16,6 +16,8 @@ import {
 import { IPC } from './constants';
 import { state as delayAppState } from '../delayApp';
 
+import UserAgent from '../../models/UserAgent';
+
 const debug = require('debug')('Ferdi:feature:todos:store');
 
 export default class TodoStore extends FeatureStore {
@@ -25,6 +27,8 @@ export default class TodoStore extends FeatureStore {
 
   @observable webview = null;
 
+  @observable userAgentModel = new UserAgent();
+
   isInitialized = false;
 
   @computed get width() {
@@ -51,6 +55,10 @@ export default class TodoStore extends FeatureStore {
     return localStorage.getItem('todos') || {};
   }
 
+  @computed get userAgent() {
+    return this.userAgentModel.userAgent;
+  }
+
   // ========== PUBLIC API ========= //
 
   @action start(stores, actions) {
@@ -123,7 +131,10 @@ export default class TodoStore extends FeatureStore {
 
   @action _setTodosWebview = ({ webview }) => {
     debug('_setTodosWebview', webview);
-    this.webview = webview;
+    if (this.webview !== webview) {
+      this.webview = webview;
+      this.userAgentModel.setWebviewReference(webview);
+    }
   };
 
   @action _handleHostMessage = (message) => {
diff --git a/src/helpers/userAgent-helpers.js b/src/helpers/userAgent-helpers.js
index c5eee0082..73c454304 100644
--- a/src/helpers/userAgent-helpers.js
+++ b/src/helpers/userAgent-helpers.js
@@ -19,6 +19,10 @@ function linux() {
   return 'X11; Ubuntu; Linux x86_64';
 }
 
+export function isChromeless(url) {
+  return url.startsWith('https://accounts.google.com');
+}
+
 export default function userAgent(removeChromeVersion = false, addFerdiVersion = false) {
   let platformString = '';
 
diff --git a/src/i18n/locales/defaultMessages.json b/src/i18n/locales/defaultMessages.json
index ef25aa3d9..c44357396 100644
--- a/src/i18n/locales/defaultMessages.json
+++ b/src/i18n/locales/defaultMessages.json
@@ -6412,39 +6412,39 @@
         "defaultMessage": "!!!Franz Todos are available to premium users now!",
         "end": {
           "column": 3,
-          "line": 36
+          "line": 34
         },
         "file": "src/features/todos/components/TodosWebview.js",
         "id": "feature.todos.premium.info",
         "start": {
           "column": 15,
-          "line": 33
+          "line": 31
         }
       },
       {
         "defaultMessage": "!!!Upgrade Account",
         "end": {
           "column": 3,
-          "line": 40
+          "line": 38
         },
         "file": "src/features/todos/components/TodosWebview.js",
         "id": "feature.todos.premium.upgrade",
         "start": {
           "column": 14,
-          "line": 37
+          "line": 35
         }
       },
       {
         "defaultMessage": "!!!Everyone else will have to wait a little longer.",
         "end": {
           "column": 3,
-          "line": 44
+          "line": 42
         },
         "file": "src/features/todos/components/TodosWebview.js",
         "id": "feature.todos.premium.rollout",
         "start": {
           "column": 15,
-          "line": 41
+          "line": 39
         }
       }
     ],
diff --git a/src/i18n/messages/src/features/todos/components/TodosWebview.json b/src/i18n/messages/src/features/todos/components/TodosWebview.json
index 0652b0a33..ff6e037fc 100644
--- a/src/i18n/messages/src/features/todos/components/TodosWebview.json
+++ b/src/i18n/messages/src/features/todos/components/TodosWebview.json
@@ -4,11 +4,11 @@
     "defaultMessage": "!!!Franz Todos are available to premium users now!",
     "file": "src/features/todos/components/TodosWebview.js",
     "start": {
-      "line": 33,
+      "line": 31,
       "column": 15
     },
     "end": {
-      "line": 36,
+      "line": 34,
       "column": 3
     }
   },
@@ -17,11 +17,11 @@
     "defaultMessage": "!!!Upgrade Account",
     "file": "src/features/todos/components/TodosWebview.js",
     "start": {
-      "line": 37,
+      "line": 35,
       "column": 14
     },
     "end": {
-      "line": 40,
+      "line": 38,
       "column": 3
     }
   },
@@ -30,11 +30,11 @@
     "defaultMessage": "!!!Everyone else will have to wait a little longer.",
     "file": "src/features/todos/components/TodosWebview.js",
     "start": {
-      "line": 41,
+      "line": 39,
       "column": 15
     },
     "end": {
-      "line": 44,
+      "line": 42,
       "column": 3
     }
   }
diff --git a/src/models/Service.js b/src/models/Service.js
index b01881beb..0d1dff431 100644
--- a/src/models/Service.js
+++ b/src/models/Service.js
@@ -4,9 +4,9 @@ import { webContents } from '@electron/remote';
 import normalizeUrl from 'normalize-url';
 import path from 'path';
 
-import userAgent from '../helpers/userAgent-helpers';
 import { TODOS_RECIPE_ID, todosStore } from '../features/todos';
 import { isValidExternalURL } from '../helpers/url-helpers';
+import UserAgent from './UserAgent';
 
 const debug = require('debug')('Ferdi:Service');
 
@@ -94,7 +94,7 @@ export default class Service {
 
   @observable lostRecipeReloadAttempt = 0;
 
-  @observable chromelessUserAgent = false;
+  @observable userAgentModel = null;
 
   constructor(data, recipe) {
     if (!data) {
@@ -156,6 +156,8 @@ export default class Service {
       this.isHibernating = true;
     }
 
+    this.userAgentModel = new UserAgent(recipe.overrideUserAgent);
+
     autorun(() => {
       if (!this.isEnabled) {
         this.webview = null;
@@ -234,12 +236,7 @@ export default class Service {
   }
 
   @computed get userAgent() {
-    let ua = userAgent(this.chromelessUserAgent);
-    if (typeof this.recipe.overrideUserAgent === 'function') {
-      ua = this.recipe.overrideUserAgent();
-    }
-
-    return ua;
+    return this.userAgentModel.userAgent;
   }
 
   @computed get partition() {
@@ -250,6 +247,8 @@ export default class Service {
   initializeWebViewEvents({ handleIPCMessage, openWindow, stores }) {
     const webviewWebContents = webContents.fromId(this.webview.getWebContentsId());
 
+    this.userAgentModel.setWebviewReference(this.webview);
+
     // If the recipe has implemented modifyRequestHeaders,
     // Send those headers to ipcMain so that it can be set in session
     if (typeof this.recipe.modifyRequestHeaders === 'function') {
@@ -263,23 +262,6 @@ export default class Service {
       debug(this.name, 'modifyRequestHeaders is not defined in the recipe');
     }
 
-    const handleUserAgent = (url, forwardingHack = false) => {
-      if (url.startsWith('https://accounts.google.com')) {
-        if (!this.chromelessUserAgent) {
-          debug('Setting user agent to chromeless for url', url);
-          this.webview.setUserAgent(userAgent(true));
-          if (forwardingHack) {
-            this.webview.loadURL(url);
-          }
-          this.chromelessUserAgent = true;
-        }
-      } else if (this.chromelessUserAgent) {
-        debug('Setting user agent to contain chrome');
-        this.webview.setUserAgent(this.userAgent);
-        this.chromelessUserAgent = false;
-      }
-    };
-
     this.webview.addEventListener('ipc-message', e => handleIPCMessage({
       serviceId: this.id,
       channel: e.channel,
@@ -306,8 +288,6 @@ export default class Service {
       }
     });
 
-    this.webview.addEventListener('will-navigate', event => handleUserAgent(event.url, true));
-
     this.webview.addEventListener('did-start-loading', (event) => {
       debug('Did start load', this.name, event);
 
@@ -325,10 +305,7 @@ export default class Service {
     };
 
     this.webview.addEventListener('did-frame-finish-load', didLoad.bind(this));
-    this.webview.addEventListener('did-navigate', (event) => {
-      handleUserAgent(event.url);
-      didLoad();
-    });
+    this.webview.addEventListener('did-navigate', didLoad.bind(this));
 
     this.webview.addEventListener('did-fail-load', (event) => {
       debug('Service failed to load', this.name, event);
diff --git a/src/models/UserAgent.js b/src/models/UserAgent.js
new file mode 100644
index 000000000..f51f2e5a6
--- /dev/null
+++ b/src/models/UserAgent.js
@@ -0,0 +1,80 @@
+import {
+  action,
+  computed,
+  observe,
+  observable,
+} from 'mobx';
+
+import defaultUserAgent, { isChromeless } from '../helpers/userAgent-helpers';
+
+const debug = require('debug')('Ferdi:UserAgent');
+
+export default class UserAgent {
+  _willNavigateListener = null;
+
+  _didNavigateListener = null;
+
+  @observable.ref webview = null;
+
+  @observable chromelessUserAgent = false;
+
+  @observable getUserAgent = defaultUserAgent;
+
+  constructor(overrideUserAgent = null) {
+    if (typeof overrideUserAgent === 'function') {
+      this.getUserAgent = overrideUserAgent;
+    }
+
+    observe(this, 'webview', (change) => {
+      const { oldValue, newValue } = change;
+      if (oldValue !== null) {
+        this._removeWebviewEvents(oldValue);
+      }
+      if (newValue !== null) {
+        this._addWebviewEvents(newValue);
+      }
+    });
+  }
+
+  @computed get userAgent() {
+    return this.chromelessUserAgent ? defaultUserAgent(true) : this.getUserAgent();
+  }
+
+  @action setWebviewReference(webview) {
+    this.webview = webview;
+  }
+
+  @action _handleNavigate(url, forwardingHack = false) {
+    if (isChromeless(url)) {
+      if (!this.chromelessUserAgent) {
+        debug('Setting user agent to chromeless for url', url);
+        this.chromelessUserAgent = true;
+        this.webview.userAgent = this.userAgent;
+        if (forwardingHack) {
+          this.webview.loadURL(url);
+        }
+      }
+    } else if (this.chromelessUserAgent) {
+      debug('Setting user agent to contain chrome for url', url);
+      this.chromelessUserAgent = false;
+      this.webview.userAgent = this.userAgent;
+    }
+  }
+
+  _addWebviewEvents(webview) {
+    debug('Adding event handlers');
+
+    this._willNavigateListener = event => this._handleNavigate(event.url, true);
+    webview.addEventListener('will-navigate', this._willNavigateListener);
+
+    this._didNavigateListener = event => this._handleNavigate(event.url);
+    webview.addEventListener('did-navigate', this._didNavigateListener);
+  }
+
+  _removeWebviewEvents(webview) {
+    debug('Removing event handlers');
+
+    webview.removeEventListener('will-navigate', this._willNavigateListener);
+    webview.removeEventListener('did-navigate', this._didNavigateListener);
+  }
+}