Various fixes for IE11 (#2021)

* Revert escape-string-regexp  back to 1.0.5

This broke IE 11. See issue https://github.com/sindresorhus/escape-string-regexp/issues/18Signed-off-by: Kyle Keating <kkeating@hypothes.is>

* Fix es2018 polyfill so it does not break IE11

Promise is undefined in IE11.
Signed-off-by: Kyle Keating <kkeating@hypothes.is>

* Add user-agent shared util

Adds 2 helper methods:
- isIE11
- isMacOS
Signed-off-by: Kyle Keating <kkeating@hypothes.is>

* Fix IE11 keyboard events

- IE11 uses special names for arrow keys that are different than the standard key names.
- Fix handleOnInput in tag-editor for IE11
- use isMacOS in markdown-editor

* Add renderer-options.js module

This module holds any renderer options to globally fix browser quirks or any other unique customizations we wish to add.

The dir=“auto” causes an exception in IE11. ie11DomReassignments() will simply replace that attribute’s value with an empty value so it does not break.

* Add browser-compatiblity-utils module

Current only holds one method used to translate key names used by IE11 to standardized names that all components can assume. This removes the need for special cases in each component when listening to keyboard events.

* missing code cov

package.json

@@ -45,7 +45,7 @@
     "enzyme": "^3.8.0",
     "enzyme-adapter-preact-pure": "^2.0.0",
     "escape-html": "^1.0.3",
-    "escape-string-regexp": "^3.0.0",
+    "escape-string-regexp": "^1.0.5",
     "eslint": "^6.0.1",
     "eslint-config-hypothesis": "^2.0.0",
     "eslint-plugin-jsx-a11y": "^6.2.3",

src/shared/browser-compatibility-utils.js

+/**
+ * Normalize a keyboard event key name.
+ *
+ * Several old browsers, such as IE11, use alternate key
+ * names for keyboard events. If any abnormal keys are used,
+ * this method returns the normalized name so our UI
+ * components don't require a special case.
+ *
+ * @param {string} key - The keyboard event `key` name
+ * @return {string} - Normalized `key` name
+ */
+export function normalizeKeyName(key) {
+  const mappings = {
+    Left: 'ArrowLeft',
+    Up: 'ArrowUp',
+    Down: 'ArrowDown',
+    Right: 'ArrowRight',
+    Spacebar: ' ',
+    Del: 'Delete',
+  };
+  return mappings[key] ? mappings[key] : key;
+}

src/shared/polyfills/index.js

@@ -58,6 +58,10 @@ const needsPolyfill = {
   },
 
   es2018: () => {
+    if (!window.Promise) {
+      // IE11 does not have a Promise object.
+      return true;
+    }
     return !hasMethods(Promise.prototype, 'finally');
   },
 

src/shared/polyfills/test/index-test.js

@@ -40,6 +40,10 @@ describe('shared/polyfills/index', () => {
         set: 'es2018',
         providesMethod: [Promise.prototype, 'finally'],
       },
+      {
+        set: 'es2018',
+        providesMethod: [window, 'Promise'],
+      },
       {
         set: 'string.prototype.normalize',
         providesMethod: [String.prototype, 'normalize'],

src/shared/renderer-options.js

+import { options } from 'preact';
+
+import { isIE11 } from './user-agent';
+
+/**
+ * Force the dir="auto" attribute to be dir="" as this otherwise causes
+ * an exception in IE11 and breaks subsequent rendering.
+ *
+ * @param {Object} _options - Test seam
+ */
+export function setupIE11Fixes(_options = options) {
+  if (isIE11()) {
+    const prevHook = _options.vnode;
+    _options.vnode = vnode => {
+      if (typeof vnode.type === 'string') {
+        if ('dir' in vnode.props && vnode.props.dir === 'auto') {
+          // Re-assign `vnode.props.dir` if its value is "auto"
+          vnode.props.dir = '';
+        }
+      }
+      // Call previously defined hook if there was any
+      if (prevHook) {
+        prevHook(vnode);
+      }
+    };
+  }
+}

src/shared/test/browser-compatiblity-utils-test.js

+import { normalizeKeyName } from '../browser-compatibility-utils';
+
+describe('shared/browser-compatibility-utils', () => {
+  describe('normalizeKeyName', () => {
+    [
+      {
+        from: 'Left',
+        to: 'ArrowLeft',
+      },
+      {
+        from: 'Up',
+        to: 'ArrowUp',
+      },
+      {
+        from: 'Down',
+        to: 'ArrowDown',
+      },
+      {
+        from: 'Right',
+        to: 'ArrowRight',
+      },
+      {
+        from: 'Spacebar',
+        to: ' ',
+      },
+      {
+        from: 'Del',
+        to: 'Delete',
+      },
+    ].forEach(test => {
+      it(`changes the key value '${test.from}' to '${test.to}'`, () => {
+        assert.equal(normalizeKeyName(test.from), test.to);
+      });
+    });
+  });
+});

src/shared/test/renderer-options-test.js

+import { createElement } from 'preact';
+
+import { setupIE11Fixes } from '../renderer-options';
+import { $imports } from '../renderer-options';
+
+describe('shared/renderer-options', () => {
+  let fakeIsIE11;
+
+  beforeEach(() => {
+    fakeIsIE11 = sinon.stub().returns(true);
+    $imports.$mock({
+      './user-agent': {
+        isIE11: fakeIsIE11,
+      },
+    });
+  });
+
+  afterEach(() => {
+    $imports.$restore();
+  });
+
+  describe('setupIE11Fixes', () => {
+    let fakeOptions;
+    let prevHook;
+
+    beforeEach(() => {
+      prevHook = sinon.stub();
+      fakeOptions = {
+        vnode: undefined,
+      };
+    });
+
+    context('when isIE11 is false', () => {
+      it('does not set a new vnode option if isIE11 is false', () => {
+        fakeIsIE11.returns(false);
+        setupIE11Fixes(fakeOptions);
+        assert.isNotOk(fakeOptions.vnode);
+      });
+    });
+
+    context('when isIE11 is true', () => {
+      it('sets a new vnode option', () => {
+        setupIE11Fixes(fakeOptions);
+        assert.isOk(fakeOptions.vnode);
+      });
+
+      it('does not override an existing option if one exists', () => {
+        fakeOptions.vnode = prevHook;
+        setupIE11Fixes(fakeOptions);
+        fakeOptions.vnode({});
+        assert.called(prevHook);
+      });
+
+      it("alters the `dir` attribute when its equal to 'auto'", () => {
+        setupIE11Fixes(fakeOptions);
+        const vDiv = createElement('div', { dir: 'auto' }, 'text');
+        fakeOptions.vnode(vDiv);
+        assert.equal(vDiv.props.dir, '');
+      });
+
+      it('does not alter the `dir` attribute when vnode.type is not a string', () => {
+        setupIE11Fixes(fakeOptions);
+        const vDiv = createElement('div', { dir: 'auto' }, 'text');
+        vDiv.type = () => {}; // force it to be a function
+        fakeOptions.vnode(vDiv);
+        assert.equal(vDiv.props.dir, 'auto');
+      });
+
+      it("does not alter the `dir` attribute when its value is not 'auto'", () => {
+        setupIE11Fixes(fakeOptions);
+        const vDiv = createElement('function', { dir: 'ltr' }, 'text');
+        fakeOptions.vnode(vDiv);
+        assert.equal(vDiv.props.dir, 'ltr');
+      });
+    });
+  });
+});

src/shared/test/user-agent-test.js

+import { isIE11, isMacOS } from '../user-agent';
+
+describe('shared/user-agent', () => {
+  describe('isIE11', () => {
+    it('returns true when the user agent is IE 11', () => {
+      assert.isTrue(
+        isIE11('Mozilla/5.0 (Windows NT 10.0; Trident/7.0; rv:11.0) like Gecko')
+      );
+    });
+
+    it('returns false when the user agent is not IE 11', () => {
+      assert.isFalse(
+        isIE11(
+          'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.92 Safari/537.36 Edg/80.0.361.109'
+        )
+      );
+    });
+  });
+
+  describe('isMacOS', () => {
+    it('returns true when the user agent is a Mac', () => {
+      assert.isTrue(
+        isMacOS(
+          'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.163 Safari/537.36'
+        )
+      );
+    });
+
+    it('returns false when the user agent is not a Mac', () => {
+      assert.isFalse(
+        isMacOS(
+          'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.92 Safari/537.36 Edg/80.0.361.109'
+        )
+      );
+    });
+  });
+});

src/shared/user-agent.js

+/**
+ * Helper methods to identify browser versions and os types
+ */
+
+/**
+ * Returns true when the browser is IE11.
+ *
+ * @param _userAgent {string} - Test seam
+ */
+export const isIE11 = (_userAgent = window.navigator.userAgent) => {
+  return _userAgent.indexOf('Trident/7.0') >= 0;
+};
+
+/**
+ * Returns true when the OS is Mac OS.
+ *
+ * @param _userAgent {string} - Test seam
+ */
+export const isMacOS = (_userAgent = window.navigator.userAgent) => {
+  return _userAgent.indexOf('Mac OS') >= 0;
+};

src/sidebar/components/markdown-editor.js

 import classnames from 'classnames';
 import { createElement, createRef } from 'preact';
-import { useEffect, useRef, useState } from 'preact/hooks';
+import { useEffect, useMemo, useRef, useState } from 'preact/hooks';
 import propTypes from 'prop-types';
 
 import {
@@ -9,6 +9,8 @@ import {
   toggleBlockStyle,
   toggleSpanStyle,
 } from '../markdown-commands';
+import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
+import { isMacOS } from '../../shared/user-agent';
 
 import MarkdownView from './markdown-view';
 import SvgIcon from '../../shared/components/svg-icon';
@@ -102,10 +104,10 @@ function ToolbarButton({
   tabIndex,
   title = '',
 }) {
+  const modifierKey = useMemo(() => (isMacOS() ? 'Cmd' : 'Ctrl'), []);
+
   let tooltip = title;
   if (shortcutKey) {
-    const modifierKey =
-      window.navigator.userAgent.indexOf('Mac OS') !== -1 ? 'Cmd' : 'Ctrl';
     tooltip += ` (${modifierKey}-${shortcutKey.toUpperCase()})`;
   }
 
@@ -211,7 +213,7 @@ function Toolbar({ isPreviewing, onCommand, onTogglePreview }) {
       lowerLimit = buttonIds.help;
     }
     let newFocusedElement = null;
-    switch (e.key) {
+    switch (normalizeKeyName(e.key)) {
       case 'ArrowLeft':
         if (rovingElement <= lowerLimit) {
           newFocusedElement = upperLimit;
@@ -379,10 +381,10 @@ export default function MarkdownEditor({
   onEditText = () => {},
   text = '',
 }) {
-  /** Whether the preview mode is currently active. */
+  // Whether the preview mode is currently active.
   const [preview, setPreview] = useState(false);
 
-  /** The input element where the user inputs their comment. */
+  // The input element where the user inputs their comment.
   const input = useRef(null);
 
   useEffect(() => {

src/sidebar/components/tag-editor.js

 import { createElement } from 'preact';
-import { useRef, useState } from 'preact/hooks';
+import { useMemo, useRef, useState } from 'preact/hooks';
 import propTypes from 'prop-types';
 
 import { withServices } from '../util/service-context';
+import { isIE11 } from '../../shared/user-agent';
 
 import AutocompleteList from './autocomplete-list';
+import { normalizeKeyName } from '../../shared/browser-compatibility-utils';
 import SvgIcon from '../../shared/components/svg-icon';
 import useElementShouldClose from './hooks/use-element-should-close';
 
@@ -34,6 +36,8 @@ function TagEditor({ onEditTags, tags: tagsService, tagList }) {
     setSuggestionsListOpen(false);
   });
 
+  const ie11 = useMemo(() => isIE11(), []);
+
   /**
    * Helper function that returns a list of suggestions less any
    * results also found from the duplicates list.
@@ -121,7 +125,8 @@ function TagEditor({ onEditTags, tags: tagsService, tagList }) {
   const handleOnInput = e => {
     if (
       e.inputType === 'insertText' ||
-      e.inputType === 'deleteContentBackward'
+      e.inputType === 'deleteContentBackward' ||
+      ie11 // inputType is not defined in IE 11, so trigger on any input in this case.
     ) {
       updateSuggestions();
     }
@@ -173,7 +178,7 @@ function TagEditor({ onEditTags, tags: tagsService, tagList }) {
    * found in the suggestions list
    */
   const handleKeyDown = e => {
-    switch (e.key) {
+    switch (normalizeKeyName(e.key)) {
       case 'ArrowUp':
         changeSelectedItem(-1);
         e.preventDefault();

src/sidebar/components/test/markdown-editor-test.js

@@ -21,13 +21,14 @@ describe('MarkdownEditor', () => {
     toggleSpanStyle: sinon.stub().returns(formatResult),
     LinkType,
   };
-
+  let fakeIsMacOS;
   let MarkdownView;
 
   beforeEach(() => {
     fakeMarkdownCommands.convertSelectionToLink.resetHistory();
     fakeMarkdownCommands.toggleBlockStyle.resetHistory();
     fakeMarkdownCommands.toggleSpanStyle.resetHistory();
+    fakeIsMacOS = sinon.stub().returns(false);
 
     MarkdownView = function MarkdownView() {
       return null;
@@ -37,6 +38,9 @@ describe('MarkdownEditor', () => {
     $imports.$mock({
       '../markdown-commands': fakeMarkdownCommands,
       './markdown-view': MarkdownView,
+      '../../shared/user-agent': {
+        isMacOS: fakeIsMacOS,
+      },
     });
   });
 
@@ -129,45 +133,36 @@ describe('MarkdownEditor', () => {
 
       if (key) {
         describe('renders appropriate tooltip for user OS', () => {
-          let fakeUserAgent;
-          let stubbedUserAgent;
-
-          beforeEach(() => {
-            stubbedUserAgent = sinon
-              .stub(window.navigator, 'userAgent')
-              .get(() => fakeUserAgent);
-          });
-
-          afterEach(() => {
-            stubbedUserAgent.restore();
-          });
-
-          // Test that button `title` shows the correct modifier for user OS:
-          // `Cmd-shortcut` for Mac users and `Ctrl-shortcut` for everyone else
           [
             {
-              userAgent:
-                'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.116 Safari/537.36',
+              setOs: () => {
+                fakeIsMacOS.returns(true);
+              },
               expectedModifier: 'Cmd',
             },
-            { userAgent: 'literally anything else', expectedModifier: 'Ctrl' },
-          ].forEach(testCase => {
-            it('should the correct modifier key for user OS in button `title`', () => {
-              fakeUserAgent = testCase.userAgent;
-
+            {
+              setOs: () => {
+                fakeIsMacOS.returns(false);
+              },
+              expectedModifier: 'Ctrl',
+            },
+          ].forEach(test => {
+            it('should show the correct modifier key for user OS in button `title`', () => {
+              // Test that button `title` shows the correct modifier for user OS:
+              // `Cmd-shortcut` for Mac users and `Ctrl-shortcut` for everyone else
+              test.setOs();
               const wrapper = createComponent();
               const button = wrapper.find(
                 `ToolbarButton[title="${command}"] > button`
               );
 
               const buttonTitlePattern = new RegExp(
-                `${testCase.expectedModifier}-${key.toUpperCase()}`
+                `${test.expectedModifier}-${key.toUpperCase()}`
               );
               assert.match(button.props().title, buttonTitlePattern);
             });
           });
         });
-
         // Test that shortcuts are executed with different Ctrl- and Cmd- combos
         const keyEventDetails = [
           { ctrlKey: true, metaKey: false, key },

src/sidebar/components/test/tag-editor-test.js

@@ -15,6 +15,7 @@ describe('TagEditor', function () {
   let fakeTagsService;
   let fakeServiceUrl;
   let fakeOnEditTags;
+  let fakeIsIE11;
 
   function createComponent(props) {
     // Use an array of containers so we can test more
@@ -36,11 +37,28 @@ describe('TagEditor', function () {
     );
   }
 
+  beforeEach(function () {
+    fakeOnEditTags = sinon.stub();
+    fakeServiceUrl = sinon.stub().returns('http://serviceurl.com');
+    fakeIsIE11 = sinon.stub().returns(false);
+    fakeTagsService = {
+      filter: sinon.stub().returns(['tag4', 'tag3']),
+      store: sinon.stub(),
+    };
+    $imports.$mock(mockImportedComponents());
+    $imports.$mock({
+      '../../shared/user-agent': {
+        isIE11: fakeIsIE11,
+      },
+    });
+  });
+
   afterEach(function () {
     containers.forEach(container => {
       container.remove();
     });
     containers = [];
+    $imports.$restore();
   });
 
   // Simulates a selection event from autocomplete-list
@@ -68,23 +86,14 @@ describe('TagEditor', function () {
   }
   // Simulates typing text
   function typeInput(wrapper) {
-    wrapper.find('input').simulate('input', { inputType: 'insertText' });
+    if (!fakeIsIE11()) {
+      wrapper.find('input').simulate('input', { inputType: 'insertText' });
+    } else {
+      // IE11 does not have an inputType key.
+      wrapper.find('input').simulate('input', {});
+    }
   }
 
-  beforeEach(function () {
-    fakeOnEditTags = sinon.stub();
-    fakeServiceUrl = sinon.stub().returns('http://serviceurl.com');
-    fakeTagsService = {
-      filter: sinon.stub().returns(['tag4', 'tag3']),
-      store: sinon.stub(),
-    };
-    $imports.$mock(mockImportedComponents());
-  });
-
-  afterEach(() => {
-    $imports.$restore();
-  });
-
   it('adds appropriate tag values to the elements', () => {
     const wrapper = createComponent();
     wrapper.find('li').forEach((tag, i) => {
@@ -322,51 +331,56 @@ describe('TagEditor', function () {
   });
 
   describe('navigating suggestions via keyboard', () => {
-    it('should set the initial `activeItem` value to -1 when opening suggestions', () => {
-      const wrapper = createComponent();
-      wrapper.find('input').instance().value = 'non-empty';
-      typeInput(wrapper);
-      assert.equal(wrapper.find('AutocompleteList').prop('open'), true);
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
-    });
-    it('should increment the `activeItem` when pressing down circularly', () => {
-      const wrapper = createComponent();
-      wrapper.find('input').instance().value = 'non-empty';
-      typeInput(wrapper);
-      // 2 suggestions: ['tag3', 'tag4'];
-      navigateDown(wrapper);
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
-      navigateDown(wrapper);
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 1);
-      navigateDown(wrapper);
-      // back to unselected
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
-    });
-
-    it('should decrement the `activeItem` when pressing up circularly', () => {
-      const wrapper = createComponent();
-      wrapper.find('input').instance().value = 'non-empty';
-      typeInput(wrapper);
-      // 2 suggestions: ['tag3', 'tag4'];
-      navigateUp(wrapper);
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 1);
-      navigateUp(wrapper);
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
-      navigateUp(wrapper);
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
-    });
+    [true, false].forEach(ie11 => {
+      it('should set the initial `activeItem` value to -1 when opening suggestions', () => {
+        fakeIsIE11.returns(ie11);
+        const wrapper = createComponent();
+        wrapper.find('input').instance().value = 'non-empty';
+        typeInput(wrapper);
+        assert.equal(wrapper.find('AutocompleteList').prop('open'), true);
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
+      });
+      it('should increment the `activeItem` when pressing down circularly', () => {
+        fakeIsIE11.returns(ie11);
+        const wrapper = createComponent();
+        wrapper.find('input').instance().value = 'non-empty';
+        typeInput(wrapper);
+        // 2 suggestions: ['tag3', 'tag4'];
+        navigateDown(wrapper);
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
+        navigateDown(wrapper);
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 1);
+        navigateDown(wrapper);
+        // back to unselected
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
+      });
 
-    it('should set `activeItem` to -1 when clearing the suggestions', () => {
-      const wrapper = createComponent();
-      wrapper.find('input').instance().value = 'non-empty';
-      typeInput(wrapper);
-      navigateDown(wrapper);
-      // change to non-default value
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
-      // clear suggestions
-      wrapper.find('input').instance().value = '';
-      typeInput(wrapper);
-      assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
+      it('should decrement the `activeItem` when pressing up circularly', () => {
+        fakeIsIE11.returns(ie11);
+        const wrapper = createComponent();
+        wrapper.find('input').instance().value = 'non-empty';
+        typeInput(wrapper);
+        // 2 suggestions: ['tag3', 'tag4'];
+        navigateUp(wrapper);
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 1);
+        navigateUp(wrapper);
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
+        navigateUp(wrapper);
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
+      });
+      it('should set `activeItem` to -1 when clearing the suggestions', () => {
+        fakeIsIE11.returns(ie11);
+        const wrapper = createComponent();
+        wrapper.find('input').instance().value = 'non-empty';
+        typeInput(wrapper);
+        navigateDown(wrapper);
+        // change to non-default value
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), 0);
+        // clear suggestions
+        wrapper.find('input').instance().value = '';
+        typeInput(wrapper);
+        assert.equal(wrapper.find('AutocompleteList').prop('activeItem'), -1);
+      });
     });
   });
 

src/sidebar/index.js

 /* global process */
 
 import { jsonConfigsFrom } from '../shared/settings';
+import * as rendererOptions from '../shared/renderer-options';
 
 import {
   startServer as startRPCServer,
@@ -56,6 +57,9 @@ const isSidebar = !(
   window.location.pathname.startsWith('/a/')
 );
 
+// Install Preact renderer options to work around IE11 quirks
+rendererOptions.setupIE11Fixes();
+
 // @ngInject
 function configureToastr(toastrConfig) {
   angular.extend(toastrConfig, {

yarn.lock

@@ -3101,11 +3101,6 @@ escape-string-regexp@1.0.5, escape-string-regexp@^1.0.3, escape-string-regexp@^1
   resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-1.0.5.tgz#1b61c0562190a8dff6ae3bb2cf0200ca130b86d4"
   integrity sha1-G2HAViGQqN/2rjuyzwIAyhMLhtQ=
 
-escape-string-regexp@^3.0.0:
-  version "3.0.0"
-  resolved "https://registry.yarnpkg.com/escape-string-regexp/-/escape-string-regexp-3.0.0.tgz#1dad9cc28aed682be0de197280f79911a5fccd61"
-  integrity sha512-11dXIUC3umvzEViLP117d0KN6LJzZxh5+9F4E/7WLAAw7GrHk8NpUR+g9iJi/pe9C0py4F8rs0hreyRCwlAuZg==
-
 eslint-config-hypothesis@^2.0.0:
   version "2.3.0"
   resolved "https://registry.yarnpkg.com/eslint-config-hypothesis/-/eslint-config-hypothesis-2.3.0.tgz#846512b19f920597551162eef4003ca1e668c8f5"