Optimize and fix a bug in `isNodeInRange` if node had no parent

`isNodeInRange` in range would throw an exception if the passed node had
no parent because `range.selectNode(...)` requires its argument to have
a parent.

This commit rewrites `isNodeInRange` to use `range.comparePoint` which
avoids creating a temporary live range and handle exceptions that
`comparePoint` may throw. It also updates the JSDoc to more accurately
describe what the function does.

src/annotator/range-util.js

@@ -14,41 +14,36 @@ export function isSelectionBackwards(selection) {
 }
 
 /**
- * Returns true if `node` lies within a range.
- *
- * This is a simplified version of `Range.isPointInRange()` for compatibility
- * with IE.
+ * Returns true if any part of `node` lies within `range`.
  *
  * @param {Range} range
  * @param {Node} node
  */
 export function isNodeInRange(range, node) {
-  if (node === range.startContainer || node === range.endContainer) {
-    return true;
+  try {
+    const length = node.nodeValue?.length ?? node.childNodes.length;
+    return (
+      // Check start of node is before end of range.
+      range.comparePoint(node, 0) <= 0 &&
+      // Check end of node is after start of range.
+      range.comparePoint(node, length) >= 0
+    );
+  } catch (e) {
+    // `comparePoint` may fail if the `range` and `node` do not share a common
+    // ancestor or `node` is a doctype.
+    return false;
   }
-
-  const nodeRange = /** @type {Document} */ (node.ownerDocument).createRange();
-  nodeRange.selectNode(node);
-  const isAtOrBeforeStart =
-    range.compareBoundaryPoints(Range.START_TO_START, nodeRange) <= 0;
-  const isAtOrAfterEnd =
-    range.compareBoundaryPoints(Range.END_TO_END, nodeRange) >= 0;
-  nodeRange.detach();
-  return isAtOrBeforeStart && isAtOrAfterEnd;
 }
 
 /**
- * Iterate over all Node(s) in `range` in document order and invoke `callback`
- * for each of them.
+ * Iterate over all Node(s) which overlap `range` in document order and invoke
+ * `callback` for each of them.
  *
  * @param {Range} range
  * @param {(n: Node) => any} callback
  */
 function forEachNodeInRange(range, callback) {
   const root = range.commonAncestorContainer;
-
-  // The `whatToShow`, `filter` and `expandEntityReferences` arguments are
-  // mandatory in IE although optional according to the spec.
   const nodeIter = /** @type {Document} */ (root.ownerDocument).createNodeIterator(
     root,
     NodeFilter.SHOW_ALL
@@ -56,7 +51,6 @@ function forEachNodeInRange(range, callback) {
 
   let currentNode;
   while ((currentNode = nodeIter.nextNode())) {
-    // eslint-disable-line no-cond-assign
     if (isNodeInRange(range, currentNode)) {
       callback(currentNode);
     }

src/annotator/test/range-util-test.js

@@ -44,26 +44,39 @@ describe('annotator.range-util', function () {
     selection.addRange(range);
   }
 
-  describe('#isNodeInRange', function () {
-    it('is true for a node in the range', function () {
-      const rng = createRange(testNode, 0, 1);
-      assert.equal(rangeUtil.isNodeInRange(rng, testNode.firstChild), true);
+  describe('#isNodeInRange', () => {
+    it('returns true for a node in the range', () => {
+      const range = createRange(testNode, 0, 1);
+      assert.isTrue(rangeUtil.isNodeInRange(range, testNode.firstChild));
     });
 
-    it('is false for a node before the range', function () {
+    it('returns false for a node before the range', () => {
       testNode.innerHTML = 'one <b>two</b> three';
-      const rng = createRange(testNode, 1, 2);
-      assert.equal(rangeUtil.isNodeInRange(rng, testNode.firstChild), false);
+      const range = createRange(testNode, 1, 2);
+      assert.isFalse(rangeUtil.isNodeInRange(range, testNode.firstChild));
     });
 
-    it('is false for a node after the range', function () {
+    it('returns false for a node after the range', () => {
       testNode.innerHTML = 'one <b>two</b> three';
-      const rng = createRange(testNode, 1, 2);
-      assert.equal(
-        rangeUtil.isNodeInRange(rng, testNode.childNodes.item(2)),
-        false
+      const range = createRange(testNode, 1, 2);
+      assert.isFalse(
+        rangeUtil.isNodeInRange(range, testNode.childNodes.item(2))
       );
     });
+
+    it('can test a node with no parent', () => {
+      const node = document.createElement('span');
+      const range = new Range();
+      range.setStart(node, 0);
+      range.setEnd(node, 0);
+      assert.isTrue(rangeUtil.isNodeInRange(range, node));
+    });
+
+    it('can test a node against an empty range', () => {
+      const node = document.createElement('span');
+      const range = new Range();
+      assert.isFalse(rangeUtil.isNodeInRange(range, node));
+    });
   });
 
   describe('#getTextBoundingBoxes', function () {