diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 55c191255bd4..15425e9d0ba4 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -1609,39 +1609,39 @@ NS_IMETHODIMP EditorBase::DeleteNode(nsIDOMNode* aNode) { nsCOMPtr node = do_QueryInterface(aNode); - NS_ENSURE_STATE(node); - return DeleteNode(node); + if (NS_WARN_IF(!node)) { + return NS_ERROR_INVALID_ARG; + } + return DeleteNodeWithTransaction(*node); } nsresult -EditorBase::DeleteNode(nsINode* aNode) +EditorBase::DeleteNodeWithTransaction(nsINode& aNode) { - if (NS_WARN_IF(!aNode)) { - return NS_ERROR_INVALID_ARG; - } - AutoRules beginRulesSniffing(this, EditAction::createNode, nsIEditor::ePrevious); if (mRules && mRules->AsHTMLEditRules()) { RefPtr htmlEditRules = mRules->AsHTMLEditRules(); - htmlEditRules->WillDeleteNode(aNode); + htmlEditRules->WillDeleteNode(&aNode); } + // FYI: DeleteNodeTransaction grabs aNode while it's alive. So, it's safe + // to refer aNode even after calling DoTransaction(). RefPtr deleteNodeTransaction = - DeleteNodeTransaction::MaybeCreate(*this, *aNode); + DeleteNodeTransaction::MaybeCreate(*this, aNode); nsresult rv = deleteNodeTransaction ? DoTransaction(deleteNodeTransaction) : NS_ERROR_FAILURE; if (mTextServicesDocument && NS_SUCCEEDED(rv)) { RefPtr textServicesDocument = mTextServicesDocument; - textServicesDocument->DidDeleteNode(aNode); + textServicesDocument->DidDeleteNode(&aNode); } if (!mActionListeners.IsEmpty()) { AutoActionListenerArray listeners(mActionListeners); for (auto& listener : listeners) { - listener->DidDeleteNode(aNode->AsDOMNode(), rv); + listener->DidDeleteNode(aNode.AsDOMNode(), rv); } } @@ -1695,8 +1695,10 @@ EditorBase::ReplaceContainer(Element* aOldContainer, // Move all children from the old container to the new container. while (aOldContainer->HasChildren()) { nsCOMPtr child = aOldContainer->GetFirstChild(); - - nsresult rv = DeleteNode(child); + if (NS_WARN_IF(!child)) { + return nullptr; + } + nsresult rv = DeleteNodeWithTransaction(*child); if (NS_WARN_IF(NS_FAILED(rv))) { return nullptr; } @@ -1719,7 +1721,7 @@ EditorBase::ReplaceContainer(Element* aOldContainer, } // Delete old container. - rv = DeleteNode(aOldContainer); + rv = DeleteNodeWithTransaction(*aOldContainer); if (NS_WARN_IF(NS_FAILED(rv))) { return nullptr; } @@ -1750,7 +1752,10 @@ EditorBase::RemoveContainer(nsIContent* aNode) // Move all children from aNode to its parent. while (aNode->HasChildren()) { nsCOMPtr child = aNode->GetLastChild(); - nsresult rv = DeleteNode(child); + if (NS_WARN_IF(!child)) { + return NS_ERROR_FAILURE; + } + nsresult rv = DeleteNodeWithTransaction(*child); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -1767,7 +1772,11 @@ EditorBase::RemoveContainer(nsIContent* aNode) } } - return DeleteNode(aNode); + nsresult rv = DeleteNodeWithTransaction(*aNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } /** @@ -1814,7 +1823,7 @@ EditorBase::InsertContainerAbove(nsIContent* aNode, AutoInsertContainerSelNotify selNotify(mRangeUpdater); // Put aNode in the new container, first. - nsresult rv = DeleteNode(aNode); + nsresult rv = DeleteNodeWithTransaction(*aNode); if (NS_WARN_IF(NS_FAILED(rv))) { return nullptr; } @@ -1877,14 +1886,14 @@ EditorBase::MoveNode(nsIContent* aNode, } // Hold a reference so aNode doesn't go away when we remove it (bug 772282) - nsCOMPtr kungFuDeathGrip = aNode; - - nsresult rv = DeleteNode(aNode); + nsCOMPtr nodeToBeMoved(aNode); + nsresult rv = DeleteNodeWithTransaction(*nodeToBeMoved); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } - rv = InsertNodeWithTransaction(*aNode, EditorRawDOMPoint(aParent, aOffset)); + rv = InsertNodeWithTransaction(*nodeToBeMoved, + EditorRawDOMPoint(aParent, aOffset)); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -2875,7 +2884,7 @@ EditorBase::InsertTextIntoTextNodeImpl(const nsAString& aStringToInsert, if (isIMETransaction && mComposition) { Text* textNode = mComposition->GetContainerTextNode(); if (textNode && !textNode->Length()) { - DeleteNode(textNode); + DeleteNodeWithTransaction(*textNode); mComposition->OnTextNodeRemoved(); static_cast(transaction.get())->MarkFixed(); } diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h index 482af9708d49..f63538bbf8ec 100644 --- a/editor/libeditor/EditorBase.h +++ b/editor/libeditor/EditorBase.h @@ -335,9 +335,11 @@ public: already_AddRefed DeleteSelectionAndCreateElement(nsAtom& aTag); /** - * Helper routines for node/parent manipulations. + * DeleteNodeWithTransaction() removes aNode from the DOM tree. + * + * @param aNode The node which will be removed form the DOM tree. */ - nsresult DeleteNode(nsINode* aNode); + nsresult DeleteNodeWithTransaction(nsINode& aNode); /** * InsertNodeWithTransaction() inserts aContentToInsert before the child diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index ac8894f8d0e9..72ccbeccdc9e 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -1574,7 +1574,10 @@ HTMLEditRules::WillLoadHTML(Selection* aSelection, if (NS_WARN_IF(!mHTMLEditor)) { return NS_ERROR_UNEXPECTED; } - mHTMLEditor->DeleteNode(mBogusNode); + RefPtr htmlEditor(mHTMLEditor); + DebugOnly rv = htmlEditor->DeleteNodeWithTransaction(*mBogusNode); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), + "Failed to remove the bogus node"); mBogusNode = nullptr; } @@ -2132,7 +2135,7 @@ HTMLEditRules::SplitMailCites(Selection* aSelection, return rv; } if (bEmptyCite) { - rv = htmlEditor->DeleteNode(previousNodeOfSplitPoint); + rv = htmlEditor->DeleteNodeWithTransaction(*previousNodeOfSplitPoint); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -2145,7 +2148,7 @@ HTMLEditRules::SplitMailCites(Selection* aSelection, return rv; } if (bEmptyCite) { - rv = htmlEditor->DeleteNode(citeNode); + rv = htmlEditor->DeleteNodeWithTransaction(*citeNode); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -2367,9 +2370,14 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, // Short circuit for invisible breaks. delete them and recurse. if (visNode->IsHTMLElement(nsGkAtoms::br) && (!mHTMLEditor || !mHTMLEditor->IsVisibleBRElement(visNode))) { - NS_ENSURE_STATE(mHTMLEditor); - rv = mHTMLEditor->DeleteNode(visNode); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(!mHTMLEditor)) { + return NS_ERROR_FAILURE; + } + RefPtr htmlEditor(mHTMLEditor); + rv = htmlEditor->DeleteNodeWithTransaction(*visNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return WillDeleteSelection(aSelection, aAction, aStripWrappers, aCancel, aHandled); } @@ -2430,14 +2438,20 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, if (otherWSType == WSType::br) { // Delete the
- - NS_ENSURE_STATE(mHTMLEditor); - nsCOMPtr otherContent(do_QueryInterface(otherNode)); - rv = WSRunObject::PrepareToDeleteNode(mHTMLEditor, otherContent); - NS_ENSURE_SUCCESS(rv, rv); - NS_ENSURE_STATE(mHTMLEditor); - rv = mHTMLEditor->DeleteNode(otherNode); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(!mHTMLEditor) || + NS_WARN_IF(!otherNode->IsContent())) { + return NS_ERROR_FAILURE; + } + nsIContent* otherContent = otherNode->AsContent(); + RefPtr htmlEditor(mHTMLEditor); + rv = WSRunObject::PrepareToDeleteNode(htmlEditor, otherContent); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + rv = htmlEditor->DeleteNodeWithTransaction(*otherContent); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } return NS_OK; @@ -2445,25 +2459,29 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, // Else continue with normal delete code } + if (NS_WARN_IF(!mHTMLEditor) || + NS_WARN_IF(!visNode->IsContent())) { + return NS_ERROR_FAILURE; + } + RefPtr htmlEditor(mHTMLEditor); // Found break or image, or hr. - NS_ENSURE_STATE(mHTMLEditor); - NS_ENSURE_STATE(visNode->IsContent()); - rv = WSRunObject::PrepareToDeleteNode(mHTMLEditor, visNode->AsContent()); - NS_ENSURE_SUCCESS(rv, rv); + rv = WSRunObject::PrepareToDeleteNode(htmlEditor, visNode->AsContent()); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // Remember sibling to visnode, if any - NS_ENSURE_STATE(mHTMLEditor); - nsCOMPtr sibling = mHTMLEditor->GetPriorHTMLSibling(visNode); + nsCOMPtr sibling = htmlEditor->GetPriorHTMLSibling(visNode); // Delete the node, and join like nodes if appropriate - NS_ENSURE_STATE(mHTMLEditor); - rv = mHTMLEditor->DeleteNode(visNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*visNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // We did something, so let's say so. *aHandled = true; // Is there a prior node and are they siblings? nsCOMPtr stepbrother; if (sibling) { - NS_ENSURE_STATE(mHTMLEditor); - stepbrother = mHTMLEditor->GetNextHTMLSibling(sibling); + stepbrother = htmlEditor->GetNextHTMLSibling(sibling); } // Are they both text nodes? If so, join them! if (startNode == stepbrother && startNode->GetAsText() && @@ -2529,9 +2547,14 @@ HTMLEditRules::WillDeleteSelection(Selection* aSelection, } if (otherNode->IsHTMLElement(nsGkAtoms::br)) { - NS_ENSURE_STATE(mHTMLEditor); - rv = mHTMLEditor->DeleteNode(otherNode); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(!mHTMLEditor)) { + return NS_ERROR_FAILURE; + } + RefPtr htmlEditor(mHTMLEditor); + rv = htmlEditor->DeleteNodeWithTransaction(*otherNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // XXX Only in this case, setting "handled" to true only when it // succeeds? *aHandled = true; @@ -2885,7 +2908,8 @@ HTMLEditRules::DeleteNodeIfCollapsedText(nsINode& aNode) if (!mHTMLEditor->IsVisibleTextNode(*text)) { RefPtr htmlEditor(mHTMLEditor); - htmlEditor->DeleteNode(&aNode); + DebugOnly rv = htmlEditor->DeleteNodeWithTransaction(aNode); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to remove aNode"); } } @@ -3109,7 +3133,7 @@ HTMLEditRules::TryToJoinBlocksWithTransaction(nsIContent& aLeftNode, } // Do br adjustment. - nsCOMPtr brNode = + RefPtr brNode = CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd); EditActionResult ret(NS_OK); if (NS_WARN_IF(mergeLists)) { @@ -3141,7 +3165,8 @@ HTMLEditRules::TryToJoinBlocksWithTransaction(nsIContent& aLeftNode, // atRightBlockChild is now invalid. atRightBlockChild.Clear(); } - if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) { + if (brNode && + NS_SUCCEEDED(htmlEditor->DeleteNodeWithTransaction(*brNode))) { ret.MarkAsHandled(); } return ret; @@ -3185,7 +3210,7 @@ HTMLEditRules::TryToJoinBlocksWithTransaction(nsIContent& aLeftNode, } } // Do br adjustment. - nsCOMPtr brNode = + RefPtr brNode = CheckForInvisibleBR(*leftBlock, BRLocation::beforeBlock, leftBlockChild.Offset()); EditActionResult ret(NS_OK); @@ -3261,7 +3286,8 @@ HTMLEditRules::TryToJoinBlocksWithTransaction(nsIContent& aLeftNode, return ret; } } - if (brNode && NS_SUCCEEDED(htmlEditor->DeleteNode(brNode))) { + if (brNode && + NS_SUCCEEDED(htmlEditor->DeleteNodeWithTransaction(*brNode))) { ret.MarkAsHandled(); } return ret; @@ -3303,9 +3329,10 @@ HTMLEditRules::TryToJoinBlocksWithTransaction(nsIContent& aLeftNode, } } if (brNode) { - rv = htmlEditor->DeleteNode(brNode); - // XXX In other top level if blocks, the result of DeleteNode() - // is ignored. Why does only this result is respected? + rv = htmlEditor->DeleteNodeWithTransaction(*brNode); + // XXX In other top level if blocks, the result of + // DeleteNodeWithTransaction() is ignored. Why does only this result + // is respected? if (NS_WARN_IF(NS_FAILED(rv))) { return ret.SetResult(rv); } @@ -3329,6 +3356,10 @@ HTMLEditRules::MoveBlock(Element& aLeftBlock, return EditActionIgnored(rv); } + if (NS_WARN_IF(!mHTMLEditor)) { + return EditActionIgnored(NS_ERROR_NOT_AVAILABLE); + } + RefPtr htmlEditor(mHTMLEditor); EditActionResult ret(NS_OK); for (uint32_t i = 0; i < arrayOfNodes.Length(); i++) { // get the node to act on @@ -3342,7 +3373,9 @@ HTMLEditRules::MoveBlock(Element& aLeftBlock, if (NS_WARN_IF(!mHTMLEditor)) { return ret.SetResult(NS_ERROR_UNEXPECTED); } - rv = mHTMLEditor->DeleteNode(arrayOfNodes[i]); + rv = htmlEditor->DeleteNodeWithTransaction(*arrayOfNodes[i]); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), + "Failed to remove a block node"); ret.MarkAsHandled(); } else { // Otherwise move the content as is, checking against the DTD. @@ -3396,7 +3429,7 @@ HTMLEditRules::MoveNodeSmart(nsIContent& aNode, } } - nsresult rv = htmlEditor->DeleteNode(&aNode); + nsresult rv = htmlEditor->DeleteNodeWithTransaction(aNode); if (NS_WARN_IF(NS_FAILED(rv))) { return ret.SetResult(rv); } @@ -3431,8 +3464,15 @@ HTMLEditRules::DeleteNonTableElements(nsINode* aNode) { MOZ_ASSERT(aNode); if (!HTMLEditUtils::IsTableElementButNotTable(aNode)) { - NS_ENSURE_STATE(mHTMLEditor); - return mHTMLEditor->DeleteNode(aNode); + if (NS_WARN_IF(!mHTMLEditor)) { + return NS_ERROR_NOT_AVAILABLE; + } + RefPtr htmlEditor(mHTMLEditor); + nsresult rv = htmlEditor->DeleteNodeWithTransaction(*aNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } AutoTArray, 10> childList; @@ -3469,7 +3509,7 @@ HTMLEditRules::DidDeleteSelection(Selection* aSelection, } // find any enclosing mailcite - nsCOMPtr citeNode = + RefPtr citeNode = GetTopEnclosingMailCite(*atStartOfSelection.GetContainer()); if (citeNode) { bool isEmpty = true, seenBR = false; @@ -3479,7 +3519,7 @@ HTMLEditRules::DidDeleteSelection(Selection* aSelection, EditorDOMPoint atCiteNode(citeNode); { AutoEditorDOMPointChildInvalidator lockOffset(atCiteNode); - nsresult rv = htmlEditor->DeleteNode(citeNode); + nsresult rv = htmlEditor->DeleteNodeWithTransaction(*citeNode); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -3572,8 +3612,10 @@ HTMLEditRules::WillMakeList(Selection* aSelection, // if only breaks, delete them if (bOnlyBreaks) { for (auto& node : arrayOfNodes) { - rv = htmlEditor->DeleteNode(node); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*node); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } } @@ -3657,7 +3699,7 @@ HTMLEditRules::WillMakeList(Selection* aSelection, // item. if (htmlEditor->IsEditable(curNode) && (TextEditUtils::IsBreak(curNode) || IsEmptyInline(curNode))) { - rv = htmlEditor->DeleteNode(curNode); + rv = htmlEditor->DeleteNodeWithTransaction(*curNode); NS_ENSURE_SUCCESS(rv, rv); if (TextEditUtils::IsBreak(curNode)) { prevListItem = nullptr; @@ -3965,8 +4007,10 @@ HTMLEditRules::MakeBasicBlock(Selection& aSelection, nsAtom& blockType) htmlEditor->GetNextEditableHTMLNode(pointToInsertBlock); if (brNode && brNode->IsHTMLElement(nsGkAtoms::br)) { AutoEditorDOMPointChildInvalidator lockOffset(pointToInsertBlock); - rv = htmlEditor->DeleteNode(brNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*brNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } // Do the splits! SplitNodeResult splitNodeResult = @@ -3997,8 +4041,10 @@ HTMLEditRules::MakeBasicBlock(Selection& aSelection, nsAtom& blockType) htmlEditor->GetNextEditableHTMLNodeInBlock(pointToInsertBlock); if (brNode && brNode->IsHTMLElement(nsGkAtoms::br)) { AutoEditorDOMPointChildInvalidator lockOffset(pointToInsertBlock); - rv = htmlEditor->DeleteNode(brNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*brNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // We don't need to act on this node any more arrayOfNodes.RemoveElement(brNode); } @@ -4020,8 +4066,10 @@ HTMLEditRules::MakeBasicBlock(Selection& aSelection, nsAtom& blockType) // Delete anything that was in the list of nodes while (!arrayOfNodes.IsEmpty()) { OwningNonNull curNode = arrayOfNodes[0]; - rv = htmlEditor->DeleteNode(curNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*curNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } arrayOfNodes.RemoveElementAt(0); } // Put selection in new block @@ -4174,8 +4222,10 @@ HTMLEditRules::WillCSSIndent(Selection* aSelection, // delete anything that was in the list of nodes while (!arrayOfNodes.IsEmpty()) { OwningNonNull curNode = arrayOfNodes[0]; - rv = htmlEditor->DeleteNode(curNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*curNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } arrayOfNodes.RemoveElementAt(0); } // put selection in new block @@ -4381,8 +4431,10 @@ HTMLEditRules::WillHTMLIndent(Selection* aSelection, // delete anything that was in the list of nodes while (!arrayOfNodes.IsEmpty()) { OwningNonNull curNode = arrayOfNodes[0]; - rv = htmlEditor->DeleteNode(curNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*curNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } arrayOfNodes.RemoveElementAt(0); } // put selection in new block @@ -4759,8 +4811,10 @@ HTMLEditRules::WillOutdent(Selection& aSelection, NS_ENSURE_SUCCESS(rv, rv); } else { // Delete any non-list items for now - rv = htmlEditor->DeleteNode(child); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*child); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } child = curNode->GetLastChild(); } @@ -5185,8 +5239,10 @@ HTMLEditRules::WillAlign(Selection& aSelection, } if (sibling && !IsBlockNode(*sibling)) { AutoEditorDOMPointChildInvalidator lockOffset(pointToInsertDiv); - rv = htmlEditor->DeleteNode(brContent); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*brContent); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } } RefPtr div = @@ -5427,9 +5483,9 @@ HTMLEditRules::CheckForEmptyBlock(nsINode* aStartNode, // If we are inside an empty block, delete it. Note: do NOT delete table // elements this way. - nsCOMPtr block = htmlEditor->GetBlock(*aStartNode); + RefPtr block = htmlEditor->GetBlock(*aStartNode); bool bIsEmptyNode; - nsCOMPtr emptyBlock; + RefPtr emptyBlock; if (block && block != aBodyNode) { // Efficiency hack, avoiding IsEmptyNode() call when in body nsresult rv = htmlEditor->IsEmptyNode(block, &bIsEmptyNode, true, false); @@ -5526,8 +5582,10 @@ HTMLEditRules::CheckForEmptyBlock(nsINode* aStartNode, } } *aHandled = true; - nsresult rv = htmlEditor->DeleteNode(emptyBlock); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = htmlEditor->DeleteNodeWithTransaction(*emptyBlock); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } return NS_OK; } @@ -6924,8 +6982,10 @@ HTMLEditRules::ReturnInHeader(Selection& aSelection, // If the new (righthand) header node is empty, delete it if (IsEmptyBlockElement(aHeader, IgnoreSingleBR::eYes)) { - rv = htmlEditor->DeleteNode(&aHeader); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(aHeader); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // Layout tells the caret to blink in a weird place if we don't place a // break after the header. nsCOMPtr sibling; @@ -7213,8 +7273,10 @@ HTMLEditRules::SplitParagraph( // Get rid of the break, if it is visible (otherwise it may be needed to // prevent an empty p). if (aNextBRNode && htmlEditor->IsVisibleBRElement(aNextBRNode)) { - rv = htmlEditor->DeleteNode(aNextBRNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*aNextBRNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } // Remove ID attribute on the paragraph from the existing right node. @@ -7304,8 +7366,10 @@ HTMLEditRules::ReturnInListItem(Selection& aSelection, NS_ENSURE_SUCCESS(rv, rv); } else { // Otherwise kill this item - nsresult rv = htmlEditor->DeleteNode(&aListItem); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = htmlEditor->DeleteNodeWithTransaction(aListItem); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // Time to insert a paragraph nsAtom& paraAtom = DefaultParagraphSeparator(); @@ -7386,11 +7450,15 @@ HTMLEditRules::ReturnInListItem(Selection& aSelection, if (NS_WARN_IF(!newListItem)) { return NS_ERROR_FAILURE; } - rv = htmlEditor->DeleteNode(&aListItem); - NS_ENSURE_SUCCESS(rv, rv); - rv = aSelection.Collapse(newListItem, 0); - NS_ENSURE_SUCCESS(rv, rv); - + rv = htmlEditor->DeleteNodeWithTransaction(aListItem); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + ErrorResult error; + aSelection.Collapse(EditorRawDOMPoint(newListItem, 0), error); + if (NS_WARN_IF(error.Failed())) { + return error.StealNSResult(); + } return NS_OK; } @@ -7699,8 +7767,10 @@ HTMLEditRules::ApplyBlockStyle(nsTArray>& aNodeArray, if (curBlock) { // Forget any previous block used for previous inline nodes curBlock = nullptr; - nsresult rv = htmlEditor->DeleteNode(curNode); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = htmlEditor->DeleteNodeWithTransaction(*curNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } continue; } @@ -8578,8 +8648,10 @@ HTMLEditRules::RemoveEmptyNodes() // now delete the empty nodes for (OwningNonNull& delNode : arrayOfEmptyNodes) { if (htmlEditor->IsModifiableNode(delNode)) { - rv = htmlEditor->DeleteNode(delNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*delNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } } @@ -8597,8 +8669,10 @@ HTMLEditRules::RemoveEmptyNodes() return NS_ERROR_FAILURE; } } - rv = htmlEditor->DeleteNode(delNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*delNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } return NS_OK; @@ -8803,8 +8877,10 @@ HTMLEditRules::RemoveListStructure(Element& aList) NS_ENSURE_SUCCESS(rv, rv); } else { // Delete any non-list items for now - nsresult rv = htmlEditor->DeleteNode(child); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = htmlEditor->DeleteNodeWithTransaction(*child); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } } @@ -9433,8 +9509,10 @@ HTMLEditRules::WillAbsolutePosition(Selection& aSelection, // Delete anything that was in the list of nodes while (!arrayOfNodes.IsEmpty()) { OwningNonNull curNode = arrayOfNodes[0]; - rv = htmlEditor->DeleteNode(curNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = htmlEditor->DeleteNodeWithTransaction(*curNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } arrayOfNodes.RemoveElementAt(0); } // Put selection in new block @@ -9692,7 +9770,8 @@ HTMLEditRules::DocumentModifiedWorker() return; } - // DeleteNode below may cause a flush, which could destroy the editor + // DeleteNodeWithTransaction() below may cause a flush, which could destroy + // the editor nsAutoScriptBlockerSuppressNodeRemoved scriptBlocker; RefPtr htmlEditor(mHTMLEditor); @@ -9704,7 +9783,9 @@ HTMLEditRules::DocumentModifiedWorker() // Delete our bogus node, if we have one, since the document might not be // empty any more. if (mBogusNode) { - htmlEditor->DeleteNode(mBogusNode); + DebugOnly rv = htmlEditor->DeleteNodeWithTransaction(*mBogusNode); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), + "Failed to remove the bogus node"); mBogusNode = nullptr; } diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index fa295a2bbd5c..9f869a5bb5b3 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -1256,7 +1256,7 @@ HTMLEditor::ReplaceHeadContentsWithHTML(const nsAString& aSourceToInsert) // First delete all children in head while (nsCOMPtr child = headNode->GetFirstChild()) { - nsresult rv = DeleteNode(child); + nsresult rv = DeleteNodeWithTransaction(*child); NS_ENSURE_SUCCESS(rv, rv); } @@ -3183,7 +3183,7 @@ HTMLEditor::DeleteSelectionImpl(EDirection aAction, content->GetParent() != content->GetEditingHost()) { content = content->GetParent(); } - rv = DeleteNode(content); + rv = DeleteNodeWithTransaction(*content); NS_ENSURE_SUCCESS(rv, rv); } @@ -3191,22 +3191,37 @@ HTMLEditor::DeleteSelectionImpl(EDirection aAction, } nsresult -HTMLEditor::DeleteNode(nsINode* aNode) +HTMLEditor::DeleteNodeWithTransaction(nsINode& aNode) { - return DeleteNode(aNode->AsDOMNode()); + if (NS_WARN_IF(!aNode.IsContent())) { + return NS_ERROR_INVALID_ARG; + } + // Do nothing if the node is read-only. + // XXX This is not a override method of EditorBase's method. This might + // cause not called accidentally. We need to investigate this issue. + if (NS_WARN_IF(!IsModifiableNode(aNode.AsContent()) && + !IsMozEditorBogusNode(aNode.AsContent()))) { + return NS_ERROR_FAILURE; + } + nsresult rv = EditorBase::DeleteNodeWithTransaction(aNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } NS_IMETHODIMP -HTMLEditor::DeleteNode(nsIDOMNode* aNode) +HTMLEditor::DeleteNode(nsIDOMNode* aDOMNode) { - // do nothing if the node is read-only - nsCOMPtr content = do_QueryInterface(aNode); - if (NS_WARN_IF(!IsModifiableNode(content) && - !IsMozEditorBogusNode(content))) { - return NS_ERROR_FAILURE; + nsCOMPtr node = do_QueryInterface(aDOMNode); + if (NS_WARN_IF(!node)) { + return NS_ERROR_INVALID_ARG; } - - return EditorBase::DeleteNode(aNode); + nsresult rv = DeleteNodeWithTransaction(*node); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } nsresult @@ -4462,20 +4477,24 @@ HTMLEditor::CopyLastEditableChildStyles(nsINode* aPreviousBlock, nsINode* aNewBlock, Element** aOutBrNode) { - nsCOMPtr newBlock = do_QueryInterface(aNewBlock); - NS_ENSURE_STATE(newBlock || !aNewBlock); + if (NS_WARN_IF(!aNewBlock)) { + return NS_ERROR_INVALID_ARG; + } + nsCOMPtr newBlock(aNewBlock); *aOutBrNode = nullptr; - nsCOMPtr child, tmp; - // first, clear out aNewBlock. Contract is that we want only the styles from previousBlock. - child = aNewBlock->GetFirstChild(); - while (child) { - nsresult rv = DeleteNode(child); - NS_ENSURE_SUCCESS(rv, rv); - child = aNewBlock->GetFirstChild(); + // First, clear out aNewBlock. Contract is that we want only the styles + // from aPreviousBlock. + for (nsCOMPtr child = aNewBlock->GetFirstChild(); + child; + child = aNewBlock->GetFirstChild()) { + nsresult rv = DeleteNodeWithTransaction(*child); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } // now find and clone the styles - child = aPreviousBlock; - tmp = aPreviousBlock; + nsCOMPtr child = aPreviousBlock; + nsCOMPtr tmp = aPreviousBlock; while (tmp) { child = tmp; tmp = GetLastEditableChild(*child); diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index e0e9372a7129..a843f28c36e1 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -385,8 +385,18 @@ public: NS_IMETHOD DeleteSelectionImpl(EDirection aAction, EStripWrappers aStripWrappers) override; - nsresult DeleteNode(nsINode* aNode); + + /** + * DeleteNodeWithTransaction() removes aNode from the DOM tree if it's + * modifiable. Note that this is not an override of same method of + * EditorBase. + * + * @param aNode The node to be removed from the DOM tree. + */ + nsresult DeleteNodeWithTransaction(nsINode& aNode); + NS_IMETHOD DeleteNode(nsIDOMNode* aNode) override; + nsresult DeleteText(dom::CharacterData& aTextNode, uint32_t aOffset, uint32_t aLength); virtual nsresult diff --git a/editor/libeditor/HTMLEditorDataTransfer.cpp b/editor/libeditor/HTMLEditorDataTransfer.cpp index 815e3cb34427..e3db5cd15035 100644 --- a/editor/libeditor/HTMLEditorDataTransfer.cpp +++ b/editor/libeditor/HTMLEditorDataTransfer.cpp @@ -356,8 +356,10 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString, TextEditUtils::IsBreak(wsObj.mEndReasonNode) && !IsVisibleBRElement(wsObj.mEndReasonNode)) { AutoEditorDOMPointChildInvalidator lockOffset(pointToInsert); - rv = DeleteNode(wsObj.mEndReasonNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = DeleteNodeWithTransaction(*wsObj.mEndReasonNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } // Remember if we are in a link. @@ -491,7 +493,7 @@ HTMLEditor::DoInsertHTMLWithContext(const nsAString& aInputString, GetParentNode())) { // Is it an orphan node? } else { - DeleteNode(pointToInsert.GetContainer()); + DeleteNodeWithTransaction(*pointToInsert.GetContainer()); pointToInsert.Set(pointToInsert.GetContainer()); } } diff --git a/editor/libeditor/HTMLStyleEditor.cpp b/editor/libeditor/HTMLStyleEditor.cpp index 9d44df125fa1..1ec8982a0775 100644 --- a/editor/libeditor/HTMLStyleEditor.cpp +++ b/editor/libeditor/HTMLStyleEditor.cpp @@ -596,8 +596,10 @@ HTMLEditor::ClearStyle(nsCOMPtr* aNode, IsEmptyNode(leftNode, &bIsEmptyNode, false, true); if (bIsEmptyNode) { // delete leftNode if it became empty - rv = DeleteNode(leftNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = DeleteNodeWithTransaction(*leftNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } } if (rightNode) { @@ -627,8 +629,10 @@ HTMLEditor::ClearStyle(nsCOMPtr* aNode, IsEmptyNode(rightNode, &bIsEmptyNode, false, true); if (bIsEmptyNode) { // delete rightNode if it became empty - rv = DeleteNode(rightNode); - NS_ENSURE_SUCCESS(rv, rv); + rv = DeleteNodeWithTransaction(*rightNode); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } } diff --git a/editor/libeditor/HTMLTableEditor.cpp b/editor/libeditor/HTMLTableEditor.cpp index 311b46c529e7..2ec545534545 100644 --- a/editor/libeditor/HTMLTableEditor.cpp +++ b/editor/libeditor/HTMLTableEditor.cpp @@ -838,8 +838,14 @@ HTMLEditor::DeleteTableCell(int32_t aNumber) NS_ENSURE_SUCCESS(rv, rv); // Then delete the cell - rv = DeleteNode(cell); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr cellToBeRemoved = do_QueryInterface(cell); + if (NS_WARN_IF(!cellToBeRemoved)) { + return NS_ERROR_FAILURE; + } + rv = DeleteNodeWithTransaction(*cellToBeRemoved); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // The next cell to delete cell = nextCell; @@ -891,10 +897,15 @@ HTMLEditor::DeleteTableCell(int32_t aNumber) startColIndex, ePreviousColumn, false); AutoTransactionsConserveSelection dontChangeSelection(this); - - rv = DeleteNode(cell); + nsCOMPtr cellToBeRemoved = do_QueryInterface(cell); + if (NS_WARN_IF(!cellToBeRemoved)) { + return NS_ERROR_FAILURE; + } + rv = DeleteNodeWithTransaction(*cellToBeRemoved); // If we fail, don't try to delete any more cells??? - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } } } @@ -963,8 +974,10 @@ HTMLEditor::DeleteCellContents(nsIDOMElement* aCell) AutoRules beginRulesSniffing(this, EditAction::deleteNode, nsIEditor::eNext); while (nsCOMPtr child = cell->GetLastChild()) { - nsresult rv = DeleteNode(child); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = DeleteNodeWithTransaction(*child); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } return NS_OK; } @@ -1121,8 +1134,14 @@ HTMLEditor::DeleteColumn(nsIDOMElement* aTable, // row now has current rowIndex } else { // A more "normal" deletion - rv = DeleteNode(cell); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr cellToBeRemoved = do_QueryInterface(cell); + if (NS_WARN_IF(!cellToBeRemoved)) { + return NS_ERROR_FAILURE; + } + rv = DeleteNodeWithTransaction(*cellToBeRemoved); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } //Skip over any rows spanned by this cell rowIndex += actualRowSpan; @@ -1312,8 +1331,14 @@ HTMLEditor::DeleteRow(nsIDOMElement* aTable, NS_ENSURE_SUCCESS(rv, rv); if (parentRow) { - rv = DeleteNode(parentRow); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr rowToBeRemoved = do_QueryInterface(parentRow); + if (NS_WARN_IF(!rowToBeRemoved)) { + return NS_ERROR_FAILURE; + } + rv = DeleteNodeWithTransaction(*rowToBeRemoved); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } // Now we can set new rowspans for cells stored above @@ -2169,9 +2194,14 @@ HTMLEditor::JoinTableCells(bool aMergeNonContiguousContents) for (uint32_t i = 0, n = deleteList.Length(); i < n; i++) { nsIDOMElement *elementPtr = deleteList[i]; if (elementPtr) { - nsCOMPtr node = do_QueryInterface(elementPtr); - rv = DeleteNode(node); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr nodeToBeRemoved = do_QueryInterface(elementPtr); + if (NS_WARN_IF(!nodeToBeRemoved)) { + return NS_ERROR_FAILURE; + } + rv = DeleteNodeWithTransaction(*nodeToBeRemoved); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } } } // Cleanup selection: remove ranges where cells were deleted @@ -2293,8 +2323,13 @@ HTMLEditor::MergeCells(nsCOMPtr aTargetCell, if (len == 1 && IsEmptyCell(targetCell)) { // Delete the empty node nsIContent* cellChild = targetCell->GetFirstChild(); - nsresult rv = DeleteNode(cellChild->AsDOMNode()); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(!cellChild)) { + return NS_ERROR_FAILURE; + } + nsresult rv = DeleteNodeWithTransaction(*cellChild); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } insertIndex = 0; } else { insertIndex = (int32_t)len; @@ -2303,10 +2338,13 @@ HTMLEditor::MergeCells(nsCOMPtr aTargetCell, // Move the contents while (cellToMerge->HasChildren()) { nsCOMPtr cellChild = cellToMerge->GetLastChild(); - // XXX We need HTMLEditor::DeleteNode(nsINode&). - nsresult rv = DeleteNode(cellChild->AsDOMNode()); - NS_ENSURE_SUCCESS(rv, rv); - + if (NS_WARN_IF(!cellChild)) { + return NS_ERROR_FAILURE; + } + nsresult rv = DeleteNodeWithTransaction(*cellChild); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } rv = InsertNodeWithTransaction(*cellChild, EditorRawDOMPoint(aTargetCell, insertIndex)); @@ -2316,11 +2354,19 @@ HTMLEditor::MergeCells(nsCOMPtr aTargetCell, } } - // Delete cells whose contents were moved - if (aDeleteCellToMerge) { - return DeleteNode(aCellToMerge); + if (!aDeleteCellToMerge) { + return NS_OK; } + // Delete cells whose contents were moved. + nsCOMPtr cellToBeRemoved = do_QueryInterface(aCellToMerge); + if (NS_WARN_IF(!cellToBeRemoved)) { + return NS_ERROR_INVALID_ARG; + } + nsresult rv = DeleteNodeWithTransaction(*cellToBeRemoved); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return NS_OK; } diff --git a/editor/libeditor/TextEditRules.cpp b/editor/libeditor/TextEditRules.cpp index f3a381f12928..5e5ca272a30e 100644 --- a/editor/libeditor/TextEditRules.cpp +++ b/editor/libeditor/TextEditRules.cpp @@ -387,8 +387,13 @@ TextEditRules::WillInsert(Selection& aSelection, bool* aCancel) // check for the magic content node and delete it if it exists if (mBogusNode) { - NS_ENSURE_TRUE_VOID(mTextEditor); - mTextEditor->DeleteNode(mBogusNode); + if (NS_WARN_IF(!mTextEditor)) { + return; // XXX Shouldn't we release mBogusNode now? + } + RefPtr textEditor(mTextEditor); + DebugOnly rv = textEditor->DeleteNodeWithTransaction(*mBogusNode); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), + "Failed to remove the bogus node"); mBogusNode = nullptr; } } @@ -1088,7 +1093,9 @@ TextEditRules::DidDeleteSelection(Selection* aSelection, return NS_ERROR_NOT_AVAILABLE; } RefPtr textEditor(mTextEditor); - nsresult rv = textEditor->DeleteNode(selectionStartPoint.GetContainer()); + nsresult rv = + textEditor->DeleteNodeWithTransaction( + *selectionStartPoint.GetContainer()); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } diff --git a/editor/libeditor/WSRunObject.cpp b/editor/libeditor/WSRunObject.cpp index f57d0b443284..7449c3ae8dec 100644 --- a/editor/libeditor/WSRunObject.cpp +++ b/editor/libeditor/WSRunObject.cpp @@ -1362,11 +1362,14 @@ WSRunObject::DeleteRange(const EditorDOMPointBase& aStartPoint, return NS_OK; } + MOZ_ASSERT(mHTMLEditor); + RefPtr htmlEditor(mHTMLEditor); + if (aStartPoint.GetContainer() == aEndPoint.GetContainer() && aStartPoint.IsInTextNode()) { - return mHTMLEditor->DeleteText(*aStartPoint.GetContainerAsText(), - aStartPoint.Offset(), - aEndPoint.Offset() - aStartPoint.Offset()); + return htmlEditor->DeleteText(*aStartPoint.GetContainerAsText(), + aStartPoint.Offset(), + aEndPoint.Offset() - aStartPoint.Offset()); } RefPtr range; @@ -1386,16 +1389,16 @@ WSRunObject::DeleteRange(const EditorDOMPointBase& aStartPoint, if (node == aStartPoint.GetContainer()) { if (!aStartPoint.IsEndOfContainer()) { nsresult rv = - mHTMLEditor->DeleteText(*node, aStartPoint.Offset(), - aStartPoint.GetContainer()->Length() - - aStartPoint.Offset()); + htmlEditor->DeleteText(*node, aStartPoint.Offset(), + aStartPoint.GetContainer()->Length() - + aStartPoint.Offset()); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } } } else if (node == aEndPoint.GetContainer()) { if (!aEndPoint.IsStartOfContainer()) { - nsresult rv = mHTMLEditor->DeleteText(*node, 0, aEndPoint.Offset()); + nsresult rv = htmlEditor->DeleteText(*node, 0, aEndPoint.Offset()); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -1419,7 +1422,7 @@ WSRunObject::DeleteRange(const EditorDOMPointBase& aStartPoint, break; } if (!nodeBefore) { - rv = mHTMLEditor->DeleteNode(node); + rv = htmlEditor->DeleteNodeWithTransaction(*node); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; }