Use Native Notifications on Windows 7 (#2330)

*   [x] Test notifications on Windows 7.
*   [x] Switch to Electron native notifications on Window 7.
*   [x] Disable **Play audio notification** setting on Windows 7 since they are
        not natively supported.
*   [x] Improve logging for notification status.
*   [x] Investigate whether Electron notification support choosing custom sound
        on Windows. Answer: no.
        Source: 82329124ff/docs/api/notification.md (new-notificationoptions-experimental)
*   [x] Remove `node-notifier`.
*   [x] **Infrastructure:** Port `OS` and `types/Settings` to TypeScript.
*   [x] Add support for specifying minimum Windows version with
        `OS.isWindows(minVersion?: string)`.
*   [x] OT: While testing on Windows 7, I confirmed spell checking worked
        for me.

.eslintignore

@@ -36,6 +36,7 @@ ts/**/*.js
 !js/logging.js
 !js/models/conversations.js
 !js/models/messages.js
+!js/notifications.js
 !js/views/attachment_view.js
 !js/views/backbone_wrapper_view.js
 !js/views/conversation_search_view.js

js/database.js

@@ -1,6 +1,7 @@
-/* global Whisper: false */
-/* global Backbone: false */
 /* global _: false */
+/* global Backbone: false */
+
+/* global Whisper: false */
 
 // eslint-disable-next-line func-names
 (function() {

js/modules/os.js

@@ -1,7 +0,0 @@
-/* eslint-env node */
-
-exports.isMacOS = () => process.platform === 'darwin';
-
-exports.isLinux = () => process.platform === 'linux';
-
-exports.isWindows = () => process.platform === 'win32';

js/modules/types/settings.js

@@ -1,3 +0,0 @@
-const OS = require('../os');
-
-exports.isAudioNotificationSupported = () => !OS.isLinux();

js/notifications.js

@@ -1,9 +1,21 @@
+/* global Backbone: false */
+
+/* global ConversationController: false */
+/* global drawAttention: false */
+/* global i18n: false */
+/* global isFocused: false */
+/* global Signal: false */
+/* global storage: false */
+/* global Whisper: false */
+
+// eslint-disable-next-line func-names
 (function() {
   'use strict';
-  window.Whisper = window.Whisper || {};
-  const { Settings } = window.Signal.Types;
 
-  var SETTINGS = {
+  window.Whisper = window.Whisper || {};
+  const { Settings } = Signal.Types;
+
+  const SettingNames = {
     OFF: 'off',
     COUNT: 'count',
     NAME: 'name',
@@ -11,78 +23,68 @@
   };
 
   Whisper.Notifications = new (Backbone.Collection.extend({
-    initialize: function() {
+    initialize() {
       this.isEnabled = false;
       this.on('add', this.update);
       this.on('remove', this.onRemove);
     },
-    onClick: function(conversationId) {
+    onClick(conversationId) {
-      var conversation = ConversationController.get(conversationId);
+      const conversation = ConversationController.get(conversationId);
       this.trigger('click', conversation);
     },
-    update: function() {
+    update() {
       const { isEnabled } = this;
-      const isFocused = window.isFocused();
+      const isAppFocused = isFocused();
       const isAudioNotificationEnabled =
         storage.get('audio-notification') || false;
       const isAudioNotificationSupported = Settings.isAudioNotificationSupported();
-      const shouldPlayNotificationSound =
-        isAudioNotificationSupported && isAudioNotificationEnabled;
       const numNotifications = this.length;
-      console.log('Update notifications:', {
+      const userSetting = this.getUserSetting();
-        isFocused,
+
+      const status = Signal.Notifications.getStatus({
+        isAppFocused,
+        isAudioNotificationEnabled,
+        isAudioNotificationSupported,
         isEnabled,
         numNotifications,
-        shouldPlayNotificationSound,
+        userSetting,
       });
 
-      if (!isEnabled) {
+      console.log('Update notifications:', status);
+
+      if (status.type !== 'ok') {
+        if (status.shouldClearNotifications) {
+          this.clear();
+        }
+
         return;
       }
 
-      const hasNotifications = numNotifications > 0;
+      let title;
-      if (!hasNotifications) {
+      let message;
-        return;
+      let iconUrl;
-      }
-
-      const isNotificationOmitted = isFocused;
-      if (isNotificationOmitted) {
-        this.clear();
-        return;
-      }
-
-      var setting = storage.get('notification-setting') || 'message';
-      if (setting === SETTINGS.OFF) {
-        return;
-      }
-
-      window.drawAttention();
-
-      var title;
-      var message;
-      var iconUrl;
 
       // NOTE: i18n has more complex rules for pluralization than just
       // distinguishing between zero (0) and other (non-zero),
       // e.g. Russian:
       // http://docs.translatehouse.org/projects/localization-guide/en/latest/l10n/pluralforms.html
-      var newMessageCount = [
+      const newMessageCount = [
         numNotifications,
         numNotifications === 1 ? i18n('newMessage') : i18n('newMessages'),
       ].join(' ');
 
-      var last = this.last();
+      const last = this.last();
-      switch (this.getSetting()) {
+      switch (userSetting) {
-        case SETTINGS.COUNT:
+        case SettingNames.COUNT:
           title = 'Signal';
           message = newMessageCount;
           break;
-        case SETTINGS.NAME:
+        case SettingNames.NAME:
           title = newMessageCount;
-          message = 'Most recent from ' + last.get('title');
+          message = `Most recent from ${last.get('title')}`;
           iconUrl = last.get('iconUrl');
           break;
-        case SETTINGS.MESSAGE:
+        case SettingNames.MESSAGE:
           if (numNotifications === 1) {
             title = last.get('title');
           } else {
@@ -91,52 +93,43 @@
           message = last.get('message');
           iconUrl = last.get('iconUrl');
           break;
+        default:
+          console.log(`Error: Unknown user setting: '${userSetting}'`);
+          break;
       }
 
-      if (window.config.polyfillNotifications) {
+      drawAttention();
-        window.nodeNotifier.notify({
-          title: title,
-          message: message,
-          sound: false,
-        });
-        window.nodeNotifier.on('click', function(notifierObject, options) {
-          last.get('conversationId');
-        });
-      } else {
-        var notification = new Notification(title, {
-          body: message,
-          icon: iconUrl,
-          tag: 'signal',
-          silent: !shouldPlayNotificationSound,
-        });
 
-        notification.onclick = this.onClick.bind(
+      const notification = new Notification(title, {
-          this,
+        body: message,
-          last.get('conversationId')
+        icon: iconUrl,
-        );
+        tag: 'signal',
-      }
+        silent: !status.shouldPlayNotificationSound,
+      });
+
+      notification.onclick = () => this.onClick(last.get('conversationId'));
 
       // We don't want to notify the user about these same messages again
       this.clear();
     },
-    getSetting: function() {
+    getUserSetting() {
-      return storage.get('notification-setting') || SETTINGS.MESSAGE;
+      return storage.get('notification-setting') || SettingNames.MESSAGE;
     },
-    onRemove: function() {
+    onRemove() {
-      console.log('remove notification');
+      console.log('Remove notification');
     },
-    clear: function() {
+    clear() {
-      console.log('remove all notifications');
+      console.log('Remove all notifications');
       this.reset([]);
     },
-    enable: function() {
+    enable() {
       const needUpdate = !this.isEnabled;
       this.isEnabled = true;
       if (needUpdate) {
         this.update();
       }
     },
-    disable: function() {
+    disable() {
       this.isEnabled = false;
     },
   }))();

main.js

@@ -4,7 +4,6 @@ const os = require('os');
 
 const _ = require('lodash');
 const electron = require('electron');
-const semver = require('semver');
 
 const { BrowserWindow, app, Menu, shell, ipcMain: ipc } = electron;
 
@@ -99,17 +98,6 @@ const loadLocale = require('./app/locale').load;
 let logger;
 let locale;
 
-const WINDOWS_8 = '8.0.0';
-const osRelease = os.release();
-const polyfillNotifications =
-  os.platform() === 'win32' && semver.lt(osRelease, WINDOWS_8);
-console.log(
-  'OS Release:',
-  osRelease,
-  '- notifications polyfill?',
-  polyfillNotifications
-);
-
 function prepareURL(pathSegments) {
   return url.format({
     pathname: path.join.apply(null, pathSegments),
@@ -127,7 +115,6 @@ function prepareURL(pathSegments) {
       node_version: process.versions.node,
       hostname: os.hostname(),
       appInstance: process.env.NODE_APP_INSTANCE,
-      polyfillNotifications: polyfillNotifications ? true : undefined, // for stringify()
       proxyUrl: process.env.HTTPS_PROXY || process.env.https_proxy,
       importMode: importMode ? true : undefined, // for stringify()
     },

package.json

@@ -67,7 +67,6 @@
     "mkdirp": "^0.5.1",
     "moment": "^2.21.0",
     "node-fetch": "https://github.com/scottnonnenberg/node-fetch.git#3e5f51e08c647ee5f20c43b15cf2d352d61c36b4",
-    "node-notifier": "^5.1.2",
     "os-locale": "^2.1.0",
     "pify": "^3.0.0",
     "proxy-agent": "^2.1.0",
@@ -92,6 +91,8 @@
     "@types/qs": "^6.5.1",
     "@types/react": "^16.3.1",
     "@types/react-dom": "^16.0.4",
+    "@types/semver": "^5.5.0",
+    "@types/sinon": "^4.3.1",
     "arraybuffer-loader": "^1.0.3",
     "asar": "^0.14.0",
     "bower": "^1.8.2",

preload.js

@@ -101,7 +101,6 @@ window.loadImage = require('blueimp-load-image');
 
 window.nodeBuffer = Buffer;
 window.nodeFetch = require('node-fetch');
-window.nodeNotifier = require('node-notifier');
 window.ProxyAgent = require('proxy-agent');
 
 // Note: when modifying this file, consider whether our React Components or Backbone Views
@@ -200,7 +199,8 @@ window.Signal.Migrations.Migrations0DatabaseWithAttachmentData = require('./js/m
 window.Signal.Migrations.Migrations1DatabaseWithoutAttachmentData = require('./js/modules/migrations/migrations_1_database_without_attachment_data');
 
 window.Signal.Migrations.upgradeMessageSchema = upgradeMessageSchema;
-window.Signal.OS = require('./js/modules/os');
+window.Signal.Notifications = require('./ts/notifications');
+window.Signal.OS = require('./ts/OS');
 window.Signal.Settings = require('./js/modules/settings');
 window.Signal.Startup = require('./js/modules/startup');
 
@@ -211,7 +211,7 @@ window.Signal.Types.Errors = require('./js/modules/types/errors');
 
 window.Signal.Types.Message = Message;
 window.Signal.Types.MIME = require('./ts/types/MIME');
-window.Signal.Types.Settings = require('./js/modules/types/settings');
+window.Signal.Types.Settings = require('./ts/types/Settings');
 window.Signal.Util = require('./ts/util');
 
 window.Signal.Views = {};

test/modules/types/settings_test.js

@@ -1,52 +0,0 @@
-const sinon = require('sinon');
-const { assert } = require('chai');
-
-const Settings = require('../../../js/modules/types/settings');
-
-describe('Settings', () => {
-  const sandbox = sinon.createSandbox();
-
-  describe('isAudioNotificationSupported', () => {
-    context('on macOS', () => {
-      beforeEach(() => {
-        sandbox.stub(process, 'platform').value('darwin');
-      });
-
-      afterEach(() => {
-        sandbox.restore();
-      });
-
-      it('should return true', () => {
-        assert.isTrue(Settings.isAudioNotificationSupported());
-      });
-    });
-
-    context('on Windows', () => {
-      beforeEach(() => {
-        sandbox.stub(process, 'platform').value('win32');
-      });
-
-      afterEach(() => {
-        sandbox.restore();
-      });
-
-      it('should return true', () => {
-        assert.isTrue(Settings.isAudioNotificationSupported());
-      });
-    });
-
-    context('on Linux', () => {
-      beforeEach(() => {
-        sandbox.stub(process, 'platform').value('linux');
-      });
-
-      afterEach(() => {
-        sandbox.restore();
-      });
-
-      it('should return false', () => {
-        assert.isFalse(Settings.isAudioNotificationSupported());
-      });
-    });
-  });
-});

ts/OS.ts

@@ -0,0 +1,15 @@
+import is from '@sindresorhus/is';
+import os from 'os';
+import semver from 'semver';
+
+export const isMacOS = () => process.platform === 'darwin';
+export const isLinux = () => process.platform === 'linux';
+export const isWindows = (minVersion?: string) => {
+  const isPlatformValid = process.platform === 'win32';
+  const osRelease = os.release();
+  const isVersionValid = is.undefined(minVersion)
+    ? true
+    : semver.gte(osRelease, minVersion);
+
+  return isPlatformValid && isVersionValid;
+};

ts/notifications/getStatus.ts

@@ -0,0 +1,66 @@
+interface Environment {
+  isAppFocused: boolean;
+  isAudioNotificationEnabled: boolean;
+  isAudioNotificationSupported: boolean;
+  isEnabled: boolean;
+  numNotifications: number;
+  userSetting: UserSetting;
+}
+
+interface Status {
+  shouldClearNotifications: boolean;
+  shouldPlayNotificationSound: boolean;
+  shouldShowNotifications: boolean;
+  type: Type;
+}
+
+type UserSetting = 'off' | 'count' | 'name' | 'message';
+
+type Type =
+  | 'ok'
+  | 'disabled'
+  | 'appIsFocused'
+  | 'noNotifications'
+  | 'userSetting';
+
+export const getStatus = ({
+  isAppFocused,
+  isAudioNotificationEnabled,
+  isAudioNotificationSupported,
+  isEnabled,
+  numNotifications,
+  userSetting,
+}: Environment): Status => {
+  const type = ((): Type => {
+    if (!isEnabled) {
+      return 'disabled';
+    }
+
+    const hasNotifications = numNotifications > 0;
+    if (!hasNotifications) {
+      return 'noNotifications';
+    }
+
+    if (isAppFocused) {
+      return 'appIsFocused';
+    }
+
+    if (userSetting === 'off') {
+      return 'userSetting';
+    }
+
+    return 'ok';
+  })();
+
+  const shouldPlayNotificationSound =
+    isAudioNotificationSupported && isAudioNotificationEnabled;
+  const shouldShowNotifications = type === 'ok';
+  const shouldClearNotifications = type === 'appIsFocused';
+
+  return {
+    shouldClearNotifications,
+    shouldPlayNotificationSound,
+    shouldShowNotifications,
+    type,
+  };
+};

ts/notifications/index.ts

@@ -0,0 +1,3 @@
+import { getStatus } from './getStatus';
+
+export { getStatus };

ts/test/types/Settings_test.ts

@@ -0,0 +1,71 @@
+import os from 'os';
+import sinon from 'sinon';
+import { assert } from 'chai';
+
+import * as Settings from '../../../ts/types/Settings';
+
+describe('Settings', () => {
+  const sandbox = sinon.createSandbox();
+
+  describe('isAudioNotificationSupported', () => {
+    context('on macOS', () => {
+      beforeEach(() => {
+        sandbox.stub(process, 'platform').value('darwin');
+      });
+
+      afterEach(() => {
+        sandbox.restore();
+      });
+
+      it('should return true', () => {
+        assert.isTrue(Settings.isAudioNotificationSupported());
+      });
+    });
+
+    context('on Windows', () => {
+      context('version 7', () => {
+        beforeEach(() => {
+          sandbox.stub(process, 'platform').value('win32');
+          sandbox.stub(os, 'release').returns('7.0.0');
+        });
+
+        afterEach(() => {
+          sandbox.restore();
+        });
+
+        it('should return false', () => {
+          assert.isFalse(Settings.isAudioNotificationSupported());
+        });
+      });
+
+      context('version 8+', () => {
+        beforeEach(() => {
+          sandbox.stub(process, 'platform').value('win32');
+          sandbox.stub(os, 'release').returns('8.0.0');
+        });
+
+        afterEach(() => {
+          sandbox.restore();
+        });
+
+        it('should return true', () => {
+          assert.isTrue(Settings.isAudioNotificationSupported());
+        });
+      });
+    });
+
+    context('on Linux', () => {
+      beforeEach(() => {
+        sandbox.stub(process, 'platform').value('linux');
+      });
+
+      afterEach(() => {
+        sandbox.restore();
+      });
+
+      it('should return false', () => {
+        assert.isFalse(Settings.isAudioNotificationSupported());
+      });
+    });
+  });
+});

ts/types/Settings.ts

@@ -0,0 +1,6 @@
+import * as OS from '../OS';
+
+const MIN_WINDOWS_VERSION = '8.0.0';
+
+export const isAudioNotificationSupported = () =>
+  OS.isWindows(MIN_WINDOWS_VERSION) || OS.isMacOS();

yarn.lock

@@ -83,6 +83,14 @@
   version "16.3.1"
   resolved "https://registry.yarnpkg.com/@types/react/-/react-16.3.1.tgz#6f6aaffaf7dba502ff5ca15e4aa18caee9b04995"
 
+"@types/semver@^5.5.0":
+  version "5.5.0"
+  resolved "https://registry.yarnpkg.com/@types/semver/-/semver-5.5.0.tgz#146c2a29ee7d3bae4bf2fcb274636e264c813c45"
+
+"@types/sinon@^4.3.1":
+  version "4.3.1"
+  resolved "https://registry.yarnpkg.com/@types/sinon/-/sinon-4.3.1.tgz#32458f9b166cd44c23844eee4937814276f35199"
+
 "@vxna/mini-html-webpack-template@^0.1.6":
   version "0.1.6"
   resolved "https://registry.yarnpkg.com/@vxna/mini-html-webpack-template/-/mini-html-webpack-template-0.1.6.tgz#64225d564da5fe610b6445523c245572923c00b8"
@@ -3789,10 +3797,6 @@ growl@1.10.3:
   version "1.10.3"
   resolved "https://registry.yarnpkg.com/growl/-/growl-1.10.3.tgz#1926ba90cf3edfe2adb4927f5880bc22c66c790f"
 
-growly@^1.3.0:
-  version "1.3.0"
-  resolved "https://registry.yarnpkg.com/growly/-/growly-1.3.0.tgz#f10748cbe76af964b7c96c93c6bcc28af120c081"
-
 grunt-cli@^1.2.0, grunt-cli@~1.2.0:
   version "1.2.0"
   resolved "https://registry.yarnpkg.com/grunt-cli/-/grunt-cli-1.2.0.tgz#562b119ebb069ddb464ace2845501be97b35b6a8"
@@ -5882,15 +5886,6 @@ node-libs-browser@^2.0.0:
     util "^0.10.3"
     vm-browserify "0.0.4"
 
-node-notifier@^5.1.2:
-  version "5.1.2"
-  resolved "https://registry.yarnpkg.com/node-notifier/-/node-notifier-5.1.2.tgz#2fa9e12605fa10009d44549d6fcd8a63dde0e4ff"
-  dependencies:
-    growly "^1.3.0"
-    semver "^5.3.0"
-    shellwords "^0.1.0"
-    which "^1.2.12"
-
 node-pre-gyp@^0.6.39:
   version "0.6.39"
   resolved "https://registry.yarnpkg.com/node-pre-gyp/-/node-pre-gyp-0.6.39.tgz#c00e96860b23c0e1420ac7befc5044e1d78d8649"
@@ -7997,10 +7992,6 @@ shelljs@0.3.x:
   version "0.3.0"
   resolved "https://registry.yarnpkg.com/shelljs/-/shelljs-0.3.0.tgz#3596e6307a781544f591f37da618360f31db57b1"
 
-shellwords@^0.1.0:
-  version "0.1.1"
-  resolved "https://registry.yarnpkg.com/shellwords/-/shellwords-0.1.1.tgz#d6b9181c1a48d397324c84871efbcfc73fc0654b"
-
 signal-exit@^3.0.0, signal-exit@^3.0.1, signal-exit@^3.0.2:
   version "3.0.2"
   resolved "https://registry.yarnpkg.com/signal-exit/-/signal-exit-3.0.2.tgz#b5fdc08f1287ea1178628e415e25132b73646c6d"
@@ -9507,7 +9498,7 @@ which@1, which@^1.2.9, which@~1.2.1:
   dependencies:
     isexe "^2.0.0"
 
-which@^1.2.10, which@^1.2.12, which@^1.2.14, which@^1.3.0:
+which@^1.2.10, which@^1.2.14, which@^1.3.0:
   version "1.3.0"
   resolved "https://registry.yarnpkg.com/which/-/which-1.3.0.tgz#ff04bdfc010ee547d780bec38e1ac1c2777d253a"
   dependencies: