Bug 1415445 - part 1: CreateElementTransaction should use EditorRawDOMPoint and RangeBoundary r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 08 Nov 2017 16:49:06 +0900
changeset 444322 ff1aff2fa5b6beb672dd91ac475cba5e627add12
parent 444321 7e505b1a27ad28dcba4b83f7db51b35406be17fc
child 444323 80b30e8921626614049932ad55b456829eb38828
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1415445
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1415445 - part 1: CreateElementTransaction should use EditorRawDOMPoint and RangeBoundary r=m_kato The constructor of CreateElementTransaction should use EditorRawDOMPoint to receive insertion point of the new element. Then, it should be stored with RangeBoundary. With this change, CreateElementTransaction doesn't need to compute child node at insertion point. Additionally, this creates InsertNode() method. Current code works differently when DoTransaction() is called and RedoTransaction() is called. MozReview-Commit-ID: 8ujhmzn65Wg
editor/libeditor/CreateElementTransaction.cpp
editor/libeditor/CreateElementTransaction.h
editor/libeditor/EditorBase.cpp
--- a/editor/libeditor/CreateElementTransaction.cpp
+++ b/editor/libeditor/CreateElementTransaction.cpp
@@ -27,130 +27,151 @@
 #include "nsReadableUtils.h"
 #include "nsStringFwd.h"
 #include "nsString.h"
 
 namespace mozilla {
 
 using namespace dom;
 
-CreateElementTransaction::CreateElementTransaction(EditorBase& aEditorBase,
-                                                   nsAtom& aTag,
-                                                   nsINode& aParent,
-                                                   int32_t aOffsetInParent,
-                                                   nsIContent* aChildAtOffset)
+CreateElementTransaction::CreateElementTransaction(
+                            EditorBase& aEditorBase,
+                            nsAtom& aTag,
+                            const EditorRawDOMPoint& aPointToInsert)
   : EditTransactionBase()
   , mEditorBase(&aEditorBase)
   , mTag(&aTag)
-  , mParent(&aParent)
-  , mOffsetInParent(aOffsetInParent)
-  , mRefNode(aChildAtOffset)
+  , mPointToInsert(aPointToInsert)
 {
 }
 
 CreateElementTransaction::~CreateElementTransaction()
 {
 }
 
 NS_IMPL_CYCLE_COLLECTION_INHERITED(CreateElementTransaction,
                                    EditTransactionBase,
                                    mEditorBase,
-                                   mParent,
-                                   mNewNode,
-                                   mRefNode)
+                                   mPointToInsert,
+                                   mNewNode)
 
 NS_IMPL_ADDREF_INHERITED(CreateElementTransaction, EditTransactionBase)
 NS_IMPL_RELEASE_INHERITED(CreateElementTransaction, EditTransactionBase)
 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(CreateElementTransaction)
 NS_INTERFACE_MAP_END_INHERITING(EditTransactionBase)
 
 
 NS_IMETHODIMP
 CreateElementTransaction::DoTransaction()
 {
-  if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mTag) || NS_WARN_IF(!mParent)) {
+  if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mTag) ||
+      NS_WARN_IF(!mPointToInsert.IsSet())) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   mNewNode = mEditorBase->CreateHTMLContent(mTag);
   NS_ENSURE_STATE(mNewNode);
 
   // Try to insert formatting whitespace for the new node:
   mEditorBase->MarkNodeDirty(GetAsDOMNode(mNewNode));
 
   // Insert the new node
