Skip to content

Commit

Permalink
Constrain selection to editor element when probing for range
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bantic committed Mar 15, 2017
1 parent e402baa commit 63142e5
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 2 deletions.
5 changes: 4 additions & 1 deletion src/js/utils/cursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };

Expand Down Expand Up @@ -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
Expand Down
50 changes: 49 additions & 1 deletion src/js/utils/selection-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -157,5 +204,6 @@ function comparePosition(selection) {
export {
clearSelection,
comparePosition,
findOffsetInNode
findOffsetInNode,
constrainSelectionTo
};

0 comments on commit 63142e5

Please sign in to comment.