Bug 1425412 - part 8: Create JoinNodeTransaction::MaybeCreate() and remove EditorBase::CreateTxnForJoinNode() r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 15 Dec 2017 21:53:08 +0900
changeset 712701 b1bfa9bce8b280ec1d69681b4b98bb975c5663a1
parent 712700 ff07469b3761b152963cece334ab5cfcb7546523
child 712702 4d9042689661d7a26687f1a1227cdc49a4a877d6
push id93397
push usermasayuki@d-toybox.com
push dateMon, 18 Dec 2017 16:25:33 +0000
reviewersm_kato
bugs1425412
milestone59.0a1
Bug 1425412 - part 8: Create JoinNodeTransaction::MaybeCreate() and remove EditorBase::CreateTxnForJoinNode() r?m_kato EditorBase::CreateTxnForJoinNode() just hides what it does. For making the caller clearer, let's create a factory method, JoinNodeTransaction::MaybeCreate(). MozReview-Commit-ID: 8vADXdzMeuV
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/JoinNodeTransaction.cpp
editor/libeditor/JoinNodeTransaction.h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -1629,21 +1629,23 @@ EditorBase::JoinNodes(nsINode& aLeftNode
     for (auto& listener : listeners) {
       listener->WillJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(),
                               parent->AsDOMNode());
     }
   }
 
   nsresult rv = NS_OK;
   RefPtr<JoinNodeTransaction> transaction =
-    CreateTxnForJoinNode(aLeftNode, aRightNode);
+    JoinNodeTransaction::MaybeCreate(*this, aLeftNode, aRightNode);
   if (transaction)  {
     rv = DoTransaction(transaction);
   }
 
+  // XXX Some other transactions manage range updater by themselves.
+  //     Why doesn't JoinNodeTransaction do it?
   mRangeUpdater.SelAdjJoinNodes(aLeftNode, aRightNode, *parent, offset,
                                 (int32_t)oldLeftNodeLen);
 
   {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       listener->DidJoinNodes(aLeftNode.AsDOMNode(), aRightNode.AsDOMNode(),
                              parent->AsDOMNode(), rv);
@@ -3040,30 +3042,16 @@ EditorBase::DeleteText(nsGenericDOMDataN
           static_cast<nsIDOMCharacterData*>(GetAsDOMNode(&aCharData)), aOffset,
           aLength, rv);
     }
   }
 
   return rv;
 }
 
-already_AddRefed<JoinNodeTransaction>
-EditorBase::CreateTxnForJoinNode(nsINode& aLeftNode,
-                                 nsINode& aRightNode)
-{
-  RefPtr<JoinNodeTransaction> joinNodeTransaction =
-    new JoinNodeTransaction(*this, aLeftNode, aRightNode);
-  // If it's not editable, the transaction shouldn't be recorded since it
-  // should never be undone/redone.
-  if (NS_WARN_IF(!joinNodeTransaction->CanDoIt())) {
-    return nullptr;
-  }
-  return joinNodeTransaction.forget();
-}
-
 struct SavedRange final
 {
   RefPtr<Selection> mSelection;
   nsCOMPtr<nsINode> mStartContainer;
   nsCOMPtr<nsINode> mEndContainer;
   int32_t mStartOffset;
   int32_t mEndOffset;
 };
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -585,19 +585,16 @@ protected:
    * Create a transaction for removing a style sheet.
    */
   already_AddRefed<mozilla::RemoveStyleSheetTransaction>
     CreateTxnForRemoveStyleSheet(StyleSheet* aSheet);
 
   nsresult DeleteText(nsGenericDOMDataNode& aElement,
                       uint32_t aOffset, uint32_t aLength);
 
-  already_AddRefed<JoinNodeTransaction>
-    CreateTxnForJoinNode(nsINode& aLeftNode, nsINode& aRightNode);
-
   /**
    * This method first deletes the selection, if it's not collapsed.  Then if
    * the selection lies in a CharacterData node, it splits it.  If the
    * selection is at this point collapsed in a CharacterData node, it's
    * adjusted to be collapsed right before or after the node instead (which is
    * always possible, since the node was split).
    */
   nsresult DeleteSelectionAndPrepareToCreateNode();
--- a/editor/libeditor/JoinNodeTransaction.cpp
+++ b/editor/libeditor/JoinNodeTransaction.cpp
@@ -13,16 +13,30 @@
 #include "nsIDOMCharacterData.h"        // for nsIDOMCharacterData
 #include "nsIEditor.h"                  // for EditorBase::IsModifiableNode
 #include "nsISupportsImpl.h"            // for QueryInterface, etc.
 
 namespace mozilla {
 
 using namespace dom;
 
+// static
+already_AddRefed<JoinNodeTransaction>
+JoinNodeTransaction::MaybeCreate(EditorBase& aEditorBase,
+                                 nsINode& aLeftNode,
+                                 nsINode& aRightNode)
+{
+  RefPtr<JoinNodeTransaction> transaction =
+    new JoinNodeTransaction(aEditorBase, aLeftNode, aRightNode);
+  if (NS_WARN_IF(!transaction->CanDoIt())) {
+    return nullptr;
+  }
+  return transaction.forget();
+}
+
 JoinNodeTransaction::JoinNodeTransaction(EditorBase& aEditorBase,
                                          nsINode& aLeftNode,
                                          nsINode& aRightNode)
   : mEditorBase(&aEditorBase)
   , mLeftNode(&aLeftNode)
   , mRightNode(&aRightNode)
   , mOffset(0)
 {
--- a/editor/libeditor/JoinNodeTransaction.h
+++ b/editor/libeditor/JoinNodeTransaction.h
@@ -21,24 +21,31 @@ class EditorBase;
 /**
  * A transaction that joins two nodes E1 (left node) and E2 (right node) into a
  * single node E.  The children of E are the children of E1 followed by the
  * children of E2.  After DoTransaction() and RedoTransaction(), E1 is removed
  * from the content tree and E2 remains.
  */
 class JoinNodeTransaction final : public EditTransactionBase
 {
+protected:
+  JoinNodeTransaction(EditorBase& aEditorBase,
+                      nsINode& aLeftNode, nsINode& aRightNode);
+
 public:
   /**
+   * Creates a join node transaction.  This returns nullptr if cannot join the
+   * nodes.
+   *
    * @param aEditorBase     The provider of core editing operations.
    * @param aLeftNode       The first of two nodes to join.
    * @param aRightNode      The second of two nodes to join.
    */
-  JoinNodeTransaction(EditorBase& aEditorBase,
-                      nsINode& aLeftNode, nsINode& aRightNode);
+  static already_AddRefed<JoinNodeTransaction>
+  MaybeCreate(EditorBase& aEditorBase, nsINode& aLeftNode, nsINode& aRightNode);
 
   /**
    * CanDoIt() returns true if there are enough members and can join or
    * restore the nodes.  Otherwise, false.
    */
   bool CanDoIt() const;
 
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(JoinNodeTransaction,