fix(disableEditing): Disable event manager when editing is disabled (#…

…573)

Previously, manual (as opposed to programmatic) editing was being
prevented for an editor after calling `#disableEditing` by setting
"contenteditable" back to false for the editor's element. This prevents the
browser from sending most of the edit-related events (keyup, keydown,
etc) that occur when editing manually. But some events (like paste) are
still emitted by the browser even though "contenteditable" is false.

This change calls `EventManager#stop` when editing is disabled, and
internally the event manager ignores all events when it is stopped.

Also some rearrangement

Other changes
  * Change editor's internal `isEditable` prop to default to true rather
    than null (incidentally allows a user to specify that an editor
    should start disabled by passing in that prop to the constructor)
  * Add test for triggering paste event in a disabled editor
  * add assertion `assert.isBlank`
  * better assertion actual/expected values for `hasElement` and `hasNoElement`
  * add test helper `getData` (for parity with the `setData` in
    element-utils and to avoid using jQuery's `.data()` method)
  * consolidate the editing-disabled tests into a new suite
  * make `editor#render` call `disableEditing` or `enableEditing` after
    it sets `hasRendered` to ensure that the event manager is stopped if
    the user calls `editor.disableEditing()` before calling
    `editor.render()`

Fixes #572

src/js/editor/editor.js

@@ -122,7 +122,7 @@ class Editor {
     assert('editor create accepts an options object. For legacy usage passing an element for the first argument, consider the `html` option for loading DOM or HTML posts. For other cases call `editor.render(domNode)` after editor creation',
           (options && !options.nodeType));
     this._views = [];
-    this.isEditable = null;
+    this.isEditable = true;
     this._parserPlugins = options.parserPlugins || [];
 
     // FIXME: This should merge onto this.options
@@ -239,10 +239,6 @@ class Editor {
 
     this.element = element;
 
-    if (this.isEditable === null) {
-      this.enableEditing();
-    }
-
     this._addTooltip();
 
     // A call to `run` will trigger the didUpdatePostCallbacks hooks with a
@@ -258,6 +254,12 @@ class Editor {
     this._mutationHandler.init();
     this._eventManager.init();
 
+    if (this.isEditable === false) {
+      this.disableEditing();
+    } else {
+      this.enableEditing();
+    }
+
     if (this.autofocus) {
       this.selectRange(this.post.headPosition());
     }
@@ -629,10 +631,9 @@ class Editor {
    * @public
    */
   disableEditing() {
-    if (this.isEditable === false) { return; }
-
     this.isEditable = false;
     if (this.hasRendered) {
+      this._eventManager.stop();
       this.element.setAttribute('contentEditable', false);
       this.setPlaceholder('');
       this.selectRange(Range.blankRange());
@@ -648,7 +649,8 @@ class Editor {
    */
   enableEditing() {
     this.isEditable = true;
-    if (this.element) {
+    if (this.hasRendered) {
+      this._eventManager.start();
       this.element.setAttribute('contentEditable', true);
       this.setPlaceholder(this.placeholder);
     }

src/js/editor/event-manager.js

@@ -28,6 +28,7 @@ export default class EventManager {
 
     this._selectionManager = new SelectionManager(
       this.editor, this.selectionDidChange.bind(this));
+    this.started = true;
   }
 
   init() {
@@ -41,6 +42,14 @@ export default class EventManager {
     this._selectionManager.start();
   }
 
+  start() {
+    this.started = true;
+  }
+
+  stop() {
+    this.started = false;
+  }
+
   registerInputHandler(inputHandler) {
     this._textInputHandler.register(inputHandler);
   }
@@ -90,6 +99,11 @@ export default class EventManager {
 
   _handleEvent(type, event) {
     let {target: element} = event;
+    if (!this.started) {
+      // abort handling this event
+      return true;
+    }
+
     if (!this.isElementAddressable(element)) {
       // abort handling this event
       return true;

tests/acceptance/basic-editor-test.js

@@ -31,47 +31,6 @@ test('sets element as contenteditable', (assert) => {
                'element is contenteditable');
 });
 
-test('#disableEditing before render is meaningful', (assert) => {
-  editor = new Editor();
-  editor.disableEditing();
-  editor.render(editorElement);
-
-  assert.ok(!editorElement.hasAttribute('contenteditable'),
-            'element is not contenteditable');
-  editor.enableEditing();
-  assert.equal(editorElement.getAttribute('contenteditable'),
-               'true',
-               'element is contenteditable');
-});
-
-test('when editing is disabled, the placeholder is not shown', (assert) => {
-  editor = new Editor({placeholder: 'the placeholder'});
-  editor.disableEditing();
-  editor.render(editorElement);
-
-  assert.ok(!$('#editor').data('placeholder'), 'no placeholder when disabled');
-  editor.enableEditing();
-  assert.equal($('#editor').data('placeholder'), 'the placeholder',
-               'placeholder is shown when editable');
-});
-
-test('#disableEditing and #enableEditing toggle contenteditable', (assert) => {
-  editor = new Editor();
-  editor.render(editorElement);
-
-  assert.equal(editorElement.getAttribute('contenteditable'),
-               'true',
-               'element is contenteditable');
-  editor.disableEditing();
-  assert.equal(editorElement.getAttribute('contenteditable'),
-               'false',
-               'element is not contenteditable');
-  editor.enableEditing();
-  assert.equal(editorElement.getAttribute('contenteditable'),
-               'true',
-               'element is contenteditable');
-});
-
 test('clicking outside the editor does not raise an error', (assert) => {
   const done = assert.async();
   editor = new Editor({autofocus: false});

tests/acceptance/editor-disable-editing-test.js

@@ -0,0 +1,81 @@
+import { Editor } from 'mobiledoc-kit';
+import Helpers from '../test-helpers';
+import { TAB, ENTER } from 'mobiledoc-kit/utils/characters';
+import { MIME_TEXT_PLAIN } from 'mobiledoc-kit/utils/parse-utils';
+
+const { test, module } = Helpers;
+
+const cards = [{
+  name: 'my-card',
+  type: 'dom',
+  render() {},
+  edit() {}
+}];
+
+let editor, editorElement;
+
+module('Acceptance: editor: #disableEditing', {
+  beforeEach() {
+    editorElement = $('#editor')[0];
+  },
+  afterEach() {
+    if (editor) { editor.destroy(); }
+  }
+});
+
+test('#disableEditing before render is meaningful', (assert) => {
+  editor = new Editor();
+  editor.disableEditing();
+  editor.render(editorElement);
+
+  assert.equal(editorElement.getAttribute('contenteditable'),'false',
+            'element is not contenteditable');
+  editor.enableEditing();
+  assert.equal(editorElement.getAttribute('contenteditable'), 'true',
+               'element is contenteditable');
+});
+
+test('when editing is disabled, the placeholder is not shown', (assert) => {
+  editor = new Editor({placeholder: 'the placeholder'});
+  editor.disableEditing();
+  editor.render(editorElement);
+
+  assert.isBlank(Helpers.dom.getData(editorElement, 'placeholder'),
+    'no placeholder when disabled');
+  editor.enableEditing();
+  assert.equal(Helpers.dom.getData(editorElement, 'placeholder'), 'the placeholder',
+               'placeholder is shown when editable');
+});
+
+test('#disableEditing and #enableEditing toggle contenteditable', (assert) => {
+  editor = new Editor();
+  editor.render(editorElement);
+
+  assert.equal(editorElement.getAttribute('contenteditable'),
+               'true',
+               'element is contenteditable');
+  editor.disableEditing();
+  assert.equal(editorElement.getAttribute('contenteditable'),
+               'false',
+               'element is not contenteditable');
+  editor.enableEditing();
+  assert.equal(editorElement.getAttribute('contenteditable'),
+               'true',
+               'element is contenteditable');
+});
+
+// https://github.com/bustle/mobiledoc-kit/issues/572
+test('pasting after #disableEditing does not insert text', function(assert) {
+  editor = Helpers.editor.buildFromText('abc|', {element: editorElement});
+
+  Helpers.dom.setCopyData(MIME_TEXT_PLAIN, 'def');
+  Helpers.dom.triggerPasteEvent(editor);
+  assert.hasElement('#editor:contains(abcdef)', 'precond - text is pasted');
+
+  editor.disableEditing();
+
+  Helpers.dom.selectText(editor, 'def');
+  Helpers.dom.setCopyData(MIME_TEXT_PLAIN, 'ghi');
+  Helpers.dom.triggerPasteEvent(editor);
+  assert.hasNoElement('#editor:contains(ghi)', 'text is not pasted after #disableEditing');
+});

tests/helpers/assertions.js

@@ -127,13 +127,22 @@ function comparePostNode(actual, expected, assert, path='root', deepCompare=fals
 /* eslint-enable complexity */
 
 export default function registerAssertions(QUnit) {
+  QUnit.assert.isBlank = function(val, message=`value is blank`) {
+    this.pushResult({
+      result: val === null || val === undefined || val === '' || val === false,
+      actual: `${val} (typeof ${typeof val})`,
+      expected: `null|undefined|''|false`,
+      message
+    });
+  };
+
   QUnit.assert.hasElement = function(selector,
                                      message=`hasElement "${selector}"`) {
     let found = $(selector);
     this.pushResult({
       result: found.length > 0,
-      actual: found.length,
-      expected: selector,
+      actual: `${found.length} matches for '${selector}'`,
+      expected: `>0 matches for '${selector}'`,
       message: message
     });
     return found;
@@ -144,8 +153,8 @@ export default function registerAssertions(QUnit) {
     let found = $(selector);
     this.pushResult({
       result: found.length === 0,
-      actual: found.length,
-      expected: selector,
+      actual: `${found.length} matches for '${selector}'`,
+      expected: `0 matches for '${selector}'`,
       message: message
     });
     return found;

tests/helpers/dom.js

@@ -9,6 +9,7 @@ import {
   MIME_TEXT_PLAIN,
   MIME_TEXT_HTML
 } from 'mobiledoc-kit/utils/parse-utils';
+import { dasherize } from 'mobiledoc-kit/utils/string-utils';
 
 function assertEditor(editor) {
   if (!(editor instanceof Editor)) {
@@ -365,6 +366,14 @@ function blur() {
   input.focus();
 }
 
+function getData(element, name) {
+  if (element.dataset) {
+    return element.dataset[name];
+  } else {
+    return element.getAttribute(dasherize(name));
+  }
+}
+
 const DOMHelper = {
   moveCursorTo,
   moveCursorWithoutNotifyingEditorTo,
@@ -394,7 +403,8 @@ const DOMHelper = {
   clearCopyData,
   createMockEvent,
   findTextNode,
-  blur
+  blur,
+  getData
 };
 
 export { triggerEvent };