Bug 1345690 part.3 Rename DeleteTextTransaction::Init() to DeleteTextTransaction::CanDoIt() since it does not initialize anything and just checking if the text node is editable r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 10 Mar 2017 13:38:28 +0900
changeset 496445 ecd332bf08c824f5098dbcf590a98b9c6e668722
parent 496444 1be2f10d340c7c48dcd677740c448fdb51d2ae86
child 496446 7496be500e366adc20e17bf215d205677cf1c25d
push id48594
push usermasayuki@d-toybox.com
push dateFri, 10 Mar 2017 04:55:38 +0000
reviewersm_kato
bugs1345690
milestone55.0a1
Bug 1345690 part.3 Rename DeleteTextTransaction::Init() to DeleteTextTransaction::CanDoIt() since it does not initialize anything and just checking if the text node is editable r?m_kato MozReview-Commit-ID: EyqHjHF3Q8G
editor/libeditor/DeleteRangeTransaction.cpp
editor/libeditor/DeleteTextTransaction.cpp
editor/libeditor/DeleteTextTransaction.h
editor/libeditor/EditorBase.cpp
--- a/editor/libeditor/DeleteRangeTransaction.cpp
+++ b/editor/libeditor/DeleteRangeTransaction.cpp
@@ -125,24 +125,25 @@ DeleteRangeTransaction::CreateTxnsToDele
       numToDel = 1;
     } else {
       numToDel = aEndOffset - aStartOffset;
     }
 
     RefPtr<nsGenericDOMDataNode> charDataNode =
       static_cast<nsGenericDOMDataNode*>(aNode);
 
-    RefPtr<DeleteTextTransaction> transaction =
+    RefPtr<DeleteTextTransaction> deleteTextTransaction =
       new DeleteTextTransaction(mEditorBase, *charDataNode, aStartOffset,
                                 numToDel, mRangeUpdater);
-
-    nsresult rv = transaction->Init();
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    AppendChild(transaction);
+    // If the text node isn't editable, it should be never undone/redone.
+    // So, the transaction shouldn't be recorded.
+    if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
+      return NS_ERROR_FAILURE;
+    }
+    AppendChild(deleteTextTransaction);
     return NS_OK;
   }
 
   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)) {
@@ -178,24 +179,25 @@ DeleteRangeTransaction::CreateTxnsToDele
     } else {
       start = 0;
       numToDelete = aOffset;
     }
 
     if (numToDelete) {
       RefPtr<nsGenericDOMDataNode> dataNode =
         static_cast<nsGenericDOMDataNode*>(aNode);
-      RefPtr<DeleteTextTransaction> transaction =
+      RefPtr<DeleteTextTransaction> deleteTextTransaction =
         new DeleteTextTransaction(mEditorBase, *dataNode, start, numToDelete,
                                   mRangeUpdater);
-
-      nsresult rv = transaction->Init();
-      NS_ENSURE_SUCCESS(rv, rv);
-
-      AppendChild(transaction);
+      // If the text node isn't editable, it should be never undone/redone.
+      // So, the transaction shouldn't be recorded.
+      if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
+        return NS_ERROR_FAILURE;
+      }
+      AppendChild(deleteTextTransaction);
     }
   }
 
   return NS_OK;
 }
 
 nsresult
 DeleteRangeTransaction::CreateTxnsToDeleteNodesBetween()
--- a/editor/libeditor/DeleteTextTransaction.cpp
+++ b/editor/libeditor/DeleteTextTransaction.cpp
@@ -36,25 +36,23 @@ DeleteTextTransaction::DeleteTextTransac
 }
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(DeleteTextTransaction, EditTransactionBase,
                                    mCharData)
 
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(DeleteTextTransaction)
 NS_INTERFACE_MAP_END_INHERITING(EditTransactionBase)
 
-nsresult
-DeleteTextTransaction::Init()
+bool
+DeleteTextTransaction::CanDoIt() const
 {
-  // Do nothing if the node is read-only
-  if (!mEditorBase.IsModifiableNode(mCharData)) {
-    return NS_ERROR_FAILURE;
+  if (NS_WARN_IF(!mCharData)) {
+    return false;
   }
-
-  return NS_OK;
+  return mEditorBase.IsModifiableNode(mCharData);
 }
 
 NS_IMETHODIMP
 DeleteTextTransaction::DoTransaction()
 {
   MOZ_ASSERT(mCharData);
 
   // Get the text that we're about to delete
--- a/editor/libeditor/DeleteTextTransaction.h
+++ b/editor/libeditor/DeleteTextTransaction.h
@@ -34,17 +34,21 @@ public:
    *                            number of bytes!
    */
   DeleteTextTransaction(EditorBase& aEditorBase,
                         nsGenericDOMDataNode& aCharData,
                         uint32_t aOffset,
                         uint32_t aNumCharsToDelete,
                         RangeUpdater* aRangeUpdater);
 
-  nsresult Init();
+  /**
+   * CanDoIt() returns true if there are enough members and can modify the
+   * text.  Otherwise, false.
+   */
+  bool CanDoIt() const;
 
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(DeleteTextTransaction,
                                            EditTransactionBase)
   NS_IMETHOD QueryInterface(REFNSIID aIID, void** aInstancePtr) override;
 
   NS_DECL_EDITTRANSACTIONBASE
 
   uint32_t GetOffset() { return mOffset; }
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2713,23 +2713,25 @@ EditorBase::DeleteText(nsGenericDOMDataN
   return rv;
 }
 
 already_AddRefed<DeleteTextTransaction>
 EditorBase::CreateTxnForDeleteText(nsGenericDOMDataNode& aCharData,
                                    uint32_t aOffset,
                                    uint32_t aLength)
 {
-  RefPtr<DeleteTextTransaction> transaction =
+  RefPtr<DeleteTextTransaction> deleteTextTransaction =
     new DeleteTextTransaction(*this, aCharData, aOffset, aLength,
                               &mRangeUpdater);
-  nsresult rv = transaction->Init();
-  NS_ENSURE_SUCCESS(rv, nullptr);
-
-  return transaction.forget();
+  // If it's not editable, the transaction shouldn't be recorded since it
+  // should never be undone/redone.
+  if (NS_WARN_IF(!deleteTextTransaction->CanDoIt())) {
+    return nullptr;
+  }
+  return deleteTextTransaction.forget();
 }
 
 already_AddRefed<SplitNodeTransaction>
 EditorBase::CreateTxnForSplitNode(nsIContent& aNode,
                                   uint32_t aOffset)
 {
   RefPtr<SplitNodeTransaction> transaction =
     new SplitNodeTransaction(*this, aNode, aOffset);