Correctly interpret a reported selection of the editor element

In Firefox, typing Command-A changes the selection to have the
editor element as the focus and anchor node (other browsers select the
first and last text nodes in the editor element). Changes
Position.fromNode to correctly detect this and return the correct
Position

Also some cleanup in tests and removed usage of `isMarkerable` functions
throughout the codebase in favor of `section.isMarkerable` property.

fixes #215

src/js/editor/post.js

@@ -1,7 +1,6 @@
 import {
   DEFAULT_TAG_NAME as DEFAULT_MARKUP_SECTION_TAG_NAME
 } from '../models/markup-section';
-import { isMarkerable } from '../models/_section';
 import { POST_TYPE, MARKUP_SECTION_TYPE, LIST_ITEM_TYPE, LIST_SECTION_TYPE } from '../models/types';
 import Position from '../utils/cursor/position';
 import { isArrayEqual, forEach, filter, compact } from '../utils/array-utils';
@@ -11,14 +10,14 @@ import mixin from '../utils/mixin';
 import assert from '../utils/assert';
 
 function isJoinable(section1, section2) {
-  return isMarkerable(section1) &&
-         isMarkerable(section2) &&
+  return section1.isMarkerable &&
+         section2.isMarkerable &&
          section1.type === section2.type &&
          section1.tagName === section2.tagName;
 }
 
 function endPosition(section) {
-  if (isMarkerable(section)) {
+  if (section.isMarkerable) {
     return new Position(section, section.length);
   } else if (section.type === LIST_SECTION_TYPE) {
     return endPosition(section.items.tail);
@@ -261,7 +260,7 @@ class PostEditor {
     if (postNode.section) {
       this._markDirty(postNode.section);
     }
-    if (isMarkerable(postNode)) {
+    if (postNode.isMarkerable) {
       this.addCallback(
         CALLBACK_QUEUES.BEFORE_COMPLETE, () => this._coalesceMarkers(postNode));
     }
@@ -304,7 +303,7 @@ class PostEditor {
     const {section } = position;
     let nextPosition = position.clone();
 
-    if (!isMarkerable(section)) {
+    if (!section.isMarkerable) {
       throw new Error('Cannot join non-markerable section to previous section');
     } else if (isListItem(section)) {
       nextPosition = this._convertListItemToMarkupSection(section);
@@ -378,7 +377,7 @@ class PostEditor {
     const { section } = position;
     let nextPosition = position.clone();
 
-    if (!isMarkerable(section)) {
+    if (!section.isMarkerable) {
       throw new Error('Cannot join non-markerable section to next section');
     } else {
       const next = section.immediatelyNextMarkerableSection();

src/js/models/_section.js

@@ -2,10 +2,6 @@ import { LIST_ITEM_TYPE } from './types';
 import { normalizeTagName } from '../utils/dom-utils';
 import LinkedItem from '../utils/linked-item';
 
-export function isMarkerable(section) {
-  return !!section.markers;
-}
-
 function isChild(section) {
   return section.type === LIST_ITEM_TYPE;
 }
@@ -59,7 +55,7 @@ export default class Section extends LinkedItem {
 
   immediatelyNextMarkerableSection() {
     let next = this.nextLeafSection();
-    while (next && !isMarkerable(next)) {
+    while (next && !next.isMarkerable) {
       next = next.nextLeafSection();
     }
     return next;

src/js/models/post.js

@@ -2,7 +2,6 @@ import { POST_TYPE } from './types';
 import LinkedList from 'content-kit-editor/utils/linked-list';
 import { forEach, compact } from 'content-kit-editor/utils/array-utils';
 import Set from 'content-kit-editor/utils/set';
-import { isMarkerable } from 'content-kit-editor/models/_section';
 import MobiledocRenderer from 'content-kit-editor/renderers/mobiledoc';
 
 export default class Post {
@@ -135,7 +134,7 @@ export default class Post {
 
   walkMarkerableSections(range, callback) {
     this.walkLeafSections(range, section => {
-      if (isMarkerable(section)) {
+      if (section.isMarkerable) {
         callback(section);
       }
     });
@@ -173,7 +172,7 @@ export default class Post {
   _nextMarkerableSection(section) {
     let nextSection = this._nextLeafSection(section);
 
-    while (nextSection && !isMarkerable(nextSection)) {
+    while (nextSection && !nextSection.isMarkerable) {
       nextSection = this._nextLeafSection(nextSection);
     }
 
@@ -216,7 +215,7 @@ export default class Post {
 
     this.walkPostSections(range, section => {
       let newSection;
-      if (isMarkerable(section)) {
+      if (section.isMarkerable) {
         newSection = builder.createMarkupSection(section.tagName);
         let currentRange = range.trimTo(section);
         forEach(

src/js/utils/cursor.js

@@ -131,13 +131,14 @@ const Cursor = class Cursor {
   }
 
   /**
-   * @private
    * @param {textNode} node
    * @param {integer} offset
-   * @param {textNode} endNode (default: node)
-   * @param {integer} endOffset (default: offset)
+   * @param {textNode} endNode
+   * @param {integer} endOffset
+   * @param {integer} direction forward or backward, default forward
+   * @private
    */
-  _moveToNode(node, offset, endNode, endOffset, direction) {
+  _moveToNode(node, offset, endNode, endOffset, direction=DIRECTION.FORWARD) {
     this.clearSelection();
 
     if (direction === DIRECTION.BACKWARD) {

src/js/utils/cursor/position.js

@@ -1,21 +1,12 @@
 import {
   isTextNode, findOffsetInElement
 } from 'content-kit-editor/utils/dom-utils';
-import {
-  MARKUP_SECTION_TYPE, LIST_ITEM_TYPE, CARD_TYPE
-} from 'content-kit-editor/models/types';
 import { DIRECTION } from 'content-kit-editor/utils/key';
 import assert from 'content-kit-editor/utils/assert';
 
 function isSection(postNode) {
   if (!(postNode && postNode.type)) { return false; }
-  return postNode.type === MARKUP_SECTION_TYPE ||
-    postNode.type === LIST_ITEM_TYPE ||
-    postNode.type === CARD_TYPE;
-}
-
-function isCardSection(section) {
-  return section.type === CARD_TYPE;
+  return postNode.isMarkerable || postNode.isCardSection;
 }
 
 function findParentSectionFromNode(renderTree, node) {
@@ -28,7 +19,7 @@ function findParentSectionFromNode(renderTree, node) {
 }
 
 function findOffsetInSection(section, node, offset) {
-  if (!isCardSection(section)) {
+  if (!section.isCardSection) {
     return findOffsetInElement(section.renderNode.element,
                                node, offset);
   }
@@ -46,7 +37,7 @@ const Position = class Position {
   constructor(section, offset=0) {
     this.section = section;
     this.offset = offset;
-    this._inCard = isCardSection(section);
+    this._inCard = section.isCardSection;
   }
 
   static emptyPosition() {
@@ -143,32 +134,37 @@ const Position = class Position {
   }
 
   static fromElementNode(renderTree, elementNode, offset) {
+    let section, offsetInSection;
+
     // The browser may change the reported selection to equal the editor's root
     // element if the user clicks an element that is immediately removed,
     // which can happen when clicking to remove a card.
     if (elementNode === renderTree.rootElement) {
-      return Position.emptyPosition();
-    }
-
-    let section = findParentSectionFromNode(renderTree, elementNode);
-    if (!section) { throw new Error('Could not find parent section from element node'); }
-
-    let offsetInSection;
-
-    if (isCardSection(section)) {
-      // Selections in cards are usually made on a text node containing a ‌ 
-      // on one side or the other of the card but some scenarios will result in
-      // selecting the card's wrapper div. If the offset is 2 we've selected
-      // the final zwnj and should consider the cursor at the end of the card (offset 1). Otherwise,
-      // the cursor is at the start of the card
-      offsetInSection = offset < 2 ? 0 : 1;
+      let post = renderTree.rootNode.postNode;
+      if (offset === 0) {
+        section = post.sections.head;
+        offsetInSection = 0;
+      } else {
+        section = post.sections.tail;
+        offsetInSection = section.length;
+      }
     } else {
-      offsetInSection = 0;
+      section = findParentSectionFromNode(renderTree, elementNode);
+      assert('Could not find parent section from element node', !!section);
+
+      if (section.isCardSection) {
+        // Selections in cards are usually made on a text node containing a &zwnj; 
+        // on one side or the other of the card but some scenarios (Firefox) will result in
+        // selecting the card's wrapper div. If the offset is 2 we've selected
+        // the final zwnj and should consider the cursor at the end of the card (offset 1). Otherwise,
+        // the cursor is at the start of the card
+        offsetInSection = offset < 2 ? 0 : 1;
+      } else {
+        // The offset is 0 if the cursor is on an element node (e.g., a <br> tag in
+        // a blank markup section)
+        offsetInSection = 0;
+      }
     }
-
-    // FIXME We assume that offsetInSection will always be 0 because we assume
-    // that only empty br tags (offsetInSection=0) will be those that cause
-    // us to call `fromElementNode`. This may not be a reliable assumption.
     return new Position(section, offsetInSection);
   }
 

src/js/utils/dom-utils.js

@@ -36,11 +36,15 @@ function clearChildNodes(element) {
 }
 
 /**
- * @return {Boolean} true when the child node is contained by (and not
- * the same as) the parent node
+ * @return {Boolean} true when the child node is contained or the same as
+ * (e.g., inclusive containment)  the parent node
  *  see https://github.com/webmodules/node-contains/blob/master/index.js
+ *  Mimics the behavior of `Node.contains`, which is broken in IE 10
  */
 function containsNode(parentNode, childNode) {
+  if (parentNode === childNode) {
+    return true;
+  }
   const position = parentNode.compareDocumentPosition(childNode);
   return !!(position & Node.DOCUMENT_POSITION_CONTAINED_BY);
 }

tests/acceptance/basic-editor-test.js

@@ -151,3 +151,25 @@ test('typing when on the start of a card is blocked', (assert) => {
   Helpers.dom.insertText(editor, 'Y');
   assert.hasNoElement('#editor div:contains(Y)');
 });
+
+// see https://github.com/bustlelabs/content-kit-editor/issues/215
+test('select-all and type text works ok', (assert) => {
+  const mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => {
+    return post([
+      markupSection('p', [marker('abc')])
+    ]);
+  });
+  editor = new Editor({mobiledoc, cards});
+  editor.render(editorElement);
+
+  Helpers.dom.moveCursorTo(editorElement.firstChild, 0);
+  document.execCommand('selectAll');
+
+  assert.selectedText('abc', 'precond - abc is selected');
+  assert.hasElement('#editor p:contains(abc)', 'precond - renders p');
+
+  Helpers.dom.insertText(editor, 'X');
+
+  assert.hasNoElement('#editor p:contains(abc)', 'replaces existing text');
+  assert.hasElement('#editor p:contains(X)', 'inserts text');
+});

tests/acceptance/cursor-position-test.js

@@ -199,8 +199,28 @@ test('cursor focused on card wrapper with 0 offset', (assert) => {
   assert.equal(offsets.tail.offset, 0,
                'Cursor tail is positioned at offset 0');
 });
-
 
+// see https://github.com/bustlelabs/content-kit-editor/issues/215
+test('selecting the entire editor element reports a selection range of the entire post', (assert) => {
+  let mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => {
+    return post([
+      markupSection('p', [marker('abc')]),
+      markupSection('p', [marker('1234')])
+    ]);
+  });
+  editor = new Editor({mobiledoc});
+  editor.render(editorElement);
+
+  Helpers.dom.moveCursorTo(editorElement, 0,
+                           editorElement, editorElement.childNodes.length);
+  let { offsets } = editor.cursor;
+  assert.ok(offsets.head.section === editor.post.sections.head,
+            'head section correct');
+  assert.equal(offsets.head.offset, 0, 'head offset 0');
+  assert.ok(offsets.tail.section === editor.post.sections.tail,
+            'tail section correct');
+  assert.equal(offsets.tail.offset, 4, 'tail offset equals section length');
+});
 
 //inside card wrapper div and before starting zwnj reports its position correctly');
 

tests/unit/utils/cursor-range-test.js

@@ -1,18 +1,13 @@
 import Helpers from '../../test-helpers';
-import Position from 'content-kit-editor/utils/cursor/position';
 import Range from 'content-kit-editor/utils/cursor/range';
 
 const {module, test} = Helpers;
 
-function makeRange(s1, o1, s2, o2) {
-  return new Range(new Position(s1, o1), new Position(s2, o2));
-}
-
 module('Unit: Utils: Range');
 
 test('#trimTo(section) when range covers only one section', (assert) => {
   const section = Helpers.postAbstract.build(({markupSection}) => markupSection());
-  const range = makeRange(section, 0, section, 5);
+  const range = Range.create(section, 0, section, 5);
 
   const newRange = range.trimTo(section);
   assert.ok(newRange.head.section === section, 'head section is correct');
@@ -28,7 +23,7 @@ test('#trimTo head section', (assert) => {
   const section2 = Helpers.postAbstract.build(
     ({markupSection, marker}) => markupSection('p', [marker(text)]));
 
-  const range = makeRange(section1, 0, section2, 5);
+  const range = Range.create(section1, 0, section2, 5);
   const newRange = range.trimTo(section1);
 
   assert.ok(newRange.head.section === section1, 'head section');
@@ -44,7 +39,7 @@ test('#trimTo tail section', (assert) => {
   const section2 = Helpers.postAbstract.build(
     ({markupSection, marker}) => markupSection('p', [marker(text)]));
 
-  const range = makeRange(section1, 0, section2, 5);
+  const range = Range.create(section1, 0, section2, 5);
   const newRange = range.trimTo(section2);
 
   assert.ok(newRange.head.section === section2, 'head section');
@@ -62,12 +57,11 @@ test('#trimTo middle section', (assert) => {
   const section3 = Helpers.postAbstract.build(
     ({markupSection, marker}) => markupSection('p', [marker(text)]));
 
-  const range = makeRange(section1, 0, section3, 5);
+  const range = Range.create(section1, 0, section3, 5);
   const newRange = range.trimTo(section2);
 
   assert.ok(newRange.head.section === section2, 'head section');
   assert.ok(newRange.tail.section === section2, 'tail section');
   assert.equal(newRange.head.offset, 0, 'head offset');
   assert.equal(newRange.tail.offset, section2.text.length, 'tail offset');
-
 });