Skip to content

Commit

Permalink
Bug 1740859 - part 2: Make `HTMLEditor::SplitNodeDeepWithTransaction(…
Browse files Browse the repository at this point in the history 
…)` callers omit checking whether the editor is destroyed r=m_kato

It touches the DOM tree only with `SplitNodeTransaction()` and it now returns
`NS_ERROR_EDITOR_DESTROYED` so that the callers don't need to check whether
the editor is destroyed or alive by themselves.

Depends on D131043

Differential Revision: https://phabricator.services.mozilla.com/D131044
  • Loading branch information
@masayuki-nakano
masayuki-nakano committed Nov 15, 2021
1 parent d4ef627 commit fac818f
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 69 deletions.
98 changes: 35 additions & 63 deletions editor/libeditor/HTMLEditSubActionHandler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1862,10 +1862,7 @@ nsresult HTMLEditor::HandleInsertBRElement(const EditorDOMPoint& aPointToBreak,
if (linkNode) {
SplitNodeResult splitLinkNodeResult = SplitNodeDeepWithTransaction(
*linkNode, pointToBreak, SplitAtEdges::eDoNotCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (splitLinkNodeResult.Failed()) {
if (MOZ_UNLIKELY(splitLinkNodeResult.Failed())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction(SplitAtEdges::"
"eDoNotCreateEmptyContainer) failed");
Expand Down Expand Up @@ -2128,10 +2125,7 @@ EditActionResult HTMLEditor::SplitMailCiteElements(

SplitNodeResult splitCiteNodeResult = SplitNodeDeepWithTransaction(
*citeNode, pointToSplit, SplitAtEdges::eDoNotCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return EditActionIgnored(NS_ERROR_EDITOR_DESTROYED);
}
if (splitCiteNodeResult.Failed()) {
if (MOZ_UNLIKELY(splitCiteNodeResult.Failed())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction(SplitAtEdges::"
"eDoNotCreateEmptyContainer) failed");
Expand Down Expand Up @@ -3583,10 +3577,7 @@ nsresult HTMLEditor::FormatBlockContainerWithTransaction(nsAtom& blockType) {
SplitNodeResult splitNodeResult = SplitNodeDeepWithTransaction(
*editableBlockElement, pointToInsertBlock,
SplitAtEdges::eDoNotCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (splitNodeResult.Failed()) {
if (MOZ_UNLIKELY(splitNodeResult.Failed())) {
NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
return splitNodeResult.Rv();
}
Expand Down Expand Up @@ -4950,7 +4941,7 @@ SplitRangeOffFromNodeResult HTMLEditor::SplitRangeOffFromBlock(
SplitNodeResult splitAtStartResult = SplitNodeDeepWithTransaction(
aBlockElement, EditorDOMPoint(&aStartOfMiddleElement),
SplitAtEdges::eDoNotCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
if (MOZ_UNLIKELY(NS_WARN_IF(splitAtStartResult.EditorDestroyed()))) {
return SplitRangeOffFromNodeResult(NS_ERROR_EDITOR_DESTROYED);
}
NS_WARNING_ASSERTION(splitAtStartResult.Succeeded(),
Expand All @@ -4963,7 +4954,7 @@ SplitRangeOffFromNodeResult HTMLEditor::SplitRangeOffFromBlock(
NS_WARNING_ASSERTION(advanced, "Failed to advance offset after the end node");
SplitNodeResult splitAtEndResult = SplitNodeDeepWithTransaction(
aBlockElement, atAfterEnd, SplitAtEdges::eDoNotCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
if (MOZ_UNLIKELY(NS_WARN_IF(splitAtEndResult.EditorDestroyed()))) {
return SplitRangeOffFromNodeResult(NS_ERROR_EDITOR_DESTROYED);
}
NS_WARNING_ASSERTION(splitAtEndResult.Succeeded(),
Expand Down Expand Up @@ -5124,10 +5115,7 @@ nsresult HTMLEditor::CreateStyleForInsertText(
SplitNodeResult splitTextNodeResult = SplitNodeDeepWithTransaction(
MOZ_KnownLive(*pointToPutCaret.GetContainerAsText()), pointToPutCaret,
SplitAtEdges::eAllowToCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (splitTextNodeResult.Failed()) {
if (MOZ_UNLIKELY(splitTextNodeResult.Failed())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction(SplitAtEdges::"
"eAllowToCreateEmptyContainer) failed");
Expand Down Expand Up @@ -6750,16 +6738,13 @@ nsresult HTMLEditor::SplitParentInlineElementsAtRangeEdges(
SplitNodeResult splitEndInlineResult = SplitNodeDeepWithTransaction(
*mostAncestorInlineContentAtEnd, aRangeItem.EndPoint(),
SplitAtEdges::eDoNotCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (splitEndInlineResult.Failed()) {
if (MOZ_UNLIKELY(splitEndInlineResult.Failed())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction(SplitAtEdges::"
"eDoNotCreateEmptyContainer) failed");
return splitEndInlineResult.Rv();
}
if (editingHost != GetActiveEditingHost()) {
if (MOZ_UNLIKELY(editingHost != GetActiveEditingHost())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction(SplitAtEdges::"
"eDoNotCreateEmptyContainer) caused changing editing host");
Expand Down Expand Up @@ -6789,10 +6774,7 @@ nsresult HTMLEditor::SplitParentInlineElementsAtRangeEdges(
SplitNodeResult splitStartInlineResult = SplitNodeDeepWithTransaction(
*mostAncestorInlineContentAtStart, aRangeItem.StartPoint(),
SplitAtEdges::eDoNotCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (splitStartInlineResult.Failed()) {
if (MOZ_UNLIKELY(splitStartInlineResult.Failed())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction(SplitAtEdges::"
"eDoNotCreateEmptyContainer) failed");
Expand All @@ -6802,7 +6784,7 @@ nsresult HTMLEditor::SplitParentInlineElementsAtRangeEdges(
// only start point of aRangeItem. Shouldn't we modify end point here
// if it's collapsed?
EditorRawDOMPoint splitPointAtStart(splitStartInlineResult.SplitPoint());
if (!splitPointAtStart.IsSet()) {
if (MOZ_UNLIKELY(!splitPointAtStart.IsSet())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction(SplitAtEdges::"
"eDoNotCreateEmptyContainer) didn't return split point");
Expand Down Expand Up @@ -6840,20 +6822,17 @@ nsresult HTMLEditor::SplitElementsAtEveryBRElement(
}
SplitNodeResult splitNodeResult = SplitNodeDeepWithTransaction(
*nextContent, atBRNode, SplitAtEdges::eAllowToCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (splitNodeResult.Failed()) {
if (MOZ_UNLIKELY(splitNodeResult.Failed())) {
NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
return splitNodeResult.Rv();
}

// Put previous node at the split point.
if (splitNodeResult.GetPreviousNode()) {
if (nsIContent* previousContent = splitNodeResult.GetPreviousNode()) {
// Might not be a left node. A break might have been at the very
// beginning of inline container, in which case
// SplitNodeDeepWithTransaction() would not actually split anything.
aOutArrayOfContents.AppendElement(*splitNodeResult.GetPreviousNode());
aOutArrayOfContents.AppendElement(*previousContent);
}

// Move break outside of container and also put in node list
Expand Down Expand Up @@ -6905,27 +6884,26 @@ nsresult HTMLEditor::HandleInsertParagraphInHeadingElement(
AutoEditorDOMPointChildInvalidator rememberOnlyOffset(atHeader);
}

// Get ws code to adjust any ws
Result<EditorDOMPoint, nsresult> preparationResult =
WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement(
*this, aPointToSplit, aHeader);
if (preparationResult.isErr()) {
NS_WARNING(
"WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement() failed");
return preparationResult.unwrapErr();
}
EditorDOMPoint pointToSplit = preparationResult.unwrap();
MOZ_ASSERT(pointToSplit.IsInContentNode());
{
// Get ws code to adjust any ws
Result<EditorDOMPoint, nsresult> preparationResult =
WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement(
*this, aPointToSplit, aHeader);
if (preparationResult.isErr()) {
NS_WARNING(
"WhiteSpaceVisibilityKeeper::PrepareToSplitBlockElement() failed");
return preparationResult.unwrapErr();
}
EditorDOMPoint pointToSplit = preparationResult.unwrap();
MOZ_ASSERT(pointToSplit.IsInContentNode());

// Split the header
SplitNodeResult splitHeaderResult = SplitNodeDeepWithTransaction(
aHeader, pointToSplit, SplitAtEdges::eAllowToCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (splitHeaderResult.Failed()) {
NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
return NS_ERROR_FAILURE;
// Split the header
SplitNodeResult splitHeaderResult = SplitNodeDeepWithTransaction(
aHeader, pointToSplit, SplitAtEdges::eAllowToCreateEmptyContainer);
if (MOZ_UNLIKELY(splitHeaderResult.Failed())) {
NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
return NS_ERROR_FAILURE;
}
}

// If the previous heading of split point is empty, put a padding <br>
Expand Down Expand Up @@ -7252,14 +7230,11 @@ nsresult HTMLEditor::SplitParagraph(Element& aParentDivOrP,
// Split the paragraph.
SplitNodeResult splitDivOrPResult = SplitNodeDeepWithTransaction(
aParentDivOrP, pointToSplit, SplitAtEdges::eAllowToCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return NS_ERROR_EDITOR_DESTROYED;
}
if (splitDivOrPResult.Failed()) {
if (MOZ_UNLIKELY(splitDivOrPResult.Failed())) {
NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
return splitDivOrPResult.Rv();
}
if (!splitDivOrPResult.DidSplit()) {
if (MOZ_UNLIKELY(!splitDivOrPResult.DidSplit())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction() didn't split any nodes");
return NS_ERROR_FAILURE;
Expand Down Expand Up @@ -7443,7 +7418,7 @@ nsresult HTMLEditor::HandleInsertParagraphInListItemElement(
// Now split the list item.
SplitNodeResult splitListItemResult = SplitNodeDeepWithTransaction(
aListItem, pointToSplit, SplitAtEdges::eAllowToCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
if (MOZ_UNLIKELY(NS_WARN_IF(splitListItemResult.EditorDestroyed()))) {
return NS_ERROR_EDITOR_DESTROYED;
}
NS_WARNING_ASSERTION(
Expand Down Expand Up @@ -8110,9 +8085,6 @@ SplitNodeResult HTMLEditor::MaybeSplitAncestorsForInsertWithTransaction(
SplitNodeResult splitNodeResult = SplitNodeDeepWithTransaction(
MOZ_KnownLive(*pointToInsert.GetChild()), aStartOfDeepestRightNode,
SplitAtEdges::eAllowToCreateEmptyContainer);
if (NS_WARN_IF(Destroyed())) {
return SplitNodeResult(NS_ERROR_EDITOR_DESTROYED);
}
NS_WARNING_ASSERTION(splitNodeResult.Succeeded(),
"HTMLEditor::SplitNodeDeepWithTransaction(SplitAtEdges::"
"eAllowToCreateEmptyContainer) failed");
Expand Down
4 changes: 2 additions & 2 deletions editor/libeditor/HTMLEditor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1919,9 +1919,9 @@ EditorDOMPoint HTMLEditor::InsertNodeIntoProperAncestorWithTransaction(
SplitNodeResult splitNodeResult =
SplitNodeDeepWithTransaction(MOZ_KnownLive(*pointToInsert.GetChild()),
aPointToInsert, aSplitAtEdges);
if (splitNodeResult.Failed()) {
if (MOZ_UNLIKELY(splitNodeResult.Failed())) {
NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
return EditorDOMPoint();
return EditorDOMPoint(); // TODO: Should return error with `Result`
}
pointToInsert = splitNodeResult.SplitPoint();
MOZ_ASSERT(pointToInsert.IsSet());
Expand Down
2 changes: 1 addition & 1 deletion editor/libeditor/HTMLEditor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2110,7 +2110,7 @@ class HTMLEditor final : public EditorBase,
* be good to insert something if the
* caller want to do it.
*/
MOZ_CAN_RUN_SCRIPT SplitNodeResult
[[nodiscard]] MOZ_CAN_RUN_SCRIPT SplitNodeResult
SplitNodeDeepWithTransaction(nsIContent& aMostAncestorToSplit,
const EditorDOMPoint& aDeepestStartOfRightNode,
SplitAtEdges aSplitAtEdges);
Expand Down
7 changes: 5 additions & 2 deletions editor/libeditor/HTMLEditorDataTransfer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -714,12 +714,12 @@ nsresult HTMLEditor::HTMLWithContextInserter::Run(
.SplitNodeDeepWithTransaction(
MOZ_KnownLive(*pointToInsert.GetContainerAsContent()),
pointToInsert, SplitAtEdges::eAllowToCreateEmptyContainer);
if (splitNodeResult.Failed()) {
if (MOZ_UNLIKELY(splitNodeResult.Failed())) {
NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
return splitNodeResult.Rv();
}
pointToInsert = splitNodeResult.SplitPoint();
if (!pointToInsert.IsSet()) {
if (MOZ_UNLIKELY(!pointToInsert.IsSet())) {
NS_WARNING(
"HTMLEditor::SplitNodeDeepWithTransaction() didn't return split "
"point");
Expand Down Expand Up @@ -1033,6 +1033,9 @@ nsresult HTMLEditor::HTMLWithContextInserter::MoveCaretOutsideOfLink(
.SplitNodeDeepWithTransaction(
aLinkElement, aPointToPutCaret,
SplitAtEdges::eDoNotCreateEmptyContainer);
if (MOZ_UNLIKELY(NS_WARN_IF(splitLinkResult.EditorDestroyed()))) {
return NS_ERROR_EDITOR_DESTROYED;
}
NS_WARNING_ASSERTION(
splitLinkResult.Succeeded(),
"HTMLEditor::SplitNodeDeepWithTransaction() failed, but ignored");
Expand Down
2 changes: 1 addition & 1 deletion editor/libeditor/HTMLStyleEditor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ SplitNodeResult HTMLEditor::SplitAncestorStyledInlineElementsAt(
SplitNodeResult splitNodeResult = SplitNodeDeepWithTransaction(
MOZ_KnownLive(content), result.SplitPoint(),
SplitAtEdges::eAllowToCreateEmptyContainer);
if (splitNodeResult.Failed()) {
if (MOZ_UNLIKELY(splitNodeResult.Failed())) {
NS_WARNING("HTMLEditor::SplitNodeDeepWithTransaction() failed");
return splitNodeResult;
}
Expand Down

0 comments on commit fac818f

Please sign in to comment.