Bug 1054735 part 3 - Clean up nsEditor::InsertContainerAbove; r=ehsan
authorAryeh Gregor <ayg@aryeh.name>
Wed, 20 Aug 2014 15:25:16 +0300
changeset 200614 dbe8c5709d09c3545bdd447fac7d671b0770d4a9
parent 200613 8d121ead4ff775c871adbb1e0ec368066748c867
child 200615 4439b62ad7702017dcc5ef1c1c54df48bb702ae6
push id27350
push userryanvm@gmail.com
push dateWed, 20 Aug 2014 20:19:55 +0000
treeherdermozilla-central@6ab867edb95a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs1054735
milestone34.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 1054735 part 3 - Clean up nsEditor::InsertContainerAbove; r=ehsan
editor/libeditor/nsEditor.cpp
editor/libeditor/nsEditor.h
editor/libeditor/nsHTMLEditRules.cpp
editor/libeditor/nsHTMLEditor.cpp
editor/libeditor/nsHTMLEditorStyle.cpp
--- a/editor/libeditor/nsEditor.cpp
+++ b/editor/libeditor/nsEditor.cpp
@@ -1610,84 +1610,63 @@ nsEditor::RemoveContainer(nsIContent* aN
     rv = InsertNode(child, parent, offset);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
   return DeleteNode(aNode);
 }
 
 
-///////////////////////////////////////////////////////////////////////////
-// InsertContainerAbove:  insert a new parent for inNode, returned in outNode,
-//                   which is contructed to be of type aNodeType.  outNode becomes
-//                   a child of inNode's earlier parent.
-//                   Callers responsibility to make sure inNode's can be child
-//                   of outNode, and outNode can be child of old parent.
-nsresult
-nsEditor::InsertContainerAbove( nsIDOMNode *inNode, 
-                                nsCOMPtr<nsIDOMNode> *outNode, 
-                                const nsAString &aNodeType,
-                                const nsAString *aAttribute,
-                                const nsAString *aValue)
-{
-  NS_ENSURE_TRUE(inNode && outNode, NS_ERROR_NULL_POINTER);
-
-  nsCOMPtr<nsIContent> node = do_QueryInterface(inNode);
-  NS_ENSURE_STATE(node);
-
-  nsCOMPtr<dom::Element> element;
-  nsresult rv = InsertContainerAbove(node, getter_AddRefs(element), aNodeType,
-                                     aAttribute, aValue);
-  *outNode = element ? element->AsDOMNode() : nullptr;
-  return rv;
-}
-
-nsresult
+///////////////////////////////////////////////////////////////////////////////
+// InsertContainerAbove: Insert a new parent for inNode, which is contructed to
+//                       be of type aNodeType.  outNode becomes a child of
+//                       inNode's earlier parent.  Caller's responsibility to
+//                       make sure inNode's can be child of outNode, and
+//                       outNode can be child of old parent.
+already_AddRefed<Element>
 nsEditor::InsertContainerAbove(nsIContent* aNode,
-                               dom::Element** aOutNode,
-                               const nsAString& aNodeType,
-                               const nsAString* aAttribute,
+                               nsIAtom* aNodeType,
+                               nsIAtom* aAttribute,
                                const nsAString* aValue)
 {
-  MOZ_ASSERT(aNode);
+  MOZ_ASSERT(aNode && aNodeType);
 
   nsCOMPtr<nsIContent> parent = aNode->GetParent();
-  NS_ENSURE_STATE(parent);
+  NS_ENSURE_TRUE(parent, nullptr);
   int32_t offset = parent->IndexOf(aNode);
 
-  // create new container
-  nsCOMPtr<Element> newContent =
-    CreateHTMLContent(nsCOMPtr<nsIAtom>(do_GetAtom(aNodeType)));
-  NS_ENSURE_STATE(newContent);
-
-  // set attribute if needed
+  // Create new container
+  nsCOMPtr<Element> newContent = CreateHTMLContent(aNodeType);
+  NS_ENSURE_TRUE(newContent, nullptr);
+
+  // Set attribute if needed
   nsresult res;
-  if (aAttribute && aValue && !aAttribute->IsEmpty()) {
-    nsIDOMNode* elem = newContent->AsDOMNode();
-    res = static_cast<nsIDOMElement*>(elem)->SetAttribute(*aAttribute, *aValue);
-    NS_ENSURE_SUCCESS(res, res);
-  }
-  
-  // notify our internal selection state listener
+  if (aAttribute && aValue && aAttribute != nsGkAtoms::_empty) {
+    res = newContent->SetAttr(kNameSpaceID_None, aAttribute, *aValue, true);
+    NS_ENSURE_SUCCESS(res, nullptr);
+  }
+
+  // Notify our internal selection state listener
   nsAutoInsertContainerSelNotify selNotify(mRangeUpdater);
-  
-  // put inNode in new parent, outNode
-  res = DeleteNode(aNode->AsDOMNode());
-  NS_ENSURE_SUCCESS(res, res);
+
+  // Put inNode in new parent, outNode
+  res = DeleteNode(aNode);
+  NS_ENSURE_SUCCESS(res, nullptr);
 
   {
     nsAutoTxnsConserveSelection conserveSelection(this);
-    res = InsertNode(aNode->AsDOMNode(), newContent->AsDOMNode(), 0);
-    NS_ENSURE_SUCCESS(res, res);
-  }
-
-  // put new parent in doc
-  res = InsertNode(newContent->AsDOMNode(), parent->AsDOMNode(), offset);
-  newContent.forget(aOutNode);
-  return res;  
+    res = InsertNode(aNode, newContent, 0);
+    NS_ENSURE_SUCCESS(res, nullptr);
+  }
+
+  // Put new parent in doc
+  res = InsertNode(newContent, parent, offset);
+  NS_ENSURE_SUCCESS(res, nullptr);
+
+  return newContent.forget();
 }
 
 ///////////////////////////////////////////////////////////////////////////
 // MoveNode:  move aNode to {aParent,aOffset}
 nsresult
 nsEditor::MoveNode(nsIDOMNode* aNode, nsIDOMNode* aParent, int32_t aOffset)
 {
   nsCOMPtr<nsINode> node = do_QueryInterface(aNode);
--- a/editor/libeditor/nsEditor.h
+++ b/editor/libeditor/nsEditor.h
@@ -230,26 +230,21 @@ public:
                             nsIAtom* aNodeType,
                             nsIAtom* aAttribute = nullptr,
                             const nsAString* aValue = nullptr,
                             ECloneAttributes aCloneAttributes = eDontCloneAttributes);
   void CloneAttributes(mozilla::dom::Element* aDest,
                        mozilla::dom::Element* aSource);
 
   nsresult RemoveContainer(nsIContent* aNode);
