Merge pull request #218 from bustlelabs/select-all-215

Correctly interpret a reported selection of the editor element

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