From 01bac70fb944c2bb2dcad756ac8dfdba7b6cb372 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Mon, 10 Feb 2025 01:29:40 +0000 Subject: [PATCH] Bug 1945711 - part 4: Rename the `aParent`/`aOffset` version of `nsContentUtils::ComparePoints` to `ComparePointsWithIndices` r=jjaschke,dom-core To make it easier to find the callers, we should give different name instead of overloading. Differential Revision: https://phabricator.services.mozilla.com/D236794 --- dom/base/RangeUtils.cpp | 2 +- dom/base/Selection.cpp | 15 ++++---- dom/base/nsContentUtils.cpp | 14 ++++---- dom/base/nsContentUtils.h | 37 +++++++++++++++----- dom/base/nsRange.cpp | 8 ++--- editor/spellchecker/TextServicesDocument.cpp | 16 ++++----- 6 files changed, 56 insertions(+), 36 deletions(-) diff --git a/dom/base/RangeUtils.cpp b/dom/base/RangeUtils.cpp index 8f21b3e0c8b6..5198e3c8fc4f 100644 --- a/dom/base/RangeUtils.cpp +++ b/dom/base/RangeUtils.cpp @@ -217,7 +217,7 @@ nsresult RangeUtils::CompareNodeToRangeBoundaries( } *aNodeIsBeforeRange = *order > 0; // is RANGE(end) >= NODE(end) ? - order = nsContentUtils::ComparePoints( + order = nsContentUtils::ComparePointsWithIndices( aEndBoundary.Container(), *aEndBoundary.Offset( RangeBoundaryBase::OffsetFilter::kValidOrInvalidOffsets), diff --git a/dom/base/Selection.cpp b/dom/base/Selection.cpp index dcee19218ded..b7540b60ba7f 100644 --- a/dom/base/Selection.cpp +++ b/dom/base/Selection.cpp @@ -3082,14 +3082,17 @@ void Selection::Extend(nsINode& aContainer, uint32_t aOffset, const uint32_t endOffset = range->MayCrossShadowBoundaryEndOffset(); bool shouldClearRange = false; - const Maybe anchorOldFocusOrder = nsContentUtils::ComparePoints( - anchorNode, anchorOffset, focusNode, focusOffset); + const Maybe anchorOldFocusOrder = + nsContentUtils::ComparePointsWithIndices(anchorNode, anchorOffset, + focusNode, focusOffset); shouldClearRange |= !anchorOldFocusOrder; - const Maybe oldFocusNewFocusOrder = nsContentUtils::ComparePoints( - focusNode, focusOffset, &aContainer, aOffset); + const Maybe oldFocusNewFocusOrder = + nsContentUtils::ComparePointsWithIndices(focusNode, focusOffset, + &aContainer, aOffset); shouldClearRange |= !oldFocusNewFocusOrder; - const Maybe anchorNewFocusOrder = nsContentUtils::ComparePoints( - anchorNode, anchorOffset, &aContainer, aOffset); + const Maybe anchorNewFocusOrder = + nsContentUtils::ComparePointsWithIndices(anchorNode, anchorOffset, + &aContainer, aOffset); shouldClearRange |= !anchorNewFocusOrder; // If the points are disconnected, the range will be collapsed below, diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 5c9a2d4eafd8..c2f2285aec4a 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -3388,11 +3388,9 @@ Maybe nsContentUtils::CompareChildNodeAndChildOffset( } /* static */ -Maybe nsContentUtils::ComparePoints(const nsINode* aParent1, - uint32_t aOffset1, - const nsINode* aParent2, - uint32_t aOffset2, - NodeIndexCache* aIndexCache) { +Maybe nsContentUtils::ComparePointsWithIndices( + const nsINode* aParent1, uint32_t aOffset1, const nsINode* aParent2, + uint32_t aOffset2, NodeIndexCache* aIndexCache) { MOZ_ASSERT(aParent1); MOZ_ASSERT(aParent2); @@ -3555,9 +3553,9 @@ Maybe nsContentUtils::ComparePoints( // RangeBoundaryBase instances may be initialized only with a child node or an // offset in the container. If both instances have computed offset, we can - // use the other ComparePoints() which works with offsets. + // use ComparePointsWithIndices() which works with offsets. if (aBoundary1.HasOffset() && aBoundary2.HasOffset()) { - return ComparePoints( + return ComparePointsWithIndices( aBoundary1.Container(), *aBoundary1.Offset(kValidOrInvalidOffsets1), aBoundary2.Container(), *aBoundary2.Offset(kValidOrInvalidOffsets2), aIndexCache); @@ -3581,7 +3579,7 @@ Maybe nsContentUtils::ComparePoints( // Otherwise, we need to compare the common ancestor children which is the // most distant different inclusive ancestors of the containers. So, the - // following implementation is similar to the other ComparePoints(), but we + // following implementation is similar to ComparePointsWithIndices(), but we // don't have offset, so, we cannot use offset when we compare the boundaries // whose one is a descendant of the other. const CommonAncestors commonAncestors(*aBoundary1.Container(), diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 5ff1503bdc83..f9c6c284fd94 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -688,17 +688,28 @@ class nsContentUtils { /** * Utility routine to compare two "points", where a point is a node/offset * pair. - * Pass a cache object as aParent1Cache if you expect to repeatedly - * call this function with the same value as aParent1. + * Pass a cache object as aIndexCache if you expect to repeatedly + * call this function with the same value as aParent1 or aParent2. * * @return -1 if point1 < point2, * 1 if point1 > point2, * 0 if point1 == point2. * `Nothing` if the two nodes aren't in the same connected subtree. */ - static mozilla::Maybe ComparePoints( + static mozilla::Maybe ComparePointsWithIndices( const nsINode* aParent1, uint32_t aOffset1, const nsINode* aParent2, uint32_t aOffset2, NodeIndexCache* aIndexCache = nullptr); + + /** + * Utility routine to compare two "points", where a point is a RangeBoundary. + * Pass a cache object as aIndexCache if you expect to repeatedly call this + * function with the same value as aBoundary1 or aBoundary2. + * + * @return -1 if point1 < point2, + * 1 if point1 > point2, + * 0 if point1 == point2. + * `Nothing` if the two nodes aren't in the same connected subtree. + */ template static mozilla::Maybe ComparePoints( const mozilla::RangeBoundaryBase& aBoundary1, @@ -737,12 +748,19 @@ class nsContentUtils { } // Otherwise, aOffset1 nor aOffset2 is referred so that any value is fine // if negative. - return ComparePoints( - aParent1, aOffset1 < 0 ? UINT32_MAX : static_cast(aOffset1), + return ComparePointsWithIndices( + aParent1, + // Avoid warnings. + aOffset1 < 0 ? aParent1->GetChildCount() + : std::min(static_cast(aOffset1), + aParent1->GetChildCount()), aParent2, - aOffset2 < 0 ? UINT32_MAX : static_cast(aOffset2)); + // Avoid warnings. + aOffset2 < 0 ? aParent2->GetChildCount() + : std::min(static_cast(aOffset2), + aParent2->GetChildCount())); } - return ComparePoints(aParent1, aOffset1, aParent2, aOffset2); + return ComparePointsWithIndices(aParent1, aOffset1, aParent2, aOffset2); } /** @@ -3585,8 +3603,9 @@ class nsContentUtils { NodeIndexCache* aIndexCache = nullptr); /** - * Helper method for ComparePoints(). This includes odd traditional behavior. - * Therefore, do not use this method as a utility method. + * Helper method for ComparePoints() and ComparePointsWithIndices(). This + * includes odd traditional behavior. Therefore, do not use this method as a + * utility method. */ static mozilla::Maybe CompareClosestCommonAncestorChildren( const nsINode&, const nsINode*, const nsINode*, NodeIndexCache*); diff --git a/dom/base/nsRange.cpp b/dom/base/nsRange.cpp index e17cf6297798..eca980cc4c93 100644 --- a/dom/base/nsRange.cpp +++ b/dom/base/nsRange.cpp @@ -1836,10 +1836,10 @@ void nsRange::CutContents(DocumentFragment** aFragment, ErrorResult& aRv) { // has a common ancestor with start and end, hence both have to be // comparable to it. if (doctype && - *nsContentUtils::ComparePoints(startContainer, startOffset, doctype, - 0) < 0 && - *nsContentUtils::ComparePoints(doctype, 0, endContainer, endOffset) < - 0) { + *nsContentUtils::ComparePointsWithIndices(startContainer, startOffset, + doctype, 0) < 0 && + *nsContentUtils::ComparePointsWithIndices(doctype, 0, endContainer, + endOffset) < 0) { aRv.ThrowHierarchyRequestError("Start or end position isn't valid."); return; } diff --git a/editor/spellchecker/TextServicesDocument.cpp b/editor/spellchecker/TextServicesDocument.cpp index ad1e3ca03769..13aec08eb19b 100644 --- a/editor/spellchecker/TextServicesDocument.cpp +++ b/editor/spellchecker/TextServicesDocument.cpp @@ -1832,9 +1832,9 @@ nsresult TextServicesDocument::GetCollapsedSelection( uint32_t offset = range->StartOffset(); - const Maybe e1s1 = nsContentUtils::ComparePoints( + const Maybe e1s1 = nsContentUtils::ComparePointsWithIndices( eStart->mTextNode, eStartOffset, parent, offset); - const Maybe e2s1 = nsContentUtils::ComparePoints( + const Maybe e2s1 = nsContentUtils::ComparePointsWithIndices( eEnd->mTextNode, eEndOffset, parent, offset); if (MOZ_UNLIKELY(NS_WARN_IF(!e1s1) || NS_WARN_IF(!e2s1))) { @@ -2029,14 +2029,14 @@ nsresult TextServicesDocument::GetUncollapsedSelection( NS_ENSURE_SUCCESS(rv, rv); - e1s2 = nsContentUtils::ComparePoints(eStart->mTextNode, eStartOffset, - endContainer, endOffset); + e1s2 = nsContentUtils::ComparePointsWithIndices( + eStart->mTextNode, eStartOffset, endContainer, endOffset); if (NS_WARN_IF(!e1s2)) { return NS_ERROR_FAILURE; } - e2s1 = nsContentUtils::ComparePoints(eEnd->mTextNode, eEndOffset, - startContainer, startOffset); + e2s1 = nsContentUtils::ComparePointsWithIndices( + eEnd->mTextNode, eEndOffset, startContainer, startOffset); if (NS_WARN_IF(!e2s1)) { return NS_ERROR_FAILURE; } @@ -2057,13 +2057,13 @@ nsresult TextServicesDocument::GetUncollapsedSelection( } // Now that we have an intersecting range, find out more info: - const Maybe e1s1 = nsContentUtils::ComparePoints( + const Maybe e1s1 = nsContentUtils::ComparePointsWithIndices( eStart->mTextNode, eStartOffset, startContainer, startOffset); if (NS_WARN_IF(!e1s1)) { return NS_ERROR_FAILURE; } - const Maybe e2s2 = nsContentUtils::ComparePoints( + const Maybe e2s2 = nsContentUtils::ComparePointsWithIndices( eEnd->mTextNode, eEndOffset, endContainer, endOffset); if (NS_WARN_IF(!e2s2)) { return NS_ERROR_FAILURE;