Bug 1402469 - Part 1. Return value of ConvertListType should use Element instead of nsresult. r?masayuki draft
authorMakoto Kato <m_kato@ga2.so-net.ne.jp>
Mon, 25 Sep 2017 14:15:50 +0900
changeset 669655 f0c1aa6f0b9a0a8cde7754664695c40aa1cdd59b
parent 669596 7e962631ba4298bcefa571008661983d77c3e652
child 669656 b63396bc092329153e06fe3d010f240ce7a8c937
push id81386
push userbmo:m_kato@ga2.so-net.ne.jp
push dateMon, 25 Sep 2017 05:20:01 +0000
reviewersmasayuki
bugs1402469, 1053779
milestone58.0a1
Bug 1402469 - Part 1. Return value of ConvertListType should use Element instead of nsresult. r?masayuki This is a typo bug of Bug 1053779 Part 2. ConvertListType might return NS_OK even if ReplaceContainer doesn't return valid value. So, to clean up code, we should return Element instead of nsresult since out parameter of this function is Element only. MozReview-Commit-ID: 44UHETzcdGy
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -2955,19 +2955,18 @@ HTMLEditRules::TryToJoinBlocks(nsIConten
   nsCOMPtr<Element> brNode =
     CheckForInvisibleBR(*leftBlock, BRLocation::blockEnd);
   EditActionResult ret(NS_OK);
   if (mergeLists || leftBlock->NodeInfo()->NameAtom() ==
                     rightBlock->NodeInfo()->NameAtom()) {
     // Nodes are same type.  merge them.
     EditorDOMPoint pt = JoinNodesSmart(*leftBlock, *rightBlock);
     if (pt.node && mergeLists) {
-      nsCOMPtr<Element> newBlock;
-      ConvertListType(rightBlock, getter_AddRefs(newBlock),
-                      existingList, nsGkAtoms::li);
+      RefPtr<Element> newBlock =
+        ConvertListType(rightBlock, existingList, nsGkAtoms::li);
     }
     ret.MarkAsHandled();
   } else {
     // Nodes are dissimilar types.
     ret |= MoveBlock(*leftBlock, *rightBlock, leftOffset, rightOffset);
     if (NS_WARN_IF(ret.Failed())) {
       return ret;
     }
@@ -3315,28 +3314,29 @@ HTMLEditRules::WillMakeList(Selection* a
       if (curList && !EditorUtils::IsDescendantOf(curNode, curList)) {
         // move all of our children into curList.  cheezy way to do it: move
         // whole list and then RemoveContainer() on the list.  ConvertListType
         // first: that routine handles converting the list item types, if
         // needed
         NS_ENSURE_STATE(mHTMLEditor);
         rv = mHTMLEditor->MoveNode(curNode, curList, -1);
         NS_ENSURE_SUCCESS(rv, rv);
-        rv = ConvertListType(curNode->AsElement(), getter_AddRefs(newBlock),
-                             listType, itemType);
-        NS_ENSURE_SUCCESS(rv, rv);
+        newBlock = ConvertListType(curNode->AsElement(), listType, itemType);
+        if (NS_WARN_IF(!newBlock)) {
+          return NS_ERROR_FAILURE;
+        }
         NS_ENSURE_STATE(mHTMLEditor);
         rv = mHTMLEditor->RemoveBlockContainer(*newBlock);
         NS_ENSURE_SUCCESS(rv, rv);
       } else {
         // replace list with new list type
-        rv = ConvertListType(curNode->AsElement(), getter_AddRefs(newBlock),
-                             listType, itemType);
-        NS_ENSURE_SUCCESS(rv, rv);
-        curList = newBlock;
+        curList = ConvertListType(curNode->AsElement(), listType, itemType);
+        if (NS_WARN_IF(!curList)) {
+          return NS_ERROR_FAILURE;
+        }
       }
       prevListItem = nullptr;
       continue;
     }
 
     if (HTMLEditUtils::IsListItem(curNode)) {
       NS_ENSURE_STATE(mHTMLEditor);
       if (!curParent->IsHTMLElement(listType)) {
@@ -4490,57 +4490,52 @@ HTMLEditRules::OutdentPartOfBlock(Elemen
   }
 
   return NS_OK;
 }
 
 /**
  * ConvertListType() converts list type and list item type.
  */
-nsresult
+already_AddRefed<Element>
 HTMLEditRules::ConvertListType(Element* aList,
-                               Element** aOutList,
                                nsIAtom* aListType,
                                nsIAtom* aItemType)
 {
   MOZ_ASSERT(aList);
-  MOZ_ASSERT(aOutList);
   MOZ_ASSERT(aListType);
   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);
-        NS_ENSURE_STATE(child);
+        if (NS_WARN_IF(!child)) {
+          return nullptr;
+        }
       } else if (HTMLEditUtils::IsList(element) &&
                  !element->IsHTMLElement(aListType)) {
-        nsCOMPtr<dom::Element> temp;
-        nsresult rv = ConvertListType(child->AsElement(), getter_AddRefs(temp),
-                                      aListType, aItemType);
-        NS_ENSURE_SUCCESS(rv, rv);
-        child = temp.forget();
+        child = ConvertListType(child->AsElement(), aListType, aItemType);
+        if (NS_WARN_IF(!child)) {
+          return nullptr;
+        }
       }
     }
     child = child->GetNextSibling();
   }
 
   if (aList->IsHTMLElement(aListType)) {
-    nsCOMPtr<dom::Element> list = aList->AsElement();
-    list.forget(aOutList);
-    return NS_OK;
-  }
-
-  *aOutList = mHTMLEditor->ReplaceContainer(aList, aListType).take();
-  NS_ENSURE_STATE(aOutList);
-
-  return NS_OK;
+    RefPtr<dom::Element> list = aList->AsElement();
+    return list.forget();
+  }
+
+  return mHTMLEditor->ReplaceContainer(aList, aListType);
 }
 
 
 /**
  * CreateStyleForInsertText() takes care of clearing and setting appropriate
  * style nodes for text insertion.
  */
 nsresult
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -311,18 +311,18 @@ protected:
                   nsIContent** aOutMiddleNode = nullptr);
   nsresult OutdentPartOfBlock(Element& aBlock,
                               nsIContent& aStartChild,
                               nsIContent& aEndChild,
                               bool aIsBlockIndentedWithCSS,
                               nsIContent** aOutLeftNode,
                               nsIContent** aOutRightNode);
 
-  nsresult ConvertListType(Element* aList, Element** aOutList,
-                           nsIAtom* aListType, nsIAtom* aItemType);
+  already_AddRefed<Element> ConvertListType(Element* aList, nsIAtom* aListType,
+                                            nsIAtom* aItemType);
 
   nsresult CreateStyleForInsertText(Selection& aSelection, nsIDocument& aDoc);
   enum class MozBRCounts { yes, no };
   nsresult IsEmptyBlock(Element& aNode, bool* aOutIsEmptyBlock,
                         MozBRCounts aMozBRCounts = MozBRCounts::yes);
   nsresult CheckForEmptyBlock(nsINode* aStartNode, Element* aBodyNode,
                               Selection* aSelection,
                               nsIEditor::EDirection aAction, bool* aHandled);