Bug 1425412 - part 1: Create InsertTextTransaction::Create() and remove EditorBase::CreateTxnForInsertText() r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 15 Dec 2017 17:26:52 +0900
changeset 712694 67a745f7ad618507c7aee8b26567aa16cd04598a
parent 712693 eefc72a8349f7d323d09f1d572ed777865b88f35
child 712695 9e937b7e951c4aea13db46d7995070b8937db97c
push id93397
push usermasayuki@d-toybox.com
push dateMon, 18 Dec 2017 16:25:33 +0000
reviewersm_kato
bugs1425412
milestone59.0a1
Bug 1425412 - part 1: Create InsertTextTransaction::Create() and remove EditorBase::CreateTxnForInsertText() r?m_kato EditorBase::CreateTxnForInsertText() just hides what it exactly does. Rewriting it with a static factory method, InsertTextTransaction::Create() should be clearer for its caller. MozReview-Commit-ID: Er7Zlhtbnb0
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/InsertTextTransaction.cpp
editor/libeditor/InsertTextTransaction.h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2790,17 +2790,18 @@ EditorBase::InsertTextIntoTextNodeImpl(c
     mComposition->DidCreateCompositionTransaction(aStringToInsert);
     isIMETransaction = true;
     // All characters of the composition string will be replaced with
     // aStringToInsert.  So, we need to emulate to remove the composition
     // string.
     insertedTextNode = mComposition->GetContainerTextNode();
     insertedOffset = mComposition->XPOffsetInTextNode();
   } else {
-    transaction = CreateTxnForInsertText(aStringToInsert, aTextNode, aOffset);
+    transaction =
+      InsertTextTransaction::Create(*this, aStringToInsert, aTextNode, aOffset);
   }
 
   // Let listeners know what's up
   {
     AutoActionListenerArray listeners(mActionListeners);
     for (auto& listener : listeners) {
       listener->WillInsertText(
         static_cast<nsIDOMCharacterData*>(insertedTextNode->AsDOMNode()),
@@ -2993,27 +2994,16 @@ EditorBase::SetTextImpl(Selection& aSele
           aString, rv);
       }
     }
   }
 
   return rv;
 }
 
