Bug 1798207 - Use shadow-including tree order to sort selection ranges. r=smaug

So that painting code doesn't get confused when trying to paint
selections that span across shadow boundaries.

Differential Revision: https://phabricator.services.mozilla.com/D160787

dom/base/Selection.cpp

@@ -692,17 +692,10 @@ static int32_t CompareToRangeStart(const nsINode& aCompareNode,
                                    const nsRange& aRange) {
   MOZ_ASSERT(aRange.GetStartContainer());
   nsINode* start = aRange.GetStartContainer();
-  // If the nodes that we're comparing are not in the same document or in the
-  // same subtree, assume that aCompareNode will fall at the end of the ranges.
-  // NOTE(emilio): This is broken (bug 1590379). When fixed, shadow-including
-  // tree order[1] seems the most reasonable order, but if we choose other order
-  // than that code in nsPrintJob.cpp to deal with selection printing might need
-  // to be fixed.
-  //
-  // [1]: https://dom.spec.whatwg.org/#concept-shadow-including-tree-order
+  // If the nodes that we're comparing are not in the same document, assume that
+  // aCompareNode will fall at the end of the ranges.
   if (aCompareNode.GetComposedDoc() != start->GetComposedDoc() ||
-      !start->GetComposedDoc() ||
-      aCompareNode.SubtreeRoot() != start->SubtreeRoot()) {
+      !start->GetComposedDoc()) {
     NS_WARNING(
         "`CompareToRangeStart` couldn't compare nodes, pretending some order.");
     return 1;
@@ -721,8 +714,7 @@ static int32_t CompareToRangeEnd(const nsINode& aCompareNode,
   // If the nodes that we're comparing are not in the same document or in the
   // same subtree, assume that aCompareNode will fall at the end of the ranges.
   if (aCompareNode.GetComposedDoc() != end->GetComposedDoc() ||
-      !end->GetComposedDoc() ||
-      aCompareNode.SubtreeRoot() != end->SubtreeRoot()) {
+      !end->GetComposedDoc()) {
     NS_WARNING(
         "`CompareToRangeEnd` couldn't compare nodes, pretending some order.");
     return 1;
@@ -774,11 +766,18 @@ size_t Selection::StyledRanges::FindInsertionPoint(
 nsresult Selection::StyledRanges::SubtractRange(
     StyledRange& aRange, nsRange& aSubtract, nsTArray<StyledRange>* aOutput) {
   nsRange* range = aRange.mRange;
-
   if (NS_WARN_IF(!range->IsPositioned())) {
     return NS_ERROR_UNEXPECTED;
   }
 
+  if (range->GetStartContainer()->SubtreeRoot() !=
+      aSubtract.GetStartContainer()->SubtreeRoot()) {
+    // These are ranges for different shadow trees, we can't subtract them in
+    // any sensible way.
+    aOutput->InsertElementAt(0, aRange);
+    return NS_OK;
+  }
+
   // First we want to compare to the range start
   int32_t cmp{CompareToRangeStart(*range->GetStartContainer(),
                                   range->StartOffset(), aSubtract)};
@@ -1041,13 +1040,11 @@ nsresult Selection::StyledRanges::MaybeAddRangeAndTruncateOverlaps(
   if (maybeEndIndex.isNothing()) {
     // All ranges start after the given range. We can insert our range at
     // position 0, knowing there are no overlaps (handled below)
-    startIndex = 0;
-    endIndex = 0;
+    startIndex = endIndex = 0;
   } else if (maybeStartIndex.isNothing()) {
     // All ranges end before the given range. We can insert our range at
     // the end of the array, knowing there are no overlaps (handled below)
-    startIndex = mRanges.Length();
-    endIndex = startIndex;
+    startIndex = endIndex = mRanges.Length();
   } else {
     startIndex = *maybeStartIndex;
     endIndex = *maybeEndIndex;
@@ -1076,16 +1073,11 @@ nsresult Selection::StyledRanges::MaybeAddRangeAndTruncateOverlaps(
   // the two end points, startIndex and endIndex - 1 (which may point to the
   // same range) as these may partially overlap the new range. Any ranges
   // between these indices are fully overlapped by the new range, and so can be
-  // removed
-  nsTArray<StyledRange> overlaps;
-  // XXX(Bug 1631371) Check if this should use a fallible operation as it
-  // pretended earlier.
-  overlaps.InsertElementAt(0, mRanges[startIndex]);
-
+  // removed.
+  AutoTArray<StyledRange, 2> overlaps;
+  overlaps.AppendElement(mRanges[startIndex]);
   if (endIndex - 1 != startIndex) {
-    // XXX(Bug 1631371) Check if this should use a fallible operation as it
-    // pretended earlier.
-    overlaps.InsertElementAt(1, mRanges[endIndex - 1]);
+    overlaps.AppendElement(mRanges[endIndex - 1]);
   }
 
   // Remove all the overlapping ranges
@@ -1094,7 +1086,7 @@ nsresult Selection::StyledRanges::MaybeAddRangeAndTruncateOverlaps(
   }
   mRanges.RemoveElementsAt(startIndex, endIndex - startIndex);
 
-  nsTArray<StyledRange> temp;
+  AutoTArray<StyledRange, 3> temp;
   for (const size_t i : Reversed(IntegerRange(overlaps.Length()))) {
     nsresult rv = SubtractRange(overlaps[i], *aRange, &temp);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -1106,13 +1098,9 @@ nsresult Selection::StyledRanges::MaybeAddRangeAndTruncateOverlaps(
                                            aRange->StartOffset(),
                                            CompareToRangeStart)};
 
-  // XXX(Bug 1631371) Check if this should use a fallible operation as it
-  // pretended earlier.
   temp.InsertElementAt(insertionPoint, StyledRange(aRange));
 
   // Merge the leftovers back in to mRanges
-  // XXX(Bug 1631371) Check if this should use a fallible operation as it
-  // pretended earlier.
   mRanges.InsertElementsAt(startIndex, temp);
 
   for (uint32_t i = 0; i < temp.Length(); ++i) {

dom/base/nsContentUtils.cpp

@@ -2857,11 +2857,11 @@ int32_t nsContentUtils::ComparePoints_Deprecated(
   const nsINode* node2 = aParent2;
   do {
     parents1.AppendElement(node1);
-    node1 = node1->GetParentNode();
+    node1 = node1->GetParentOrShadowHostNode();
   } while (node1);
   do {
     parents2.AppendElement(node2);
-    node2 = node2->GetParentNode();
+    node2 = node2->GetParentOrShadowHostNode();
   } while (node2);
 
   uint32_t pos1 = parents1.Length() - 1;
@@ -2883,6 +2883,15 @@ int32_t nsContentUtils::ComparePoints_Deprecated(
     const nsINode* child1 = parents1.ElementAt(--pos1);
     const nsINode* child2 = parents2.ElementAt(--pos2);
     if (child1 != child2) {
+      if (MOZ_UNLIKELY(child1->IsShadowRoot())) {
+        // Shadow roots come before light DOM per
+        // https://dom.spec.whatwg.org/#concept-shadow-including-tree-order
+        MOZ_ASSERT(!child2->IsShadowRoot(), "Two shadow roots?");
+        return -1;
+      }
+      if (MOZ_UNLIKELY(child2->IsShadowRoot())) {
+        return 1;
+      }
       const Maybe<uint32_t> child1Index =
           aParent1Cache ? aParent1Cache->ComputeIndexOf(parent, child1)
                         : parent->ComputeIndexOf(child1);

dom/base/nsRange.cpp

@@ -730,15 +730,15 @@ void nsRange::ParentChainChanged(nsIContent* aContent) {
 
 bool nsRange::IsPointComparableToRange(const nsINode& aContainer,
                                        uint32_t aOffset,
-                                       ErrorResult& aErrorResult) const {
+                                       ErrorResult& aRv) const {
   // our range is in a good state?
   if (!mIsPositioned) {
-    aErrorResult.Throw(NS_ERROR_NOT_INITIALIZED);
+    aRv.Throw(NS_ERROR_NOT_INITIALIZED);
     return false;
   }
 
   if (!aContainer.IsInclusiveDescendantOf(mRoot)) {
-    aErrorResult.Throw(NS_ERROR_DOM_WRONG_DOCUMENT_ERR);
+    aRv.ThrowWrongDocumentError("Node is not in the same document as the range");
     return false;
   }
 
@@ -747,17 +747,18 @@ bool nsRange::IsPointComparableToRange(const nsINode& aContainer,
                "Start and end of a range must be either both native anonymous "
                "content or not.");
   if (aContainer.ChromeOnlyAccess() != chromeOnlyAccess) {
-    aErrorResult.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR);
+    aRv.ThrowInvalidNodeTypeError(
+        "Trying to compare restricted with unrestricted nodes");
     return false;
   }
 
   if (aContainer.NodeType() == nsINode::DOCUMENT_TYPE_NODE) {
-    aErrorResult.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR);
+    aRv.ThrowInvalidNodeTypeError("Trying to compare with a document");
     return false;
   }
 
   if (aOffset > aContainer.Length()) {
-    aErrorResult.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR);
+    aRv.ThrowInvalidNodeTypeError("Offset is out of bounds");
     return false;
   }
 
@@ -814,6 +815,10 @@ bool nsRange::IntersectsNode(nsINode& aNode, ErrorResult& aRv) {
     return false;
   }
 
+  if (!IsPointComparableToRange(*parent, *nodeIndex, IgnoreErrors())) {
+    return false;
+  }
+
   const Maybe<int32_t> startOrder = nsContentUtils::ComparePoints(
       mStart.Container(),
       *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets), parent,

layout/reftests/selection/reftest.list

@@ -55,3 +55,6 @@ needs-focus fuzzy-if(!nativeThemePref,0-5,0-1) == 1478604.html 1478604-ref.html
 
 needs-focus fuzzy-if(!nativeThemePref,0-3,0-13) == disabled-1.html disabled-1-notref.html
 needs-focus != disabled-2.html disabled-2-notref.html
+
+== shadow-tree-order-1.html shadow-tree-order-1-ref.html
+!= shadow-tree-order-1.html shadow-tree-order-1-notref.html

layout/reftests/selection/shadow-tree-order-1-notref.html

@@ -0,0 +1,4 @@
+<!doctype html>
+<p>Something to <span>find</span> above</p>
+<p>Something to <span>find</span> in the shadow</p>
+<p>Something to <span>find</span> below</p>

layout/reftests/selection/shadow-tree-order-1-ref.html

@@ -0,0 +1,13 @@
+<!doctype html>
+<p>Something to <span>find</span> above</p>
+<p>Something to <span>find</span> in the shadow</p>
+<p>Something to <span>find</span> below</p>
+<script>
+  let selection = getSelection();
+  selection.removeAllRanges();
+  for (let span of document.querySelectorAll("span")) {
+    let range = document.createRange();
+    range.selectNode(span);
+    selection.addRange(range);
+  }
+</script>

layout/reftests/selection/shadow-tree-order-1.html

@@ -0,0 +1,18 @@
+<!doctype html>
+<p id="above">Something to <span>find</span> above</p>
+<p id="host"></p>
+<p id="below">Something to <span>find</span> below</p>
+<script>
+  document.getElementById("host").attachShadow({ mode: "open" }).innerHTML = `
+    Something to <span>find</span> in the shadow
+  `.trim();
+  let selection = getSelection();
+  selection.removeAllRanges();
+  for (let id of ["above", "host", "below"]) {
+    let element = document.getElementById(id);
+    let span = (element.shadowRoot || element).querySelector("span");
+    let range = document.createRange();
+    range.selectNode(span);
+    selection.addRange(range);
+  }
+</script>

layout/reftests/webcomponents/cross-tree-selection-1-ref.html

@@ -1,8 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<body>
-<div>
-Hello World
-</div>
-</body>
-</html>

layout/reftests/webcomponents/cross-tree-selection-1.html

@@ -1,32 +0,0 @@
-<!DOCTYPE HTML>
-<html class="reftest-wait">
-<head>
-  <script>
-    function tweak() {
-      var host = document.getElementById("host");
-      var shadow = host.attachShadow({ mode: "open" });
-
-      var textNode = document.createTextNode(" World");
-      shadow.appendChild(textNode);
-
-      // Create a selection with focus preceeding anchor
-      var selection = window.getSelection();
-      var range = document.createRange();
-      range.setStart(shadow, 1);
-      range.setEnd(shadow, 1);
-      selection.addRange(range);
-      selection.extend(shadow, 0);
-
-      // Extend selection into a different node tree
-      // (from ShadowRoot into the previous node in the parent node tree).
-      setTimeout(function() {
-        selection.extend(document.getElementById("previous"), 0);
-        document.documentElement.className = '';
-      }, 100);
-    }
-  </script>
-</head>
-<body onload="tweak()">
-<span id="previous">Hello</span><span id="host"></span>
-</body>
-</html>

layout/reftests/webcomponents/reftest.list

@@ -1,4 +1,3 @@
-== cross-tree-selection-1.html cross-tree-selection-1-ref.html
 == basic-shadow-1.html basic-shadow-1-ref.html
 == basic-shadow-2.html basic-shadow-2-ref.html
 == basic-shadow-3.html basic-shadow-3-ref.html

testing/web-platform/tests/dom/ranges/Range-intersectsNode-shadow.html

@@ -0,0 +1,19 @@
+<!doctype html>
+<title>Range.intersectsNode with Shadow DOM</title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="host"></div>
+<script>
+test(() => {
+  const host = document.getElementById("host");
+  host.attachShadow({ mode: "open" }).innerHTML = `<span>ABC</span>`;
+
+  const range = document.createRange();
+  range.selectNode(document.body);
+
+  assert_true(range.intersectsNode(host), "Should intersect host");
+  assert_false(range.intersectsNode(host.shadowRoot), "Should not intersect shadow root");
+  assert_false(range.intersectsNode(host.shadowRoot.firstElementChild), "Should not intersect shadow span");
+}, "Range.intersectsNode() doesn't return true for shadow children in other trees");
+</script>
+