Bug 1587433: part 7.4) Adapt callers of `RangeBoundaryBase::Offset()`. r=smaug
☠☠ backed out by c0c22dbdd5b8 ☠ ☠
authorMirko Brodesser <mbrodesser@mozilla.com>
Wed, 11 Dec 2019 12:26:08 +0000
changeset 507075 f8a7e23843b842bb01c695a0ba16f7d86bd08ec3
parent 507074 f9255884980fc6a89687af026785c672b702aed2
child 507076 79ec11ba7fde31452a85d2f854e9f4831ffb5582
push id36922
push userncsoregi@mozilla.com
push dateMon, 16 Dec 2019 17:21:47 +0000
treeherdermozilla-central@27d0d6cc2131 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1587433
milestone73.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1587433: part 7.4) Adapt callers of `RangeBoundaryBase::Offset()`. r=smaug Calls around nsContentUtils::ComparePoint might be strengthened to retrieve only valid offsets in a separate commit. The change in `nsRange::CloneContents` is according to https://dom.spec.whatwg.org/#concept-range-clone. Differential Revision: https://phabricator.services.mozilla.com/D54494
dom/base/nsRange.cpp
--- a/dom/base/nsRange.cpp
+++ b/dom/base/nsRange.cpp
@@ -436,51 +436,57 @@ nsRange::DetermineNewRangeBoundariesAndR
   RawRangeBoundary newStart;
   RawRangeBoundary newEnd;
   nsINode* newRoot = nullptr;
 
   // normalize(), aInfo.mDetails->mNextSibling is the merged text node
   // that will be removed
   nsIContent* removed = aInfo.mDetails->mNextSibling;
   if (removed == mStart.Container()) {
-    CheckedUint32 newStartOffset{mStart.Offset()};
+    CheckedUint32 newStartOffset{
+        *mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets)};
     newStartOffset += aInfo.mChangeStart;
 
     // newStartOffset.isValid() isn't checked explicitly here, because
     // newStartOffset.value() contains an assertion.
     newStart = {aContent, newStartOffset.value()};
     if (MOZ_UNLIKELY(removed == mRoot)) {
       newRoot = RangeUtils::ComputeRootNode(newStart.Container());
     }
   }
   if (removed == mEnd.Container()) {
-    CheckedUint32 newEndOffset{mEnd.Offset()};
+    CheckedUint32 newEndOffset{
+        *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets)};
     newEndOffset += aInfo.mChangeStart;
 
     // newEndOffset.isValid() isn't checked explicitly here, because
     // newEndOffset.value() contains an assertion.
     newEnd = {aContent, newEndOffset.value()};
     if (MOZ_UNLIKELY(removed == mRoot)) {
       newRoot = {RangeUtils::ComputeRootNode(newEnd.Container())};
     }
   }
   // When the removed text node's parent is one of our boundary nodes we may
   // need to adjust the offset to account for the removed node. However,
   // there will also be a ContentRemoved notification later so the only cases
   // we need to handle here is when the removed node is the text node after
   // the boundary.  (The m*Offset > 0 check is an optimization - a boundary
   // point before the first child is never affected by normalize().)
   nsINode* parentNode = aContent->GetParentNode();
-  if (parentNode == mStart.Container() && mStart.Offset() > 0 &&
-      mStart.Offset() < parentNode->GetChildCount() &&
+  if (parentNode == mStart.Container() &&
+      *mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) > 0 &&
+      *mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) <
+          parentNode->GetChildCount() &&
       removed == mStart.GetChildAtOffset()) {
     newStart = {aContent, aInfo.mChangeStart};
   }