-  nsresult InsertContainerAbove(nsIContent* aNode,
-                                mozilla::dom::Element** aOutNode,
-                                const nsAString& aNodeType,
-                                const nsAString* aAttribute = nullptr,
+  already_AddRefed<mozilla::dom::Element> InsertContainerAbove(
+                                nsIContent* aNode,
+                                nsIAtom* aNodeType,
+                                nsIAtom* aAttribute = nullptr,
                                 const nsAString* aValue = nullptr);
-  nsresult InsertContainerAbove(nsIDOMNode *inNode, 
-                                nsCOMPtr<nsIDOMNode> *outNode, 
-                                const nsAString &aNodeType,
-                                const nsAString *aAttribute = nullptr,
-                                const nsAString *aValue = nullptr);
   nsresult JoinNodes(nsINode* aNodeToKeep, nsIContent* aNodeToMove);
   nsresult MoveNode(nsINode* aNode, nsINode* aParent, int32_t aOffset);
   nsresult MoveNode(nsIDOMNode *aNode, nsIDOMNode *aParent, int32_t aOffset);
 
   /* Method to replace certain CreateElementNS() calls. 
      Arguments:
       nsIAtom* aTag          - tag you want
   */
--- a/editor/libeditor/nsHTMLEditRules.cpp
+++ b/editor/libeditor/nsHTMLEditRules.cpp
@@ -3251,17 +3251,19 @@ nsHTMLEditRules::WillMakeList(Selection*
   nsCOMPtr<nsIDOMNode> curParent;
   nsCOMPtr<nsIDOMNode> curList;
   nsCOMPtr<nsIDOMNode> prevListItem;
 
   for (int32_t i = 0; i < listCount; i++) {
     // here's where we actually figure out what to do
     nsCOMPtr<nsIDOMNode> newBlock;
     nsCOMPtr<nsIDOMNode> curNode = arrayOfNodes[i];
-    nsCOMPtr<Element> curNodeAsElement = do_QueryInterface(curNode);
+    nsCOMPtr<nsIContent> curNodeAsContent = do_QueryInterface(curNode);
+    nsCOMPtr<Element> curNodeAsElement = curNodeAsContent->IsElement()
+      ? curNodeAsContent->AsElement() : nullptr;
     int32_t offset;
     curParent = nsEditor::GetNodeLocation(curNode, &offset);
 
     // make sure we don't assemble content that is in different table cells
     // into the same list.  respect table cell boundaries when listifying.
     if (curList && InDifferentTableElements(curList, curNode)) {
       curList = nullptr;
     }
@@ -3414,19 +3416,21 @@ nsHTMLEditRules::WillMakeList(Selection*
         if (nsHTMLEditUtils::IsParagraph(curNode)) {
           NS_ENSURE_STATE(mHTMLEditor);
           NS_ENSURE_STATE(curNodeAsElement);
           listItem = dont_AddRef(GetAsDOMNode(
             mHTMLEditor->ReplaceContainer(curNodeAsElement, itemType).take()));
           NS_ENSURE_STATE(listItem);
         } else {
           NS_ENSURE_STATE(mHTMLEditor);
-          res = mHTMLEditor->InsertContainerAbove(curNode, address_of(listItem),
-                                                  nsDependentAtomString(itemType));
-          NS_ENSURE_SUCCESS(res, res);
+          NS_ENSURE_STATE(curNodeAsContent);
+          listItem = dont_AddRef(GetAsDOMNode(
+            mHTMLEditor->InsertContainerAbove(curNodeAsContent,
+                                              itemType).take()));
+          NS_ENSURE_STATE(listItem);
         }
         if (IsInlineNode(curNode)) {
           prevListItem = listItem;
         } else {
           prevListItem = nullptr;
         }
       }
     } else {
--- a/editor/libeditor/nsHTMLEditor.cpp
+++ b/editor/libeditor/nsHTMLEditor.cpp
@@ -4904,48 +4904,43 @@ nsHTMLEditor::CopyLastEditableChildStyle
     NS_ENSURE_SUCCESS(res, res);
   }
   while (child && nsTextEditUtils::IsBreak(child)) {
     nsCOMPtr<nsIDOMNode> priorNode;
     res = GetPriorHTMLNode(child, address_of(priorNode));
     NS_ENSURE_SUCCESS(res, res);
     child = priorNode;
   }
-  nsCOMPtr<nsIDOMNode> newStyles = nullptr, deepestStyle = nullptr;
-  while (child && (child != aPreviousBlock)) {
-    if (nsHTMLEditUtils::IsInlineStyle(child) ||
-        nsEditor::NodeIsType(child, nsEditProperty::span)) {
-      nsAutoString domTagName;
-      child->GetNodeName(domTagName);
-      ToLowerCase(domTagName);
+  nsCOMPtr<Element> newStyles, deepestStyle;
+  nsCOMPtr<nsINode> childNode = do_QueryInterface(child);
+  nsCOMPtr<Element> childElement;
+  if (childNode) {
+    childElement = childNode->IsElement() ? childNode->AsElement()
+                                          : childNode->GetParentElement();
+  }
+  while (childElement && (childElement->AsDOMNode() != aPreviousBlock)) {
+    if (nsHTMLEditUtils::IsInlineStyle(childElement) ||
+        childElement->Tag() == nsGkAtoms::span) {
       if (newStyles) {
-        nsCOMPtr<nsIDOMNode> newContainer;
-        res = InsertContainerAbove(newStyles, address_of(newContainer), domTagName);
+        newStyles = InsertContainerAbove(newStyles, childElement->Tag());
+        NS_ENSURE_STATE(newStyles);
+      } else {
+        nsCOMPtr<nsIDOMNode> newStylesDOM;
+        res = CreateNode(nsDependentAtomString(childElement->Tag()), aNewBlock,
+                         0, getter_AddRefs(newStylesDOM));
         NS_ENSURE_SUCCESS(res, res);
-        newStyles = newContainer;
+        deepestStyle = newStyles = do_QueryInterface(newStylesDOM);
       }
-      else {
-        res = CreateNode(domTagName, aNewBlock, 0, getter_AddRefs(newStyles));
-        NS_ENSURE_SUCCESS(res, res);
-        deepestStyle = newStyles;
-      }
-      res = CloneAttributes(newStyles, child);
-      NS_ENSURE_SUCCESS(res, res);
+      CloneAttributes(newStyles, childElement);
     }
-    nsCOMPtr<nsIDOMNode> tmp;
-    res = child->GetParentNode(getter_AddRefs(tmp));
-    NS_ENSURE_SUCCESS(res, res);
-    child = tmp;
+    childElement = childElement->GetParentElement();
   }
   if (deepestStyle) {
-    nsCOMPtr<nsIDOMNode> outBRNode;
-    res = CreateBR(deepestStyle, 0, address_of(outBRNode));
-    NS_ENSURE_SUCCESS(res, res);
-    // Getters must addref
-    outBRNode.forget(aOutBrNode);
+    *aOutBrNode = GetAsDOMNode(CreateBR(deepestStyle, 0).take());
+    NS_ENSURE_STATE(*aOutBrNode);
   }
   return NS_OK;
 }
 
 nsresult
 nsHTMLEditor::GetElementOrigin(nsIDOMElement * aElement, int32_t & aX, int32_t & aY)
 {
   aX = 0;
--- a/editor/libeditor/nsHTMLEditorStyle.cpp
+++ b/editor/libeditor/nsHTMLEditorStyle.cpp
@@ -406,16 +406,18 @@ nsresult
 nsHTMLEditor::SetInlinePropertyOnNodeImpl(nsIContent* aNode,
                                           nsIAtom* aProperty,
                                           const nsAString* aAttribute,
                                           const nsAString* aValue)
 {
   MOZ_ASSERT(aNode && aProperty);
   MOZ_ASSERT(aValue);
 
+  nsCOMPtr<nsIAtom> attrAtom = aAttribute ? do_GetAtom(*aAttribute) : nullptr;
+
   // If this is an element that can't be contained in a span, we have to
   // recurse to its children.
   if (!TagCanContain(nsGkAtoms::span, aNode->AsDOMNode())) {
     if (aNode->HasChildren()) {
       nsCOMArray<nsIContent> arrayOfNodes;
 
       // Populate the list.
       for (nsIContent* child = aNode->GetFirstChild();
@@ -475,20 +477,18 @@ nsHTMLEditor::SetInlinePropertyOnNodeImp
   if (useCSS) {
     nsCOMPtr<dom::Element> tmp;
     // We only add style="" to <span>s with no attributes (bug 746515).  If we
     // don't have one, we need to make one.
     if (aNode->IsElement() && aNode->AsElement()->IsHTML(nsGkAtoms::span) &&
         !aNode->AsElement()->GetAttrCount()) {
       tmp = aNode->AsElement();
     } else {
-      res = InsertContainerAbove(aNode, getter_AddRefs(tmp),
-                                 NS_LITERAL_STRING("span"),
-                                 nullptr, nullptr);
-      NS_ENSURE_SUCCESS(res, res);
+      tmp = InsertContainerAbove(aNode, nsGkAtoms::span);
+      NS_ENSURE_STATE(tmp);
     }
 
     // Add the CSS styles corresponding to the HTML style request
     int32_t count;
     res = mHTMLCSSUtils->SetCSSEquivalentToHTMLStyle(tmp->AsDOMNode(),
                                                      aProperty, aAttribute,
                                                      aValue, &count, false);
     NS_ENSURE_SUCCESS(res, res);
@@ -498,22 +498,21 @@ nsHTMLEditor::SetInlinePropertyOnNodeImp
   // is it already the right kind of node, but with wrong attribute?
   if (aNode->Tag() == aProperty) {
     // Just set the attribute on it.
     nsCOMPtr<nsIDOMElement> elem = do_QueryInterface(aNode);
     return SetAttribute(elem, *aAttribute, *aValue);
   }
 
   // ok, chuck it in its very own container
-  nsAutoString tag;
-  aProperty->ToString(tag);
-  ToLowerCase(tag);
-  nsCOMPtr<nsIDOMNode> tmp;
-  return InsertContainerAbove(aNode->AsDOMNode(), address_of(tmp), tag,
-                              aAttribute, aValue);
+  nsCOMPtr<Element> tmp = InsertContainerAbove(aNode, aProperty, attrAtom,
+                                               aValue);
+  NS_ENSURE_STATE(tmp);
+
+  return NS_OK;
 }
 
 
 nsresult
 nsHTMLEditor::SetInlinePropertyOnNode(nsIDOMNode *aNode,
                                       nsIAtom *aProperty,
                                       const nsAString *aAttribute,
                                       const nsAString *aValue)
@@ -815,23 +814,22 @@ nsresult nsHTMLEditor::RemoveStyleInside
       NS_NAMED_LITERAL_STRING(classAttr, "class");
       bool hasStyleAttr = HasAttr(aNode, &styleAttr);
       bool hasClassAttr = HasAttr(aNode, &classAttr);
       if (aProperty && (hasStyleAttr || hasClassAttr)) {
         // aNode carries inline styles or a class attribute so we can't
         // just remove the element... We need to create above the element
         // a span that will carry those styles or class, then we can delete
         // the node.
-        nsCOMPtr<nsIDOMNode> spanNode;
-        res = InsertContainerAbove(aNode, address_of(spanNode),
-                                   NS_LITERAL_STRING("span"));
+        nsCOMPtr<Element> spanNode =
+          InsertContainerAbove(content, nsGkAtoms::span);
+        NS_ENSURE_STATE(spanNode);
+        res = CloneAttribute(styleAttr, spanNode->AsDOMNode(), aNode);
         NS_ENSURE_SUCCESS(res, res);
-        res = CloneAttribute(styleAttr, spanNode, aNode);
-        NS_ENSURE_SUCCESS(res, res);
-        res = CloneAttribute(classAttr, spanNode, aNode);
+        res = CloneAttribute(classAttr, spanNode->AsDOMNode(), aNode);
         NS_ENSURE_SUCCESS(res, res);
       }
       res = RemoveContainer(content);
       NS_ENSURE_SUCCESS(res, res);
     } else {
       // otherwise we just want to eliminate the attribute
       if (HasAttr(aNode, aAttribute)) {
         // if this matching attribute is the ONLY one on the node,
@@ -1671,19 +1669,16 @@ nsHTMLEditor::RelativeFontChangeOnTextNo
   }
   if ( aStartOffset )
   {
     // we need to split off front of text node
     res = SplitNode(node, aStartOffset, getter_AddRefs(tmp));
     NS_ENSURE_SUCCESS(res, res);
   }
 
-  NS_NAMED_LITERAL_STRING(bigSize, "big");
-  NS_NAMED_LITERAL_STRING(smallSize, "small");
-  const nsAString& nodeType = (aSizeChange==1) ? static_cast<const nsAString&>(bigSize) : static_cast<const nsAString&>(smallSize);
   // look for siblings that are correct type of node
   nsCOMPtr<nsIDOMNode> sibling;
   GetPriorHTMLSibling(node, address_of(sibling));
   if (sibling && NodeIsType(sibling, (aSizeChange==1) ? nsEditProperty::big : nsEditProperty::small))
   {
     // previous sib is already right kind of inline node; slide this over into it
     res = MoveNode(node, sibling, -1);
     return res;
@@ -1693,18 +1688,23 @@ nsHTMLEditor::RelativeFontChangeOnTextNo
   if (sibling && NodeIsType(sibling, (aSizeChange==1) ? nsEditProperty::big : nsEditProperty::small))
   {
     // following sib is already right kind of inline node; slide this over into it
     res = MoveNode(node, sibling, 0);
     return res;
   }
   
   // else reparent the node inside font node with appropriate relative size
-  res = InsertContainerAbove(node, address_of(tmp), nodeType);
-  return res;
+  nsIAtom* nodeType = aSizeChange == 1 ? nsGkAtoms::big : nsGkAtoms::small;
+  nsCOMPtr<nsIContent> content = do_QueryInterface(node);
+  NS_ENSURE_STATE(content);
+  nsCOMPtr<Element> newElement = InsertContainerAbove(content, nodeType);
+  NS_ENSURE_STATE(newElement);
+
+  return NS_OK;
 }
 
 
 nsresult
 nsHTMLEditor::RelativeFontChangeHelper(int32_t aSizeChange, nsINode* aNode)
 {
   MOZ_ASSERT(aNode);
 
@@ -1787,19 +1787,20 @@ nsHTMLEditor::RelativeFontChangeOnNode(i
 
     sibling = GetNextHTMLSibling(aNode);
     if (sibling && sibling->IsHTML(atom)) {
       // following sib is already right kind of inline node; slide this over into it
       return MoveNode(aNode->AsDOMNode(), sibling->AsDOMNode(), 0);
     }
 
     // else insert it above aNode
-    nsCOMPtr<nsIDOMNode> tmp;
-    return InsertContainerAbove(aNode->AsDOMNode(), address_of(tmp),
-                                nsAtomString(atom));
+    nsCOMPtr<Element> newElement = InsertContainerAbove(aNode, atom);
+    NS_ENSURE_STATE(newElement);
+
+    return NS_OK;
   }
 
   // none of the above?  then cycle through the children.
   // MOOSE: we should group the children together if possible
   // into a single "big" or "small".  For the moment they are
   // each getting their own.  
   for (uint32_t i = aNode->GetChildCount(); i--; ) {
     nsresult rv = RelativeFontChangeOnNode(aSizeChange, aNode->GetChildAt(i));