Merge pull request #67 from bustlelabs/fix-selection-issues-56

Correct the browser's reported selection

src/js/models/cursor.js

@@ -8,9 +8,33 @@ import {
 } from '../utils/selection-utils';
 
 import {
-  detectParentNode
+  detectParentNode,
+  isTextNode
 } from '../utils/dom-utils';
 
+
+function comparePosition(selection) {
+  let { anchorNode, focusNode, anchorOffset, focusOffset } = selection;
+  let leftNode, rightNode, leftOffset, rightOffset;
+
+  const position = anchorNode.compareDocumentPosition(focusNode);
+
+  if (position & Node.DOCUMENT_POSITION_FOLLOWING) {
+    leftNode = anchorNode; rightNode = focusNode;
+    leftOffset = anchorOffset; rightOffset = focusOffset;
+  } else if (position & Node.DOCUMENT_POSITION_PRECEDING) {
+    leftNode = focusNode; rightNode = anchorNode;
+    leftOffset = focusOffset; rightOffset = anchorOffset;
+  } else { // same node
+    leftNode = anchorNode;
+    rightNode = focusNode;
+    leftOffset = Math.min(anchorOffset, focusOffset);
+    rightOffset = Math.max(anchorOffset, focusOffset);
+  }
+
+  return {leftNode, leftOffset, rightNode, rightOffset};
+}
+
 export default class Cursor {
   constructor(editor) {
     this.editor = editor;
@@ -32,30 +56,24 @@ export default class Cursor {
   }
 
   get offsets() {
-    let leftNode, rightNode,
-        leftOffset, rightOffset;
     const selection = this.selection;
-    const { anchorNode, focusNode, anchorOffset, focusOffset } = selection;
     const { rangeCount } = selection;
     const range = rangeCount > 0 && selection.getRangeAt(0);
 
     if (!range) {
       return {};
     }
 
-    const position = anchorNode.compareDocumentPosition(focusNode);
-
-    if (position & Node.DOCUMENT_POSITION_FOLLOWING) {
-      leftNode = anchorNode; rightNode = focusNode;
-      leftOffset = anchorOffset; rightOffset = focusOffset;
-    } else if (position & Node.DOCUMENT_POSITION_PRECEDING) {
-      leftNode = focusNode; rightNode = anchorNode;
-      leftOffset = focusOffset; rightOffset = anchorOffset;
-    } else { // same node
-      leftNode = anchorNode;
-      rightNode = focusNode;
-      leftOffset = Math.min(anchorOffset, focusOffset);
-      rightOffset = Math.max(anchorOffset, focusOffset);
+    let {leftNode, leftOffset, rightNode, rightOffset} =
+      comparePosition(selection);
+
+    // The selection should contain two text nodes.
+    // On Chrome/Safari using shift+<Up arrow> can create a selection with
+    // a tag rather than a text node. This fixes that.
+    // See https://github.com/bustlelabs/content-kit-editor/issues/56
+    if (!isTextNode(rightNode)) {
+      rightNode = rightNode.previousSibling.lastChild;
+      rightOffset = rightNode.textContent.length;
     }
 
     const leftRenderNode = this.renderTree.elements.get(leftNode),

src/js/utils/dom-utils.js

@@ -129,5 +129,6 @@ export {
   walkDOMUntil,
   walkTextNodes,
   addClassName,
-  normalizeTagName
+  normalizeTagName,
+  isTextNode
 };

tests/acceptance/editor-sections-test.js

@@ -340,4 +340,62 @@ test('keystroke of delete at start of first section does nothing', (assert) => {
                   'cursor stays at start of first section');
 });
 
+test('when selection incorrectly contains P end tag, editor reports correct selection', (assert) => {
+  const done = assert.async();
+
+  editor = new Editor(editorElement, {mobiledoc: mobileDocWith2Sections});
+
+  let secondSectionTextNode = editor.element.childNodes[1].firstChild;
+  let firstSectionPNode = editor.element.childNodes[0];
+
+  Helpers.dom.moveCursorTo(firstSectionPNode, 0,
+                           secondSectionTextNode, 0);
+  Helpers.dom.triggerEvent(document, 'mouseup');
+
+  setTimeout(() => {
+    assert.ok(true, 'No error should occur');
+
+    let firstSectionTextNode = editor.element.childNodes[0].childNodes[0];
+    let firstSectionLength = firstSectionTextNode.textContent.length;
+
+    let {leftNode, rightNode, leftOffset, rightOffset} = editor.cursor.offsets;
+
+    assert.equal(leftNode, firstSectionTextNode, 'returns first section text node as left');
+    assert.equal(rightNode, firstSectionTextNode, 'returns first section text node as right');
+    assert.equal(leftOffset, 0, 'leftOffset correct');
+    assert.equal(rightOffset, firstSectionLength, 'rightOffset correct');
+
+    done();
+  });
+});
+
+test('when selection incorrectly contains P start tag, editor reports correct selection', (assert) => {
+  const done = assert.async();
+
+  editor = new Editor(editorElement, {mobiledoc: mobileDocWith2Sections});
+
+  let firstSectionTextNode = editor.element.childNodes[0].firstChild;
+  let secondSectionPNode = editor.element.childNodes[1];
+
+  Helpers.dom.moveCursorTo(firstSectionTextNode, 0,
+                           secondSectionPNode, 0);
+  Helpers.dom.triggerEvent(document, 'mouseup');
+
+  setTimeout(() => {
+    assert.ok(true, 'No error should occur');
+
+    let firstSectionTextNode = editor.element.childNodes[0].childNodes[0];
+    let firstSectionLength = firstSectionTextNode.textContent.length;
+
+    let {leftNode, rightNode, leftOffset, rightOffset} = editor.cursor.offsets;
+
+    assert.equal(leftNode, firstSectionTextNode, 'returns first section text node as left');
+    assert.equal(rightNode, firstSectionTextNode, 'returns first section text node as right');
+    assert.equal(leftOffset, 0, 'leftOffset correct');
+    assert.equal(rightOffset, firstSectionLength, 'rightOffset correct');
+
+    done();
+  });
+});
+
 // test: deleting at start of section when previous section is a non-markup section

tests/helpers/dom.js

@@ -38,8 +38,8 @@ function selectText(startText,
   selectRange(startTextNode, startOffset, endTextNode, endOffset);
 }
 
-function moveCursorTo(element, offset=0) {
-  selectRange(element, offset, element, offset);
+function moveCursorTo(node, offset=0, endNode=node, endOffset=offset) {
+  selectRange(node, offset, endNode, endOffset);
 }
 
 function triggerEvent(node, eventType) {