From c66ed1275f1039fdf23a8f2c172d7c0b4691d1c2 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 15 Apr 2020 01:55:25 -0400 Subject: Bug 1336011 - Fix Crash in InvalidArrayIndex_CRASH in mozilla::EditorBase::DeleteSelectionImpl * EditorBase shouldn't refer mActionListeners directly in loops because it might be removed during a loop * Create an alias of the type of mEditorObservers * Create an alias of the type of mDocStateListeners Tag #1375 --- editor/libeditor/EditorBase.cpp | 183 +++++++++++++++++++++++++--------------- editor/libeditor/EditorBase.h | 12 ++- 2 files changed, 124 insertions(+), 71 deletions(-) (limited to 'editor') diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index f7988cd1a..f0f3095d6 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -1385,9 +1385,12 @@ EditorBase::CreateNode(nsIAtom* aTag, AutoRules beginRulesSniffing(this, EditAction::createNode, nsIEditor::eNext); - for (auto& listener : mActionListeners) { - listener->WillCreateNode(nsDependentAtomString(aTag), - GetAsDOMNode(aParent), aPosition); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->WillCreateNode(nsDependentAtomString(aTag), + GetAsDOMNode(aParent), aPosition); + } } nsCOMPtr ret; @@ -1402,9 +1405,12 @@ EditorBase::CreateNode(nsIAtom* aTag, mRangeUpdater.SelAdjCreateNode(aParent, aPosition); - for (auto& listener : mActionListeners) { - listener->DidCreateNode(nsDependentAtomString(aTag), GetAsDOMNode(ret), - GetAsDOMNode(aParent), aPosition, rv); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->DidCreateNode(nsDependentAtomString(aTag), GetAsDOMNode(ret), + GetAsDOMNode(aParent), aPosition, rv); + } } return ret.forget(); @@ -1429,9 +1435,12 @@ EditorBase::InsertNode(nsIContent& aNode, { AutoRules beginRulesSniffing(this, EditAction::insertNode, nsIEditor::eNext); - for (auto& listener : mActionListeners) { - listener->WillInsertNode(aNode.AsDOMNode(), aParent.AsDOMNode(), - aPosition); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->WillInsertNode(aNode.AsDOMNode(), aParent.AsDOMNode(), + aPosition); + } } RefPtr transaction = @@ -1440,9 +1449,12 @@ EditorBase::InsertNode(nsIContent& aNode, mRangeUpdater.SelAdjInsertNode(aParent.AsDOMNode(), aPosition); - for (auto& listener : mActionListeners) { - listener->DidInsertNode(aNode.AsDOMNode(), aParent.AsDOMNode(), aPosition, - rv); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->DidInsertNode(aNode.AsDOMNode(), aParent.AsDOMNode(), aPosition, + rv); + } } return rv; @@ -1468,8 +1480,11 @@ EditorBase::SplitNode(nsIContent& aNode, { AutoRules beginRulesSniffing(this, EditAction::splitNode, nsIEditor::eNext); - for (auto& listener : mActionListeners) { - listener->WillSplitNode(aNode.AsDOMNode(), aOffset); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->WillSplitNode(aNode.AsDOMNode(), aOffset); + } } RefPtr transaction = @@ -1482,9 +1497,12 @@ EditorBase::SplitNode(nsIContent& aNode, mRangeUpdater.SelAdjSplitNode(aNode, aOffset, newNode); nsresult rv = aResult.StealNSResult(); - for (auto& listener : mActionListeners) { - listener->DidSplitNode(aNode.AsDOMNode(), aOffset, GetAsDOMNode(newNode), - rv); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->DidSplitNode(aNode.AsDOMNode(), aOffset, GetAsDOMNode(newNode), + rv); + } } // Note: result might be a success code, so we can't use Throw() to // set it on aResult. @@ -1520,9 +1538,12 @@ EditorBase::JoinNodes(nsINode& aLeftNode, // Find the number of children of the lefthand node uint32_t oldLeftNodeLen = aLeftNode.Length(); - for (auto& listener : mActionListeners) { - listener->WillJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(), - parent->AsDOMNode()); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->WillJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(), + parent->AsDOMNode()); + } } nsresult rv = NS_OK; @@ -1535,9 +1556,12 @@ EditorBase::JoinNodes(nsINode& aLeftNode, mRangeUpdater.SelAdjJoinNodes(aLeftNode, aRightNode, *parent, offset, (int32_t)oldLeftNodeLen); - for (auto& listener : mActionListeners) { - listener->DidJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(), - parent->AsDOMNode(), rv); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->DidJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(), + parent->AsDOMNode(), rv); + } } return rv; @@ -1558,8 +1582,11 @@ EditorBase::DeleteNode(nsINode* aNode) nsIEditor::ePrevious); // save node location for selection updating code. - for (auto& listener : mActionListeners) { - listener->WillDeleteNode(aNode->AsDOMNode()); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->WillDeleteNode(aNode->AsDOMNode()); + } } RefPtr transaction; @@ -1568,8 +1595,11 @@ EditorBase::DeleteNode(nsINode* aNode) rv = DoTransaction(transaction); } - for (auto& listener : mActionListeners) { - listener->DidDeleteNode(aNode->AsDOMNode(), rv); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->DidDeleteNode(aNode->AsDOMNode(), rv); + } } NS_ENSURE_SUCCESS(rv, rv); @@ -1844,7 +1874,7 @@ void EditorBase::NotifyEditorObservers(NotificationForEditorObservers aNotification) { // Copy the observers since EditAction()s can modify mEditorObservers. - nsTArray> observers(mEditorObservers); + AutoEditorObserverArray observers(mEditorObservers); switch (aNotification) { case eNotifyEditorObserversOfEnd: mIsInEditAction = false; @@ -2505,10 +2535,13 @@ EditorBase::InsertTextIntoTextNodeImpl(const nsAString& aStringToInsert, } // Let listeners know what's up - for (auto& listener : mActionListeners) { - listener->WillInsertText( - static_cast(insertedTextNode->AsDOMNode()), - insertedOffset, aStringToInsert); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->WillInsertText( + static_cast(insertedTextNode->AsDOMNode()), + insertedOffset, aStringToInsert); + } } // XXX We may not need these view batches anymore. This is handled at a @@ -2518,10 +2551,13 @@ EditorBase::InsertTextIntoTextNodeImpl(const nsAString& aStringToInsert, EndUpdateViewBatch(); // let listeners know what happened - for (auto& listener : mActionListeners) { - listener->DidInsertText( - static_cast(insertedTextNode->AsDOMNode()), - insertedOffset, aStringToInsert, rv); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->DidInsertText( + static_cast(insertedTextNode->AsDOMNode()), + insertedOffset, aStringToInsert, rv); + } } // Added some cruft here for bug 43366. Layout was crashing because we left @@ -2583,8 +2619,7 @@ EditorBase::NotifyDocumentListeners( return NS_OK; } - nsTArray> - listeners(mDocStateListeners); + AutoDocumentStateListenerArray listeners(mDocStateListeners); nsresult rv = NS_OK; switch (aNotificationType) { @@ -2656,19 +2691,25 @@ EditorBase::DeleteText(nsGenericDOMDataNode& aCharData, nsIEditor::ePrevious); // Let listeners know what's up - for (auto& listener : mActionListeners) { - listener->WillDeleteText( - static_cast(GetAsDOMNode(&aCharData)), aOffset, - aLength); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->WillDeleteText( + static_cast(GetAsDOMNode(&aCharData)), aOffset, + aLength); + } } nsresult rv = DoTransaction(transaction); // Let listeners know what happened - for (auto& listener : mActionListeners) { - listener->DidDeleteText( - static_cast(GetAsDOMNode(&aCharData)), aOffset, - aLength, rv); + { + AutoActionListenerArray listeners(mActionListeners); + for (auto& listener : listeners) { + listener->DidDeleteText( + static_cast(GetAsDOMNode(&aCharData)), aOffset, + aLength, rv); + } } return rv; @@ -4034,17 +4075,20 @@ EditorBase::DeleteSelectionImpl(EDirection aAction, if (NS_SUCCEEDED(rv)) { AutoRules beginRulesSniffing(this, EditAction::deleteSelection, aAction); // Notify nsIEditActionListener::WillDelete[Selection|Text|Node] - if (!deleteNode) { - for (auto& listener : mActionListeners) { - listener->WillDeleteSelection(selection); - } - } else if (deleteCharData) { - for (auto& listener : mActionListeners) { - listener->WillDeleteText(deleteCharData, deleteCharOffset, 1); - } - } else { - for (auto& listener : mActionListeners) { - listener->WillDeleteNode(deleteNode->AsDOMNode()); + { + AutoActionListenerArray listeners(mActionListeners); + if (!deleteNode) { + for (auto& listener : listeners) { + listener->WillDeleteSelection(selection); + } + } else if (deleteCharData) { + for (auto& listener : listeners) { + listener->WillDeleteText(deleteCharData, deleteCharOffset, 1); + } + } else { + for (auto& listener : listeners) { + listener->WillDeleteNode(deleteNode->AsDOMNode()); + } } } @@ -4052,17 +4096,20 @@ EditorBase::DeleteSelectionImpl(EDirection aAction, rv = DoTransaction(transaction); // Notify nsIEditActionListener::DidDelete[Selection|Text|Node] - if (!deleteNode) { - for (auto& listener : mActionListeners) { - listener->DidDeleteSelection(selection); - } - } else if (deleteCharData) { - for (auto& listener : mActionListeners) { - listener->DidDeleteText(deleteCharData, deleteCharOffset, 1, rv); - } - } else { - for (auto& listener : mActionListeners) { - listener->DidDeleteNode(deleteNode->AsDOMNode(), rv); + { + AutoActionListenerArray listeners(mActionListeners); + if (!deleteNode) { + for (auto& listener : mActionListeners) { + listener->DidDeleteSelection(selection); + } + } else if (deleteCharData) { + for (auto& listener : mActionListeners) { + listener->DidDeleteText(deleteCharData, deleteCharOffset, 1, rv); + } + } else { + for (auto& listener : mActionListeners) { + listener->DidDeleteNode(deleteNode->AsDOMNode(), rv); + } } } } diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h index dbd00771e..8680b57ae 100644 --- a/editor/libeditor/EditorBase.h +++ b/editor/libeditor/EditorBase.h @@ -987,11 +987,17 @@ protected: RefPtr mComposition; // Listens to all low level actions on the doc. - nsTArray> mActionListeners; + typedef AutoTArray, 5> + AutoActionListenerArray; + AutoActionListenerArray mActionListeners; // Just notify once per high level change. - nsTArray> mEditorObservers; + typedef AutoTArray, 3> + AutoEditorObserverArray; + AutoEditorObserverArray mEditorObservers; // Listen to overall doc state (dirty or not, just created, etc.). - nsTArray> mDocStateListeners; + typedef AutoTArray, 1> + AutoDocumentStateListenerArray; + AutoDocumentStateListenerArray mDocStateListeners; // Cached selection for AutoSelectionRestorer. SelectionState mSavedSel; -- cgit v1.2.3 From ef689a705ffdd79cdeeca8e68438b4ad6597f38d Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:27:56 -0400 Subject: Bug 1348851 - Use new block when better selection isn't found. Tag #1375 --- editor/libeditor/HTMLEditRules.cpp | 26 +++++++++++++++----------- editor/libeditor/crashtests/1348851.html | 19 +++++++++++++++++++ editor/libeditor/crashtests/crashtests.list | 1 + 3 files changed, 35 insertions(+), 11 deletions(-) create mode 100644 editor/libeditor/crashtests/1348851.html (limited to 'editor') diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index 545e22f70..fcbb75ee6 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -7179,15 +7179,18 @@ HTMLEditRules::PinSelectionToNewBlock(Selection* aSelection) return NS_OK; } + if (NS_WARN_IF(!mNewBlock)) { + return NS_ERROR_NULL_POINTER; + } + // get the (collapsed) selection location - nsCOMPtr selNode, temp; + nsCOMPtr selNode; int32_t selOffset; NS_ENSURE_STATE(mHTMLEditor); nsresult rv = mHTMLEditor->GetStartNodeAndOffset(aSelection, getter_AddRefs(selNode), &selOffset); NS_ENSURE_SUCCESS(rv, rv); - temp = selNode; // use ranges and sRangeHelper to compare sel point to new block nsCOMPtr node = do_QueryInterface(selNode); @@ -7197,24 +7200,23 @@ HTMLEditRules::PinSelectionToNewBlock(Selection* aSelection) NS_ENSURE_SUCCESS(rv, rv); rv = range->SetEnd(selNode, selOffset); NS_ENSURE_SUCCESS(rv, rv); - nsCOMPtr block = mNewBlock.get(); - NS_ENSURE_TRUE(block, NS_ERROR_NO_INTERFACE); bool nodeBefore, nodeAfter; - rv = nsRange::CompareNodeToRange(block, range, &nodeBefore, &nodeAfter); + rv = nsRange::CompareNodeToRange(mNewBlock, range, &nodeBefore, &nodeAfter); NS_ENSURE_SUCCESS(rv, rv); if (nodeBefore && nodeAfter) { return NS_OK; // selection is inside block } else if (nodeBefore) { // selection is after block. put at end of block. - nsCOMPtr tmp = GetAsDOMNode(mNewBlock); NS_ENSURE_STATE(mHTMLEditor); - tmp = GetAsDOMNode(mHTMLEditor->GetLastEditableChild(*block)); + nsCOMPtr tmp = mHTMLEditor->GetLastEditableChild(*mNewBlock); + if (!tmp) { + tmp = mNewBlock; + } uint32_t endPoint; if (mHTMLEditor->IsTextNode(tmp) || mHTMLEditor->IsContainer(tmp)) { - rv = EditorBase::GetLengthOfDOMNode(tmp, endPoint); - NS_ENSURE_SUCCESS(rv, rv); + endPoint = tmp->Length(); } else { tmp = EditorBase::GetNodeLocation(tmp, (int32_t*)&endPoint); endPoint++; // want to be after this node @@ -7222,9 +7224,11 @@ HTMLEditRules::PinSelectionToNewBlock(Selection* aSelection) return aSelection->Collapse(tmp, (int32_t)endPoint); } else { // selection is before block. put at start of block. - nsCOMPtr tmp = GetAsDOMNode(mNewBlock); NS_ENSURE_STATE(mHTMLEditor); - tmp = GetAsDOMNode(mHTMLEditor->GetFirstEditableChild(*block)); + nsCOMPtr tmp = mHTMLEditor->GetFirstEditableChild(*mNewBlock); + if (!tmp) { + tmp = mNewBlock; + } int32_t offset; if (mHTMLEditor->IsTextNode(tmp) || mHTMLEditor->IsContainer(tmp)) { diff --git a/editor/libeditor/crashtests/1348851.html b/editor/libeditor/crashtests/1348851.html new file mode 100644 index 000000000..d618f049e --- /dev/null +++ b/editor/libeditor/crashtests/1348851.html @@ -0,0 +1,19 @@ + + + + + + + + + + diff --git a/editor/libeditor/crashtests/crashtests.list b/editor/libeditor/crashtests/crashtests.list index 3fbc6b196..7b1c57dbf 100644 --- a/editor/libeditor/crashtests/crashtests.list +++ b/editor/libeditor/crashtests/crashtests.list @@ -68,4 +68,5 @@ load 1158452.html load 1158651.html load 1244894.xhtml load 1272490.html +load 1348851.html load 1317704.html -- cgit v1.2.3 From 940d191ef8b61309f4ea83d0fea77828f361251b Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:28:43 -0400 Subject: Bug 1367683 - Optimize initializing nsRange Tag #1375 --- editor/libeditor/HTMLEditRules.cpp | 111 +++++++++++++++++-------------- editor/libeditor/HTMLEditor.cpp | 4 +- editor/libeditor/HTMLStyleEditor.cpp | 26 +++++--- editor/libeditor/SelectionState.cpp | 3 +- editor/libeditor/WSRunObject.cpp | 2 +- editor/txtsvc/nsTextServicesDocument.cpp | 12 ++-- 6 files changed, 88 insertions(+), 70 deletions(-) (limited to 'editor') diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index fcbb75ee6..0aa2bde8c 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -1422,16 +1422,15 @@ HTMLEditRules::WillInsertText(EditAction aAction, if (!mDocChangeRange) { mDocChangeRange = new nsRange(selNode); } - rv = mDocChangeRange->SetStart(selNode, selOffset); - NS_ENSURE_SUCCESS(rv, rv); if (curNode) { - rv = mDocChangeRange->SetEnd(curNode, curOffset); + rv = mDocChangeRange->SetStartAndEnd(selNode, selOffset, + curNode, curOffset); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } } else { - rv = mDocChangeRange->SetEnd(selNode, selOffset); + rv = mDocChangeRange->CollapseTo(selNode, selOffset); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -5093,10 +5092,11 @@ HTMLEditRules::ExpandSelectionForDeletion(Selection& aSelection) // Create a range that represents expanded selection RefPtr range = new nsRange(selStartNode); - nsresult rv = range->SetStart(selStartNode, selStartOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = range->SetEnd(selEndNode, selEndOffset); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = range->SetStartAndEnd(selStartNode, selStartOffset, + selEndNode, selEndOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } // Check if block is entirely inside range if (brBlock) { @@ -5558,26 +5558,27 @@ HTMLEditRules::PromoteRange(nsRange& aRange, // This is tricky. The basic idea is to push out the range endpoints to // truly enclose the blocks that we will affect. - nsCOMPtr opStartNode; - nsCOMPtr opEndNode; + nsCOMPtr opDOMStartNode; + nsCOMPtr opDOMEndNode; int32_t opStartOffset, opEndOffset; GetPromotedPoint(kStart, GetAsDOMNode(startNode), startOffset, - aOperationType, address_of(opStartNode), &opStartOffset); + aOperationType, address_of(opDOMStartNode), &opStartOffset); GetPromotedPoint(kEnd, GetAsDOMNode(endNode), endOffset, aOperationType, - address_of(opEndNode), &opEndOffset); + address_of(opDOMEndNode), &opEndOffset); // Make sure that the new range ends up to be in the editable section. if (!htmlEditor->IsDescendantOfEditorRoot( - EditorBase::GetNodeAtRangeOffsetPoint(opStartNode, opStartOffset)) || + EditorBase::GetNodeAtRangeOffsetPoint(opDOMStartNode, opStartOffset)) || !htmlEditor->IsDescendantOfEditorRoot( - EditorBase::GetNodeAtRangeOffsetPoint(opEndNode, opEndOffset - 1))) { + EditorBase::GetNodeAtRangeOffsetPoint(opDOMEndNode, opEndOffset - 1))) { return; } - DebugOnly rv = aRange.SetStart(opStartNode, opStartOffset); - MOZ_ASSERT(NS_SUCCEEDED(rv)); - rv = aRange.SetEnd(opEndNode, opEndOffset); + nsCOMPtr opStartNode = do_QueryInterface(opDOMStartNode); + nsCOMPtr opEndNode = do_QueryInterface(opDOMEndNode); + DebugOnly rv = + aRange.SetStartAndEnd(opStartNode, opStartOffset, opEndNode, opEndOffset); MOZ_ASSERT(NS_SUCCEEDED(rv)); } @@ -7196,10 +7197,10 @@ HTMLEditRules::PinSelectionToNewBlock(Selection* aSelection) nsCOMPtr node = do_QueryInterface(selNode); NS_ENSURE_STATE(node); RefPtr range = new nsRange(node); - rv = range->SetStart(selNode, selOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = range->SetEnd(selNode, selOffset); - NS_ENSURE_SUCCESS(rv, rv); + rv = range->CollapseTo(node, selOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } bool nodeBefore, nodeAfter; rv = nsRange::CompareNodeToRange(mNewBlock, range, &nodeBefore, &nodeAfter); NS_ENSURE_SUCCESS(rv, rv); @@ -8095,10 +8096,13 @@ HTMLEditRules::DidSplitNode(nsIDOMNode* aExistingRightNode, if (!mListenerEnabled) { return NS_OK; } - nsresult rv = mUtilRange->SetStart(aNewLeftNode, 0); - NS_ENSURE_SUCCESS(rv, rv); - rv = mUtilRange->SetEnd(aExistingRightNode, 0); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr newLeftNode = do_QueryInterface(aNewLeftNode); + nsCOMPtr existingRightNode = do_QueryInterface(aExistingRightNode); + nsresult rv = mUtilRange->SetStartAndEnd(newLeftNode, 0, + existingRightNode, 0); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return UpdateDocChangeRange(mUtilRange); } @@ -8123,11 +8127,12 @@ HTMLEditRules::DidJoinNodes(nsIDOMNode* aLeftNode, if (!mListenerEnabled) { return NS_OK; } + nsCOMPtr rightNode = do_QueryInterface(aRightNode); // assumption that Join keeps the righthand node - nsresult rv = mUtilRange->SetStart(aRightNode, mJoinOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = mUtilRange->SetEnd(aRightNode, mJoinOffset); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = mUtilRange->CollapseTo(rightNode, mJoinOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return UpdateDocChangeRange(mUtilRange); } @@ -8149,11 +8154,12 @@ HTMLEditRules::DidInsertText(nsIDOMCharacterData* aTextNode, return NS_OK; } int32_t length = aString.Length(); - nsCOMPtr theNode = do_QueryInterface(aTextNode); - nsresult rv = mUtilRange->SetStart(theNode, aOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = mUtilRange->SetEnd(theNode, aOffset+length); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr theNode = do_QueryInterface(aTextNode); + nsresult rv = mUtilRange->SetStartAndEnd(theNode, aOffset, + theNode, aOffset + length); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return UpdateDocChangeRange(mUtilRange); } @@ -8174,11 +8180,11 @@ HTMLEditRules::DidDeleteText(nsIDOMCharacterData* aTextNode, if (!mListenerEnabled) { return NS_OK; } - nsCOMPtr theNode = do_QueryInterface(aTextNode); - nsresult rv = mUtilRange->SetStart(theNode, aOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = mUtilRange->SetEnd(theNode, aOffset); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr theNode = do_QueryInterface(aTextNode); + nsresult rv = mUtilRange->CollapseTo(theNode, aOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return UpdateDocChangeRange(mUtilRange); } @@ -8193,22 +8199,27 @@ HTMLEditRules::WillDeleteSelection(nsISelection* aSelection) } RefPtr selection = aSelection->AsSelection(); // get the (collapsed) selection location - nsCOMPtr selNode; - int32_t selOffset; - + nsCOMPtr startNode; + int32_t startOffset; NS_ENSURE_STATE(mHTMLEditor); nsresult rv = mHTMLEditor->GetStartNodeAndOffset(selection, - getter_AddRefs(selNode), &selOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = mUtilRange->SetStart(selNode, selOffset); - NS_ENSURE_SUCCESS(rv, rv); + getter_AddRefs(startNode), &startOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + nsCOMPtr endNode; + int32_t endOffset; NS_ENSURE_STATE(mHTMLEditor); rv = mHTMLEditor->GetEndNodeAndOffset(selection, - getter_AddRefs(selNode), &selOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = mUtilRange->SetEnd(selNode, selOffset); - NS_ENSURE_SUCCESS(rv, rv); + getter_AddRefs(endNode), &endOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + rv = mUtilRange->SetStartAndEnd(startNode, startOffset, endNode, endOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return UpdateDocChangeRange(mUtilRange); } diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 532da7a15..73dd1673b 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -3284,8 +3284,8 @@ HTMLEditor::DoContentInserted(nsIDocument* aDocument, sibling = sibling->GetNextSibling(); } } - nsresult rv = range->Set(aContainer, aIndexInContainer, - aContainer, endIndex); + nsresult rv = range->SetStartAndEnd(aContainer, aIndexInContainer, + aContainer, endIndex); if (NS_SUCCEEDED(rv)) { mInlineSpellChecker->SpellCheckRange(range); } diff --git a/editor/libeditor/HTMLStyleEditor.cpp b/editor/libeditor/HTMLStyleEditor.cpp index 6a1ffe8b4..7141cfd61 100644 --- a/editor/libeditor/HTMLStyleEditor.cpp +++ b/editor/libeditor/HTMLStyleEditor.cpp @@ -541,9 +541,11 @@ HTMLEditor::SplitStyleAboveRange(nsRange* inRange, NS_ENSURE_SUCCESS(rv, rv); // reset the range - rv = inRange->SetStart(startNode, startOffset); - NS_ENSURE_SUCCESS(rv, rv); - return inRange->SetEnd(endNode, endOffset); + rv = inRange->SetStartAndEnd(startNode, startOffset, endNode, endOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } nsresult @@ -885,10 +887,11 @@ HTMLEditor::PromoteRangeIfStartsOrEndsInNamedAnchor(nsRange& aRange) endOffset = endNode ? endNode->IndexOf(parent) + 1 : 0; } - nsresult rv = aRange.SetStart(startNode, startOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = aRange.SetEnd(endNode, endOffset); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = aRange.SetStartAndEnd(startNode, startOffset, + endNode, endOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return NS_OK; } @@ -918,10 +921,11 @@ HTMLEditor::PromoteInlineRange(nsRange& aRange) endNode = parent; } - nsresult rv = aRange.SetStart(startNode, startOffset); - NS_ENSURE_SUCCESS(rv, rv); - rv = aRange.SetEnd(endNode, endOffset); - NS_ENSURE_SUCCESS(rv, rv); + nsresult rv = aRange.SetStartAndEnd(startNode, startOffset, + endNode, endOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } return NS_OK; } diff --git a/editor/libeditor/SelectionState.cpp b/editor/libeditor/SelectionState.cpp index f9ad5947a..057e04875 100644 --- a/editor/libeditor/SelectionState.cpp +++ b/editor/libeditor/SelectionState.cpp @@ -686,7 +686,8 @@ already_AddRefed RangeItem::GetRange() { RefPtr range = new nsRange(startNode); - if (NS_FAILED(range->Set(startNode, startOffset, endNode, endOffset))) { + if (NS_FAILED(range->SetStartAndEnd(startNode, startOffset, + endNode, endOffset))) { return nullptr; } return range.forget(); diff --git a/editor/libeditor/WSRunObject.cpp b/editor/libeditor/WSRunObject.cpp index 39ac3fee8..0748f09e5 100644 --- a/editor/libeditor/WSRunObject.cpp +++ b/editor/libeditor/WSRunObject.cpp @@ -1315,7 +1315,7 @@ WSRunObject::DeleteChars(nsINode* aStartNode, if (!range) { range = new nsRange(aStartNode); nsresult rv = - range->Set(aStartNode, aStartOffset, aEndNode, aEndOffset); + range->SetStartAndEnd(aStartNode, aStartOffset, aEndNode, aEndOffset); NS_ENSURE_SUCCESS(rv, rv); } bool nodeBefore, nodeAfter; diff --git a/editor/txtsvc/nsTextServicesDocument.cpp b/editor/txtsvc/nsTextServicesDocument.cpp index e0c779683..ccf964d2c 100644 --- a/editor/txtsvc/nsTextServicesDocument.cpp +++ b/editor/txtsvc/nsTextServicesDocument.cpp @@ -406,11 +406,13 @@ nsTextServicesDocument::ExpandRangeToWordBoundaries(nsIDOMRange *aRange) // Now adjust the range so that it uses our new // end points. - - rv = range->SetEnd(rngEndNode, rngEndOffset); - NS_ENSURE_SUCCESS(rv, rv); - - return range->SetStart(rngStartNode, rngStartOffset); + nsCOMPtr startNode = do_QueryInterface(rngStartNode); + nsCOMPtr endNode = do_QueryInterface(rngEndNode); + rv = range->SetStartAndEnd(startNode, rngStartOffset, endNode, rngEndOffset); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } NS_IMETHODIMP -- cgit v1.2.3 From 516fd67d506b8dd3c2721dfd1aa1bbef4a2eda6f Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:43:18 -0400 Subject: Bug 1337698 - Use UniquePtr instead of nsAutoPtr in editor * PlaceholderTransaction should use UniquePtr * HTMLEditor should use UniquePtr * TypeInState should use UniquePtr Tag #1375 --- editor/libeditor/EditorBase.cpp | 8 ++------ editor/libeditor/EditorBase.h | 3 ++- editor/libeditor/HTMLEditRules.cpp | 10 ++++++---- editor/libeditor/HTMLEditor.cpp | 2 +- editor/libeditor/HTMLEditor.h | 4 ++-- editor/libeditor/PlaceholderTransaction.cpp | 23 ++++++++--------------- editor/libeditor/PlaceholderTransaction.h | 10 ++++------ editor/libeditor/TypeInState.cpp | 8 ++++---- editor/libeditor/TypeInState.h | 5 +++-- editor/libeditor/nsIAbsorbingTransaction.h | 3 --- 10 files changed, 32 insertions(+), 44 deletions(-) (limited to 'editor') diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index f0f3095d6..9bae5dfa0 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -688,13 +688,10 @@ EditorBase::DoTransaction(nsITransaction* aTxn) { if (mPlaceHolderBatch && !mPlaceHolderTxn) { nsCOMPtr placeholderTransaction = - new PlaceholderTransaction(); + new PlaceholderTransaction(*this, mPlaceHolderName, Move(mSelState)); // Save off weak reference to placeholder transaction mPlaceHolderTxn = do_GetWeakReference(placeholderTransaction); - placeholderTransaction->Init(mPlaceHolderName, mSelState, this); - // placeholder txn took ownership of this pointer - mSelState = nullptr; // QI to an nsITransaction since that's what DoTransaction() expects nsCOMPtr transaction = @@ -944,7 +941,7 @@ EditorBase::BeginPlaceHolderTransaction(nsIAtom* aName) mPlaceHolderName = aName; RefPtr selection = GetSelection(); if (selection) { - mSelState = new SelectionState(); + mSelState = MakeUnique(); mSelState->SaveSelection(selection); // Composition transaction can modify multiple nodes and it merges text // node for ime into single text node. @@ -1008,7 +1005,6 @@ EditorBase::EndPlaceHolderTransaction() if (mPlaceHolderName == nsGkAtoms::IMETxnName) { mRangeUpdater.DropSelectionState(*mSelState); } - delete mSelState; mSelState = nullptr; } // We might have never made a placeholder if no action took place. diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h index 8680b57ae..157623287 100644 --- a/editor/libeditor/EditorBase.h +++ b/editor/libeditor/EditorBase.h @@ -11,6 +11,7 @@ #include "mozilla/OwningNonNull.h" // for OwningNonNull #include "mozilla/SelectionState.h" // for RangeUpdater, etc. #include "mozilla/StyleSheet.h" // for StyleSheet +#include "mozilla/UniquePtr.h" #include "mozilla/dom/Text.h" #include "nsCOMPtr.h" // for already_AddRefed, nsCOMPtr #include "nsCycleCollectionParticipant.h" @@ -980,7 +981,7 @@ protected: // Name of placeholder transaction. nsIAtom* mPlaceHolderName; // Saved selection state for placeholder transaction batching. - SelectionState* mSelState; + mozilla::UniquePtr mSelState; nsString* mPhonetic; // IME composition this is not null between compositionstart and // compositionend. diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index 0aa2bde8c..c97ebf27f 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -16,12 +16,13 @@ #include "mozilla/EditorUtils.h" #include "mozilla/HTMLEditor.h" #include "mozilla/MathAlgorithms.h" +#include "mozilla/Move.h" #include "mozilla/Preferences.h" +#include "mozilla/UniquePtr.h" #include "mozilla/dom/Selection.h" #include "mozilla/dom/Element.h" #include "mozilla/OwningNonNull.h" #include "mozilla/mozalloc.h" -#include "nsAutoPtr.h" #include "nsAString.h" #include "nsAlgorithm.h" #include "nsCRT.h" @@ -4410,20 +4411,21 @@ HTMLEditRules::CreateStyleForInsertText(Selection& aSelection, NS_ENSURE_STATE(rootElement); // process clearing any styles first - nsAutoPtr item(mHTMLEditor->mTypeInState->TakeClearProperty()); + UniquePtr item = + Move(mHTMLEditor->mTypeInState->TakeClearProperty()); while (item && node != rootElement) { NS_ENSURE_STATE(mHTMLEditor); nsresult rv = mHTMLEditor->ClearStyle(address_of(node), &offset, item->tag, &item->attr); NS_ENSURE_SUCCESS(rv, rv); - item = mHTMLEditor->mTypeInState->TakeClearProperty(); + item = Move(mHTMLEditor->mTypeInState->TakeClearProperty()); weDidSomething = true; } // then process setting any styles int32_t relFontSize = mHTMLEditor->mTypeInState->TakeRelativeFontSize(); - item = mHTMLEditor->mTypeInState->TakeSetProperty(); + item = Move(mHTMLEditor->mTypeInState->TakeSetProperty()); if (item || relFontSize) { // we have at least one style to add; make a new text node to insert style diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 73dd1673b..56e607200 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -287,7 +287,7 @@ HTMLEditor::Init(nsIDOMDocument* aDoc, } // Init the HTML-CSS utils - mCSSEditUtils = new CSSEditUtils(this); + mCSSEditUtils = MakeUnique(this); // disable links nsCOMPtr presShell = GetPresShell(); diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index dfcdd8d6b..dc1a41b70 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -10,11 +10,11 @@ #include "mozilla/CSSEditUtils.h" #include "mozilla/StyleSheet.h" #include "mozilla/TextEditor.h" +#include "mozilla/UniquePtr.h" #include "mozilla/dom/Element.h" #include "mozilla/dom/File.h" #include "nsAttrName.h" -#include "nsAutoPtr.h" #include "nsCOMPtr.h" #include "nsIContentFilter.h" #include "nsICSSLoaderObserver.h" @@ -896,7 +896,7 @@ protected: bool mCRInParagraphCreatesParagraph; bool mCSSAware; - nsAutoPtr mCSSEditUtils; + UniquePtr mCSSEditUtils; // Used by GetFirstSelectedCell and GetNextSelectedCell int32_t mSelectedCellIndex; diff --git a/editor/libeditor/PlaceholderTransaction.cpp b/editor/libeditor/PlaceholderTransaction.cpp index fa808afad..5a76e391a 100644 --- a/editor/libeditor/PlaceholderTransaction.cpp +++ b/editor/libeditor/PlaceholderTransaction.cpp @@ -8,6 +8,7 @@ #include "CompositionTransaction.h" #include "mozilla/EditorBase.h" #include "mozilla/dom/Selection.h" +#include "mozilla/Move.h" #include "nsGkAtoms.h" #include "nsQueryObject.h" @@ -15,13 +16,18 @@ namespace mozilla { using namespace dom; -PlaceholderTransaction::PlaceholderTransaction() +PlaceholderTransaction::PlaceholderTransaction( + EditorBase& aEditorBase, + nsIAtom* aName, + UniquePtr aSelState) : mAbsorb(true) , mForwarding(nullptr) , mCompositionTransaction(nullptr) , mCommitted(false) - , mEditorBase(nullptr) + , mStartSel(Move(aSelState)) + , mEditorBase(&aEditorBase) { + mName = aName; } PlaceholderTransaction::~PlaceholderTransaction() @@ -56,19 +62,6 @@ NS_INTERFACE_MAP_END_INHERITING(EditAggregateTransaction) NS_IMPL_ADDREF_INHERITED(PlaceholderTransaction, EditAggregateTransaction) NS_IMPL_RELEASE_INHERITED(PlaceholderTransaction, EditAggregateTransaction) -NS_IMETHODIMP -PlaceholderTransaction::Init(nsIAtom* aName, - SelectionState* aSelState, - EditorBase* aEditorBase) -{ - NS_ENSURE_TRUE(aEditorBase && aSelState, NS_ERROR_NULL_POINTER); - - mName = aName; - mStartSel = aSelState; - mEditorBase = aEditorBase; - return NS_OK; -} - NS_IMETHODIMP PlaceholderTransaction::DoTransaction() { diff --git a/editor/libeditor/PlaceholderTransaction.h b/editor/libeditor/PlaceholderTransaction.h index 8193239be..7e592cc03 100644 --- a/editor/libeditor/PlaceholderTransaction.h +++ b/editor/libeditor/PlaceholderTransaction.h @@ -8,12 +8,12 @@ #include "EditAggregateTransaction.h" #include "mozilla/EditorUtils.h" +#include "mozilla/UniquePtr.h" #include "nsIAbsorbingTransaction.h" #include "nsIDOMNode.h" #include "nsCOMPtr.h" #include "nsWeakPtr.h" #include "nsWeakReference.h" -#include "nsAutoPtr.h" namespace mozilla { @@ -33,7 +33,8 @@ class PlaceholderTransaction final : public EditAggregateTransaction, public: NS_DECL_ISUPPORTS_INHERITED - PlaceholderTransaction(); + PlaceholderTransaction(EditorBase& aEditorBase, nsIAtom* aName, + UniquePtr aSelState); NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(PlaceholderTransaction, EditAggregateTransaction) @@ -46,9 +47,6 @@ public: // ------------ nsIAbsorbingTransaction ----------------------- - NS_IMETHOD Init(nsIAtom* aName, SelectionState* aSelState, - EditorBase* aEditorBase) override; - NS_IMETHOD GetTxnName(nsIAtom** aName) override; NS_IMETHOD StartSelectionEquals(SelectionState* aSelState, @@ -80,7 +78,7 @@ protected: // restore the selection properly. // Use a pointer because this is constructed before we exist. - nsAutoPtr mStartSel; + UniquePtr mStartSel; SelectionState mEndSel; // The editor for this transaction. diff --git a/editor/libeditor/TypeInState.cpp b/editor/libeditor/TypeInState.cpp index ce43e5e4d..840139fee 100644 --- a/editor/libeditor/TypeInState.cpp +++ b/editor/libeditor/TypeInState.cpp @@ -193,7 +193,7 @@ TypeInState::ClearProp(nsIAtom* aProp, * TakeClearProperty() hands back next property item on the clear list. * Caller assumes ownership of PropItem and must delete it. */ -PropItem* +UniquePtr TypeInState::TakeClearProperty() { size_t count = mClearedArray.Length(); @@ -204,14 +204,14 @@ TypeInState::TakeClearProperty() --count; // indices are zero based PropItem* propItem = mClearedArray[count]; mClearedArray.RemoveElementAt(count); - return propItem; + return UniquePtr(propItem); } /** * TakeSetProperty() hands back next poroperty item on the set list. * Caller assumes ownership of PropItem and must delete it. */ -PropItem* +UniquePtr TypeInState::TakeSetProperty() { size_t count = mSetArray.Length(); @@ -221,7 +221,7 @@ TypeInState::TakeSetProperty() count--; // indices are zero based PropItem* propItem = mSetArray[count]; mSetArray.RemoveElementAt(count); - return propItem; + return UniquePtr(propItem); } /** diff --git a/editor/libeditor/TypeInState.h b/editor/libeditor/TypeInState.h index 540b2d9c1..e1b949508 100644 --- a/editor/libeditor/TypeInState.h +++ b/editor/libeditor/TypeInState.h @@ -6,6 +6,7 @@ #ifndef TypeInState_h #define TypeInState_h +#include "mozilla/UniquePtr.h" #include "nsCOMPtr.h" #include "nsCycleCollectionParticipant.h" #include "nsISelectionListener.h" @@ -63,13 +64,13 @@ public: * TakeClearProperty() hands back next property item on the clear list. * Caller assumes ownership of PropItem and must delete it. */ - PropItem* TakeClearProperty(); + UniquePtr TakeClearProperty(); /** * TakeSetProperty() hands back next property item on the set list. * Caller assumes ownership of PropItem and must delete it. */ - PropItem* TakeSetProperty(); + UniquePtr TakeSetProperty(); /** * TakeRelativeFontSize() hands back relative font value, which is then diff --git a/editor/libeditor/nsIAbsorbingTransaction.h b/editor/libeditor/nsIAbsorbingTransaction.h index e22caed4a..b2d7b2c79 100644 --- a/editor/libeditor/nsIAbsorbingTransaction.h +++ b/editor/libeditor/nsIAbsorbingTransaction.h @@ -35,9 +35,6 @@ public: NS_DECLARE_STATIC_IID_ACCESSOR(NS_IABSORBINGTRANSACTION_IID) - NS_IMETHOD Init(nsIAtom* aName, mozilla::SelectionState* aSelState, - mozilla::EditorBase* aEditorBase) = 0; - NS_IMETHOD EndPlaceHolderBatch()=0; NS_IMETHOD GetTxnName(nsIAtom **aName)=0; -- cgit v1.2.3 From 5a379a4b15b4da55f5fda0be56c43a85e0162f05 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:43:47 -0400 Subject: Bug 1371170 - Add non-virtual EditorBase::GetSelectionController Tag #1375 --- editor/libeditor/EditorBase.cpp | 20 +++++++++++++------- editor/libeditor/EditorBase.h | 1 + 2 files changed, 14 insertions(+), 7 deletions(-) (limited to 'editor') diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 9bae5dfa0..d37070849 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -629,6 +629,17 @@ EditorBase::GetSelectionController(nsISelectionController** aSel) { NS_ENSURE_TRUE(aSel, NS_ERROR_NULL_POINTER); *aSel = nullptr; // init out param + nsCOMPtr selCon = GetSelectionController(); + if (NS_WARN_IF(!selCon)) { + return NS_ERROR_NOT_INITIALIZED; + } + selCon.forget(aSel); + return NS_OK; +} + +already_AddRefed +EditorBase::GetSelectionController() +{ nsCOMPtr selCon; if (mSelConWeak) { selCon = do_QueryReferent(mSelConWeak); @@ -636,11 +647,7 @@ EditorBase::GetSelectionController(nsISelectionController** aSel) nsCOMPtr presShell = GetPresShell(); selCon = do_QueryInterface(presShell); } - if (!selCon) { - return NS_ERROR_NOT_INITIALIZED; - } - NS_ADDREF(*aSel = selCon); - return NS_OK; + return selCon.forget(); } NS_IMETHODIMP @@ -663,8 +670,7 @@ EditorBase::GetSelection(SelectionType aSelectionType, { NS_ENSURE_TRUE(aSelection, NS_ERROR_NULL_POINTER); *aSelection = nullptr; - nsCOMPtr selcon; - GetSelectionController(getter_AddRefs(selcon)); + nsCOMPtr selcon = GetSelectionController(); if (!selcon) { return NS_ERROR_NOT_INITIALIZED; } diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h index 157623287..83917c150 100644 --- a/editor/libeditor/EditorBase.h +++ b/editor/libeditor/EditorBase.h @@ -453,6 +453,7 @@ protected: */ bool EnsureComposition(WidgetCompositionEvent* aCompositionEvent); + already_AddRefed GetSelectionController(); nsresult GetSelection(SelectionType aSelectionType, nsISelection** aSelection); -- cgit v1.2.3 From ea3a2ce279f92457bfd6168f97b106be193ea740 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:45:53 -0400 Subject: Bug 1372829 - Part 1: Make mozilla::PlaceholderTransaction inherit mozilla::SupportsWeakPtr instead of nsSupportsWeakReference Tag #1375 --- editor/libeditor/EditorBase.cpp | 64 ++++++++++++++--------------- editor/libeditor/EditorBase.h | 10 +++-- editor/libeditor/PlaceholderTransaction.cpp | 1 - editor/libeditor/PlaceholderTransaction.h | 15 +++++-- editor/libeditor/TextEditor.cpp | 2 +- editor/libeditor/nsIAbsorbingTransaction.h | 4 ++ 6 files changed, 53 insertions(+), 43 deletions(-) (limited to 'editor') diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index d37070849..f068e9179 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -127,13 +127,13 @@ using namespace widget; *****************************************************************************/ EditorBase::EditorBase() - : mPlaceHolderName(nullptr) + : mPlaceholderName(nullptr) , mSelState(nullptr) , mPhonetic(nullptr) , mModCount(0) , mFlags(0) , mUpdateCount(0) - , mPlaceHolderBatch(0) + , mPlaceholderBatch(0) , mAction(EditAction::none) , mIMETextOffset(0) , mIMETextLength(0) @@ -692,29 +692,29 @@ EditorBase::GetSelection(SelectionType aSelectionType) NS_IMETHODIMP EditorBase::DoTransaction(nsITransaction* aTxn) { - if (mPlaceHolderBatch && !mPlaceHolderTxn) { - nsCOMPtr placeholderTransaction = - new PlaceholderTransaction(*this, mPlaceHolderName, Move(mSelState)); + if (mPlaceholderBatch && !mPlaceholderTransactionWeak) { + RefPtr placeholderTransaction = + new PlaceholderTransaction(*this, mPlaceholderName, Move(mSelState)); // Save off weak reference to placeholder transaction - mPlaceHolderTxn = do_GetWeakReference(placeholderTransaction); + mPlaceholderTransactionWeak = placeholderTransaction; - // QI to an nsITransaction since that's what DoTransaction() expects - nsCOMPtr transaction = - do_QueryInterface(placeholderTransaction); // We will recurse, but will not hit this case in the nested call - DoTransaction(transaction); + DoTransaction(placeholderTransaction); if (mTxnMgr) { - nsCOMPtr topTxn = mTxnMgr->PeekUndoStack(); - if (topTxn) { - placeholderTransaction = do_QueryInterface(topTxn); - if (placeholderTransaction) { + nsCOMPtr topTransaction = mTxnMgr->PeekUndoStack(); + nsCOMPtr topAbsorbingTransaction = + do_QueryInterface(topTransaction); + if (topAbsorbingTransaction) { + RefPtr topPlaceholderTransaction = + topAbsorbingTransaction->AsPlaceholderTransaction(); + if (topPlaceholderTransaction) { // there is a placeholder transaction on top of the undo stack. It // is either the one we just created, or an earlier one that we are // now merging into. From here on out remember this placeholder // instead of the one we just created. - mPlaceHolderTxn = do_GetWeakReference(placeholderTransaction); + mPlaceholderTransactionWeak = topPlaceholderTransaction; } } } @@ -938,13 +938,13 @@ EditorBase::EndTransaction() NS_IMETHODIMP EditorBase::BeginPlaceHolderTransaction(nsIAtom* aName) { - NS_PRECONDITION(mPlaceHolderBatch >= 0, "negative placeholder batch count!"); - if (!mPlaceHolderBatch) { + MOZ_ASSERT(mPlaceholderBatch >= 0, "negative placeholder batch count!"); + if (!mPlaceholderBatch) { NotifyEditorObservers(eNotifyEditorObserversOfBefore); // time to turn on the batch BeginUpdateViewBatch(); - mPlaceHolderTxn = nullptr; - mPlaceHolderName = aName; + mPlaceholderTransactionWeak = nullptr; + mPlaceholderName = aName; RefPtr selection = GetSelection(); if (selection) { mSelState = MakeUnique(); @@ -954,12 +954,12 @@ EditorBase::BeginPlaceHolderTransaction(nsIAtom* aName) // So if current selection is into IME text node, it might be failed // to restore selection by UndoTransaction. // So we need update selection by range updater. - if (mPlaceHolderName == nsGkAtoms::IMETxnName) { + if (mPlaceholderName == nsGkAtoms::IMETxnName) { mRangeUpdater.RegisterSelectionState(*mSelState); } } } - mPlaceHolderBatch++; + mPlaceholderBatch++; return NS_OK; } @@ -967,8 +967,9 @@ EditorBase::BeginPlaceHolderTransaction(nsIAtom* aName) NS_IMETHODIMP EditorBase::EndPlaceHolderTransaction() { - NS_PRECONDITION(mPlaceHolderBatch > 0, "zero or negative placeholder batch count when ending batch!"); - if (mPlaceHolderBatch == 1) { + MOZ_ASSERT(mPlaceholderBatch > 0, + "zero or negative placeholder batch count when ending batch!"); + if (mPlaceholderBatch == 1) { RefPtr selection = GetSelection(); // By making the assumption that no reflow happens during the calls @@ -1008,21 +1009,16 @@ EditorBase::EndPlaceHolderTransaction() if (mSelState) { // we saved the selection state, but never got to hand it to placeholder // (else we ould have nulled out this pointer), so destroy it to prevent leaks. - if (mPlaceHolderName == nsGkAtoms::IMETxnName) { + if (mPlaceholderName == nsGkAtoms::IMETxnName) { mRangeUpdater.DropSelectionState(*mSelState); } mSelState = nullptr; } // We might have never made a placeholder if no action took place. - if (mPlaceHolderTxn) { - nsCOMPtr plcTxn = do_QueryReferent(mPlaceHolderTxn); - if (plcTxn) { - plcTxn->EndPlaceHolderBatch(); - } else { - // in the future we will check to make sure undo is off here, - // since that is the only known case where the placeholdertxn would disappear on us. - // For now just removing the assert. - } + if (mPlaceholderTransactionWeak) { + RefPtr placeholderTransaction = + mPlaceholderTransactionWeak.get(); + placeholderTransaction->EndPlaceHolderBatch(); // notify editor observers of action but if composing, it's done by // compositionchange event handler. if (!mComposition) { @@ -1032,7 +1028,7 @@ EditorBase::EndPlaceHolderTransaction() NotifyEditorObservers(eNotifyEditorObserversOfCancel); } } - mPlaceHolderBatch--; + mPlaceholderBatch--; return NS_OK; } diff --git a/editor/libeditor/EditorBase.h b/editor/libeditor/EditorBase.h index 83917c150..7ea0f4dab 100644 --- a/editor/libeditor/EditorBase.h +++ b/editor/libeditor/EditorBase.h @@ -10,8 +10,9 @@ #include "mozFlushType.h" // for mozFlushType enum #include "mozilla/OwningNonNull.h" // for OwningNonNull #include "mozilla/SelectionState.h" // for RangeUpdater, etc. -#include "mozilla/StyleSheet.h" // for StyleSheet +#include "mozilla/StyleSheet.h" // for StyleSheet #include "mozilla/UniquePtr.h" +#include "mozilla/WeakPtr.h" // for WeakPtr #include "mozilla/dom/Text.h" #include "nsCOMPtr.h" // for already_AddRefed, nsCOMPtr #include "nsCycleCollectionParticipant.h" @@ -116,6 +117,7 @@ class ErrorResult; class InsertNodeTransaction; class InsertTextTransaction; class JoinNodeTransaction; +class PlaceholderTransaction; class RemoveStyleSheetTransaction; class SplitNodeTransaction; class TextComposition; @@ -976,11 +978,11 @@ protected: // Weak reference to the nsISelectionController. nsWeakPtr mSelConWeak; // Weak reference to placeholder for begin/end batch purposes. - nsWeakPtr mPlaceHolderTxn; + WeakPtr mPlaceholderTransactionWeak; // Weak reference to the nsIDOMDocument. nsWeakPtr mDocWeak; // Name of placeholder transaction. - nsIAtom* mPlaceHolderName; + nsIAtom* mPlaceholderName; // Saved selection state for placeholder transaction batching. mozilla::UniquePtr mSelState; nsString* mPhonetic; @@ -1014,7 +1016,7 @@ protected: int32_t mUpdateCount; // Nesting count for batching. - int32_t mPlaceHolderBatch; + int32_t mPlaceholderBatch; // The current editor action. EditAction mAction; diff --git a/editor/libeditor/PlaceholderTransaction.cpp b/editor/libeditor/PlaceholderTransaction.cpp index 5a76e391a..142a85075 100644 --- a/editor/libeditor/PlaceholderTransaction.cpp +++ b/editor/libeditor/PlaceholderTransaction.cpp @@ -56,7 +56,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(PlaceholderTransaction) NS_INTERFACE_MAP_ENTRY(nsIAbsorbingTransaction) - NS_INTERFACE_MAP_ENTRY(nsISupportsWeakReference) NS_INTERFACE_MAP_END_INHERITING(EditAggregateTransaction) NS_IMPL_ADDREF_INHERITED(PlaceholderTransaction, EditAggregateTransaction) diff --git a/editor/libeditor/PlaceholderTransaction.h b/editor/libeditor/PlaceholderTransaction.h index 7e592cc03..5fa58a1e9 100644 --- a/editor/libeditor/PlaceholderTransaction.h +++ b/editor/libeditor/PlaceholderTransaction.h @@ -9,6 +9,7 @@ #include "EditAggregateTransaction.h" #include "mozilla/EditorUtils.h" #include "mozilla/UniquePtr.h" +#include "mozilla/WeakPtr.h" #include "nsIAbsorbingTransaction.h" #include "nsIDOMNode.h" #include "nsCOMPtr.h" @@ -26,11 +27,14 @@ class CompositionTransaction; * transactions it has absorbed. */ -class PlaceholderTransaction final : public EditAggregateTransaction, - public nsIAbsorbingTransaction, - public nsSupportsWeakReference +class PlaceholderTransaction final + : public EditAggregateTransaction + , public nsIAbsorbingTransaction + , public SupportsWeakPtr { public: + MOZ_DECLARE_WEAKREFERENCE_TYPENAME(PlaceholderTransaction) + NS_DECL_ISUPPORTS_INHERITED PlaceholderTransaction(EditorBase& aEditorBase, nsIAtom* aName, @@ -59,6 +63,11 @@ public: NS_IMETHOD Commit() override; + NS_IMETHOD_(PlaceholderTransaction*) AsPlaceholderTransaction() override + { + return this; + } + nsresult RememberEndingSelection(); protected: diff --git a/editor/libeditor/TextEditor.cpp b/editor/libeditor/TextEditor.cpp index 1e855d769..35ea5da83 100644 --- a/editor/libeditor/TextEditor.cpp +++ b/editor/libeditor/TextEditor.cpp @@ -864,7 +864,7 @@ TextEditor::UpdateIMEComposition(WidgetCompositionEvent* aCompositionChangeEvent // of NotifiyEditorObservers(eNotifyEditorObserversOfEnd) or // NotifiyEditorObservers(eNotifyEditorObserversOfCancel) which notifies // TextComposition of a selection change. - MOZ_ASSERT(!mPlaceHolderBatch, + MOZ_ASSERT(!mPlaceholderBatch, "UpdateIMEComposition() must be called without place holder batch"); TextComposition::CompositionChangeEventHandlingMarker compositionChangeEventHandlingMarker(mComposition, aCompositionChangeEvent); diff --git a/editor/libeditor/nsIAbsorbingTransaction.h b/editor/libeditor/nsIAbsorbingTransaction.h index b2d7b2c79..06329f3d4 100644 --- a/editor/libeditor/nsIAbsorbingTransaction.h +++ b/editor/libeditor/nsIAbsorbingTransaction.h @@ -23,6 +23,7 @@ class nsIAtom; namespace mozilla { class EditorBase; +class PlaceholderTransaction; class SelectionState; } // namespace mozilla @@ -45,6 +46,9 @@ public: NS_IMETHOD ForwardEndBatchTo(nsIAbsorbingTransaction *aForwardingAddress)=0; NS_IMETHOD Commit()=0; + + NS_IMETHOD_(mozilla::PlaceholderTransaction*) + AsPlaceholderTransaction() = 0; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsIAbsorbingTransaction, -- cgit v1.2.3 From 2e2190a5044943bde31679996afdc3558d22231b Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:46:23 -0400 Subject: Bug 1332353 - Make it clearer when a stylesheet is really owned by its mDocument Tag #1375 --- editor/libeditor/HTMLEditor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'editor') diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 56e607200..3c43ade3b 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -2961,7 +2961,7 @@ HTMLEditor::EnableStyleSheet(const nsAString& aURL, // Ensure the style sheet is owned by our document. nsCOMPtr doc = do_QueryReferent(mDocWeak); - sheet->SetOwningDocument(doc); + sheet->SetAssociatedDocument(doc, StyleSheet::NotOwnedByDocument); if (sheet->IsServo()) { // XXXheycam ServoStyleSheets don't support being enabled/disabled yet. @@ -2983,7 +2983,7 @@ HTMLEditor::EnableExistingStyleSheet(const nsAString& aURL) // Ensure the style sheet is owned by our document. nsCOMPtr doc = do_QueryReferent(mDocWeak); - sheet->SetOwningDocument(doc); + sheet->SetAssociatedDocument(doc, StyleSheet::NotOwnedByDocument); if (sheet->IsServo()) { // XXXheycam ServoStyleSheets don't support being enabled/disabled yet. -- cgit v1.2.3 From 32e8155127126c187ce32f7368742057bcaf69da Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:47:12 -0400 Subject: Bug 1372829 - Part 2: mozilla::EditorBase should cache raw pointer of nsISelectionController and nsIDocument with nsWeakPtr Tag #1375 --- editor/libeditor/EditorBase.cpp | 214 ++++++++++++++++++++--------------- editor/libeditor/EditorBase.h | 68 ++++++++++- editor/libeditor/HTMLEditor.cpp | 160 +++++++++++++++----------- editor/libeditor/HTMLTableEditor.cpp | 1 - editor/libeditor/TextEditor.cpp | 2 +- 5 files changed, 281 insertions(+), 164 deletions(-) (limited to 'editor') diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index f068e9179..5df4ff2c4 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -37,6 +37,7 @@ #include "mozilla/TextComposition.h" // for TextComposition #include "mozilla/TextEvents.h" #include "mozilla/dom/Element.h" // for Element, nsINode::AsElement +#include "mozilla/dom/HTMLBodyElement.h" #include "mozilla/dom/Text.h" #include "mozilla/dom/Event.h" #include "mozilla/mozalloc.h" // for operator new, etc. @@ -71,7 +72,6 @@ #include "nsIDOMNode.h" // for nsIDOMNode, etc. #include "nsIDOMNodeList.h" // for nsIDOMNodeList #include "nsIDOMText.h" // for nsIDOMText -#include "nsIDocument.h" // for nsIDocument #include "nsIDocumentStateListener.h" // for nsIDocumentStateListener #include "nsIEditActionListener.h" // for nsIEditActionListener #include "nsIEditorObserver.h" // for nsIEditorObserver @@ -151,7 +151,8 @@ EditorBase::EditorBase() EditorBase::~EditorBase() { - NS_ASSERTION(!mDocWeak || mDidPreDestroy, "Why PreDestroy hasn't been called?"); + MOZ_ASSERT(!IsInitialized() || mDidPreDestroy, + "Why PreDestroy hasn't been called?"); if (mComposition) { mComposition->OnEditorDestroyed(); @@ -220,20 +221,21 @@ NS_IMPL_CYCLE_COLLECTING_RELEASE(EditorBase) NS_IMETHODIMP -EditorBase::Init(nsIDOMDocument* aDoc, +EditorBase::Init(nsIDOMDocument* aDOMDocument, nsIContent* aRoot, - nsISelectionController* aSelCon, + nsISelectionController* aSelectionController, uint32_t aFlags, const nsAString& aValue) { MOZ_ASSERT(mAction == EditAction::none, "Initializing during an edit action is an error"); - MOZ_ASSERT(aDoc); - if (!aDoc) + MOZ_ASSERT(aDOMDocument); + if (!aDOMDocument) { return NS_ERROR_NULL_POINTER; + } // First only set flags, but other stuff shouldn't be initialized now. - // Don't move this call after initializing mDocWeak. + // Don't move this call after initializing mDocumentWeak. // SetFlags() can check whether it's called during initialization or not by // them. Note that SetFlags() will be called by PostCreate(). #ifdef DEBUG @@ -242,19 +244,21 @@ EditorBase::Init(nsIDOMDocument* aDoc, SetFlags(aFlags); NS_ASSERTION(NS_SUCCEEDED(rv), "SetFlags() failed"); - mDocWeak = do_GetWeakReference(aDoc); // weak reference to doc + nsCOMPtr document = do_QueryInterface(aDOMDocument); + mDocumentWeak = document.get(); // HTML editors currently don't have their own selection controller, // so they'll pass null as aSelCon, and we'll get the selection controller // off of the presshell. - nsCOMPtr selCon; - if (aSelCon) { - mSelConWeak = do_GetWeakReference(aSelCon); // weak reference to selectioncontroller - selCon = aSelCon; + nsCOMPtr selectionController; + if (aSelectionController) { + mSelectionControllerWeak = aSelectionController; + selectionController = aSelectionController; } else { nsCOMPtr presShell = GetPresShell(); - selCon = do_QueryInterface(presShell); + selectionController = do_QueryInterface(presShell); } - NS_ASSERTION(selCon, "Selection controller should be available at this point"); + MOZ_ASSERT(selectionController, + "Selection controller should be available at this point"); //set up root element if we are passed one. if (aRoot) @@ -271,13 +275,14 @@ EditorBase::Init(nsIDOMDocument* aDoc, mIMETextNode = nullptr; } - /* Show the caret */ - selCon->SetCaretReadOnly(false); - selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); - - selCon->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL);//we want to see all the selection reflected to user + // Show the caret. + selectionController->SetCaretReadOnly(false); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_ON); + // Show all the selection reflected to user. + selectionController->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL); - NS_POSTCONDITION(mDocWeak, "bad state"); + MOZ_ASSERT(IsInitialized()); // Make sure that the editor will be destroyed properly mDidPreDestroy = false; @@ -356,8 +361,9 @@ EditorBase::CreateEventListeners() nsresult EditorBase::InstallEventListeners() { - NS_ENSURE_TRUE(mDocWeak && mEventListener, - NS_ERROR_NOT_INITIALIZED); + if (NS_WARN_IF(!IsInitialized()) || NS_WARN_IF(!mEventListener)) { + return NS_ERROR_NOT_INITIALIZED; + } // Initialize the event target. nsCOMPtr rootContent = GetRoot(); @@ -378,7 +384,7 @@ EditorBase::InstallEventListeners() void EditorBase::RemoveEventListeners() { - if (!mDocWeak || !mEventListener) { + if (!IsInitialized() || !mEventListener) { return; } reinterpret_cast(mEventListener.get())->Disconnect(); @@ -501,7 +507,7 @@ EditorBase::SetFlags(uint32_t aFlags) bool spellcheckerWasEnabled = CanEnableSpellCheck(); mFlags = aFlags; - if (!mDocWeak) { + if (!IsInitialized()) { // If we're initializing, we shouldn't do anything now. // SetFlags() will be called by PostCreate(), // we should synchronize some stuff for the flags at that time. @@ -567,17 +573,15 @@ EditorBase::GetIsDocumentEditable(bool* aIsDocumentEditable) already_AddRefed EditorBase::GetDocument() { - NS_PRECONDITION(mDocWeak, "bad state, mDocWeak weak pointer not initialized"); - nsCOMPtr doc = do_QueryReferent(mDocWeak); - return doc.forget(); + nsCOMPtr document = mDocumentWeak.get(); + return document.forget(); } already_AddRefed EditorBase::GetDOMDocument() { - NS_PRECONDITION(mDocWeak, "bad state, mDocWeak weak pointer not initialized"); - nsCOMPtr doc = do_QueryReferent(mDocWeak); - return doc.forget(); + nsCOMPtr domDocument = do_QueryInterface(mDocumentWeak); + return domDocument.forget(); } NS_IMETHODIMP @@ -590,11 +594,12 @@ EditorBase::GetDocument(nsIDOMDocument** aDoc) already_AddRefed EditorBase::GetPresShell() { - NS_PRECONDITION(mDocWeak, "bad state, null mDocWeak"); - nsCOMPtr doc = do_QueryReferent(mDocWeak); - NS_ENSURE_TRUE(doc, nullptr); - nsCOMPtr ps = doc->GetShell(); - return ps.forget(); + nsCOMPtr document = GetDocument(); + if (NS_WARN_IF(!document)) { + return nullptr; + } + nsCOMPtr presShell = document->GetShell(); + return presShell.forget(); } already_AddRefed @@ -640,14 +645,14 @@ EditorBase::GetSelectionController(nsISelectionController** aSel) already_AddRefed EditorBase::GetSelectionController() { - nsCOMPtr selCon; - if (mSelConWeak) { - selCon = do_QueryReferent(mSelConWeak); + nsCOMPtr selectionController; + if (mSelectionControllerWeak) { + selectionController = mSelectionControllerWeak.get(); } else { nsCOMPtr presShell = GetPresShell(); - selCon = do_QueryInterface(presShell); + selectionController = do_QueryInterface(presShell); } - return selCon.forget(); + return selectionController.forget(); } NS_IMETHODIMP @@ -1065,7 +1070,8 @@ EditorBase::GetDocumentIsEmpty(bool* aDocumentIsEmpty) NS_IMETHODIMP EditorBase::SelectAll() { - if (!mDocWeak) { + // XXX Why doesn't this check if the document is alive? + if (!IsInitialized()) { return NS_ERROR_NOT_INITIALIZED; } ForceCompositionEnd(); @@ -1078,7 +1084,8 @@ EditorBase::SelectAll() NS_IMETHODIMP EditorBase::BeginningOfDocument() { - if (!mDocWeak) { + // XXX Why doesn't this check if the document is alive? + if (!IsInitialized()) { return NS_ERROR_NOT_INITIALIZED; } @@ -1115,7 +1122,10 @@ EditorBase::BeginningOfDocument() NS_IMETHODIMP EditorBase::EndOfDocument() { - NS_ENSURE_TRUE(mDocWeak, NS_ERROR_NOT_INITIALIZED); + // XXX Why doesn't this check if the document is alive? + if (NS_WARN_IF(!IsInitialized())) { + return NS_ERROR_NOT_INITIALIZED; + } // get selection RefPtr selection = GetSelection(); @@ -1150,20 +1160,22 @@ EditorBase::GetDocumentModified(bool* outDocModified) NS_IMETHODIMP EditorBase::GetDocumentCharacterSet(nsACString& characterSet) { - nsCOMPtr doc = do_QueryReferent(mDocWeak); - NS_ENSURE_TRUE(doc, NS_ERROR_UNEXPECTED); - - characterSet = doc->GetDocumentCharacterSet(); + nsCOMPtr document = GetDocument(); + if (NS_WARN_IF(!document)) { + return NS_ERROR_UNEXPECTED; + } + characterSet = document->GetDocumentCharacterSet(); return NS_OK; } NS_IMETHODIMP EditorBase::SetDocumentCharacterSet(const nsACString& characterSet) { - nsCOMPtr doc = do_QueryReferent(mDocWeak); - NS_ENSURE_TRUE(doc, NS_ERROR_UNEXPECTED); - - doc->SetDocumentCharacterSet(characterSet); + nsCOMPtr document = GetDocument(); + if (NS_WARN_IF(!document)) { + return NS_ERROR_UNEXPECTED; + } + document->SetDocumentCharacterSet(characterSet); return NS_OK; } @@ -2004,12 +2016,17 @@ NS_IMETHODIMP EditorBase::DebugDumpContent() { #ifdef DEBUG - nsCOMPtr doc = do_QueryReferent(mDocWeak); - NS_ENSURE_TRUE(doc, NS_ERROR_NOT_INITIALIZED); - - nsCOMPtrbodyElem; - doc->GetBody(getter_AddRefs(bodyElem)); - nsCOMPtr content = do_QueryInterface(bodyElem); + nsCOMPtr document = GetDocument(); + if (NS_WARN_IF(!document)) { + return NS_ERROR_NOT_INITIALIZED; + } + nsCOMPtr domHTMLDocument = do_QueryInterface(document); + if (NS_WARN_IF(!domHTMLDocument)) { + return NS_ERROR_NOT_INITIALIZED; + } + nsCOMPtr bodyElement; + domHTMLDocument->GetBody(getter_AddRefs(bodyElement)); + nsCOMPtr content = do_QueryInterface(bodyElement); if (content) { content->List(); } @@ -2323,18 +2340,20 @@ EditorBase::CloneAttributes(Element* aDest, NS_IMETHODIMP EditorBase::ScrollSelectionIntoView(bool aScrollToAnchor) { - nsCOMPtr selCon; - if (NS_SUCCEEDED(GetSelectionController(getter_AddRefs(selCon))) && selCon) { - int16_t region = nsISelectionController::SELECTION_FOCUS_REGION; - - if (aScrollToAnchor) { - region = nsISelectionController::SELECTION_ANCHOR_REGION; - } - - selCon->ScrollSelectionIntoView(nsISelectionController::SELECTION_NORMAL, - region, nsISelectionController::SCROLL_OVERFLOW_HIDDEN); + nsCOMPtr selectionController = + GetSelectionController(); + if (!selectionController) { + return NS_OK; } + int16_t region = nsISelectionController::SELECTION_FOCUS_REGION; + if (aScrollToAnchor) { + region = nsISelectionController::SELECTION_ANCHOR_REGION; + } + selectionController->ScrollSelectionIntoView( + nsISelectionController::SELECTION_NORMAL, + region, + nsISelectionController::SCROLL_OVERFLOW_HIDDEN); return NS_OK; } @@ -4784,22 +4803,27 @@ EditorBase::InitializeSelection(nsIDOMEventTarget* aFocusEventTarget) nsCOMPtr presShell = GetPresShell(); NS_ENSURE_TRUE(presShell, NS_ERROR_NOT_INITIALIZED); - nsCOMPtr selCon; - nsresult rv = GetSelectionController(getter_AddRefs(selCon)); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr selectionController = + GetSelectionController(); + if (NS_WARN_IF(!selectionController)) { + return NS_ERROR_FAILURE; + } // Init the caret RefPtr caret = presShell->GetCaret(); NS_ENSURE_TRUE(caret, NS_ERROR_UNEXPECTED); caret->SetIgnoreUserModify(false); caret->SetSelection(selection); - selCon->SetCaretReadOnly(IsReadonly()); - selCon->SetCaretEnabled(true); + selectionController->SetCaretReadOnly(IsReadonly()); + selectionController->SetCaretEnabled(true); // Init selection - selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); - selCon->SetSelectionFlags(nsISelectionDisplay::DISPLAY_ALL); - selCon->RepaintSelection(nsISelectionController::SELECTION_NORMAL); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_ON); + selectionController->SetSelectionFlags( + nsISelectionDisplay::DISPLAY_ALL); + selectionController->RepaintSelection( + nsISelectionController::SELECTION_NORMAL); // If the computed selection root isn't root content, we should set it // as selection ancestor limit. However, if that is root element, it means // there is not limitation of the selection, then, we must set nullptr. @@ -4867,9 +4891,11 @@ private: NS_IMETHODIMP EditorBase::FinalizeSelection() { - nsCOMPtr selCon; - nsresult rv = GetSelectionController(getter_AddRefs(selCon)); - NS_ENSURE_SUCCESS(rv, rv); + nsCOMPtr selectionController = + GetSelectionController(); + if (NS_WARN_IF(!selectionController)) { + return NS_ERROR_FAILURE; + } RefPtr selection = GetSelection(); NS_ENSURE_STATE(selection); @@ -4879,7 +4905,7 @@ EditorBase::FinalizeSelection() nsCOMPtr presShell = GetPresShell(); NS_ENSURE_TRUE(presShell, NS_ERROR_NOT_INITIALIZED); - selCon->SetCaretEnabled(false); + selectionController->SetCaretEnabled(false); nsFocusManager* fm = nsFocusManager::GetFocusManager(); NS_ENSURE_TRUE(fm, NS_ERROR_NOT_INITIALIZED); @@ -4894,29 +4920,33 @@ EditorBase::FinalizeSelection() ErrorResult ret; if (!doc || !doc->HasFocus(ret)) { // If the document already lost focus, mark the selection as disabled. - selCon->SetDisplaySelection(nsISelectionController::SELECTION_DISABLED); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_DISABLED); } else { // Otherwise, mark selection as normal because outside of a // contenteditable element should be selected with normal selection // color after here. - selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON); + selectionController->SetDisplaySelection( + nsISelectionController::SELECTION_ON); } } else if (IsFormWidget() || IsPasswordEditor() || IsReadonly() || IsDisabled() || IsInputFiltered()) { // In or