Bug 1460509 - part 25: Make HTMLEditRules::RemoveEmptyNodes() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 14 May 2018 19:07:59 +0900
changeset 798743 a5c1b6709a9979a75a21a81cd44185314133c64e
parent 798742 55aececc29764efadc654cb547c325a1a43bbf8c
child 798744 cd0a5bf636e7703a9cf541189834f1367e4d6932
push id110840
push usermasayuki@d-toybox.com
push dateWed, 23 May 2018 13:41:58 +0000
reviewersm_kato
bugs1460509
milestone62.0a1
Bug 1460509 - part 25: Make HTMLEditRules::RemoveEmptyNodes() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato And this patch renames it to RemoveEmptyNodesInChangedRange(). MozReview-Commit-ID: Kr2xmUOH1ZZ
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -519,17 +519,17 @@ HTMLEditRules::AfterEditInner(EditAction
         aAction != EditAction::insertIMEText) {
       nsresult rv = HTMLEditorRef().CollapseAdjacentTextNodes(mDocChangeRange);
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
 
     // clean up any empty nodes in the selection
-    nsresult rv = RemoveEmptyNodes();
+    nsresult rv = RemoveEmptyNodesInChangedRange();
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
 
     // attempt to transform any unneeded nbsp's into spaces after doing various operations
     if (aAction == EditAction::insertText ||
         aAction == EditAction::insertIMEText ||
         aAction == EditAction::deleteSelection ||
@@ -9001,17 +9001,17 @@ HTMLEditRules::InDifferentTableElements(
     aNode2 = aNode2->GetParentNode();
   }
 
   return aNode1 != aNode2;
 }
 
 
 nsresult
-HTMLEditRules::RemoveEmptyNodes()
+HTMLEditRules::RemoveEmptyNodesInChangedRange()
 {
   MOZ_ASSERT(IsEditorDataAvailable());
 
   // Some general notes on the algorithm used here: the goal is to examine all
   // the nodes in mDocChangeRange, and remove the empty ones.  We do this by
   // using a content iterator to traverse all the nodes in the range, and
   // placing the empty nodes into an array.  After finishing the iteration, we
   // delete the empty nodes in the array.  (They cannot be deleted as we find
@@ -9035,17 +9035,19 @@ HTMLEditRules::RemoveEmptyNodes()
   // Unfortunately I can't use that strategy here, because the range may
   // include some children of a node while excluding others.  Thus I could find
   // all the _examined_ children empty, but still not have an empty parent.
 
   // need an iterator
   nsCOMPtr<nsIContentIterator> iter = NS_NewContentIterator();
 
   nsresult rv = iter->Init(mDocChangeRange);
-  NS_ENSURE_SUCCESS(rv, rv);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return rv;
+  }
 
   nsTArray<OwningNonNull<nsINode>> arrayOfEmptyNodes, arrayOfEmptyCites, skipList;
 
   // Check for empty nodes
   while (!iter->IsDone()) {
     OwningNonNull<nsINode> node = *iter->GetCurrentNode();
 
     nsCOMPtr<nsINode> parent = node->GetParentNode();
@@ -9053,101 +9055,112 @@ HTMLEditRules::RemoveEmptyNodes()
     size_t idx = skipList.IndexOf(node);
     if (idx != skipList.NoIndex) {
       // This node is on our skip list.  Skip processing for this node, and
       // replace its value in the skip list with the value of its parent
       if (parent) {
         skipList[idx] = parent;
       }
     } else {
-      bool bIsCandidate = false;
-      bool bIsEmptyNode = false;
-      bool bIsMailCite = false;
+      bool isCandidate = false;
+      bool isEmptyNode = false;
+      bool isMailCite = false;
 
       if (node->IsElement()) {
         if (node->IsHTMLElement(nsGkAtoms::body)) {
           // Don't delete the body
-        } else if ((bIsMailCite = HTMLEditUtils::IsMailCite(node)) ||
+        } else if ((isMailCite = HTMLEditUtils::IsMailCite(node)) ||
                    node->IsHTMLElement(nsGkAtoms::a) ||
                    HTMLEditUtils::IsInlineStyle(node) ||
                    HTMLEditUtils::IsList(node) ||
                    node->IsHTMLElement(nsGkAtoms::div)) {
           // Only consider certain nodes to be empty for purposes of removal
-          bIsCandidate = true;
+          isCandidate = true;
         } else if (HTMLEditUtils::IsFormatNode(node) ||
                    HTMLEditUtils::IsListItem(node) ||
                    node->IsHTMLElement(nsGkAtoms::blockquote)) {
           // These node types are candidates if selection is not in them.  If
           // it is one of these, don't delete if selection inside.  This is so
           // we can create empty headings, etc., for the user to type into.
-          bool bIsSelInNode;
-          rv = SelectionEndpointInNode(node, &bIsSelInNode);
-          NS_ENSURE_SUCCESS(rv, rv);
-          if (!bIsSelInNode) {
-            bIsCandidate = true;
+          bool isSelectionEndInNode;
+          rv = SelectionEndpointInNode(node, &isSelectionEndInNode);
+          if (NS_WARN_IF(NS_FAILED(rv))) {
+            return rv;
           }
-        }
-      }
-
-      if (bIsCandidate) {
+          if (!isSelectionEndInNode) {
+            isCandidate = true;
+          }
+        }
+      }
+
+      if (isCandidate) {
         // We delete mailcites even if they have a solo br in them.  Other
         // nodes we require to be empty.
-        rv = HTMLEditorRef().IsEmptyNode(node, &bIsEmptyNode,
-                                         bIsMailCite, true);
+        rv = HTMLEditorRef().IsEmptyNode(node, &isEmptyNode,
+                                         isMailCite, true);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
-        if (bIsEmptyNode) {
-          if (bIsMailCite) {
+        if (isEmptyNode) {
+          if (isMailCite) {
             // mailcites go on a separate list from other empty nodes
             arrayOfEmptyCites.AppendElement(*node);
           } else {
             arrayOfEmptyNodes.AppendElement(*node);
           }
         }
       }
 
-      if (!bIsEmptyNode && parent) {
+      if (!isEmptyNode && parent) {
         // put parent on skip list
         skipList.AppendElement(*parent);
       }
     }
 
     iter->Next();
   }
 
   // now delete the empty nodes
   for (OwningNonNull<nsINode>& delNode : arrayOfEmptyNodes) {
     if (HTMLEditorRef().IsModifiableNode(delNode)) {
       rv = HTMLEditorRef().DeleteNodeWithTransaction(*delNode);
+      if (NS_WARN_IF(!CanHandleEditAction())) {
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
     }
   }
 
   // Now delete the empty mailcites.  This is a separate step because we want
   // to pull out any br's and preserve them.
   for (OwningNonNull<nsINode>& delNode : arrayOfEmptyCites) {
-    bool bIsEmptyNode;
-    rv = HTMLEditorRef().IsEmptyNode(delNode, &bIsEmptyNode, false, true);
+    bool isEmptyNode;
+    rv = HTMLEditorRef().IsEmptyNode(delNode, &isEmptyNode, false, true);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
-    if (!bIsEmptyNode) {
+    if (!isEmptyNode) {
       // We are deleting a cite that has just a br.  We want to delete cite,
       // but preserve br.
       RefPtr<Element> brElement =
         HTMLEditorRef().InsertBrElementWithTransaction(
                           SelectionRef(), EditorRawDOMPoint(delNode));
+      if (NS_WARN_IF(!CanHandleEditAction())) {
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
       if (NS_WARN_IF(!brElement)) {
         return NS_ERROR_FAILURE;
       }
     }
     rv = HTMLEditorRef().DeleteNodeWithTransaction(*delNode);
+    if (NS_WARN_IF(!CanHandleEditAction())) {
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
   return NS_OK;
 }
 
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -618,17 +618,29 @@ protected:
   /**
    * Returns true if aNode1 or aNode2 or both is the descendant of some type of
    * table element, but their nearest table element ancestors differ.  "Table
    * element" here includes not just <table> but also <td>, <tbody>, <tr>, etc.
    * The nodes count as being their own descendants for this purpose, so a
    * table element is its own nearest table element ancestor.
    */
   bool InDifferentTableElements(nsINode* aNode1, nsINode* aNode2);
-  nsresult RemoveEmptyNodes();
+
+  /**
+   * RemoveEmptyNodesInChangedRange() removes all empty nodes in
+   * mDocChangeRange.  However, if mail-cite node has only a <br> element,
+   * the node will be removed but <br> element is moved to where the
+   * mail-cite node was.
+   * XXX This method is expensive if mDocChangeRange is too wide and may
+   *     remove unexpected empty element, e.g., it was created by JS, but
+   *     we haven't touched it.  Cannot we remove this method and make
+   *     guarantee that empty nodes won't be created?
+   */
+  MOZ_MUST_USE nsresult RemoveEmptyNodesInChangedRange();
+
   nsresult SelectionEndpointInNode(nsINode* aNode, bool* aResult);
   nsresult UpdateDocChangeRange(nsRange* aRange);
 
   /**
    * ConfirmSelectionInBody() makes sure that Selection is in editor root
    * element typically <body> element (see HTMLEditor::UpdateRootElement())
    * and only one Selection range.
    * XXX This method is not necessary because even if selection is outside the