Bug 1460509 - part 75: Make HTMLEditRules::DeleteNonTableElements() 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 23:46:00 +0900
changeset 798793 2790be26fe1665a79b37887ebd6123a7939beaaa
parent 798792 1aacc148d12799d83bc74b115da1e2377f063c69
child 798794 42b5fcdcebaef787f5746fe750a1f8df5f5ad84c
push id110840
push usermasayuki@d-toybox.com
push dateWed, 23 May 2018 13:41:58 +0000
reviewersm_kato
bugs1460509
milestone62.0a1
Bug 1460509 - part 75: Make HTMLEditRules::DeleteNonTableElements() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato MozReview-Commit-ID: 9SIBECF8s1R
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3120,35 +3120,37 @@ HTMLEditRules::WillDeleteSelection(nsIEd
           if (NS_WARN_IF(NS_FAILED(rv))) {
             return rv;
           }
           iter.AppendList(functor, arrayOfNodes);
 
           // Now that we have the list, delete non-table elements
           int32_t listCount = arrayOfNodes.Length();
           for (int32_t j = 0; j < listCount; j++) {
-            nsCOMPtr<nsINode> somenode = do_QueryInterface(arrayOfNodes[0]);
-            if (NS_WARN_IF(!somenode)) {
-              return NS_ERROR_FAILURE;
+            OwningNonNull<nsINode> node = arrayOfNodes[0];
+            nsresult rv = DeleteElementsExceptTableRelatedElements(node);
+            if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
+              return NS_ERROR_EDITOR_DESTROYED;
             }
-            DeleteNonTableElements(somenode);
+            NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+              "Failed to elements except table related elements");
             arrayOfNodes.RemoveElementAt(0);
             // If something visible is deleted, no need to join.  Visible means
             // all nodes except non-visible textnodes and breaks.
             if (join && origCollapsed) {
-              if (!somenode->IsContent()) {
+              if (!node->IsContent()) {
                 join = false;
                 continue;
               }
-              nsCOMPtr<nsIContent> content = somenode->AsContent();
+              nsIContent* content = node->AsContent();
               if (Text* text = content->GetAsText()) {
                 join = !HTMLEditorRef().IsInVisibleTextFrames(*text);
               } else {
                 join = content->IsHTMLElement(nsGkAtoms::br) &&
-                       !HTMLEditorRef().IsVisibleBRElement(somenode);
+                       !HTMLEditorRef().IsVisibleBRElement(node);
               }
             }
           }
         }
 
         // Check endpoints for possible text deletion.  We can assume that if
         // text node is found, we can delete to end or to begining as
         // appropriate, since the case where both sel endpoints in same text
@@ -3782,39 +3784,48 @@ HTMLEditRules::MoveContents(Element& aEl
       MoveNodeSmart(*aElement.GetFirstChild(), aDestElement, aInOutDestOffset);
     if (NS_WARN_IF(ret.Failed())) {
       return ret;
     }
   }
   return ret;
 }
 
-
 nsresult
-HTMLEditRules::DeleteNonTableElements(nsINode* aNode)
-{
-  MOZ_ASSERT(IsEditorDataAvailable());
-  MOZ_ASSERT(aNode);
-  if (!HTMLEditUtils::IsTableElementButNotTable(aNode)) {
-    nsresult rv = HTMLEditorRef().DeleteNodeWithTransaction(*aNode);
+HTMLEditRules::DeleteElementsExceptTableRelatedElements(nsINode& aNode)
+{
+  MOZ_ASSERT(IsEditorDataAvailable());
+
+  if (!HTMLEditUtils::IsTableElementButNotTable(&aNode)) {
+    nsresult rv = HTMLEditorRef().DeleteNodeWithTransaction(aNode);
+    if (NS_WARN_IF(!CanHandleEditAction())) {
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
     return NS_OK;
   }
 
-  AutoTArray<nsCOMPtr<nsIContent>, 10> childList;
-  for (nsIContent* child = aNode->GetFirstChild();
+  // XXX For performance, this should just call
+  //     DeleteElementsExceptTableRelatedElements() while there are children
+  //     in aNode.  If we need to avoid infinite loop because mutation event
+  //     listeners can add unexpected nodes into aNode, we should just loop
+  //     only original count of the children.
+  AutoTArray<OwningNonNull<nsIContent>, 10> childList;
+  for (nsIContent* child = aNode.GetFirstChild();
        child; child = child->GetNextSibling()) {
-    childList.AppendElement(child);
+    childList.AppendElement(*child);
   }
 
   for (const auto& child: childList) {
-    nsresult rv = DeleteNonTableElements(child);
-    NS_ENSURE_SUCCESS(rv, rv);
+    nsresult rv = DeleteElementsExceptTableRelatedElements(child);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
   }
   return NS_OK;
 }
 
 nsresult
 HTMLEditRules::DidDeleteSelection()
 {
   MOZ_ASSERT(IsEditorDataAvailable());
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -321,17 +321,29 @@ protected:
    * @param aInOutDestOffset        updated to point after inserted content.
    * @return                        Sets true to handled if this actually moves
    *                                the nodes.
    *                                canceled is always false.
    */
   EditActionResult MoveContents(Element& aElement, Element& aDestElement,
                                 int32_t* aInOutDestOffset);
 
-  nsresult DeleteNonTableElements(nsINode* aNode);
+  /**
+   * DeleteElementsExceptTableRelatedElements() removes elements except
+   * table related elements (except <table> itself) and their contents
+   * from the DOM tree.
+   *
+   * @param aNode               If this is not a table related element, this
+   *                            node will be removed from the DOM tree.
+   *                            Otherwise, this method calls itself recursively
+   *                            with its children.
+   *
+   */
+  MOZ_MUST_USE nsresult
+  DeleteElementsExceptTableRelatedElements(nsINode& aNode);
 
   /**
    * XXX Should document what this does.
    */
   MOZ_MUST_USE nsresult
   WillMakeList(const nsAString* aListType, bool aEntireList,
                const nsAString* aBulletType,
                bool* aCancel, bool* aHandled,