-  if (parentNode == mEnd.Container() && mEnd.Offset() > 0 &&
-      mEnd.Offset() < parentNode->GetChildCount() &&
+  if (parentNode == mEnd.Container() &&
+      *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) > 0 &&
+      *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) <
+          parentNode->GetChildCount() &&
       removed == mEnd.GetChildAtOffset()) {
     newEnd = {aContent, aInfo.mChangeEnd};
   }
 
   return {newStart, newEnd, newRoot};
 }
 
 /******************************************************
@@ -506,25 +512,31 @@ void nsRange::CharacterDataChanged(nsICo
 
   if (aInfo.mDetails &&
       aInfo.mDetails->mType == CharacterDataChangeInfo::Details::eSplit) {
     AdjustNextRefsOnCharacterDataSplit(*aContent, aInfo);
   }
 
   // If the changed node contains our start boundary and the change starts
   // before the boundary we'll need to adjust the offset.
-  if (aContent == mStart.Container() && aInfo.mChangeStart < mStart.Offset()) {
+  if (aContent == mStart.Container() &&
+      aInfo.mChangeStart <
+          *mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets)) {
     if (aInfo.mDetails) {
       // splitText(), aInfo->mDetails->mNextSibling is the new text node
       NS_ASSERTION(
           aInfo.mDetails->mType == CharacterDataChangeInfo::Details::eSplit,
           "only a split can start before the end");
-      NS_ASSERTION(mStart.Offset() <= aInfo.mChangeEnd + 1,
-                   "mStart.Offset() is beyond the end of this node");
-      const uint32_t newStartOffset = mStart.Offset() - aInfo.mChangeStart;
+      NS_ASSERTION(
+          *mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) <=
+              aInfo.mChangeEnd + 1,
+          "mStart.Offset() is beyond the end of this node");
+      const uint32_t newStartOffset =
+          *mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) -
+          aInfo.mChangeStart;
       newStart = {aInfo.mDetails->mNextSibling, newStartOffset};
       if (MOZ_UNLIKELY(aContent == mRoot)) {
         newRoot = RangeUtils::ComputeRootNode(newStart.Container());
       }
 
       bool isCommonAncestor =
           IsInSelection() && mStart.Container() == mEnd.Container();
       if (isCommonAncestor) {
@@ -535,62 +547,72 @@ void nsRange::CharacterDataChanged(nsICo
               ->IsDescendantOfCommonAncestorForRangeInSelection()) {
         newStart.Container()
             ->SetDescendantOfCommonAncestorForRangeInSelection();
       }
     } else {
       // If boundary is inside changed text, position it before change
       // else adjust start offset for the change in length.
       CheckedUint32 newStartOffset{0};
-      if (mStart.Offset() <= aInfo.mChangeEnd) {
+      if (*mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) <=
+          aInfo.mChangeEnd) {
         newStartOffset = aInfo.mChangeStart;
       } else {
-        newStartOffset = mStart.Offset();
+        newStartOffset =
+            *mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets);
         newStartOffset -= aInfo.LengthOfRemovedText();
         newStartOffset += aInfo.mReplaceLength;
       }
 
       // newStartOffset.isValid() isn't checked explicitly here, because
       // newStartOffset.value() contains an assertion.
       newStart = {mStart.Container(), newStartOffset.value()};
     }
   }
 
   // Do the same thing for the end boundary, except for splitText of a node
   // with no parent then only switch to the new node if the start boundary
   // did so too (otherwise the range would end up with disconnected nodes).
-  if (aContent == mEnd.Container() && aInfo.mChangeStart < mEnd.Offset()) {
+  if (aContent == mEnd.Container() &&
+      aInfo.mChangeStart <
+          *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets)) {
     if (aInfo.mDetails && (aContent->GetParentNode() || newStart.Container())) {
       // splitText(), aInfo.mDetails->mNextSibling is the new text node
       NS_ASSERTION(
           aInfo.mDetails->mType == CharacterDataChangeInfo::Details::eSplit,
           "only a split can start before the end");
-      MOZ_ASSERT(mEnd.Offset() <= aInfo.mChangeEnd + 1,
-                 "mEnd.Offset() is beyond the end of this node");
-
-      const uint32_t newEndOffset{mEnd.Offset() - aInfo.mChangeStart};
+      MOZ_ASSERT(
+          *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) <=
+              aInfo.mChangeEnd + 1,
+          "mEnd.Offset() is beyond the end of this node");
+
+      const uint32_t newEndOffset{
+          *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) -
+          aInfo.mChangeStart};
       newEnd = {aInfo.mDetails->mNextSibling, newEndOffset};
 
       bool isCommonAncestor =
           IsInSelection() && mStart.Container() == mEnd.Container();
       if (isCommonAncestor && !newStart.Container()) {
         // The split occurs inside the range.
         UnregisterCommonAncestor(mStart.Container(), false);
         RegisterCommonAncestor(mStart.Container()->GetParentNode());
         newEnd.Container()->SetDescendantOfCommonAncestorForRangeInSelection();
       } else if (mEnd.Container()
                      ->IsDescendantOfCommonAncestorForRangeInSelection()) {
         newEnd.Container()->SetDescendantOfCommonAncestorForRangeInSelection();
       }
     } else {
       CheckedUint32 newEndOffset{0};
-      if (mEnd.Offset() <= aInfo.mChangeEnd) {
+      if (*mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets) <=
+          aInfo.mChangeEnd) {
         newEndOffset = aInfo.mChangeStart;
       } else {
-        newEndOffset = mEnd.Offset();
+        newEndOffset =
+            *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets);
         newEndOffset -= aInfo.LengthOfRemovedText();
         newEndOffset += aInfo.mReplaceLength;
       }
 
       // newEndOffset.isValid() isn't checked explicitly here, because
       // newEndOffset.value() contains an assertion.
       newEnd = {mEnd.Container(), newEndOffset.value()};
     }
@@ -877,21 +899,24 @@ bool nsRange::IntersectsNode(nsINode& aN
   }
 
   // Step 5.
   int32_t nodeIndex = parent->ComputeIndexOf(&aNode);
 
   // Steps 6-7.
   // Note: if disconnected is true, ComparePoints returns 1.
   bool disconnected = false;
-  bool result =
-      nsContentUtils::ComparePoints(mStart.Container(), mStart.Offset(), parent,
-                                    nodeIndex + 1, &disconnected) < 0 &&
-      nsContentUtils::ComparePoints(parent, nodeIndex, mEnd.Container(),
-                                    mEnd.Offset(), &disconnected) < 0;
+  bool result = nsContentUtils::ComparePoints(
+                    mStart.Container(),
+                    *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+                    parent, nodeIndex + 1, &disconnected) < 0 &&
+                nsContentUtils::ComparePoints(
+                    parent, nodeIndex, mEnd.Container(),
+                    *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+                    &disconnected) < 0;
 
   // Step 2.
   if (disconnected) {
     result = false;
   }
   return result;
 }
 
@@ -1654,19 +1679,21 @@ nsresult nsRange::CutContents(DocumentFr
 
   // Batch possible DOMSubtreeModified events.
   mozAutoSubtreeModified subtree(mRoot ? mRoot->OwnerDoc() : nullptr, nullptr);
 
   // Save the range end points locally to avoid interference
   // of Range gravity during our edits!
 
   nsCOMPtr<nsINode> startContainer = mStart.Container();
-  uint32_t startOffset = mStart.Offset();
+  uint32_t startOffset =
+      *mStart.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets);
   nsCOMPtr<nsINode> endContainer = mEnd.Container();
-  uint32_t endOffset = mEnd.Offset();
+  uint32_t endOffset =
+      *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOrInvalidOffsets);
 
   if (retval) {
     // For extractContents(), abort early if there's a doctype (bug 719533).
     // This can happen only if the common ancestor is a document, in which case
     // we just need to find its doctype child and check if that's in the range.
     nsCOMPtr<Document> commonAncestorDocument =
         do_QueryInterface(commonAncestor);
     if (commonAncestorDocument) {
@@ -1955,35 +1982,35 @@ int16_t nsRange::CompareBoundaryPoints(u
   }
 
   nsINode *ourNode, *otherNode;
   uint32_t ourOffset, otherOffset;
 
   switch (aHow) {
     case Range_Binding::START_TO_START:
       ourNode = mStart.Container();
-      ourOffset = mStart.Offset();
+      ourOffset = *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets);
       otherNode = aOtherRange.GetStartContainer();
       otherOffset = aOtherRange.StartOffset();
       break;
     case Range_Binding::START_TO_END:
       ourNode = mEnd.Container();
-      ourOffset = mEnd.Offset();
+      ourOffset = *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets);
       otherNode = aOtherRange.GetStartContainer();
       otherOffset = aOtherRange.StartOffset();
       break;
     case Range_Binding::END_TO_START:
       ourNode = mStart.Container();
-      ourOffset = mStart.Offset();
+      ourOffset = *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets);
       otherNode = aOtherRange.GetEndContainer();
       otherOffset = aOtherRange.EndOffset();
       break;
     case Range_Binding::END_TO_END:
       ourNode = mEnd.Container();
-      ourOffset = mEnd.Offset();
+      ourOffset = *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets);
       otherNode = aOtherRange.GetEndContainer();
       otherOffset = aOtherRange.EndOffset();
       break;
     default:
       // We were passed an illegal value
       rv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
       return 0;
   }
@@ -2061,16 +2088,20 @@ already_AddRefed<DocumentFragment> nsRan
   }
 
   // Create a new document fragment in the context of this document,
   // which might be null
 
   RefPtr<DocumentFragment> clonedFrag =
       new DocumentFragment(doc->NodeInfoManager());
 
+  if (Collapsed()) {
+    return clonedFrag.forget();
+  }
+
   nsCOMPtr<nsINode> commonCloneAncestor = clonedFrag.get();
 
   // Create and initialize a subtree iterator that will give
   // us all the subtrees within the range.
 
   RangeSubtreeIterator iter;
 
   aRv = iter.Init(this);
@@ -2092,20 +2123,23 @@ already_AddRefed<DocumentFragment> nsRan
   //
   // Unfortunately these subtrees don't contain the parent hierarchy/context
   // that the Range spec requires us to return. This loop clones the
   // parent hierarchy, adds a cloned version of the subtree, to it, then
   // correctly places this new subtree into the doc fragment.
 
   while (!iter.IsDone()) {
     nsCOMPtr<nsINode> node = iter.GetCurrentNode();
-    bool deepClone = !node->IsElement() ||
-                     (!(node == mEnd.Container() && mEnd.Offset() == 0) &&
-                      !(node == mStart.Container() &&
-                        mStart.Offset() == node->AsElement()->GetChildCount()));
+    bool deepClone =
+        !node->IsElement() ||
+        (!(node == mEnd.Container() &&
+           *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets) == 0) &&
+         !(node == mStart.Container() &&
+           *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets) ==
+               node->AsElement()->GetChildCount()));
 
     // Clone the current subtree!
 
     nsCOMPtr<nsINode> clone = node->CloneNode(deepClone, aRv);
     if (aRv.Failed()) {
       return nullptr;
     }
 
@@ -2116,30 +2150,37 @@ already_AddRefed<DocumentFragment> nsRan
     // XXX_kin: according to the spec.
 
     if (auto charData = CharacterData::FromNode(clone)) {
       if (node == mEnd.Container()) {
         // We only need the data before mEndOffset, so get rid of any
         // data after it.
 
         uint32_t dataLength = charData->Length();
-        if (dataLength > (uint32_t)mEnd.Offset()) {
-          charData->DeleteData(mEnd.Offset(), dataLength - mEnd.Offset(), aRv);
+        if (dataLength >
+            *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets)) {
+          charData->DeleteData(
+              *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+              dataLength -
+                  *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+              aRv);
           if (aRv.Failed()) {
             return nullptr;
           }
         }
       }
 
       if (node == mStart.Container()) {
         // We don't need any data before mStartOffset, so just
         // delete it!
 
-        if (mStart.Offset() > 0) {
-          charData->DeleteData(0, mStart.Offset(), aRv);
+        if (*mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets) > 0) {
+          charData->DeleteData(
+              0, *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+              aRv);
           if (aRv.Failed()) {
             return nullptr;
           }
         }
       }
     }
 
     // Clone the parent hierarchy between commonAncestor and node.
@@ -2447,18 +2488,21 @@ void nsRange::ToString(nsAString& aRetur
     if (textNode) {
 #ifdef DEBUG_range
       // If debug, dump it:
       textNode->List(stdout);
       printf("End Range dump: -----------------------\n");
 #endif /* DEBUG */
 
       // grab the text
