Bug 1425412 - part 6: Create DeleteNodeTransaction::MaybeCreate() and remove EditorBaseTransaction::CreateTxnForDeleteNode() r?m_kato
EditorBaseTransaction::CreateTxnForDeleteNode() just hides what it does.
Instead, let's create a factory method, DeleteNodeTransaction::MaybeCreate()
for making callers clearer.
MozReview-Commit-ID: 8WUYN0BjKSU
--- a/editor/libeditor/DeleteNodeTransaction.cpp
+++ b/editor/libeditor/DeleteNodeTransaction.cpp
@@ -7,28 +7,35 @@
#include "mozilla/EditorBase.h"
#include "mozilla/SelectionState.h" // RangeUpdater
#include "nsDebug.h"
#include "nsError.h"
#include "nsAString.h"
namespace mozilla {
+// static
+already_AddRefed<DeleteNodeTransaction>
+DeleteNodeTransaction::MaybeCreate(EditorBase& aEditorBase,
+ nsINode& aNodeToDelete)
+{
+ RefPtr<DeleteNodeTransaction> transaction =
+ new DeleteNodeTransaction(aEditorBase, aNodeToDelete);
+ if (NS_WARN_IF(!transaction->CanDoIt())) {
+ return nullptr;
+ }
+ return transaction.forget();
+}
+
DeleteNodeTransaction::DeleteNodeTransaction(EditorBase& aEditorBase,
- nsINode& aNodeToDelete,
- RangeUpdater* aRangeUpdater)
+ nsINode& aNodeToDelete)
: mEditorBase(&aEditorBase)
, mNodeToDelete(&aNodeToDelete)
, mParentNode(aNodeToDelete.GetParentNode())
- , mRangeUpdater(aRangeUpdater)
{
- // XXX We're not sure if this is really necessary.
- if (!CanDoIt()) {
- mRangeUpdater = nullptr;
- }
}
DeleteNodeTransaction::~DeleteNodeTransaction()
{
}
NS_IMPL_CYCLE_COLLECTION_INHERITED(DeleteNodeTransaction, EditTransactionBase,
mEditorBase,
@@ -60,19 +67,17 @@ DeleteNodeTransaction::DoTransaction()
// Remember which child mNodeToDelete was (by remembering which child was
// next). Note that mRefNode can be nullptr.
mRefNode = mNodeToDelete->GetNextSibling();
// give range updater a chance. SelAdjDeleteNode() needs to be called
// *before* we do the action, unlike some of the other RangeItem update
// methods.
- if (mRangeUpdater) {
- mRangeUpdater->SelAdjDeleteNode(mNodeToDelete);
- }
+ mEditorBase->RangeUpdaterRef().SelAdjDeleteNode(mNodeToDelete);
ErrorResult error;
mParentNode->RemoveChild(*mNodeToDelete, error);
return error.StealNSResult();
}
NS_IMETHODIMP
DeleteNodeTransaction::UndoTransaction()
@@ -90,19 +95,17 @@ DeleteNodeTransaction::UndoTransaction()
NS_IMETHODIMP
DeleteNodeTransaction::RedoTransaction()
{
if (NS_WARN_IF(!CanDoIt())) {
// This is a legal state, the transaction is a no-op.
return NS_OK;
}
- if (mRangeUpdater) {
- mRangeUpdater->SelAdjDeleteNode(mNodeToDelete);
- }
+ mEditorBase->RangeUpdaterRef().SelAdjDeleteNode(mNodeToDelete);
ErrorResult error;
mParentNode->RemoveChild(*mNodeToDelete, error);
return error.StealNSResult();
}
NS_IMETHODIMP
DeleteNodeTransaction::GetTxnDescription(nsAString& aString)
--- a/editor/libeditor/DeleteNodeTransaction.h
+++ b/editor/libeditor/DeleteNodeTransaction.h
@@ -12,26 +12,35 @@
#include "nsIContent.h"
#include "nsINode.h"
#include "nsISupportsImpl.h"
#include "nscore.h"
namespace mozilla {
class EditorBase;
-class RangeUpdater;
/**
* A transaction that deletes a single element
*/
class DeleteNodeTransaction final : public EditTransactionBase
{
+protected:
+ DeleteNodeTransaction(EditorBase& aEditorBase, nsINode& aNodeToDelete);
+
public:
- DeleteNodeTransaction(EditorBase& aEditorBase, nsINode& aNodeToDelete,
- RangeUpdater* aRangeUpdater);
+ /**
+ * Creates a delete node transaction instance. This returns nullptr if
+ * it cannot remove the node from its parent.
+ *
+ * @param aEditorBase The editor.
+ * @param aNodeToDelete The node to be removed from the DOM tree.
+ */
+ static already_AddRefed<DeleteNodeTransaction>
+ MaybeCreate(EditorBase& aEditorBase, nsINode& aNodeToDelete);
/**
* CanDoIt() returns true if there are enough members and can modify the
* parent. Otherwise, false.
*/
bool CanDoIt() const;
NS_DECL_ISUPPORTS_INHERITED
@@ -51,16 +60,13 @@ protected:
// The element to delete.
nsCOMPtr<nsINode> mNodeToDelete;
// Parent of node to delete.
nsCOMPtr<nsINode> mParentNode;
// Next sibling to remember for undo/redo purposes.
nsCOMPtr<nsIContent> mRefNode;
-
- // Range updater object.
- RangeUpdater* mRangeUpdater;
};
} // namespace mozilla
#endif // #ifndef DeleteNodeTransaction_h
--- a/editor/libeditor/DeleteRangeTransaction.cpp
+++ b/editor/libeditor/DeleteRangeTransaction.cpp
@@ -172,22 +172,22 @@ DeleteRangeTransaction::CreateTxnsToDele
}
// Even if we detect invalid range, we should ignore it for removing
// specified range's nodes as far as possible.
for (nsIContent* child = aStart.GetChildAtOffset();
child && child != aEnd.GetChildAtOffset();
child = child->GetNextSibling()) {
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
- new DeleteNodeTransaction(*mEditorBase, *child, mRangeUpdater);
+ DeleteNodeTransaction::MaybeCreate(*mEditorBase, *child);
// 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()) {
+ if (deleteNodeTransaction) {
AppendChild(deleteNodeTransaction);
}
}
return NS_OK;
}
nsresult
@@ -250,22 +250,22 @@ DeleteRangeTransaction::CreateTxnsToDele
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);
+ DeleteNodeTransaction::MaybeCreate(*mEditorBase, *node);
// 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())) {
+ if (NS_WARN_IF(!deleteNodeTransaction)) {
return NS_ERROR_FAILURE;
}
AppendChild(deleteNodeTransaction);
iter->Next();
}
return NS_OK;
}
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -1657,29 +1657,33 @@ EditorBase::DeleteNode(nsIDOMNode* aNode
nsCOMPtr<nsINode> node = do_QueryInterface(aNode);
NS_ENSURE_STATE(node);
return DeleteNode(node);
}
nsresult
EditorBase::DeleteNode(nsINode* aNode)
{
+ if (NS_WARN_IF(!aNode)) {
+ return NS_ERROR_INVALID_ARG;
+ }
+
AutoRules beginRulesSniffing(this, EditAction::createNode,
nsIEditor::ePrevious);
// save node location for selection updating code.
{
AutoActionListenerArray listeners(mActionListeners);
for (auto& listener : listeners) {
listener->WillDeleteNode(aNode->AsDOMNode());
}
}
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
- CreateTxnForDeleteNode(aNode);
+ DeleteNodeTransaction::MaybeCreate(*this, *aNode);
nsresult rv = deleteNodeTransaction ? DoTransaction(deleteNodeTransaction) :
NS_ERROR_FAILURE;
{
AutoActionListenerArray listeners(mActionListeners);
for (auto& listener : listeners) {
listener->DidDeleteNode(aNode->AsDOMNode(), rv);
}
@@ -4620,33 +4624,16 @@ EditorBase::CreateTxnForRemoveAttribute(
nsAtom& aAttribute)
{
RefPtr<ChangeAttributeTransaction> transaction =
new ChangeAttributeTransaction(aElement, aAttribute, nullptr);
return transaction.forget();
}
-already_AddRefed<DeleteNodeTransaction>
-EditorBase::CreateTxnForDeleteNode(nsINode* aNode)
-{
- if (NS_WARN_IF(!aNode)) {
- return nullptr;
- }
-
- RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
- new DeleteNodeTransaction(*this, *aNode, &mRangeUpdater);
- // This should be OK because if currently it cannot delete the node,
- // it should never be able to undo/redo.
- if (!deleteNodeTransaction->CanDoIt()) {
- return nullptr;
- }
- return deleteNodeTransaction.forget();
-}
-
already_AddRefed<AddStyleSheetTransaction>
EditorBase::CreateTxnForAddStyleSheet(StyleSheet* aSheet)
{
RefPtr<AddStyleSheetTransaction> transaction =
new AddStyleSheetTransaction(*this, aSheet);
return transaction.forget();
}
@@ -4774,17 +4761,17 @@ EditorBase::CreateTxnForDeleteRange(nsRa
*aOffset = deleteTextTransaction->Offset();
*aLength = deleteTextTransaction->LengthToDelete();
priorNode.forget(aRemovingNode);
return deleteTextTransaction.forget();
}
// priorNode is not chardata, so tell its parent to delete it
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
- CreateTxnForDeleteNode(priorNode);
+ DeleteNodeTransaction::MaybeCreate(*this, *priorNode);
if (NS_WARN_IF(!deleteNodeTransaction)) {
return nullptr;
}
priorNode.forget(aRemovingNode);
return deleteNodeTransaction.forget();
}
if (aAction == eNext && isLast) {
@@ -4814,17 +4801,17 @@ EditorBase::CreateTxnForDeleteRange(nsRa
*aOffset = deleteTextTransaction->Offset();
*aLength = deleteTextTransaction->LengthToDelete();
nextNode.forget(aRemovingNode);
return deleteTextTransaction.forget();
}
// nextNode is not chardata, so tell its parent to delete it
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
- CreateTxnForDeleteNode(nextNode);
+ DeleteNodeTransaction::MaybeCreate(*this, *nextNode);
if (NS_WARN_IF(!deleteNodeTransaction)) {
return nullptr;
}
nextNode.forget(aRemovingNode);
return deleteNodeTransaction.forget();
}
if (node->IsNodeOfType(nsINode::eDATA_NODE)) {
@@ -4896,17 +4883,17 @@ EditorBase::CreateTxnForDeleteRange(nsRa
}
*aOffset = deleteTextTransaction->Offset();
*aLength = deleteTextTransaction->LengthToDelete();
selectedNode.forget(aRemovingNode);
return deleteTextTransaction.forget();
}
RefPtr<DeleteNodeTransaction> deleteNodeTransaction =
- CreateTxnForDeleteNode(selectedNode);
+ DeleteNodeTransaction::MaybeCreate(*this, *selectedNode);
if (NS_WARN_IF(!deleteNodeTransaction)) {
return nullptr;
}
selectedNode.forget(aRemovingNode);
return deleteNodeTransaction.forget();
}
nsresult
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -532,22 +532,16 @@ protected:
* Otherwise, will insert the element before the
* child node referred by this.
* @return The created new element node.
*/
already_AddRefed<Element> CreateNode(nsAtom* aTag,
const EditorRawDOMPoint& aPointToInsert);
/**
- * Create a transaction for removing aNode from its parent.
- */
- already_AddRefed<DeleteNodeTransaction>
- CreateTxnForDeleteNode(nsINode* aNode);
-
- /**
* Create an aggregate transaction for delete selection. The result may
* include DeleteNodeTransactions and/or DeleteTextTransactions as its
* children.
*
* @param aAction The action caused removing the selection.
* @param aRemovingNode The node to be removed.
* @param aOffset The start offset of the range in aRemovingNode.
* @param aLength The length of the range in aRemovingNode.