-  ErrorResult rv;
-  if (mOffsetInParent == -1) {
-    mParent->AppendChild(*mNewNode, rv);
-    return rv.StealNSResult();
+  ErrorResult error;
+  InsertNewNode(error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
   }
 
-  mOffsetInParent = std::min(mOffsetInParent,
-                             static_cast<int32_t>(mParent->GetChildCount()));
-
-  if (!mRefNode) {
-    // Note, it's ok for mRefNode to be null. That means append
-    mRefNode = mParent->GetChildAt(mOffsetInParent);
-  }
-
-  nsCOMPtr<nsIContent> refNode = mRefNode;
-  mParent->InsertBefore(*mNewNode, refNode, rv);
-  NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
-
   // Only set selection to insertion point if editor gives permission
   if (!mEditorBase->GetShouldTxnSetSelection()) {
     // Do nothing - DOM range gravity will adjust selection
     return NS_OK;
   }
 
   RefPtr<Selection> selection = mEditorBase->GetSelection();
   NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
 
   EditorRawDOMPoint afterNewNode(mNewNode);
   if (NS_WARN_IF(!afterNewNode.AdvanceOffset())) {
     // If mutation observer or mutation event listener moved or removed the
     // new node, we hit this case.  Should we use script blocker while we're
     // in this method?
     return NS_ERROR_FAILURE;
   }
-  rv = selection->Collapse(afterNewNode);
-  NS_ASSERTION(!rv.Failed(),
-               "selection could not be collapsed after insert");
+  selection->Collapse(afterNewNode, error);
+  if (error.Failed()) {
+    NS_WARNING("selection could not be collapsed after insert");
+    error.SuppressException();
+  }
   return NS_OK;
 }
 