-      textNode->SubstringData(mStart.Offset(), mEnd.Offset() - mStart.Offset(),
-                              aReturn, aErr);
+      textNode->SubstringData(
+          *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+          *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets) -
+              *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+          aReturn, aErr);
       return;
     }
   }
 
   /* complex case: mStart.Container() != mEnd.Container(), or mStartParent not a
      text node revisit - there are potential optimizations here and also
      tradeoffs.
   */
@@ -2481,22 +2525,27 @@ void nsRange::ToString(nsAString& aRetur
     // If debug, dump it:
     n->List(stdout);
 #endif /* DEBUG */
     Text* textNode = n->GetAsText();
     if (textNode)  // if it's a text node, get the text
     {
       if (n == mStart.Container()) {  // only include text past start offset
         uint32_t strLength = textNode->Length();
-        textNode->SubstringData(mStart.Offset(), strLength - mStart.Offset(),
-                                tempString, IgnoreErrors());
+        textNode->SubstringData(
+            *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+            strLength -
+                *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+            tempString, IgnoreErrors());
         aReturn += tempString;
       } else if (n ==
                  mEnd.Container()) {  // only include text before end offset
-        textNode->SubstringData(0, mEnd.Offset(), tempString, IgnoreErrors());
+        textNode->SubstringData(
+            0, *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+            tempString, IgnoreErrors());
         aReturn += tempString;
       } else {  // grab the whole kit-n-kaboodle
         textNode->GetData(tempString);
         aReturn += tempString;
       }
     }
   }
 
