fix(486): Constrain selection to editor element when probing for range (

#542) * clean up unnecessary text-with-atoms builder in tests * Update debug page Add toggle-editable button, error display, selection display, special styles for when editing is disabled. * failing test for demonstrate 486 * Constrain selection to editor element when probing for range This fixes an issue where `Position.fromNode` would be called with a node that is outside the editor element, triggering a failed assertion. Now, Cursor constrains the selection to only include the extent inside the editor element before looking up Positions from the anchor and focus nodes. Most of the time this situation is prevented by the browser (it refuses to allow one to create a selection that crosses into or out of the contentEditable div), but when `editor.disableEditing()` is called, a user can triple-click the last part of the mobiledoc document which causes the browser (for Chrome and Safari, but not Firefox) to extend the selection *outside* the editor's element. Fixes #486 as reported by @YoranBrondsema
bustle · Mar 15, 2017 · 9b7f58c · 9b7f58c
1 parent 749eec0
commit 9b7f58c
Show file tree

Hide file tree

Showing 6 changed files with 229 additions and 80 deletions.
diff --git a/assets/demo/debug.css b/assets/demo/debug.css
@@ -1,3 +1,22 @@
 #toolbar button.active {
   font-weight: bold;
 }
+
+#error {
+  color: red;
+}
+
+#error .name {
+  font-weight: bold;
+}
+
+#editor {
+  outline: 1px solid black;
+  width: 200px;
+  min-height: 300px;
+}
+
+#editor[contenteditable="false"] {
+  color: #9e9e9e;
+  background-color: #f1f1f1;
+}
diff --git a/assets/demo/debug.html b/assets/demo/debug.html
@@ -3,43 +3,68 @@
 <head>
   <meta charset="utf-8">
   <title>Mobiledoc Kit Debug</title>
-</head>
-<body>
   <script src="../tests/jquery/jquery.js"></script>
   <script src="../global/mobiledoc-kit.js"></script>
   <script src="./debug.js"></script>
   <link rel="stylesheet" href="../css/mobiledoc-kit.css">
   <link rel="stylesheet" href="./debug.css">
-</body>
+</head>
+<body>
 
-<div id='toolbar'>
-  <button class='toggle' data-action='toggleSection' data-toggle='h1'>h1</button>
-  <button class='toggle' data-action='toggleSection' data-toggle='h2'>h2</button>
-  <button class='toggle' data-action='toggleMarkup' data-toggle='strong'>strong</button>
-  <button class='toggle' data-action='toggleMarkup' data-toggle='em'>em</button>
-  <button class='insert-atom' data-name='mention'>@mention</button>
-  <button class='insert-atom' data-name='click'>@click</button>
-  <button class='insert-card' data-name='movable'>movable card</button>
-</div>
-
-<div id="editor" style='outline: 1px solid black; width: 200px; min-height: 300px;'></div>
-
-<div>
-  <h3>Cursor</h3>
-  <div id='cursor'>
-  </div>
-</div>
+  <h1>Mobiledoc Kit Debug Page</h1>
 
-<div>
-  <h3>Post</h3>
-  <div id='post'>
+  <div id='toolbar'>
+    <button class='toggle' data-action='toggleSection' data-toggle='h1'>h1</button>
+    <button class='toggle' data-action='toggleSection' data-toggle='h2'>h2</button>
+    <button class='toggle' data-action='toggleMarkup' data-toggle='strong'>strong</button>
+    <button class='toggle' data-action='toggleMarkup' data-toggle='em'>em</button>
+    <button class='insert-atom' data-name='mention'>@mention</button>
+    <button class='insert-atom' data-name='click'>@click</button>
+    <button class='insert-card' data-name='movable'>movable card</button>
+    <button class='toggle-method' data-on='disableEditing' data-off='enableEditing'>
+      toggle editable
+    </button>
   </div>
-</div>
 
-<div>
-  <h3>Input Mode</h3>
-  <div id='input-mode'>
+  <div id="editor"></div>
+
+  <div>
+    <h2>Editor Info</h2>
+
+    <div>
+      <h3>Cursor</h3>
+      <div id='cursor'>
+      </div>
+    </div>
+
+    <div>
+      <h3>Post</h3>
+      <div id='post'>
+      </div>
+    </div>
+
+    <div>
+      <h3>Input Mode</h3>
+      <div id='input-mode'>
+      </div>
+    </div>
+
+    <div>
+      <h3>Error</h3>
+      <div id='error'>
+        <span class='name'></span>
+        <span class='message'></span>
+      </div>
+    </div>
   </div>
-</div>
 
+  <div>
+    <h2>Browser Info</h2>
+
+    <div>
+      <h3>Selection</h3>
+      <div id="selection"></div>
+    </div>
+  </div>
+</body>
 </html>
diff --git a/assets/demo/debug.js b/assets/demo/debug.js
@@ -3,6 +3,14 @@
 
 var editor;
 