+void
+CreateElementTransaction::InsertNewNode(ErrorResult& aError)
+{
+  if (mPointToInsert.IsSetAndValid()) {
+    if (mPointToInsert.IsEndOfContainer()) {
+      mPointToInsert.Container()->AppendChild(*mNewNode, aError);
+      NS_WARNING_ASSERTION(!aError.Failed(), "Failed to append the new node");
+      return;
+    }
+    mPointToInsert.Container()->InsertBefore(*mNewNode,
+                                             mPointToInsert.GetChildAtOffset(),
+                                             aError);
+    NS_WARNING_ASSERTION(!aError.Failed(), "Failed to insert the new node");
+    return;
+  }
+
+  if (NS_WARN_IF(mPointToInsert.GetChildAtOffset() &&
+                 mPointToInsert.Container() !=
+                   mPointToInsert.GetChildAtOffset()->GetParentNode())) {
+    aError.Throw(NS_ERROR_FAILURE);
+    return;
+  }
+
+  // If mPointToInsert has only offset and it's not valid, we need to treat
+  // it as pointing end of the container.
+  mPointToInsert.Container()->AppendChild(*mNewNode, aError);
+  NS_WARNING_ASSERTION(!aError.Failed(), "Failed to append the new node");
+}
+
 NS_IMETHODIMP
 CreateElementTransaction::UndoTransaction()
 {
-  if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mParent)) {
+  if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mPointToInsert.IsSet())) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
-  ErrorResult rv;
-  mParent->RemoveChild(*mNewNode, rv);
-
-  return rv.StealNSResult();
+  ErrorResult error;
+  mPointToInsert.Container()->RemoveChild(*mNewNode, error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
+  }
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 CreateElementTransaction::RedoTransaction()
 {
-  if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mParent)) {
+  if (NS_WARN_IF(!mEditorBase) || NS_WARN_IF(!mPointToInsert.IsSet())) {
     return NS_ERROR_NOT_INITIALIZED;
   }
 
   // First, reset mNewNode so it has no attributes or content
   // XXX We never actually did this, we only cleared mNewNode's contents if it
   // was a CharacterData node (which it's not, it's an Element)
+  // XXX Don't we need to set selection like DoTransaction()?
 
   // Now, reinsert mNewNode
-  ErrorResult rv;
-  nsCOMPtr<nsIContent> refNode = mRefNode;
-  mParent->InsertBefore(*mNewNode, refNode, rv);
-  return rv.StealNSResult();
+  ErrorResult error;
+  InsertNewNode(error);
+  if (NS_WARN_IF(error.Failed())) {
+    return error.StealNSResult();
+  }
+  return NS_OK;
 }
 
 NS_IMETHODIMP
 CreateElementTransaction::GetTxnDescription(nsAString& aString)
 {
   aString.AssignLiteral("CreateElementTransaction: ");
   aString += nsDependentAtomString(mTag);
   return NS_OK;
--- a/editor/libeditor/CreateElementTransaction.h
+++ b/editor/libeditor/CreateElementTransaction.h
@@ -1,16 +1,17 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #ifndef CreateElementTransaction_h
 #define CreateElementTransaction_h
 
+#include "mozilla/EditorDOMPoint.h"
 #include "mozilla/EditTransactionBase.h"
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsISupportsImpl.h"
 
 class nsAtom;
 class nsIContent;
 class nsINode;
@@ -27,56 +28,51 @@ class Element;
 
 class CreateElementTransaction final : public EditTransactionBase
 {
 public:
   /**
    * Initialize the transaction.
    * @param aEditorBase     The provider of basic editing functionality.
    * @param aTag            The tag (P, HR, TABLE, etc.) for the new element.
-   * @param aParent         The node into which the new element will be
-   *                        inserted.
-   * @param aOffsetInParent The location in aParent to insert the new element.
-   *                        If eAppend, the new element is appended as the last
-   *                        child.
+   * @param aPointToInsert  The new node will be inserted before the child at
+   *                        aPointToInsert.  If this refers end of the container
+   *                        or after, the new node will be appended to the
+   *                        container.
    */
   CreateElementTransaction(EditorBase& aEditorBase,
                            nsAtom& aTag,
-                           nsINode& aParent,
-                           int32_t aOffsetInParent,
-                           nsIContent* aChildAtOffset);
+                           const EditorRawDOMPoint& aPointToInsert);
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(CreateElementTransaction,
                                            EditTransactionBase)
 
   NS_DECL_EDITTRANSACTIONBASE
 
   NS_IMETHOD RedoTransaction() override;
 
   already_AddRefed<dom::Element> GetNewNode();
 
 protected:
   virtual ~CreateElementTransaction();
 
+  /**
+   * InsertNewNode() inserts mNewNode before the child node at mPointToInsert.
+   */
+  void InsertNewNode(ErrorResult& aError);
+
   // The document into which the new node will be inserted.
   RefPtr<EditorBase> mEditorBase;
 
   // The tag (mapping to object type) for the new element.
   RefPtr<nsAtom> mTag;
 
-  // The node into which the new node will be inserted.
-  nsCOMPtr<nsINode> mParent;
-
-  // The index in mParent for the new node.
-  int32_t mOffsetInParent;
+  // The DOM point we will insert mNewNode.
+  RangeBoundary mPointToInsert;
 
   // The new node to insert.
   nsCOMPtr<dom::Element> mNewNode;
-
-  // The node we will insert mNewNode before.  We compute this ourselves if it
-  // is not set by the constructor.
-  nsCOMPtr<nsIContent> mRefNode;
 };
 
 } // namespace mozilla
 
 #endif // #ifndef CreateElementTransaction_h
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -4367,19 +4367,26 @@ EditorBase::CreateTxnForRemoveAttribute(
 }
 
 already_AddRefed<CreateElementTransaction>
 EditorBase::CreateTxnForCreateElement(nsAtom& aTag,
                                       nsINode& aParent,
                                       int32_t aPosition,
                                       nsIContent* aChildAtPosition)
 {
+  EditorRawDOMPoint pointToInsert;
+  if (aPosition == -1) {
+    pointToInsert.Set(&aParent, aParent.Length());
+  } else if (aChildAtPosition) {
+    pointToInsert.Set(aChildAtPosition);
+  } else {
+    pointToInsert.Set(&aParent, aPosition);
+  }
   RefPtr<CreateElementTransaction> transaction =
-    new CreateElementTransaction(*this, aTag, aParent, aPosition,
-                                 aChildAtPosition);
+    new CreateElementTransaction(*this, aTag, pointToInsert);
 
   return transaction.forget();
 }
 
 
 already_AddRefed<InsertNodeTransaction>
 EditorBase::CreateTxnForInsertNode(nsIContent& aNode,
                                    nsINode& aParent,