From bab72d08e8444d092026d44160c5eff84ccc335f Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 5 Dec 2017 17:50:13 +0900 Subject: [PATCH] Bug 1423097 - part 2: Add overloads of nsRange::SetStart(), nsRange::SetEnd(), nsRange::IsPointInRange() and nsRange::ComparePoint() to use them with RawRangeBoundary r=smaug nsRange::SetStart(), nsRange::SetEnd(), nsRange::IsPointInRange() and nsRange::ComparePoint() take a set of container node and offset in it to specifying a DOM point. However, the caller may not have computed the offset but may know the child node at the point. In such case, they can avoid computing the offset with nsINode::IndexOf() if they have overloads which take RawRangeBoundary. Therefore, this patch implements the overloads and changes the callers in editor. MozReview-Commit-ID: E4DLbAgTTCI --HG-- extra : rebase_source : 8d1632a030f1e0a0dd2b81c3996c19d427e8b0bd --- dom/base/nsRange.cpp | 111 +++++++++++++---------------- dom/base/nsRange.h | 44 ++++++++---- editor/libeditor/HTMLEditRules.cpp | 42 +++++++---- 3 files changed, 105 insertions(+), 92 deletions(-) diff --git a/dom/base/nsRange.cpp b/dom/base/nsRange.cpp index 02ac0772baf1..61b26f3ddbf6 100644 --- a/dom/base/nsRange.cpp +++ b/dom/base/nsRange.cpp @@ -285,7 +285,7 @@ nsRange::~nsRange() NS_ASSERTION(!IsInSelection(), "deleting nsRange that is in use"); // we want the side effects (releases and list removals) - DoSetRange(nullptr, 0, nullptr, 0, nullptr); + DoSetRange(RawRangeBoundary(), RawRangeBoundary(), nullptr); } nsRange::nsRange(nsINode* aNode) @@ -859,14 +859,14 @@ nsRange::IsPointInRange(nsIDOMNode* aContainer, uint32_t aOffset, bool* aResult) } ErrorResult rv; - *aResult = IsPointInRange(*container, aOffset, rv); + *aResult = IsPointInRange(RawRangeBoundary(container, aOffset), rv); return rv.StealNSResult(); } bool -nsRange::IsPointInRange(nsINode& aContainer, uint32_t aOffset, ErrorResult& aRv) +nsRange::IsPointInRange(const RawRangeBoundary& aPoint, ErrorResult& aRv) { - uint16_t compareResult = ComparePoint(aContainer, aOffset, aRv); + uint16_t compareResult = ComparePoint(aPoint, aRv); // If the node isn't in the range's document, it clearly isn't in the range. if (aRv.ErrorCodeIs(NS_ERROR_DOM_WRONG_DOCUMENT_ERR)) { aRv.SuppressException(); @@ -886,42 +886,46 @@ nsRange::ComparePoint(nsIDOMNode* aContainer, uint32_t aOffset, NS_ENSURE_TRUE(container, NS_ERROR_DOM_HIERARCHY_REQUEST_ERR); ErrorResult rv; - *aResult = ComparePoint(*container, aOffset, rv); + *aResult = ComparePoint(RawRangeBoundary(container, aOffset), rv); return rv.StealNSResult(); } int16_t -nsRange::ComparePoint(nsINode& aContainer, uint32_t aOffset, ErrorResult& aRv) +nsRange::ComparePoint(const RawRangeBoundary& aPoint, ErrorResult& aRv) { + if (NS_WARN_IF(!aPoint.IsSet())) { + // FYI: Shouldn't reach this case if it's called by JS. Therefore, it's + // okay to warn. + aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR); + return 0; + } + // our range is in a good state? if (!mIsPositioned) { aRv.Throw(NS_ERROR_NOT_INITIALIZED); return 0; } - if (!nsContentUtils::ContentIsDescendantOf(&aContainer, mRoot)) { + if (!nsContentUtils::ContentIsDescendantOf(aPoint.Container(), mRoot)) { aRv.Throw(NS_ERROR_DOM_WRONG_DOCUMENT_ERR); return 0; } - if (aContainer.NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) { + if (aPoint.Container()->NodeType() == nsIDOMNode::DOCUMENT_TYPE_NODE) { aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR); return 0; } - if (aOffset > aContainer.Length()) { + if (aPoint.Offset() > aPoint.Container()->Length()) { aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR); return 0; } - int32_t cmp = nsContentUtils::ComparePoints(&aContainer, aOffset, - mStart.Container(), - mStart.Offset()); + int32_t cmp = nsContentUtils::ComparePoints(aPoint, mStart.AsRaw()); if (cmp <= 0) { return cmp; } - if (nsContentUtils::ComparePoints(mEnd.Container(), mEnd.Offset(), - &aContainer, aOffset) == -1) { + if (nsContentUtils::ComparePoints(mEnd.AsRaw(), aPoint) == -1) { return 1; } @@ -1378,7 +1382,7 @@ nsRange::SetStart(nsINode& aNode, uint32_t aOffset, ErrorResult& aRv) } AutoInvalidateSelection atEndOfBlock(this); - aRv = SetStart(&aNode, aOffset); + SetStart(RawRangeBoundary(&aNode, aOffset), aRv); } NS_IMETHODIMP @@ -1394,33 +1398,29 @@ nsRange::SetStart(nsIDOMNode* aContainer, uint32_t aOffset) return rv.StealNSResult(); } -/* virtual */ nsresult -nsRange::SetStart(nsINode* aContainer, uint32_t aOffset) +void +nsRange::SetStart(const RawRangeBoundary& aPoint, ErrorResult& aRv) { - nsINode* newRoot = IsValidBoundary(aContainer); + nsINode* newRoot = IsValidBoundary(aPoint.Container()); if (!newRoot) { - return NS_ERROR_DOM_INVALID_NODE_TYPE_ERR; + aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR); + return; } - if (!IsValidOffset(aContainer, aOffset)) { - return NS_ERROR_DOM_INDEX_SIZE_ERR; + if (!aPoint.IsSetAndValid()) { + aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR); + return; } // Collapse if not positioned yet, if positioned in another doc or // if the new start is after end. if (!mIsPositioned || newRoot != mRoot || - nsContentUtils::ComparePoints(aContainer, - static_cast(aOffset), - mEnd.Container(), mEnd.Offset()) == 1) { - DoSetRange(aContainer, aOffset, aContainer, aOffset, newRoot); - - return NS_OK; + nsContentUtils::ComparePoints(aPoint, mEnd.AsRaw()) == 1) { + DoSetRange(aPoint, aPoint, newRoot); + return; } - RawRangeBoundary newStart(aContainer, aOffset); - DoSetRange(newStart, mEnd.AsRaw(), mRoot); - - return NS_OK; + DoSetRange(aPoint, mEnd.AsRaw(), mRoot); } void @@ -1518,7 +1518,7 @@ nsRange::SetEnd(nsINode& aNode, uint32_t aOffset, ErrorResult& aRv) return; } AutoInvalidateSelection atEndOfBlock(this); - aRv = SetEnd(&aNode, aOffset); + SetEnd(RawRangeBoundary(&aNode, aOffset), aRv); } NS_IMETHODIMP @@ -1534,34 +1534,29 @@ nsRange::SetEnd(nsIDOMNode* aContainer, uint32_t aOffset) return rv.StealNSResult(); } -/* virtual */ nsresult -nsRange::SetEnd(nsINode* aContainer, uint32_t aOffset) +void +nsRange::SetEnd(const RawRangeBoundary& aPoint, ErrorResult& aRv) { - nsINode* newRoot = IsValidBoundary(aContainer); + nsINode* newRoot = IsValidBoundary(aPoint.Container()); if (!newRoot) { - return NS_ERROR_DOM_INVALID_NODE_TYPE_ERR; + aRv.Throw(NS_ERROR_DOM_INVALID_NODE_TYPE_ERR); + return; } - if (!IsValidOffset(aContainer, aOffset)) { - return NS_ERROR_DOM_INDEX_SIZE_ERR; + if (!aPoint.IsSetAndValid()) { + aRv.Throw(NS_ERROR_DOM_INDEX_SIZE_ERR); + return; } // Collapse if not positioned yet, if positioned in another doc or // if the new end is before start. if (!mIsPositioned || newRoot != mRoot || - nsContentUtils::ComparePoints(mStart.Container(), - static_cast(mStart.Offset()), - aContainer, - static_cast(aOffset)) == 1) { - DoSetRange(aContainer, aOffset, aContainer, aOffset, newRoot); - - return NS_OK; + nsContentUtils::ComparePoints(mStart.AsRaw(), aPoint) == 1) { + DoSetRange(aPoint, aPoint, newRoot); + return; } - RawRangeBoundary newEnd(aContainer, aOffset); - DoSetRange(mStart.AsRaw(), newEnd, mRoot); - - return NS_OK; + DoSetRange(mStart.AsRaw(), aPoint, mRoot); } void @@ -1642,18 +1637,6 @@ nsRange::SetStartAndEnd(const RawRangeBoundary& aStart, return NS_OK; } -nsresult -nsRange::SetStartAndEnd(nsINode* aStartContainer, uint32_t aStartOffset, - nsINode* aEndContainer, uint32_t aEndOffset) -{ - if (NS_WARN_IF(!aStartContainer) || NS_WARN_IF(!aEndContainer)) { - return NS_ERROR_INVALID_ARG; - } - - return SetStartAndEnd(RawRangeBoundary(aStartContainer, aStartOffset), - RawRangeBoundary(aEndContainer, aEndOffset)); -} - void nsRange::SetEndBeforeJS(nsINode& aNode, ErrorResult& aErr) { @@ -1804,7 +1787,8 @@ nsRange::SelectNode(nsINode& aNode, ErrorResult& aRv) } AutoInvalidateSelection atEndOfBlock(this); - DoSetRange(container, index, container, index + 1, newRoot); + DoSetRange(RawRangeBoundary(container, index), + RawRangeBoundary(container, index + 1), newRoot); } NS_IMETHODIMP @@ -1842,7 +1826,8 @@ nsRange::SelectNodeContents(nsINode& aNode, ErrorResult& aRv) } AutoInvalidateSelection atEndOfBlock(this); - DoSetRange(&aNode, 0, &aNode, aNode.Length(), newRoot); + DoSetRange(RawRangeBoundary(&aNode, 0), + RawRangeBoundary(&aNode, aNode.Length()), newRoot); } // The Subtree Content Iterator only returns subtrees that are diff --git a/dom/base/nsRange.h b/dom/base/nsRange.h index 1aa4f1036c11..f5acdba8853c 100644 --- a/dom/base/nsRange.h +++ b/dom/base/nsRange.h @@ -182,8 +182,18 @@ public: * When you set both start and end of a range, you should use * SetStartAndEnd() instead. */ - nsresult SetStart(nsINode* aContainer, uint32_t aOffset); - nsresult SetEnd(nsINode* aContainer, uint32_t aOffset); + nsresult SetStart(nsINode* aContainer, uint32_t aOffset) + { + ErrorResult error; + SetStart(RawRangeBoundary(aContainer, aOffset), error); + return error.StealNSResult(); + } + nsresult SetEnd(nsINode* aContainer, uint32_t aOffset) + { + ErrorResult error; + SetEnd(RawRangeBoundary(aContainer, aOffset), error); + return error.StealNSResult(); + } already_AddRefed CloneRange() const; @@ -196,7 +206,11 @@ public: * the range will be collapsed at the end point. */ nsresult SetStartAndEnd(nsINode* aStartContainer, uint32_t aStartOffset, - nsINode* aEndContainer, uint32_t aEndOffset); + nsINode* aEndContainer, uint32_t aEndOffset) + { + return SetStartAndEnd(RawRangeBoundary(aStartContainer, aStartOffset), + RawRangeBoundary(aEndContainer, aEndOffset)); + } nsresult SetStartAndEnd(const RawRangeBoundary& aStart, const RawRangeBoundary& aEnd); @@ -290,7 +304,11 @@ public: int16_t CompareBoundaryPoints(uint16_t aHow, nsRange& aOther, ErrorResult& aErr); int16_t ComparePoint(nsINode& aContainer, uint32_t aOffset, - ErrorResult& aErr); + ErrorResult& aErr) + { + return ComparePoint(RawRangeBoundary(&aContainer, aOffset), aErr); + } + int16_t ComparePoint(const RawRangeBoundary& aPoint, ErrorResult& aErr); void DeleteContents(ErrorResult& aRv); already_AddRefed ExtractContents(ErrorResult& aErr); @@ -301,7 +319,11 @@ public: uint32_t GetEndOffset(ErrorResult& aRv) const; void InsertNode(nsINode& aNode, ErrorResult& aErr); bool IntersectsNode(nsINode& aNode, ErrorResult& aRv); - bool IsPointInRange(nsINode& aContainer, uint32_t aOffset, ErrorResult& aErr); + bool IsPointInRange(nsINode& aContainer, uint32_t aOffset, ErrorResult& aErr) + { + return IsPointInRange(RawRangeBoundary(&aContainer, aOffset), aErr); + } + bool IsPointInRange(const RawRangeBoundary& aPoint, ErrorResult& aErr); // *JS() methods are mapped to Range.*() of DOM. // They may move focus only when the range represents normal selection. @@ -329,9 +351,11 @@ public: void SelectNode(nsINode& aNode, ErrorResult& aErr); void SelectNodeContents(nsINode& aNode, ErrorResult& aErr); void SetEnd(nsINode& aNode, uint32_t aOffset, ErrorResult& aErr); + void SetEnd(const RawRangeBoundary& aPoint, ErrorResult& aErr); void SetEndAfter(nsINode& aNode, ErrorResult& aErr); void SetEndBefore(nsINode& aNode, ErrorResult& aErr); void SetStart(nsINode& aNode, uint32_t aOffset, ErrorResult& aErr); + void SetStart(const RawRangeBoundary& aPoint, ErrorResult& aErr); void SetStartAfter(nsINode& aNode, ErrorResult& aErr); void SetStartBefore(nsINode& aNode, ErrorResult& aErr); @@ -469,16 +493,6 @@ protected: const RawRangeBoundary& upperBound, nsINode* aRoot, bool aNotInsertedYet = false); - void DoSetRange(nsINode* aStartContainer, uint32_t aStartOffset, - nsINode* aEndContainer, uint32_t aEndOffset, - nsINode* aRoot, bool aNotInsertedYet = false) - { - RawRangeBoundary start(aStartContainer, aStartOffset); - RawRangeBoundary end(aEndContainer, aEndOffset); - - DoSetRange(start, end, aRoot, aNotInsertedYet); - } - /** * For a range for which IsInSelection() is true, return the common ancestor * for the range, which we had to compute when the common ancestor changed or diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index 8366ff8c1e5d..09173de1f964 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -6116,8 +6116,10 @@ HTMLEditRules::GetNodesForOperation( // The new end parent becomes the parent node of the text. // XXX We want nsRange::SetEnd(const RawRangeBoundary&) EditorRawDOMPoint atContainerOfSplitNode(atEnd.Container()); - range->SetEnd(atContainerOfSplitNode.Container(), - atContainerOfSplitNode.Offset()); + range->SetEnd(atContainerOfSplitNode, error); + if (NS_WARN_IF(error.Failed())) { + error.SuppressException(); + } } } } @@ -6614,8 +6616,11 @@ HTMLEditRules::GetNodesFromPoint( return NS_ERROR_INVALID_ARG; } RefPtr range = new nsRange(aPoint.Container()); - DebugOnly rv = range->SetStart(aPoint.Container(), aPoint.Offset()); - MOZ_ASSERT(NS_SUCCEEDED(rv)); + IgnoredErrorResult error; + range->SetStart(aPoint, error); + if (NS_WARN_IF(error.Failed())) { + MOZ_ASSERT(!error.Failed()); + } // Expand the range to include adjacent inlines PromoteRange(*range, aOperation); @@ -8640,16 +8645,19 @@ HTMLEditRules::ConfirmSelectionInBody() nsresult HTMLEditRules::UpdateDocChangeRange(nsRange* aRange) { + if (NS_WARN_IF(!mHTMLEditor)) { + return NS_ERROR_NOT_AVAILABLE; + } + // first make sure aRange is in the document. It might not be if // portions of our editting action involved manipulating nodes // prior to placing them in the document (e.g., populating a list item // before placing it in its list) - nsCOMPtr startNode = aRange->GetStartContainer(); - if (NS_WARN_IF(!startNode)) { + const RangeBoundary& atStart = aRange->StartRef(); + if (NS_WARN_IF(!atStart.IsSet())) { return NS_ERROR_FAILURE; } - NS_ENSURE_STATE(mHTMLEditor); - if (!mHTMLEditor->IsDescendantOfRoot(startNode)) { + if (!mHTMLEditor->IsDescendantOfRoot(atStart.Container())) { // just return - we don't need to adjust mDocChangeRange in this case return NS_OK; } @@ -8675,8 +8683,11 @@ HTMLEditRules::UpdateDocChangeRange(nsRange* aRange) NS_ENSURE_SUCCESS(rv, rv); // Positive result means mDocChangeRange start is after aRange start. if (result > 0) { - rv = mDocChangeRange->SetStart(startNode, aRange->StartOffset()); - NS_ENSURE_SUCCESS(rv, rv); + ErrorResult error; + mDocChangeRange->SetStart(atStart.AsRaw(), error); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); + } } // compare ends of ranges @@ -8685,12 +8696,15 @@ HTMLEditRules::UpdateDocChangeRange(nsRange* aRange) NS_ENSURE_SUCCESS(rv, rv); // Negative result means mDocChangeRange end is before aRange end. if (result < 0) { - nsINode* endNode = aRange->GetEndContainer(); - if (NS_WARN_IF(!endNode)) { + const RangeBoundary& atEnd = aRange->EndRef(); + if (NS_WARN_IF(!atEnd.IsSet())) { return NS_ERROR_FAILURE; } - rv = mDocChangeRange->SetEnd(endNode, aRange->EndOffset()); - NS_ENSURE_SUCCESS(rv, rv); + ErrorResult error; + mDocChangeRange->SetEnd(atEnd.AsRaw(), error); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); + } } } return NS_OK;