Bug 1345690 part.2 Make the constructor of DeleteRangeTransaction initialize all members instead of Init() r?m_kato
Similar to DeleteNodeTransaction, DeleteRangeTransaction should take all necessary information as its arguments. However, different from DeleteNodeTransaction, this doesn't need to implement CanDoIt() since nobody checks the state.
MozReview-Commit-ID: 2Z9fNtGeJ9c
--- a/editor/libeditor/DeleteRangeTransaction.cpp
+++ b/editor/libeditor/DeleteRangeTransaction.cpp
@@ -19,60 +19,42 @@
#include "nsINode.h"
#include "nsAString.h"
namespace mozilla {
using namespace dom;
// note that aEditorBase is not refcounted
-DeleteRangeTransaction::DeleteRangeTransaction()
- : mEditorBase(nullptr)
- , mRangeUpdater(nullptr)
+DeleteRangeTransaction::DeleteRangeTransaction(EditorBase& aEditorBase,
+ nsRange& aRangeToDelete,
+ RangeUpdater* aRangeUpdater)
+ : mEditorBase(aEditorBase)
+ , mRangeToDelete(aRangeToDelete.CloneRange())
+ , mRangeUpdater(aRangeUpdater)
{
}
NS_IMPL_CYCLE_COLLECTION_INHERITED(DeleteRangeTransaction,
EditAggregateTransaction,
- mRange)
+ mRangeToDelete)
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DeleteRangeTransaction)
NS_INTERFACE_MAP_END_INHERITING(EditAggregateTransaction)
-nsresult
-DeleteRangeTransaction::Init(EditorBase* aEditorBase,
- nsRange* aRange,
- RangeUpdater* aRangeUpdater)
-{
- MOZ_ASSERT(aEditorBase && aRange);
-
- mEditorBase = aEditorBase;
- mRange = aRange->CloneRange();
- mRangeUpdater = aRangeUpdater;
-
- NS_ENSURE_TRUE(mEditorBase->IsModifiableNode(mRange->GetStartParent()),
- NS_ERROR_FAILURE);
- NS_ENSURE_TRUE(mEditorBase->IsModifiableNode(mRange->GetEndParent()),
- NS_ERROR_FAILURE);
- NS_ENSURE_TRUE(mEditorBase->IsModifiableNode(mRange->GetCommonAncestor()),
- NS_ERROR_FAILURE);
-
- return NS_OK;
-}
-
NS_IMETHODIMP
DeleteRangeTransaction::DoTransaction()
{
- MOZ_ASSERT(mRange && mEditorBase);
+ MOZ_ASSERT(mRangeToDelete);
// build the child transactions
- nsCOMPtr<nsINode> startParent = mRange->GetStartParent();
- int32_t startOffset = mRange->StartOffset();
- nsCOMPtr<nsINode> endParent = mRange->GetEndParent();
- int32_t endOffset = mRange->EndOffset();
+ nsCOMPtr<nsINode> startParent = mRangeToDelete->GetStartParent();
+ int32_t startOffset = mRangeToDelete->StartOffset();
+ nsCOMPtr<nsINode> endParent = mRangeToDelete->GetEndParent();
+ int32_t endOffset = mRangeToDelete->EndOffset();
MOZ_ASSERT(startParent && endParent);
if (startParent == endParent) {
// the selection begins and ends in the same node
nsresult rv =
CreateTxnsToDeleteBetween(startParent, startOffset, endOffset);
NS_ENSURE_SUCCESS(rv, rv);
} else {
@@ -90,40 +72,40 @@ DeleteRangeTransaction::DoTransaction()
}
// if we've successfully built this aggregate transaction, then do it.
nsresult rv = EditAggregateTransaction::DoTransaction();
NS_ENSURE_SUCCESS(rv, rv);
// only set selection to deletion point if editor gives permission
bool bAdjustSelection;
- mEditorBase->ShouldTxnSetSelection(&bAdjustSelection);
+ mEditorBase.ShouldTxnSetSelection(&bAdjustSelection);
if (bAdjustSelection) {
- RefPtr<Selection> selection = mEditorBase->GetSelection();
+ RefPtr<Selection> selection = mEditorBase.GetSelection();
NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
rv = selection->Collapse(startParent, startOffset);
NS_ENSURE_SUCCESS(rv, rv);
}
// else do nothing - dom range gravity will adjust selection
return NS_OK;
}
NS_IMETHODIMP
DeleteRangeTransaction::UndoTransaction()
{
- MOZ_ASSERT(mRange && mEditorBase);
+ MOZ_ASSERT(mRangeToDelete);
return EditAggregateTransaction::UndoTransaction();
}
NS_IMETHODIMP
DeleteRangeTransaction::RedoTransaction()
{
- MOZ_ASSERT(mRange && mEditorBase);
+ MOZ_ASSERT(mRangeToDelete);
return EditAggregateTransaction::RedoTransaction();
}
NS_IMETHODIMP
DeleteRangeTransaction::GetTxnDescription(nsAString& aString)
{
aString.AssignLiteral("DeleteRangeTransaction");
@@ -144,17 +126,17 @@ DeleteRangeTransaction::CreateTxnsToDele
} else {
numToDel = aEndOffset - aStartOffset;
}
RefPtr<nsGenericDOMDataNode> charDataNode =
static_cast<nsGenericDOMDataNode*>(aNode);
RefPtr<DeleteTextTransaction> transaction =
- new DeleteTextTransaction(*mEditorBase, *charDataNode, aStartOffset,
+ new DeleteTextTransaction(mEditorBase, *charDataNode, aStartOffset,
numToDel, mRangeUpdater);
nsresult rv = transaction->Init();
NS_ENSURE_SUCCESS(rv, rv);
AppendChild(transaction);
return NS_OK;
}
@@ -162,17 +144,17 @@ DeleteRangeTransaction::CreateTxnsToDele
nsCOMPtr<nsIContent> child = aNode->GetChildAt(aStartOffset);
for (int32_t i = aStartOffset; i < aEndOffset; ++i) {
// Even if we detect invalid range, we should ignore it for removing
// specified range's nodes as far as possible.
if (NS_WARN_IF(!child)) {
break;
}
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
- new DeleteNodeTransaction(*mEditorBase, *child, mRangeUpdater);
+ new DeleteNodeTransaction(mEditorBase, *child, mRangeUpdater);
// XXX This is odd handling. Even if some children are not editable,
// editor should append transactions because they could be editable
// at undoing/redoing. Additionally, if the transaction needs to
// delete/restore all nodes, it should at undoing/redoing.
if (deleteNodeTransaction->CanDoIt()) {
AppendChild(deleteNodeTransaction);
}
child = child->GetNextSibling();
@@ -197,17 +179,17 @@ DeleteRangeTransaction::CreateTxnsToDele
start = 0;
numToDelete = aOffset;
}
if (numToDelete) {
RefPtr<nsGenericDOMDataNode> dataNode =
static_cast<nsGenericDOMDataNode*>(aNode);
RefPtr<DeleteTextTransaction> transaction =
- new DeleteTextTransaction(*mEditorBase, *dataNode, start, numToDelete,
+ new DeleteTextTransaction(mEditorBase, *dataNode, start, numToDelete,
mRangeUpdater);
nsresult rv = transaction->Init();
NS_ENSURE_SUCCESS(rv, rv);
AppendChild(transaction);
}
}
@@ -215,27 +197,27 @@ DeleteRangeTransaction::CreateTxnsToDele
return NS_OK;
}
nsresult
DeleteRangeTransaction::CreateTxnsToDeleteNodesBetween()
{
nsCOMPtr<nsIContentIterator> iter = NS_NewContentSubtreeIterator();
- nsresult rv = iter->Init(mRange);
+ nsresult rv = iter->Init(mRangeToDelete);
NS_ENSURE_SUCCESS(rv, rv);
while (!iter->IsDone()) {
nsCOMPtr<nsINode> node = iter->GetCurrentNode();
if (NS_WARN_IF(!node)) {
return NS_ERROR_NULL_POINTER;
}
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
- new DeleteNodeTransaction(*mEditorBase, *node, mRangeUpdater);
+ new DeleteNodeTransaction(mEditorBase, *node, mRangeUpdater);
// XXX This is odd handling. Even if some nodes in the range are not
// editable, editor should append transactions because they could
// at undoing/redoing. Additionally, if the transaction needs to
// delete/restore all nodes, it should at undoing/redoing.
if (NS_WARN_IF(!deleteNodeTransaction->CanDoIt())) {
return NS_ERROR_FAILURE;
}
AppendChild(deleteNodeTransaction);
--- a/editor/libeditor/DeleteRangeTransaction.h
+++ b/editor/libeditor/DeleteRangeTransaction.h
@@ -23,56 +23,53 @@ class RangeUpdater;
/**
* A transaction that deletes an entire range in the content tree
*/
class DeleteRangeTransaction final : public EditAggregateTransaction
{
public:
/**
- * Initialize the transaction.
- * @param aEditorBase The object providing basic editing operations.
- * @param aRange The range to delete.
+ * @param aEditorBase The object providing basic editing operations.
+ * @param aRangeToDelete The range to delete.
*/
- nsresult Init(EditorBase* aEditorBase,
- nsRange* aRange,
- RangeUpdater* aRangeUpdater);
-
- DeleteRangeTransaction();
+ DeleteRangeTransaction(EditorBase& aEditorBase,
+ nsRange& aRangeToDelete,
+ RangeUpdater* aRangeUpdater);
NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(DeleteRangeTransaction,
EditAggregateTransaction)
NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
NS_DECL_EDITTRANSACTIONBASE
NS_IMETHOD RedoTransaction() override;
virtual void LastRelease() override
{
- mRange = nullptr;
+ mRangeToDelete = nullptr;
EditAggregateTransaction::LastRelease();
}
protected:
nsresult CreateTxnsToDeleteBetween(nsINode* aNode,
int32_t aStartOffset,
int32_t aEndOffset);
nsresult CreateTxnsToDeleteNodesBetween();
nsresult CreateTxnsToDeleteContent(nsINode* aParent,
int32_t aOffset,
nsIEditor::EDirection aAction);
- // P1 in the range.
- RefPtr<nsRange> mRange;
+ // The editor for this transaction.
+ EditorBase& mEditorBase;
- // The editor for this transaction.
- EditorBase* mEditorBase;
+ // P1 in the range.
+ RefPtr<nsRange> mRangeToDelete;
// Range updater object.
RangeUpdater* mRangeUpdater;
};
} // namespace mozilla
#endif // #ifndef DeleteRangeTransaction_h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -4363,19 +4363,20 @@ EditorBase::CreateTxnForDeleteSelection(
for (uint32_t rangeIdx = 0; rangeIdx < selection->RangeCount(); ++rangeIdx) {
RefPtr<nsRange> range = selection->GetRangeAt(rangeIdx);
NS_ENSURE_STATE(range);
// Same with range as with selection; if it is collapsed and action
// is eNone, do nothing.
if (!range->Collapsed()) {
- RefPtr<DeleteRangeTransaction> transaction = new DeleteRangeTransaction();
- transaction->Init(this, range, &mRangeUpdater);
- aggregateTransaction->AppendChild(transaction);
+ RefPtr<DeleteRangeTransaction> deleteRangeTransaction =
+ new DeleteRangeTransaction(*this, *range, &mRangeUpdater);
+ // XXX Oh, not checking if deleteRangeTransaction can modify the range...
+ aggregateTransaction->AppendChild(deleteRangeTransaction);
} else if (aAction != eNone) {
// we have an insertion point. delete the thing in front of it or
// behind it, depending on aAction
nsresult rv = CreateTxnForDeleteInsertionPoint(range, aAction,
aggregateTransaction,
aNode, aOffset, aLength);
NS_ENSURE_SUCCESS(rv, rv);
}