Refocus editor after toggling markup when no selection. fixes #369 (#436

)

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

@@ -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

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() {

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);