-already_AddRefed<InsertTextTransaction>
-EditorBase::CreateTxnForInsertText(const nsAString& aStringToInsert,
-                                   Text& aTextNode,
-                                   int32_t aOffset)
-{
-  RefPtr<InsertTextTransaction> transaction =
-    new InsertTextTransaction(aTextNode, aOffset, aStringToInsert, *this,
-                              &mRangeUpdater);
-  return transaction.forget();
-}
-
 nsresult
 EditorBase::DeleteText(nsGenericDOMDataNode& aCharData,
                        uint32_t aOffset,
                        uint32_t aLength)
 {
   RefPtr<DeleteTextTransaction> transaction =
     CreateTxnForDeleteText(aCharData, aOffset, aLength);
   NS_ENSURE_STATE(transaction);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -492,16 +492,18 @@ public:
    * to native IME.  Therefore, when there is composition, this can do anything.
    * For example, the editor instance, the widget or the process itself may
    * be destroyed.
    */
   nsresult CommitComposition();
 
   void SwitchTextDirectionTo(uint32_t aDirection);
 
+  RangeUpdater& RangeUpdaterRef() { return mRangeUpdater; }
+
 protected:
   nsresult DetermineCurrentDirection();
   void FireInputEvent();
 
   /**
    * Create a transaction for setting aAttribute to aValue on aElement.  Never
    * returns null.
    */
@@ -609,24 +611,16 @@ protected:
   already_AddRefed<EditTransactionBase>
     CreateTxnForDeleteRange(nsRange* aRangeToDelete,
                             EDirection aAction,
                             nsINode** aRemovingNode,
                             int32_t* aOffset,
                             int32_t* aLength);
 
   /**
-   * Create a transaction for inserting aStringToInsert into aTextNode.  Never
-   * returns null.
-   */
-  already_AddRefed<mozilla::InsertTextTransaction>
-    CreateTxnForInsertText(const nsAString& aStringToInsert, Text& aTextNode,
-                           int32_t aOffset);
-
-  /**
    * Never returns null.
    */
   already_AddRefed<mozilla::CompositionTransaction>
     CreateTxnForComposition(const nsAString& aStringToInsert);
 
   /**
    * Create a transaction for adding a style sheet.
    */
--- a/editor/libeditor/InsertTextTransaction.cpp
+++ b/editor/libeditor/InsertTextTransaction.cpp
@@ -13,26 +13,36 @@
 #include "nsDebug.h"                    // for NS_ASSERTION, etc.
 #include "nsError.h"                    // for NS_OK, etc.
 #include "nsQueryObject.h"              // for do_QueryObject
 
 namespace mozilla {
 
 using namespace dom;
 
-InsertTextTransaction::InsertTextTransaction(Text& aTextNode,
-                                             uint32_t aOffset,
+// static
+already_AddRefed<InsertTextTransaction>
+InsertTextTransaction::Create(EditorBase& aEditorBase,
+                              const nsAString& aStringToInsert,
+                              Text& aTextNode,
+                              uint32_t aOffset)
+{
+  RefPtr<InsertTextTransaction> transaction =
+    new InsertTextTransaction(aEditorBase, aStringToInsert, aTextNode, aOffset);
+  return transaction.forget();
+}
+
+InsertTextTransaction::InsertTextTransaction(EditorBase& aEditorBase,
                                              const nsAString& aStringToInsert,
-                                             EditorBase& aEditorBase,
-                                             RangeUpdater* aRangeUpdater)
+                                             Text& aTextNode,
+                                             uint32_t aOffset)
   : mTextNode(&aTextNode)
   , mOffset(aOffset)
   , mStringToInsert(aStringToInsert)
   , mEditorBase(&aEditorBase)
-  , mRangeUpdater(aRangeUpdater)
 {
 }
 
 InsertTextTransaction::~InsertTextTransaction()
 {
 }
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(InsertTextTransaction, EditTransactionBase,
@@ -51,30 +61,35 @@ NS_INTERFACE_MAP_END_INHERITING(EditTran
 NS_IMETHODIMP
 InsertTextTransaction::DoTransaction()
 {
   if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mTextNode)) {
     return NS_ERROR_NOT_AVAILABLE;
   }
 
   nsresult rv = mTextNode->InsertData(mOffset, mStringToInsert);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   // Only set selection to insertion point if editor gives permission
   if (mEditorBase->GetShouldTxnSetSelection()) {
     RefPtr<Selection> selection = mEditorBase->GetSelection();
-    NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
+    if (NS_WARN_IF(!selection)) {
+      return NS_ERROR_FAILURE;
+    }
     DebugOnly<nsresult> rv =
       selection->Collapse(mTextNode, mOffset + mStringToInsert.Length());
     NS_ASSERTION(NS_SUCCEEDED(rv),
                  "Selection could not be collapsed after insert");
   } else {
     // Do nothing - DOM Range gravity will adjust selection
   }
-  mRangeUpdater->SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
+  mEditorBase->RangeUpdaterRef().
+                 SelAdjInsertText(*mTextNode, mOffset, mStringToInsert);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 InsertTextTransaction::UndoTransaction()
 {
   return mTextNode->DeleteData(mOffset, mStringToInsert.Length());
--- a/editor/libeditor/InsertTextTransaction.h
+++ b/editor/libeditor/InsertTextTransaction.h
@@ -17,40 +17,49 @@ class nsITransaction;
 
 #define NS_INSERTTEXTTXN_IID \
 { 0x8c9ad77f, 0x22a7, 0x4d01, \
   { 0xb1, 0x59, 0x8a, 0x0f, 0xdb, 0x1d, 0x08, 0xe9 } }
 
 namespace mozilla {
 
 class EditorBase;
-class RangeUpdater;
 
 namespace dom {
 class Text;
 } // namespace dom
 
 /**
  * A transaction that inserts text into a content node.
  */
 class InsertTextTransaction final : public EditTransactionBase
 {
+protected:
+  InsertTextTransaction(EditorBase& aEditorBase,
+                        const nsAString& aStringToInsert,
+                        dom::Text& aTextNode,
+                        uint32_t aOffset);
+
 public:
-  NS_DECLARE_STATIC_IID_ACCESSOR(NS_INSERTTEXTTXN_IID)
-
   /**
-   * @param aElement        The text content node.
-   * @param aOffset         The location in aElement to do the insertion.
-   * @param aString         The new text to insert.
-   * @param aPresShell      Used to get and set the selection.
-   * @param aRangeUpdater   The range updater
+   * Creates new InsertTextTransaction instance.  This never returns nullptr.
+   *
+   * @param aEditorBase     The editor which manages the transaction.
+   * @param aTextNode       The text content node to be inserted
+   *                        aStringToInsert.
+   * @param aOffset         The offset in aTextNode to do the insertion.
+   * @param aStringToInsert The new string to insert.
    */
-  InsertTextTransaction(dom::Text& aTextNode, uint32_t aOffset,
-                        const nsAString& aString, EditorBase& aEditorBase,
-                        RangeUpdater* aRangeUpdater);
+  static already_AddRefed<InsertTextTransaction>
+  Create(EditorBase& aEditorBase,
+         const nsAString& aStringToInsert,
+         dom::Text& aTextNode,
+         uint32_t aOffset);
+
+  NS_DECLARE_STATIC_IID_ACCESSOR(NS_INSERTTEXTTXN_IID)
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(InsertTextTransaction,
                                            EditTransactionBase)
 
   NS_DECL_EDITTRANSACTIONBASE
 
   NS_IMETHOD Merge(nsITransaction* aTransaction, bool* aDidMerge) override;
@@ -72,17 +81,15 @@ private:
   // The offset into mTextNode where the insertion is to take place.
   uint32_t mOffset;
 
   // The text to insert into mTextNode at mOffset.
   nsString mStringToInsert;
 
   // The editor, which we'll need to get the selection.
   RefPtr<EditorBase> mEditorBase;
-
-  RangeUpdater* mRangeUpdater;
 };
 
 NS_DEFINE_STATIC_IID_ACCESSOR(InsertTextTransaction, NS_INSERTTEXTTXN_IID)
 
 } // namespace mozilla
 
 #endif // #ifndef InsertTextTransaction_h