Bug 1451672 - part 14: Rename EditorBase::ReplaceContainer() to EditorBase::ReplaceContainerWithTransactionInternal() and create some wrappers of it r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 12 Apr 2018 21:45:55 +0900
changeset 468367 24e17127ec4875531ef46308dde8169bf35cea67
parent 468366 979bfcb751572486dce5e69b709f916c008425ee
child 468368 3bd18c2b985162bf4fa3a5d74214861839036325
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1451672
milestone61.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 1451672 - part 14: Rename EditorBase::ReplaceContainer() to EditorBase::ReplaceContainerWithTransactionInternal() and create some wrappers of it r=m_kato The parameters of EditorBase::ReplaceContainer() are complicated. For example, if it's specified as cloning all attributes, aAttribute and aValue are ignored because CloneAttributes() removes all existing attributes but ReplaceContainer() sets attributes before calling CloneAttributes(). This method has 3 modes: 1. Just replaces aOldContainer with new element. 2. #1 and clones all attributes from aOldContainer to the new element. 3. #1 and sets aAttribute of the new element to aValue. Therefore, this patch creates 3 inline wrappers of the renamed method. MozReview-Commit-ID: IsPu2uZuU8f
editor/libeditor/EditorBase.cpp
editor/libeditor/EditorBase.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLTableEditor.cpp
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -1648,62 +1648,57 @@ EditorBase::DeleteNodeWithTransaction(ns
       listener->DidDeleteNode(aNode.AsDOMNode(), rv);
     }
   }
 
   NS_ENSURE_SUCCESS(rv, rv);
   return NS_OK;
 }
 
-/**
- * ReplaceContainer() replaces inNode with a new node (outNode) which is
- * constructed to be of type aNodeType.  Put inNodes children into outNode.
- * Callers responsibility to make sure inNode's children can go in outNode.
- */
 already_AddRefed<Element>
