From afee421a656e023e693ef6c3fc9fd5288e06b07a Mon Sep 17 00:00:00 2001
From: Cory Forsyth <cory.forsyth@gmail.com>
Date: Tue, 19 Jul 2016 12:51:06 -0400
Subject: [PATCH] Refocus editor after toggling markup when no selection. fixes
 #369

Ensures that we refocus the editor element after `toggleMarkup`.
Clicking a button can cause the button to become focused (e.g.
`document.activeElement === buttonElement`) but the window's
`getSelection` is still in the editor.
When the selection is in the editor but it is not focused, key
up/down/press events don't fire on it. Since it has the selection,
typing causes the browser to mutate the editor element's dom and bypass
the key* event handlers. In addition to being generally unwanted, this has the
downside that the mutation will insert text to match its surrounding
style, so the `toggleMarkup` ends up having no effect.

After this change, it is possible to click e.g. the "bold" button when
the selection is collapsed, and the next characters typed will be bold,
as expected.
---
 src/js/editor/editor.js                 | 36 ++++++++++++++++++++++++-
 src/js/utils/cursor.js                  |  6 ++---
 tests/unit/editor/editor-events-test.js | 35 ++++++++++++++++++++++--
 3 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js
index c1695eea7..9e320c67b 100644
--- a/src/js/editor/editor.js
+++ b/src/js/editor/editor.js
@@ -578,8 +578,10 @@ class Editor {
    */
   destroy() {
     this.isDestroyed = true;
-    if (this.hasCursor()) {
+    if (this._hasSelection()) {
       this.cursor.clearSelection();
+    }
+    if (this._hasFocus()) {
       this.element.blur(); // FIXME This doesn't blur the element on IE11
     }
     this._mutationHandler.destroy();
@@ -790,11 +792,43 @@ class Editor {
     if (range.isCollapsed) {
       this._editState.toggleMarkupState(markup);
       this._inputModeDidChange();
+
+      // when clicking a button to toggle markup, the button can end up being focused,
+      // so ensure the editor is focused
+      this._ensureFocus();
     } else {
       this.run(postEditor => postEditor.toggleMarkup(markup, range));
     }
   }
 
+  // If the editor has a selection but is not focused, focus it
+  _ensureFocus() {
+    if (this._hasSelection() && !this._hasFocus()) {
+      this.element.focus();
+    }
+  }
+
+  /**
+   * Whether there is a selection inside the editor's element.
+   * It's possible to have a selection but not have focus.
+   * @see #_hasFocus
+   * @return {Boolean}
+   */
+  _hasSelection() {
+    let { cursor } = this;
+    return this.hasRendered && (cursor._hasCollapsedSelection() || cursor._hasSelection());
+  }
+
+  /**
+   * Whether the editor's element is focused
+   * It's possible to be focused but have no selection
+   * @see #_hasSelection
+   * @return {Boolean}
+   */
+  _hasFocus() {
+    return document.activeElement === this.element;
+  }
+
   /**
    * Toggles the tagName for the current active section(s). This will skip
    * non-markerable sections. E.g. if the editor's range includes a "P" MarkupSection
diff --git a/src/js/utils/cursor.js b/src/js/utils/cursor.js
index 3403ab3ae..4d73e559b 100644
--- a/src/js/utils/cursor.js
+++ b/src/js/utils/cursor.js
@@ -116,9 +116,9 @@ const Cursor = class Cursor {
     const { node:headNode, offset:headOffset } = this._findNodeForPosition(head),
           { node:tailNode, offset:tailOffset } = this._findNodeForPosition(tail);
     this._moveToNode(headNode, headOffset, tailNode, tailOffset, direction);
-    if (document.activeElement !== this.editor.element) {
-      this.editor.element.focus();
-    }
+
+    // Firefox sometimes doesn't keep focus in the editor after adding a card
+    this.editor._ensureFocus();
   }
 
   get selection() {
diff --git a/tests/unit/editor/editor-events-test.js b/tests/unit/editor/editor-events-test.js
index 7e87156d2..bc5c333bf 100644
--- a/tests/unit/editor/editor-events-test.js
+++ b/tests/unit/editor/editor-events-test.js
@@ -214,12 +214,14 @@ test('inputModeDidChange callback fired when markup is toggled and there is a se
   });
 });
 
-test('inputModeDidChange callback fired when markup is toggled and there is no selection', (assert) => {
+test('inputModeDidChange callback fired when markup is toggled and selection is collapsed', (assert) => {
   let done = assert.async();
-  assert.expect(1);
+  assert.expect(2);
 
   editor.selectRange(new Range(editor.post.headPosition()));
 
+  assert.ok(editor.range.isCollapsed, 'precond - range is collapsed');
+
   Helpers.wait(() => {
     let inputChanged = 0;
     editor.inputModeDidChange(() => inputChanged++);
@@ -255,6 +257,35 @@ test('inputModeDidChange callback fired when moving cursor into markup', (assert
   });
 });
 
+test('after #toggleMarkup, editor refocuses if it had selection', (assert) => {
+  let done = assert.async();
+  assert.expect(3);
+
+  let button = $('<button>BOLD</button>').appendTo('#qunit-fixture').click(() => {
+    Helpers.dom.selectText(editor, 'this', editorElement); // necessary for Safari to detect a selection in the editor
+    button.focus();
+
+    assert.ok(document.activeElement !== editor.element, 'precond - editor element is not focused');
+    editor.toggleMarkup('b');
+  });
+
+  editor.selectRange(new Range(editor.post.headPosition()));
+
+  Helpers.wait(() => {
+    let inputChanged = 0;
+    editor.inputModeDidChange(() => inputChanged++);
+
+    button.click();
+
+    Helpers.wait(() => {
+      assert.equal(inputChanged, 1, 'inputModeDidChange fired once');
+      assert.ok(document.activeElement === editor.element, 'editor element is focused');
+
+      done();
+    });
+  });
+});
+
 test('inputModeDidChange callback fired when toggling section', (assert) => {
   let done = assert.async();
   assert.expect(1);