Bug 1460509 - part 80: Make HTMLEditRules::DeleteNodeIfCollapsedText() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 18 May 2018 00:31:20 +0900
changeset 798798 bdd5165b5b18b8c6cb73a5b62bf74472e462a58c
parent 798797 08075e6ebd789de21355f55131c6cb186e28719b
child 798799 94bb38460b6c811a94e5e4228fd28bb66e3e114c
push id110840
push usermasayuki@d-toybox.com
push dateWed, 23 May 2018 13:41:58 +0000
reviewersm_kato
bugs1460509
milestone62.0a1
Bug 1460509 - part 80: Make HTMLEditRules::DeleteNodeIfCollapsedText() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r?m_kato MozReview-Commit-ID: C8ZNGlKJXlq
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -2642,17 +2642,21 @@ HTMLEditRules::WillDeleteSelection(nsIEd
       //     compatibility.
       // XXX When Delete key is pressed, Edge removes all preceding empty
       //     text nodes when removing the first character of the non-empty
       //     text node.  Chromium removes only selected empty text node and
       //     following empty text nodes and the first character of the
       //     non-empty text node.  For now, we should keep our traditional
       //     behavior same as Chromium for backward compatibility.
 
-      DeleteNodeIfCollapsedText(nodeAsText);
+      rv = DeleteNodeIfCollapsedText(nodeAsText);
+      if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
+      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to remove collapsed text");
 
       rv = InsertBRIfNeeded();
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
 
       // Remember that we did a ranged delete for the benefit of
       // AfterEditInner().
@@ -3193,18 +3197,28 @@ HTMLEditRules::WillDeleteSelection(nsIEd
 
   // We might have left only collapsed whitespace in the start/end nodes
   {
     AutoTrackDOMPoint startTracker(HTMLEditorRef().mRangeUpdater,
                                    address_of(startNode), &startOffset);
     AutoTrackDOMPoint endTracker(HTMLEditorRef().mRangeUpdater,
                                  address_of(endNode), &endOffset);
 
-    DeleteNodeIfCollapsedText(*startNode);
-    DeleteNodeIfCollapsedText(*endNode);
+    nsresult rv = DeleteNodeIfCollapsedText(*startNode);
+    if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+      "Failed to delete start node even though it's collapsed text");
+    rv = DeleteNodeIfCollapsedText(*endNode);
+    if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+      "Failed to delete end node even though it's collapsed text");
   }
 
   // If we're joining blocks: if deleting forward the selection should be
   // collapsed to the end of the selection, if deleting backward the selection
   // should be collapsed to the beginning of the selection. But if we're not
   // joining then the selection should collapse to the beginning of the
   // selection if we'redeleting forward, because the end of the selection will
   // still be in the next block. And same thing for deleting backwards
@@ -3219,40 +3233,39 @@ HTMLEditRules::WillDeleteSelection(nsIEd
     rv = SelectionRef().Collapse(startNode, startOffset);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
   return NS_OK;
 }
 
-/**
- * If aNode is a text node that contains only collapsed whitespace, delete it.
- * It doesn't serve any useful purpose, and we don't want it to confuse code
- * that doesn't correctly skip over it.
- *
- * If deleting the node fails (like if it's not editable), the caller should
- * proceed as usual, so don't return any errors.
- */
-void
+nsresult
 HTMLEditRules::DeleteNodeIfCollapsedText(nsINode& aNode)
 {
   MOZ_ASSERT(IsEditorDataAvailable());
 
   Text* text = aNode.GetAsText();
   if (!text) {
-    return;
-  }
-
-  if (!HTMLEditorRef().IsVisibleTextNode(*text)) {
-    DebugOnly<nsresult> rv = HTMLEditorRef().DeleteNodeWithTransaction(aNode);
-    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to remove aNode");
-  }
-}
-
+    return NS_OK;
+  }
+
+  if (HTMLEditorRef().IsVisibleTextNode(*text)) {
+    return NS_OK;
+  }
+
+  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;
+}
 
 /**
  * InsertBRIfNeeded() determines if a br is needed for current selection to not
  * be spastic.  If so, it inserts one.  Callers responsibility to only call
  * with collapsed selection.
  *
  * @param aSelection        The collapsed selection.
  */
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -190,17 +190,25 @@ protected:
    * or something equivalent.  This method actually tries to insert new
    * paragraph or <br> element, etc.
    *
    * @param aCancel             Returns true if target node is not editable.
    * @param aHandled            Returns true if actually insert new break.
    */
   nsresult WillInsertBreak(bool* aCancel, bool* aHandled);
 
-  void DeleteNodeIfCollapsedText(nsINode& aNode);
+  /**
+   * If aNode is a text node that contains only collapsed whitespace, delete
+   * it.  It doesn't serve any useful purpose, and we don't want it to confuse
+   * code that doesn't correctly skip over it.
+   *
+   * If deleting the node fails (like if it's not editable), the caller should
+   * proceed as usual, so don't return any errors.
+   */
+  MOZ_MUST_USE nsresult DeleteNodeIfCollapsedText(nsINode& aNode);
 
   /**
    * InsertBRElement() inserts a <br> element into aInsertToBreak.
    *
    * @param aInsertToBreak      The point where new <br> element will be
    *                            inserted before.
    */
   MOZ_MUST_USE nsresult InsertBRElement(const EditorDOMPoint& aInsertToBreak);