@@ -2741,67 +2790,75 @@ void nsRange::CollectClientRectsAndText(
           aCollector, aTextList, nsLayoutUtils::RECTS_ACCOUNT_FOR_TRANSFORMS);
     }
   } while (!iter.IsDone());
 }
 
 already_AddRefed<DOMRect> nsRange::GetBoundingClientRect(bool aClampToEdge,
                                                          bool aFlushLayout) {
   RefPtr<DOMRect> rect = new DOMRect(ToSupports(this));
-  if (!mStart.Container()) {
+  if (!mIsPositioned) {
     return rect.forget();
   }
 
   nsLayoutUtils::RectAccumulator accumulator;
-  CollectClientRectsAndText(&accumulator, nullptr, this, mStart.Container(),
-                            mStart.Offset(), mEnd.Container(), mEnd.Offset(),
-                            aClampToEdge, aFlushLayout);
+  CollectClientRectsAndText(
+      &accumulator, nullptr, this, mStart.Container(),
+      *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+      mEnd.Container(),
+      *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets), aClampToEdge,
+      aFlushLayout);
 
   nsRect r = accumulator.mResultRect.IsEmpty() ? accumulator.mFirstRect
                                                : accumulator.mResultRect;
   rect->SetLayoutRect(r);
   return rect.forget();
 }
 
 already_AddRefed<DOMRectList> nsRange::GetClientRects(bool aClampToEdge,
                                                       bool aFlushLayout) {
-  if (!mStart.Container()) {
+  if (!mIsPositioned) {
     return nullptr;
   }
 
   RefPtr<DOMRectList> rectList =
       new DOMRectList(static_cast<AbstractRange*>(this));
 
   nsLayoutUtils::RectListBuilder builder(rectList);
 
-  CollectClientRectsAndText(&builder, nullptr, this, mStart.Container(),
-                            mStart.Offset(), mEnd.Container(), mEnd.Offset(),
-                            aClampToEdge, aFlushLayout);
+  CollectClientRectsAndText(
+      &builder, nullptr, this, mStart.Container(),
+      *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+      mEnd.Container(),
+      *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets), aClampToEdge,
+      aFlushLayout);
   return rectList.forget();
 }
 
 void nsRange::GetClientRectsAndTexts(mozilla::dom::ClientRectsAndTexts& aResult,
                                      ErrorResult& aErr) {
-  if (!mStart.Container()) {
+  if (!mIsPositioned) {
     return;
   }
 
   aResult.mRectList = new DOMRectList(static_cast<AbstractRange*>(this));
 
   nsLayoutUtils::RectListBuilder builder(aResult.mRectList);
 
-  CollectClientRectsAndText(&builder, &aResult.mTextList, this,
-                            mStart.Container(), mStart.Offset(),
-                            mEnd.Container(), mEnd.Offset(), true, true);
+  CollectClientRectsAndText(
+      &builder, &aResult.mTextList, this, mStart.Container(),
+      *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+      mEnd.Container(),
+      *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets), true, true);
 }
 
 nsresult nsRange::GetUsedFontFaces(nsLayoutUtils::UsedFontFaceList& aResult,
                                    uint32_t aMaxRanges,
                                    bool aSkipCollapsedWhitespace) {
-  NS_ENSURE_TRUE(mStart.Container(), NS_ERROR_UNEXPECTED);
+  NS_ENSURE_TRUE(mIsPositioned, NS_ERROR_UNEXPECTED);
 
   nsCOMPtr<nsINode> startContainer = mStart.Container();
   nsCOMPtr<nsINode> endContainer = mEnd.Container();
 
   // Flush out layout so our frames are up to date.
   Document* doc = mStart.Container()->OwnerDoc();
   NS_ENSURE_TRUE(doc, NS_ERROR_UNEXPECTED);
   doc->FlushPendingNotifications(FlushType::Frames);
@@ -2829,28 +2886,30 @@ nsresult nsRange::GetUsedFontFaces(nsLay
     }
     nsIFrame* frame = content->GetPrimaryFrame();
     if (!frame) {
       continue;
     }
 
     if (content->IsText()) {
       if (node == startContainer) {
-        int32_t offset = startContainer == endContainer
-                             ? mEnd.Offset()
-                             : content->AsText()->TextDataLength();
-        nsLayoutUtils::GetFontFacesForText(frame, mStart.Offset(), offset, true,
-                                           aResult, fontFaces, aMaxRanges,
-                                           aSkipCollapsedWhitespace);
+        int32_t offset =
+            startContainer == endContainer
+                ? *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets)
+                : content->AsText()->TextDataLength();
+        nsLayoutUtils::GetFontFacesForText(
+            frame, *mStart.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+            offset, true, aResult, fontFaces, aMaxRanges,
+            aSkipCollapsedWhitespace);
         continue;
       }
       if (node == endContainer) {
-        nsLayoutUtils::GetFontFacesForText(frame, 0, mEnd.Offset(), true,
-                                           aResult, fontFaces, aMaxRanges,
-                                           aSkipCollapsedWhitespace);
+        nsLayoutUtils::GetFontFacesForText(
+            frame, 0, *mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets),
+            true, aResult, fontFaces, aMaxRanges, aSkipCollapsedWhitespace);
         continue;
       }
     }
 
     nsLayoutUtils::GetFontFacesForFrames(frame, aResult, fontFaces, aMaxRanges,
                                          aSkipCollapsedWhitespace);
   }
 