-EditorBase::ReplaceContainer(Element* aOldContainer,
-                             nsAtom* aNodeType,
-                             nsAtom* aAttribute,
-                             const nsAString* aValue,
-                             ECloneAttributes aCloneAttributes)
-{
-  MOZ_ASSERT(aOldContainer && aNodeType);
-
-  EditorDOMPoint atOldContainer(aOldContainer);
+EditorBase::ReplaceContainerWithTransactionInternal(
+              Element& aOldContainer,
+              nsAtom& aTagName,
+              nsAtom& aAttribute,
+              const nsAString& aAttributeValue,
+              bool aCloneAllAttributes)
+{
+  EditorDOMPoint atOldContainer(&aOldContainer);
   if (NS_WARN_IF(!atOldContainer.IsSet())) {
     return nullptr;
   }
 
-  RefPtr<Element> newContainer = CreateHTMLContent(aNodeType);
+  RefPtr<Element> newContainer = CreateHTMLContent(&aTagName);
   if (NS_WARN_IF(!newContainer)) {
     return nullptr;
   }
 
-  // Set attribute if needed.
-  if (aAttribute && aValue && aAttribute != nsGkAtoms::_empty) {
+  // Set or clone attribute if needed.
+  if (aCloneAllAttributes) {
+    MOZ_ASSERT(&aAttribute == nsGkAtoms::_empty);
+    CloneAttributes(newContainer, &aOldContainer);
+  } else if (&aAttribute != nsGkAtoms::_empty) {
     nsresult rv =
-      newContainer->SetAttr(kNameSpaceID_None, aAttribute, *aValue, true);
+      newContainer->SetAttr(kNameSpaceID_None, &aAttribute, aAttributeValue,
+                            true);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return nullptr;
     }
   }
-  if (aCloneAttributes == eCloneAttributes) {
-    CloneAttributes(newContainer, aOldContainer);
-  }
 
   // Notify our internal selection state listener.
   // Note: An AutoSelectionRestorer object must be created before calling this
   // to initialize mRangeUpdater.
-  AutoReplaceContainerSelNotify selStateNotify(mRangeUpdater, aOldContainer,
+  AutoReplaceContainerSelNotify selStateNotify(mRangeUpdater, &aOldContainer,
                                                newContainer);
   {
     AutoTransactionsConserveSelection conserveSelection(this);
     // Move all children from the old container to the new container.
-    while (aOldContainer->HasChildren()) {
-      nsCOMPtr<nsIContent> child = aOldContainer->GetFirstChild();
+    while (aOldContainer.HasChildren()) {
+      nsCOMPtr<nsIContent> child = aOldContainer.GetFirstChild();
       if (NS_WARN_IF(!child)) {
         return nullptr;
       }
       nsresult rv = DeleteNodeWithTransaction(*child);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return nullptr;
       }
 
@@ -1720,17 +1715,17 @@ EditorBase::ReplaceContainer(Element* aO
   NS_WARNING_ASSERTION(atOldContainer.IsSetAndValid(),
     "The old container might be moved by mutation observer");
   nsresult rv = InsertNodeWithTransaction(*newContainer, atOldContainer);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
 
   // Delete old container.
-  rv = DeleteNodeWithTransaction(*aOldContainer);
+  rv = DeleteNodeWithTransaction(aOldContainer);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return nullptr;
   }
 
   return newContainer.forget();
 }
 
 /**
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -360,23 +360,76 @@ public:
    *                            container.  Otherwise, will insert the node
    *                            before child node referred by this.
    */
   template<typename PT, typename CT>
   nsresult
   InsertNodeWithTransaction(nsIContent& aContentToInsert,
                             const EditorDOMPointBase<PT, CT>& aPointToInsert);
 
-  enum ECloneAttributes { eDontCloneAttributes, eCloneAttributes };
-  already_AddRefed<Element> ReplaceContainer(Element* aOldContainer,
-                                             nsAtom* aNodeType,
-                                             nsAtom* aAttribute = nullptr,
-                                             const nsAString* aValue = nullptr,
-                                             ECloneAttributes aCloneAttributes
-                                             = eDontCloneAttributes);
+  /**
+   * ReplaceContainerWithTransaction() creates new element whose name is
+   * aTagName, moves all children in aOldContainer to the new element, then,
+   * removes aOldContainer from the DOM tree.
+   *
+   * @param aOldContainer       The element node which should be replaced
+   *                            with new element.
+   * @param aTagName            The name of new element node.
+   */
+  already_AddRefed<Element>
+  ReplaceContainerWithTransaction(Element& aOldContainer,
+                                  nsAtom& aTagName)
+  {
+    return ReplaceContainerWithTransactionInternal(aOldContainer, aTagName,
+                                                   *nsGkAtoms::_empty,
+                                                   EmptyString(), false);
+  }
+
+  /**
+   * ReplaceContainerAndCloneAttributesWithTransaction() creates new element
+   * whose name is aTagName, copies all attributes from aOldContainer to the
+   * new element, moves all children in aOldContainer to the new element, then,
+   * removes aOldContainer from the DOM tree.
+   *
+   * @param aOldContainer       The element node which should be replaced
+   *                            with new element.
+   * @param aTagName            The name of new element node.
+   */
+  already_AddRefed<Element>
+  ReplaceContainerAndCloneAttributesWithTransaction(Element& aOldContainer,
+                                                    nsAtom& aTagName)
+  {
+    return ReplaceContainerWithTransactionInternal(aOldContainer, aTagName,
+                                                   *nsGkAtoms::_empty,
+                                                   EmptyString(), true);
+  }
+
+  /**
+   * ReplaceContainerWithTransaction() creates new element whose name is
+   * aTagName, sets aAttributes of the new element to aAttributeValue, moves
+   * all children in aOldContainer to the new element, then, removes
+   * aOldContainer from the DOM tree.
+   *
+   * @param aOldContainer       The element node which should be replaced
+   *                            with new element.
+   * @param aTagName            The name of new element node.
+   * @param aAttribute          Attribute name to be set to the new element.
+   * @param aAttributeValue     Attribute value to be set to aAttribute.
+   */
+  already_AddRefed<Element>
+  ReplaceContainerWithTransaction(Element& aOldContainer,
+                                  nsAtom& aTagName,
+                                  nsAtom& aAttribute,
+                                  const nsAString& aAttributeValue)
+  {
+    return ReplaceContainerWithTransactionInternal(aOldContainer, aTagName,
+                                                   aAttribute,
+                                                   aAttributeValue, false);
+  }
+
   void CloneAttributes(Element* aDest, Element* aSource);
 
   nsresult RemoveContainer(nsIContent* aNode);
   already_AddRefed<Element> InsertContainerAbove(nsIContent* aNode,
                                                  nsAtom* aNodeType,
                                                  nsAtom* aAttribute = nullptr,
                                                  const nsAString* aValue =
                                                  nullptr);
@@ -625,16 +678,39 @@ protected:
    * @param aCharData           The data node which should be modified.
    * @param aOffset             Start offset of removing text in aCharData.
    * @param aLength             Length of removing text.
    */
   nsresult DeleteTextWithTransaction(dom::CharacterData& aCharacterData,
                                      uint32_t aOffset, uint32_t aLength);
 
   /**
+   * ReplaceContainerWithTransactionInternal() is implementation of
+   * ReplaceContainerWithTransaction() and
+   * ReplaceContainerAndCloneAttributesWithTransaction().
+   *
+   * @param aOldContainer       The element which will be replaced with new
+   *                            element.
+   * @param aTagName            The name of new element node.
+   * @param aAttribute          Attribute name which will be set to the new
+   *                            element.  This will be ignored if
+   *                            aCloneAllAttributes is set to true.
+   * @param aAttributeValue     Attribute value which will be set to
+   *                            aAttribute.
+   * @param aCloneAllAttributes If true, all attributes of aOldContainer will
+   *                            be copied to the new element.
+   */
+  already_AddRefed<Element>
+  ReplaceContainerWithTransactionInternal(Element& aElement,
+                                          nsAtom& aTagName,
+                                          nsAtom& aAttribute,
+                                          const nsAString& aAttributeValue,
+                                          bool aCloneAllAttributes);
+
+  /**
    * Called after a transaction is done successfully.
    */
   void DoAfterDoTransaction(nsITransaction *aTxn);
 
   /**
    * Called after a transaction is undone successfully.
    */
 
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3783,34 +3783,40 @@ HTMLEditRules::WillMakeList(Selection* a
             return NS_ERROR_FAILURE;
           }
         }
         // move list item to new list
         rv = htmlEditor->MoveNode(curNode, curList, -1);
         NS_ENSURE_SUCCESS(rv, rv);
         // convert list item type if needed
         if (!curNode->IsHTMLElement(itemType)) {
-          newBlock = htmlEditor->ReplaceContainer(curNode->AsElement(),
-                                                  itemType);
-          NS_ENSURE_STATE(newBlock);
+          newBlock =
+            htmlEditor->ReplaceContainerWithTransaction(*curNode->AsElement(),
+                                                        *itemType);
+          if (NS_WARN_IF(!newBlock)) {
+            return NS_ERROR_FAILURE;
+          }
         }
       } else {
         // item is in right type of list.  But we might still have to move it.
         // and we might need to convert list item types.
         if (!curList) {
           curList = atCurNode.GetContainerAsElement();
         } else if (atCurNode.GetContainer() != curList) {
           // move list item to new list
           rv = htmlEditor->MoveNode(curNode, curList, -1);
           NS_ENSURE_SUCCESS(rv, rv);
         }
         if (!curNode->IsHTMLElement(itemType)) {
-          newBlock = htmlEditor->ReplaceContainer(curNode->AsElement(),
-                                                  itemType);
-          NS_ENSURE_STATE(newBlock);
+          newBlock =
+            htmlEditor->ReplaceContainerWithTransaction(*curNode->AsElement(),
+                                                        *itemType);
+          if (NS_WARN_IF(!newBlock)) {
+            return NS_ERROR_FAILURE;
+          }
         }
       }
       nsCOMPtr<Element> curElement = do_QueryInterface(curNode);
       if (NS_WARN_IF(!curElement)) {
         return NS_ERROR_FAILURE;
       }
       if (aBulletType && !aBulletType->IsEmpty()) {
         rv = htmlEditor->SetAttributeWithTransaction(*curElement,
@@ -3870,19 +3876,22 @@ HTMLEditRules::WillMakeList(Selection* a
       if (IsInlineNode(curNode) && prevListItem) {
         // this is a continuation of some inline nodes that belong together in
         // the same list item.  use prevListItem
         rv = htmlEditor->MoveNode(curNode, prevListItem, -1);
         NS_ENSURE_SUCCESS(rv, rv);
       } else {
         // don't wrap li around a paragraph.  instead replace paragraph with li
         if (curNode->IsHTMLElement(nsGkAtoms::p)) {
-          listItem = htmlEditor->ReplaceContainer(curNode->AsElement(),
-                                                  itemType);
-          NS_ENSURE_STATE(listItem);
+          listItem =
+            htmlEditor->ReplaceContainerWithTransaction(*curNode->AsElement(),
+                                                        *itemType);
+          if (NS_WARN_IF(!listItem)) {
+            return NS_ERROR_FAILURE;
+          }
         } else {
           listItem = htmlEditor->InsertContainerAbove(curNode, itemType);
           NS_ENSURE_STATE(listItem);
         }
         if (IsInlineNode(curNode)) {
           prevListItem = listItem;
         } else {
           prevListItem = nullptr;
@@ -5016,17 +5025,18 @@ HTMLEditRules::ConvertListType(Element* 
   MOZ_ASSERT(aItemType);
 
   nsCOMPtr<nsINode> child = aList->GetFirstChild();
   while (child) {
     if (child->IsElement()) {
       dom::Element* element = child->AsElement();
       if (HTMLEditUtils::IsListItem(element) &&
           !element->IsHTMLElement(aItemType)) {
-        child = mHTMLEditor->ReplaceContainer(element, aItemType);
+        child =
+          mHTMLEditor->ReplaceContainerWithTransaction(*element, *aItemType);
         if (NS_WARN_IF(!child)) {
           return nullptr;
         }
       } else if (HTMLEditUtils::IsList(element) &&
                  !element->IsHTMLElement(aListType)) {
         child = ConvertListType(child->AsElement(), aListType, aItemType);
         if (NS_WARN_IF(!child)) {
           return nullptr;
@@ -5036,17 +5046,17 @@ HTMLEditRules::ConvertListType(Element* 
     child = child->GetNextSibling();
   }
 
   if (aList->IsHTMLElement(aListType)) {
     RefPtr<dom::Element> list = aList->AsElement();
     return list.forget();
   }
 
-  return mHTMLEditor->ReplaceContainer(aList, aListType);
+  return mHTMLEditor->ReplaceContainerWithTransaction(*aList, *aListType);
 }
 
 
 /**
  * CreateStyleForInsertText() takes care of clearing and setting appropriate
  * style nodes for text insertion.
  */
 nsresult
@@ -7742,19 +7752,18 @@ HTMLEditRules::ApplyBlockStyle(nsTArray<
     // If curNode is a address, p, header, address, or pre, replace it with a
     // new block of correct type.
     // XXX: pre can't hold everything the others can
     if (HTMLEditUtils::IsMozDiv(curNode) ||
         HTMLEditUtils::IsFormatNode(curNode)) {
       // Forget any previous block used for previous inline nodes
       curBlock = nullptr;
       newBlock =
-        htmlEditor->ReplaceContainer(curNode->AsElement(),
-                                     &aBlockTag, nullptr, nullptr,
-                                     EditorBase::eCloneAttributes);
+        htmlEditor->ReplaceContainerAndCloneAttributesWithTransaction(
+                      *curNode->AsElement(), aBlockTag);
       NS_ENSURE_STATE(newBlock);
       continue;
     }
 
     if (HTMLEditUtils::IsTable(curNode) ||
         HTMLEditUtils::IsList(curNode) ||
         curNode->IsAnyOfHTMLElements(nsGkAtoms::tbody,
                                      nsGkAtoms::tr,
--- a/editor/libeditor/HTMLTableEditor.cpp
+++ b/editor/libeditor/HTMLTableEditor.cpp
@@ -1965,42 +1965,51 @@ HTMLEditor::SplitCellIntoRows(nsIDOMElem
   return CopyCellBackgroundColor(newCell, cell2);
 }
 
 NS_IMETHODIMP
 HTMLEditor::SwitchTableCellHeaderType(nsIDOMElement* aSourceCell,
                                       nsIDOMElement** aNewCell)
 {
   nsCOMPtr<Element> sourceCell = do_QueryInterface(aSourceCell);
-  NS_ENSURE_TRUE(sourceCell, NS_ERROR_NULL_POINTER);
+  if (NS_WARN_IF(!sourceCell)) {
+    return NS_ERROR_INVALID_ARG;
+  }
 
   AutoPlaceholderBatch beginBatching(this);
-  // Prevent auto insertion of BR in new cell created by ReplaceContainer
+  // Prevent auto insertion of BR in new cell created by
+  // ReplaceContainerAndCloneAttributesWithTransaction().
   AutoRules beginRulesSniffing(this, EditAction::insertNode, nsIEditor::eNext);
 
-  // Save current selection to restore when done
-  // This is needed so ReplaceContainer can monitor selection
-  //  when replacing nodes
+  // Save current selection to restore when done.
+  // This is needed so ReplaceContainerAndCloneAttributesWithTransaction()
+  // can monitor selection when replacing nodes.
   RefPtr<Selection> selection = GetSelection();
-  NS_ENSURE_TRUE(selection, NS_ERROR_FAILURE);
+  if (NS_WARN_IF(!selection)) {
+    return NS_ERROR_FAILURE;
+  }
+
   AutoSelectionRestorer selectionRestorer(selection, this);
 
   // Set to the opposite of current type
-  nsAtom* newCellType =
+  nsAtom* newCellName =
     sourceCell->IsHTMLElement(nsGkAtoms::td) ? nsGkAtoms::th : nsGkAtoms::td;
 
   // This creates new node, moves children, copies attributes (true)
   //   and manages the selection!
-  nsCOMPtr<Element> newNode = ReplaceContainer(sourceCell, newCellType,
-      nullptr, nullptr, EditorBase::eCloneAttributes);
-  NS_ENSURE_TRUE(newNode, NS_ERROR_FAILURE);
+  RefPtr<Element> newCell =
+    ReplaceContainerAndCloneAttributesWithTransaction(*sourceCell,
+                                                      *newCellName);
+  if (NS_WARN_IF(!newCell)) {
+    return NS_ERROR_FAILURE;
+  }
 
   // Return the new cell
   if (aNewCell) {
-    nsCOMPtr<nsIDOMElement> newElement = do_QueryInterface(newNode);
+    nsCOMPtr<nsIDOMElement> newElement = do_QueryInterface(newCell);
     *aNewCell = newElement.get();
     NS_ADDREF(*aNewCell);
   }
 
   return NS_OK;
 }
 
 NS_IMETHODIMP