Bug 1460509 - part 19: Make HTMLEditRules::AlignBlock() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Sat, 12 May 2018 10:58:57 +0900
changeset 473697 ba7cf7a4465e1f1e5740d9da092e0aa8f6931692
parent 473696 a737ec81970747f3fcc2c40ae9d8b17fe1e9488e
child 473698 2f7af16c2b10a02832db498fed38394d4026719a
push id9374
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:43:20 +0000
treeherdermozilla-beta@160e085dfb0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1460509
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1460509 - part 19: Make HTMLEditRules::AlignBlock() return NS_ERROR_EDITOR_DESTROYED if it causes destroying the editor r=m_kato Additionally, this patch renames its specific enum class from ContentsOnly to ResetAlignOf for making its target clearer. MozReview-Commit-ID: KD4ndAsMClN
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -5533,18 +5533,21 @@ HTMLEditRules::WillAlign(const nsAString
   if (nodeArray.Length() == 1) {
     OwningNonNull<nsINode> node = nodeArray[0];
 
     if (HTMLEditUtils::SupportsAlignAttr(*node)) {
       // The node is a table element, an hr, a paragraph, a div or a section
       // header; in HTML 4, it can directly carry the ALIGN attribute and we
       // don't need to make a div! If we are in CSS mode, all the work is done
       // in AlignBlock
-      rv = AlignBlock(*node->AsElement(), aAlignType, ContentsOnly::yes);
-      NS_ENSURE_SUCCESS(rv, rv);
+      rv = AlignBlock(*node->AsElement(), aAlignType,
+                      ResetAlignOf::OnlyDescendants);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
       return NS_OK;
     }
 
     if (TextEditUtils::IsBreak(node)) {
       // The special case emptyDiv code (below) that consumes BRs can cause
       // tables to split if the start node of the selection is not in a table
       // cell or caption, for example parent is a <tr>.  Avoid this unnecessary
       // splitting if possible by leaving emptyDiv FALSE so that we fall
@@ -5614,18 +5617,20 @@ HTMLEditRules::WillAlign(const nsAString
       HTMLEditorRef().CreateNodeWithTransaction(*nsGkAtoms::div,
                                                 pointToInsertDiv);
     if (NS_WARN_IF(!div)) {
       return NS_ERROR_FAILURE;
     }
     // Remember our new block for postprocessing
     mNewBlock = div;
     // Set up the alignment on the div, using HTML or CSS
-    rv = AlignBlock(*div, aAlignType, ContentsOnly::yes);
-    NS_ENSURE_SUCCESS(rv, rv);
+    rv = AlignBlock(*div, aAlignType, ResetAlignOf::OnlyDescendants);
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
     *aHandled = true;
     // Put in a moz-br so that it won't get deleted
     CreateElementResult createMozBrResult =
       CreateMozBR(EditorRawDOMPoint(div, 0));
     if (NS_WARN_IF(createMozBrResult.Failed())) {
       return createMozBrResult.Rv();
     }
     EditorRawDOMPoint atStartOfDiv(div, 0);
@@ -5659,18 +5664,21 @@ HTMLEditRules::WillAlign(const nsAString
       continue;
     }
 
     // The node is a table element, an hr, a paragraph, a div or a section
     // header; in HTML 4, it can directly carry the ALIGN attribute and we
     // don't need to nest it, just set the alignment.  In CSS, assign the
     // corresponding CSS styles in AlignBlock
     if (HTMLEditUtils::SupportsAlignAttr(*curNode)) {
-      rv = AlignBlock(*curNode->AsElement(), aAlignType, ContentsOnly::no);
-      NS_ENSURE_SUCCESS(rv, rv);
+      rv = AlignBlock(*curNode->AsElement(), aAlignType,
+                      ResetAlignOf::ElementAndDescendants);
+      if (NS_WARN_IF(NS_FAILED(rv))) {
+        return rv;
+      }
       // Clear out curDiv so that we don't put nodes after this one into it
       curDiv = nullptr;
       continue;
     }
 
     EditorDOMPoint atCurNode(curNode);
     if (NS_WARN_IF(!atCurNode.IsSet())) {
       continue;
@@ -5733,17 +5741,21 @@ HTMLEditRules::WillAlign(const nsAString
         HTMLEditorRef().CreateNodeWithTransaction(*nsGkAtoms::div,
                                                   splitNodeResult.SplitPoint());
       if (NS_WARN_IF(!curDiv)) {
         return NS_ERROR_FAILURE;
       }
       // Remember our new block for postprocessing
       mNewBlock = curDiv;
       // Set up the alignment on the div
-      rv = AlignBlock(*curDiv, aAlignType, ContentsOnly::yes);
+      rv = AlignBlock(*curDiv, aAlignType, ResetAlignOf::OnlyDescendants);
+      if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
+        return NS_ERROR_EDITOR_DESTROYED;
+      }
+      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to align the <div>");
     }
 
     // Tuck the node into the end of the active div
     rv = HTMLEditorRef().MoveNodeToEndWithTransaction(*curNode->AsContent(),
                                                       *curDiv);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
@@ -9862,46 +9874,57 @@ HTMLEditRules::MakeSureElemStartsAndEnds
     return rv;
   }
   return NS_OK;
 }
 
 nsresult
 HTMLEditRules::AlignBlock(Element& aElement,
                           const nsAString& aAlignType,
-                          ContentsOnly aContentsOnly)
+                          ResetAlignOf aResetAlignOf)
 {
   MOZ_ASSERT(IsEditorDataAvailable());
 
   if (!IsBlockNode(aElement) && !aElement.IsHTMLElement(nsGkAtoms::hr)) {
     // We deal only with blocks; early way out
     return NS_OK;
   }
 
   nsresult rv = RemoveAlignment(aElement, aAlignType,
-                                aContentsOnly == ContentsOnly::yes);
+                                aResetAlignOf == ResetAlignOf::OnlyDescendants);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   if (HTMLEditorRef().IsCSSEnabled()) {
     // Let's use CSS alignment; we use margin-left and margin-right for tables
     // and text-align for other block-level elements
-    return HTMLEditorRef().SetAttributeOrEquivalent(
-                             &aElement, nsGkAtoms::align, aAlignType, false);
+    nsresult rv =
+      HTMLEditorRef().SetAttributeOrEquivalent(&aElement, nsGkAtoms::align,
+                                               aAlignType, false);
+    if (NS_WARN_IF(!CanHandleEditAction())) {
+      return NS_ERROR_EDITOR_DESTROYED;
+    }
+    if (NS_WARN_IF(NS_FAILED(rv))) {
+      return rv;
+    }
+    return NS_OK;
   }
 
   // HTML case; this code is supposed to be called ONLY if the element
   // supports the align attribute but we'll never know...
   if (NS_WARN_IF(!HTMLEditUtils::SupportsAlignAttr(aElement))) {
     // XXX error?
     return NS_OK;
   }
 
   rv = HTMLEditorRef().SetAttributeOrEquivalent(&aElement, nsGkAtoms::align,
                                                 aAlignType, false);
+  if (NS_WARN_IF(!CanHandleEditAction())) {
+    return NS_ERROR_EDITOR_DESTROYED;
+  }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return NS_OK;
 }
 
 nsresult
 HTMLEditRules::ChangeMarginStart(Element& aElement,
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -585,19 +585,31 @@ protected:
     return InsertBRIfNeededInternal(aNode, true);
   }
 
   bool IsEmptyInline(nsINode& aNode);
   bool ListIsEmptyLine(nsTArray<OwningNonNull<nsINode>>& arrayOfNodes);
   nsresult RemoveAlignment(nsINode& aNode, const nsAString& aAlignType,
                            bool aChildrenOnly);
   nsresult MakeSureElemStartsOrEndsOnCR(nsINode& aNode, bool aStarts);
-  enum class ContentsOnly { no, yes };
-  nsresult AlignBlock(Element& aElement,
-                      const nsAString& aAlignType, ContentsOnly aContentsOnly);
+
+  /**
+   * AlignBlock() resets align attribute, text-align property, etc first.
+   * Then, aligns contents of aElement on aAlignType.
+   *
+   * @param aElement            The element whose contents will be aligned.
+   * @param aAlignType          Boundary or "center" which contents should be
+   *                            aligned on.
+   * @param aResetAlignOf       Resets align of whether element and its
+   *                            descendants or only descendants.
+   */
+  enum class ResetAlignOf { ElementAndDescendants, OnlyDescendants };
+  MOZ_MUST_USE nsresult
+  AlignBlock(Element& aElement, const nsAString& aAlignType,
+             ResetAlignOf aResetAlignOf);
 
   /**
    * IncreaseMarginToIndent() increases the margin of aElement.  See the
    * document of ChangeMarginStart() for the detail.
    * XXX This is not aware of vertical writing-mode.
    *
    * @param aElement            The element to be indented.
    */