+function renderError(event) {
+  let error = event.error;
+  $('#error .name').text(error.name);
+  $('#error .message').text(error.message);
+}
+
+window.addEventListener('error', renderError);
+
 function renderSection(section) {
   return '[' +
     'Section: tagName ' + section.tagName +
@@ -29,6 +37,31 @@ function updateCursor() {
   $('#cursor').html(html);
 }
 
+document.addEventListener('selectionchange', renderNativeSelection);
+
+function renderNativeSelection(event) {
+  let sel = window.getSelection();
+  let { anchorNode, focusNode, anchorOffset, focusOffset, isCollapsed, rangeCount } = sel;
+  if (anchorNode === null && focusNode === null) {
+    $('#selection').html(`<em>None</em>`);
+    return;
+  }
+  $('#selection').html(`
+    <div class='node'>Anchor: ${renderNode(anchorNode)} (${anchorOffset})</div>
+    <div class='node'>Focus: ${renderNode(focusNode)} (${focusOffset})</div>
+    <div>${isCollapsed ? 'Collapsed' : 'Not collapsed'}</div>
+    <div class='ranges'>Ranges: ${rangeCount}</div>
+  `);
+}
+
+function renderNode(node) {
+  let text = node.textContent.slice(0, 22);
+  if (node.textContent.length > 22) { text += '...'; }
+
+  let type = node.nodeType === Node.TEXT_NODE ? 'text' : `el (${node.tagName})`;
+  return `<span class='type'>${type}</span>: ${text}`;
+}
+
 function renderMarkup(markup) {
   function renderAttrs(obj) {
     var str = Object.keys(obj).reduce(function (memo, key) {
@@ -195,6 +228,18 @@ $(function () {
     editor[action](toggle);
   });
 
+  $('#toolbar button.toggle-method').click(function() {
+    let isOn = $(this).data('is-on') === 'true';
+    let methodOn = $(this).data('on');
+    let methodOff = $(this).data('off');
+
+    let nextState = isOn ? 'false' : 'true';
+    let method = isOn ? methodOff : methodOn;
+
+    $(this).data('is-on', nextState);
+    editor[method]();
+  });
+
   $('#toolbar button.insert-atom').click(function() {
     let name = $(this).data('name');
     let value = $(this).data('value') || '';

diff --git a/src/js/utils/cursor.js b/src/js/utils/cursor.js
@@ -6,6 +6,7 @@ import { containsNode } from '../utils/dom-utils';
 import Position from './cursor/position';
 import Range from './cursor/range';
 import { DIRECTION } from '../utils/key';
+import { constrainSelectionTo } from '../utils/selection-utils';
 
 export { Position, Range };
 
@@ -61,7 +62,9 @@ const Cursor = class Cursor {
   get offsets() {
     if (!this.hasCursor()) { return Range.blankRange(); }
 
-    const { selection, renderTree } = this;
+    let { selection, renderTree } = this;
+    let parentNode = this.editor.element;
+    selection = constrainSelectionTo(selection, parentNode);
 
     const {
       headNode, headOffset, tailNode, tailOffset, direction

diff --git a/src/js/utils/selection-utils.js b/src/js/utils/selection-utils.js
@@ -80,6 +80,53 @@ function findOffsetInNode(node, coords) {
   return {node, offset};
 }
 
+function constrainNodeTo(node, parentNode, existingOffset) {
+  let compare = parentNode.compareDocumentPosition(node);
+  if (compare & Node.DOCUMENT_POSITION_CONTAINED_BY) {
+    // the node is inside parentNode, do nothing
+    return { node, offset: existingOffset};
+  } else if (compare & Node.DOCUMENT_POSITION_CONTAINS) {
+    // the node contains parentNode. This shouldn't happen.
+    return { node, offset: existingOffset};
+  } else if (compare & Node.DOCUMENT_POSITION_PRECEDING) {
+    // node is before parentNode. return start of deepest first child
+    let child = parentNode.firstChild;
+    while (child.firstChild) {
+      child = child.firstChild;
+    }
+    return { node: child, offset: 0};
+  } else if (compare & Node.DOCUMENT_POSITION_FOLLOWING) {
+    // node is after parentNode. return end of deepest last child
+    let child = parentNode.lastChild;
+    while (child.lastChild) {
+      child = child.lastChild;
+    }
+
+    let offset = isTextNode(child) ? child.textContent.length : 1;
+    return {node: child, offset};
+  } else {
+    return { node, offset: existingOffset};
+  }
+}
+
+/*
+ * Returns a new selection that is constrained within parentNode.
+ * If the anchorNode or focusNode are outside the parentNode, they are replaced with the beginning
+ * or end of the parentNode's children
+ */
+function constrainSelectionTo(selection, parentNode) {
+  let {
+    node: anchorNode,
+    offset: anchorOffset
+  } = constrainNodeTo(selection.anchorNode, parentNode, selection.anchorOffset);
+  let {
+    node: focusNode,
+    offset: focusOffset
+  } = constrainNodeTo(selection.focusNode, parentNode, selection.focusOffset);
+
+  return { anchorNode, anchorOffset, focusNode, focusOffset };
+}
+
 function comparePosition(selection) {
   let { anchorNode, focusNode, anchorOffset, focusOffset } = selection;
   let headNode, tailNode, headOffset, tailOffset, direction;
@@ -157,5 +204,6 @@ function comparePosition(selection) {
 export {
   clearSelection,
   comparePosition,
-  findOffsetInNode
+  findOffsetInNode,
+  constrainSelectionTo
 };