Bug 1460509 - part 60: Make HTMLEditRules::ConvertListType() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 17 May 2018 14:28:47 +0900
changeset 798778 df2b6285101f7ad3bb5c17055c0e41891d5187fa
parent 798777 7b93bc7c22082f29e55b05864ee5ed6a85e8aa91
child 798779 628d6f26bd9f2054da351592bf1dcf1e0d69e17e
push id110840
push usermasayuki@d-toybox.com
push dateWed, 23 May 2018 13:41:58 +0000
reviewersm_kato
bugs1460509
milestone62.0a1
Bug 1460509 - part 60: Make HTMLEditRules::ConvertListType() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato MozReview-Commit-ID: 1xbR3f73NR1
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3623,18 +3623,21 @@ HTMLEditRules::TryToJoinBlocksWithTransa
     // Nodes are same type.  merge them.
     EditorDOMPoint pt;
     nsresult rv =
       JoinNearestEditableNodesWithTransaction(*leftBlock, *rightBlock, &pt);
     if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
       return EditActionIgnored(NS_ERROR_EDITOR_DESTROYED);
     }
     if (pt.IsSet() && mergeLists) {
-      RefPtr<Element> newBlock =
-        ConvertListType(rightBlock, existingList, nsGkAtoms::li);
+      CreateElementResult convertListTypeResult =
+        ConvertListType(*rightBlock, *existingList, *nsGkAtoms::li);
+      if (NS_WARN_IF(convertListTypeResult.Rv() == NS_ERROR_EDITOR_DESTROYED)) {
+        return EditActionIgnored(NS_ERROR_EDITOR_DESTROYED);
+      }
     }
     ret.MarkAsHandled();
   } else {
     // Nodes are dissimilar types.
     ret |= MoveBlock(*leftBlock, *rightBlock, -1, 0);
     if (NS_WARN_IF(ret.Failed())) {
       return ret;
     }
@@ -3980,17 +3983,17 @@ HTMLEditRules::WillMakeList(const nsAStr
   // blockquote, then look inside of it until we find inner list or content.
 
   LookInsideDivBQandList(arrayOfNodes);
 
   // Ok, now go through all the nodes and put then in the list,
   // or whatever is approriate.  Wohoo!
 
   uint32_t listCount = arrayOfNodes.Length();
-  nsCOMPtr<Element> curList, prevListItem;
+  RefPtr<Element> curList, prevListItem;
 
   for (uint32_t i = 0; i < listCount; i++) {
     // here's where we actually figure out what to do
     RefPtr<Element> newBlock;
     if (NS_WARN_IF(!arrayOfNodes[i]->IsContent())) {
       return NS_ERROR_FAILURE;
     }
     OwningNonNull<nsIContent> curNode = *arrayOfNodes[i]->AsContent();
@@ -4022,30 +4025,35 @@ HTMLEditRules::WillMakeList(const nsAStr
         // move all of our children into curList.  cheezy way to do it: move
         // whole list and then RemoveContainerWithTransaction() on the list.
         // ConvertListType first: that routine handles converting the list
         // item types, if needed.
         rv = HTMLEditorRef().MoveNodeToEndWithTransaction(*curNode, *curList);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
-        newBlock = ConvertListType(curNode->AsElement(), listType, itemType);
-        if (NS_WARN_IF(!newBlock)) {
-          return NS_ERROR_FAILURE;
-        }
-        rv = HTMLEditorRef().RemoveBlockContainerWithTransaction(*newBlock);
+        CreateElementResult convertListTypeResult =
+          ConvertListType(*curNode->AsElement(), *listType, *itemType);
+        if (NS_WARN_IF(convertListTypeResult.Failed())) {
+          return convertListTypeResult.Rv();
+        }
+        rv = HTMLEditorRef().RemoveBlockContainerWithTransaction(
+                               *convertListTypeResult.GetNewNode());
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
+        newBlock = convertListTypeResult.forget();
       } else {
         // replace list with new list type
-        curList = ConvertListType(curNode->AsElement(), listType, itemType);
-        if (NS_WARN_IF(!curList)) {
-          return NS_ERROR_FAILURE;
-        }
+        CreateElementResult convertListTypeResult =
+          ConvertListType(*curNode->AsElement(), *listType, *itemType);
+        if (NS_WARN_IF(convertListTypeResult.Failed())) {
+          return convertListTypeResult.Rv();
+        }
+        curList = convertListTypeResult.forget();
       }
       prevListItem = nullptr;
       continue;
     }
 
     EditorRawDOMPoint atCurNode(curNode);
     if (NS_WARN_IF(!atCurNode.IsSet())) {
       return NS_ERROR_FAILURE;
@@ -5464,60 +5472,64 @@ HTMLEditRules::OutdentPartOfBlock(Elemen
       return rv;
     }
     return NS_OK;
   }
 
   return NS_OK;
 }
 
-/**
- * ConvertListType() converts list type and list item type.
- */
-already_AddRefed<Element>
-HTMLEditRules::ConvertListType(Element* aList,
-                               nsAtom* aListType,
-                               nsAtom* aItemType)
-{
-  MOZ_ASSERT(IsEditorDataAvailable());
-  MOZ_ASSERT(aList);
-  MOZ_ASSERT(aListType);
-  MOZ_ASSERT(aItemType);
-
-  nsCOMPtr<nsINode> child = aList->GetFirstChild();
+CreateElementResult
+HTMLEditRules::ConvertListType(Element& aListElement,
+                               nsAtom& aNewListTag,
+                               nsAtom& aNewListItemTag)
+{
+  MOZ_ASSERT(IsEditorDataAvailable());
+
+  nsCOMPtr<nsINode> child = aListElement.GetFirstChild();
   while (child) {
     if (child->IsElement()) {
       Element* element = child->AsElement();
       if (HTMLEditUtils::IsListItem(element) &&
-          !element->IsHTMLElement(aItemType)) {
+          !element->IsHTMLElement(&aNewListItemTag)) {
         child =
-          HTMLEditorRef().ReplaceContainerWithTransaction(*element, *aItemType);
+          HTMLEditorRef().ReplaceContainerWithTransaction(*element,
+                                                          aNewListItemTag);
+        if (NS_WARN_IF(!CanHandleEditAction())) {
+          return CreateElementResult(NS_ERROR_EDITOR_DESTROYED);
+        }
         if (NS_WARN_IF(!child)) {
-          return nullptr;
+          return CreateElementResult(NS_ERROR_FAILURE);
         }
       } else if (HTMLEditUtils::IsList(element) &&
-                 !element->IsHTMLElement(aListType)) {
-        child = ConvertListType(child->AsElement(), aListType, aItemType);
-        if (NS_WARN_IF(!child)) {
-          return nullptr;
-        }
+                 !element->IsHTMLElement(&aNewListTag)) {
+        // XXX List elements shouldn't have other list elements as their
+        //     child.  Why do we handle such invalid tree?
+        CreateElementResult convertListTypeResult =
+          ConvertListType(*child->AsElement(), aNewListTag, aNewListItemTag);
+        if (NS_WARN_IF(convertListTypeResult.Failed())) {
+          return convertListTypeResult;
+        }
+        child = convertListTypeResult.forget();
       }
     }
     child = child->GetNextSibling();
   }
 
-  if (aList->IsHTMLElement(aListType)) {
-    RefPtr<dom::Element> list = aList;
-    return list.forget();
+  if (aListElement.IsHTMLElement(&aNewListTag)) {
+    return CreateElementResult(&aListElement);
   }
 
   RefPtr<Element> listElement =
-    HTMLEditorRef().ReplaceContainerWithTransaction(*aList, *aListType);
+    HTMLEditorRef().ReplaceContainerWithTransaction(aListElement, aNewListTag);
+  if (NS_WARN_IF(!CanHandleEditAction())) {
+    return CreateElementResult(NS_ERROR_EDITOR_DESTROYED);
+  }
   NS_WARNING_ASSERTION(listElement != nullptr, "Failed to create list element");
-  return listElement.forget();
+  return CreateElementResult(listElement.forget());
 }
 
 nsresult
 HTMLEditRules::CreateStyleForInsertText(nsIDocument& aDocument)
 {
   MOZ_ASSERT(IsEditorDataAvailable());
   MOZ_ASSERT(HTMLEditorRef().mTypeInState);
 
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -539,18 +539,32 @@ protected:
                   nsIContent** aOutMiddleNode = nullptr);
   nsresult OutdentPartOfBlock(Element& aBlock,
                               nsIContent& aStartChild,
                               nsIContent& aEndChild,
                               bool aIsBlockIndentedWithCSS,
                               nsIContent** aOutLeftNode,
                               nsIContent** aOutRightNode);
 
-  already_AddRefed<Element> ConvertListType(Element* aList, nsAtom* aListType,
-                                            nsAtom* aItemType);
+  /**
+   * ConvertListType() replaces child list items of aListElement with
+   * new list item element whose tag name is aNewListItemTag.
+   * Note that if there are other list elements as children of aListElement,
+   * this calls itself recursively even though it's invalid structure.
+   *
+   * @param aListElement        The list element whose list items will be
+   *                            replaced.
+   * @param aNewListTag         New list tag name.
+   * @param aNewListItemTag     New list item tag name.
+   * @return                    New list element or an error code if it fails.
+   *                            New list element may be aListElement if its
+   *                            tag name is same as aNewListTag.
+   */
+  MOZ_MUST_USE CreateElementResult
+  ConvertListType(Element& aListElement, nsAtom& aListType, nsAtom& aItemType);
 
   /**
    * CreateStyleForInsertText() sets CSS properties which are stored in
    * TypeInState to proper element node.
    *
    * @param aDocument           The document of the editor.
    */
   MOZ_MUST_USE nsresult CreateStyleForInsertText(nsIDocument& aDocument);