@@ -2904,17 +2963,20 @@ already_AddRefed<nsRange> nsRange::Const
 }
 
 static bool ExcludeIfNextToNonSelectable(nsIContent* aContent) {
   return aContent->IsText() &&
          aContent->HasFlag(NS_CREATE_FRAME_IF_NON_WHITESPACE);
 }
 
 void nsRange::ExcludeNonSelectableNodes(nsTArray<RefPtr<nsRange>>* aOutRanges) {
-  MOZ_ASSERT(mIsPositioned);
+  if (!mIsPositioned) {
+    MOZ_ASSERT(false);
+    return;
+  }
   MOZ_ASSERT(mEnd.Container());
   MOZ_ASSERT(mStart.Container());
 
   nsRange* range = this;
   RefPtr<nsRange> newRange;
   while (range) {
     PreContentIterator preOrderIter;
     nsresult rv = preOrderIter.Init(range);
@@ -2970,17 +3032,18 @@ void nsRange::ExcludeNonSelectableNodes(
           range->SetStartBefore(*node, err);
           if (err.Failed()) {
             return;
           }
           break;  // restart the same range with a new iterator
         } else {
           // Save the end point before truncating the range.
           nsINode* endContainer = range->mEnd.Container();
-          int32_t endOffset = range->mEnd.Offset();
+          const int32_t endOffset =
+              *range->mEnd.Offset(RangeBoundary::OffsetFilter::kValidOffsets);
 
           // Truncate the current range before the first non-selectable node.
           range->SetEndBefore(*firstNonSelectableContent, err);
 
           // Store it in the result (strong ref) - do this before creating
           // a new range in |newRange| below so we don't drop the last ref
           // to the range created in the previous iteration.
           if (!added && !err.Failed()) {