Bug 1574852 - part 73: Move `HTMLEditRules::IndentAroundSelectionWithCSS()` to `HTMLEditor` r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 06 Sep 2019 12:57:56 +0000
changeset 492101 196ce3ca6e9d0a3adc664da8feb05b418d6f6aa2
parent 492100 4f6733afb731bc19d519ad7a5746b16bf27696e4
child 492102 962be1fcc7c5a5cf6998875cf63b3b2ec6420d8b
push id36543
push userncsoregi@mozilla.com
push dateSat, 07 Sep 2019 09:40:21 +0000
treeherdermozilla-central@de62729b366c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1574852, 1460509
milestone71.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 1574852 - part 73: Move `HTMLEditRules::IndentAroundSelectionWithCSS()` to `HTMLEditor` r=m_kato And also this patch fixes unexpected behavior change by bug 1460509: https://searchfox.org/mozilla-central/diff/d5d67de86f23655fcccc7bbcf4423bb75148fd34/editor/libeditor/HTMLEditRules.cpp#4466 Differential Revision: https://phabricator.services.mozilla.com/D44777
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
editor/libeditor/HTMLEditor.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -4966,294 +4966,275 @@ nsresult HTMLEditRules::WillCSSIndent(bo
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
   // IndentAroundSelectionWithCSS() creates AutoSelectionRestorer.
   // Therefore, even if it returns NS_OK, editor might have been destroyed
   // at restoring Selection.
-  rv = IndentAroundSelectionWithCSS();
+  rv = MOZ_KnownLive(HTMLEditorRef()).IndentAroundSelectionWithCSS();
   if (NS_WARN_IF(!CanHandleEditAction())) {
     return NS_ERROR_EDITOR_DESTROYED;
   }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
   return NS_OK;
 }
 
-nsresult HTMLEditRules::IndentAroundSelectionWithCSS() {
-  MOZ_ASSERT(IsEditorDataAvailable());
-
-  AutoSelectionRestorer restoreSelectionLater(HTMLEditorRef());
-  nsTArray<OwningNonNull<nsRange>> arrayOfRanges;
+nsresult HTMLEditor::IndentAroundSelectionWithCSS() {
+  MOZ_ASSERT(IsTopLevelEditSubActionDataAvailable());
+
+  AutoSelectionRestorer restoreSelectionLater(*this);
   AutoTArray<OwningNonNull<nsINode>, 64> arrayOfNodes;
 
   // short circuit: detect case of collapsed selection inside an <li>.
   // just sublist that <li>.  This prevents bug 97797.
 
-  nsCOMPtr<Element> liNode;
   if (SelectionRefPtr()->IsCollapsed()) {
     EditorRawDOMPoint selectionStartPoint(
         EditorBase::GetStartPoint(*SelectionRefPtr()));
     if (NS_WARN_IF(!selectionStartPoint.IsSet())) {
       return NS_ERROR_FAILURE;
     }
-    Element* block =
-        HTMLEditorRef().GetBlock(*selectionStartPoint.GetContainer());
+    Element* block = GetBlock(*selectionStartPoint.GetContainer());
     if (block && HTMLEditUtils::IsListItem(block)) {
-      liNode = block;
-    }
-  }
-
-  if (liNode) {
-    arrayOfNodes.AppendElement(*liNode);
-  } else {
+      arrayOfNodes.AppendElement(*block);
+    }
+  }
+
+  if (arrayOfNodes.IsEmpty()) {
     nsresult rv =
-        MOZ_KnownLive(HTMLEditorRef())
-            .SplitInlinesAndCollectEditTargetNodesInExtendedSelectionRanges(
-                arrayOfNodes, EditSubAction::eIndent,
-                HTMLEditor::CollectNonEditableNodes::Yes);
+        SplitInlinesAndCollectEditTargetNodesInExtendedSelectionRanges(
+            arrayOfNodes, EditSubAction::eIndent, CollectNonEditableNodes::Yes);
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
   // If there is no visible and editable nodes in the edit targets, make an
   // empty block.
   // XXX Isn't this odd if there are only non-editable visible nodes?
-  if (HTMLEditorRef().IsEmptyOneHardLine(arrayOfNodes)) {
+  if (IsEmptyOneHardLine(arrayOfNodes)) {
     // get selection location
     nsRange* firstRange = SelectionRefPtr()->GetRangeAt(0);
     if (NS_WARN_IF(!firstRange)) {
       return NS_ERROR_FAILURE;
     }
 
     EditorDOMPoint atStartOfSelection(firstRange->StartRef());
     if (NS_WARN_IF(!atStartOfSelection.IsSet())) {
       return NS_ERROR_FAILURE;
     }
 
     // make sure we can put a block here
     SplitNodeResult splitNodeResult =
-        MOZ_KnownLive(HTMLEditorRef())
-            .MaybeSplitAncestorsForInsertWithTransaction(*nsGkAtoms::div,
-                                                         atStartOfSelection);
+        MaybeSplitAncestorsForInsertWithTransaction(*nsGkAtoms::div,
+                                                    atStartOfSelection);
     if (NS_WARN_IF(splitNodeResult.Failed())) {
       return splitNodeResult.Rv();
     }
-    RefPtr<Element> theBlock =
-        MOZ_KnownLive(HTMLEditorRef())
-            .CreateNodeWithTransaction(*nsGkAtoms::div,
-                                       splitNodeResult.SplitPoint());
-    if (NS_WARN_IF(!CanHandleEditAction())) {
+    RefPtr<Element> theBlock = CreateNodeWithTransaction(
+        *nsGkAtoms::div, splitNodeResult.SplitPoint());
+    if (NS_WARN_IF(Destroyed())) {
       return NS_ERROR_EDITOR_DESTROYED;
     }
     if (NS_WARN_IF(!theBlock)) {
       return NS_ERROR_FAILURE;
     }
     // remember our new block for postprocessing
-    HTMLEditorRef().TopLevelEditSubActionDataRef().mNewBlockElement = theBlock;
-    nsresult rv =
-        MOZ_KnownLive(HTMLEditorRef())
-            .ChangeMarginStart(*theBlock, HTMLEditor::ChangeMargin::Increase);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
+    TopLevelEditSubActionDataRef().mNewBlockElement = theBlock;
+    nsresult rv = ChangeMarginStart(*theBlock, ChangeMargin::Increase);
+    if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
       return NS_ERROR_EDITOR_DESTROYED;
     }
-    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to increase indentation");
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+                         "ChangeMarginStart() failed, but ignored");
     // delete anything that was in the list of nodes
+    // XXX We don't need to remove the nodes from the array for performance.
     while (!arrayOfNodes.IsEmpty()) {
       OwningNonNull<nsINode> curNode = arrayOfNodes[0];
-      rv = MOZ_KnownLive(HTMLEditorRef()).DeleteNodeWithTransaction(*curNode);
-      if (NS_WARN_IF(!CanHandleEditAction())) {
+      rv = DeleteNodeWithTransaction(*curNode);
+      if (NS_WARN_IF(Destroyed())) {
         return NS_ERROR_EDITOR_DESTROYED;
       }
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       arrayOfNodes.RemoveElementAt(0);
     }
     // put selection in new block
     EditorRawDOMPoint atStartOfTheBlock(theBlock, 0);
     // Don't restore the selection
     restoreSelectionLater.Abort();
     ErrorResult error;
     SelectionRefPtr()->Collapse(atStartOfTheBlock, error);
-    if (NS_WARN_IF(!CanHandleEditAction())) {
+    if (NS_WARN_IF(Destroyed())) {
       error.SuppressException();
       return NS_ERROR_EDITOR_DESTROYED;
     }
     if (NS_WARN_IF(!error.Failed())) {
       return error.StealNSResult();
     }
     return NS_OK;
   }
 
   // Ok, now go through all the nodes and put them in a blockquote,
-  // or whatever is appropriate.  Wohoo!
-  nsCOMPtr<Element> curList, curQuote;
-  nsCOMPtr<nsIContent> sibling;
+  // or whatever is appropriate.
+  RefPtr<Element> curList, curQuote;
   for (OwningNonNull<nsINode>& curNode : arrayOfNodes) {
     // Here's where we actually figure out what to do.
     EditorDOMPoint atCurNode(curNode);
     if (NS_WARN_IF(!atCurNode.IsSet())) {
       continue;
     }
 
     // Ignore all non-editable nodes.  Leave them be.
-    if (!HTMLEditorRef().IsEditable(curNode)) {
+    // XXX We ignore non-editable nodes here, but not so in the above block.
+    if (!IsEditable(curNode)) {
       continue;
     }
 
     // some logic for putting list items into nested lists...
     if (HTMLEditUtils::IsList(atCurNode.GetContainer())) {
       // Check for whether we should join a list that follows curNode.
       // We do this if the next element is a list, and the list is of the
       // same type (li/ol) as curNode was a part it.
-      sibling = HTMLEditorRef().GetNextHTMLSibling(curNode);
-      if (sibling && HTMLEditUtils::IsList(sibling) &&
-          atCurNode.GetContainer()->NodeInfo()->NameAtom() ==
-              sibling->NodeInfo()->NameAtom() &&
-          atCurNode.GetContainer()->NodeInfo()->NamespaceID() ==
-              sibling->NodeInfo()->NamespaceID()) {
-        nsresult rv =
-            MOZ_KnownLive(HTMLEditorRef())
-                .MoveNodeWithTransaction(MOZ_KnownLive(*curNode->AsContent()),
-                                         EditorDOMPoint(sibling, 0));
-        if (NS_WARN_IF(!CanHandleEditAction())) {
-          return NS_ERROR_EDITOR_DESTROYED;
-        }
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return rv;
-        }
-        continue;
+      // XXX We also check namespace of the element here, but we don't do
+      //     that in other places.
+      if (nsIContent* nextEditableSibling = GetNextHTMLSibling(curNode)) {
+        if (HTMLEditUtils::IsList(nextEditableSibling) &&
+            atCurNode.GetContainer()->NodeInfo()->NameAtom() ==
+                nextEditableSibling->NodeInfo()->NameAtom() &&
+            atCurNode.GetContainer()->NodeInfo()->NamespaceID() ==
+                nextEditableSibling->NodeInfo()->NamespaceID()) {
+          nsresult rv =
+              MoveNodeWithTransaction(MOZ_KnownLive(*curNode->AsContent()),
+                                      EditorDOMPoint(nextEditableSibling, 0));
+          if (NS_WARN_IF(Destroyed())) {
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
+          if (NS_WARN_IF(NS_FAILED(rv))) {
+            return rv;
+          }
+          continue;
+        }
       }
 
       // Check for whether we should join a list that preceeds curNode.
       // We do this if the previous element is a list, and the list is of
       // the same type (li/ol) as curNode was a part of.
-      sibling = HTMLEditorRef().GetPriorHTMLSibling(curNode);
-      if (sibling && HTMLEditUtils::IsList(sibling) &&
-          atCurNode.GetContainer()->NodeInfo()->NameAtom() ==
-              sibling->NodeInfo()->NameAtom() &&
-          atCurNode.GetContainer()->NodeInfo()->NamespaceID() ==
-              sibling->NodeInfo()->NamespaceID()) {
-        nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                          .MoveNodeToEndWithTransaction(
-                              MOZ_KnownLive(*curNode->AsContent()), *sibling);
-        if (NS_WARN_IF(!CanHandleEditAction())) {
-          return NS_ERROR_EDITOR_DESTROYED;
-        }
-        if (NS_WARN_IF(NS_FAILED(rv))) {
-          return rv;
-        }
-        continue;
+      if (nsCOMPtr<nsIContent> previousEditableSibling =
+              GetPriorHTMLSibling(curNode)) {
+        if (HTMLEditUtils::IsList(previousEditableSibling) &&
+            atCurNode.GetContainer()->NodeInfo()->NameAtom() ==
+                previousEditableSibling->NodeInfo()->NameAtom() &&
+            atCurNode.GetContainer()->NodeInfo()->NamespaceID() ==
+                previousEditableSibling->NodeInfo()->NamespaceID()) {
+          nsresult rv = MoveNodeToEndWithTransaction(
+              MOZ_KnownLive(*curNode->AsContent()), *previousEditableSibling);
+          if (NS_WARN_IF(Destroyed())) {
+            return NS_ERROR_EDITOR_DESTROYED;
+          }
+          if (NS_WARN_IF(NS_FAILED(rv))) {
+            return rv;
+          }
+          continue;
+        }
       }
 
       // check to see if curList is still appropriate.  Which it is if
       // curNode is still right after it in the same list.
-      sibling = nullptr;
-      if (curList) {
-        sibling = HTMLEditorRef().GetPriorHTMLSibling(curNode);
-      }
-
-      if (!curList || (sibling && sibling != curList)) {
+      nsIContent* previousEditableSibling =
+          curList ? GetPriorHTMLSibling(curNode) : nullptr;
+      if (!curList ||
+          (previousEditableSibling && previousEditableSibling != curList)) {
         nsAtom* containerName =
             atCurNode.GetContainer()->NodeInfo()->NameAtom();
         // Create a new nested list of correct type.
         SplitNodeResult splitNodeResult =
-            MOZ_KnownLive(HTMLEditorRef())
-                .MaybeSplitAncestorsForInsertWithTransaction(
-                    MOZ_KnownLive(*containerName), atCurNode);
+            MaybeSplitAncestorsForInsertWithTransaction(
+                MOZ_KnownLive(*containerName), atCurNode);
         if (NS_WARN_IF(splitNodeResult.Failed())) {
           return splitNodeResult.Rv();
         }
-        curList = MOZ_KnownLive(HTMLEditorRef())
-                      .CreateNodeWithTransaction(MOZ_KnownLive(*containerName),
-                                                 splitNodeResult.SplitPoint());
-        if (NS_WARN_IF(!CanHandleEditAction())) {
+        curList = CreateNodeWithTransaction(MOZ_KnownLive(*containerName),
+                                            splitNodeResult.SplitPoint());
+        if (NS_WARN_IF(Destroyed())) {
           return NS_ERROR_EDITOR_DESTROYED;
         }
         if (NS_WARN_IF(!curList)) {
           return NS_ERROR_FAILURE;
         }
         // curList is now the correct thing to put curNode in
         // remember our new block for postprocessing
-        HTMLEditorRef().TopLevelEditSubActionDataRef().mNewBlockElement =
-            curList;
+        TopLevelEditSubActionDataRef().mNewBlockElement = curList;
       }
       // tuck the node into the end of the active list
-      nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                        .MoveNodeToEndWithTransaction(
-                            MOZ_KnownLive(*curNode->AsContent()), *curList);
-      if (NS_WARN_IF(!CanHandleEditAction())) {
+      nsresult rv = MoveNodeToEndWithTransaction(
+          MOZ_KnownLive(*curNode->AsContent()), *curList);
+      if (NS_WARN_IF(Destroyed())) {
         return NS_ERROR_EDITOR_DESTROYED;
       }
       if (NS_WARN_IF(NS_FAILED(rv))) {
         return rv;
       }
       continue;
     }
 
     // Not a list item.
 
     if (HTMLEditor::NodeIsBlockStatic(*curNode)) {
-      nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                        .ChangeMarginStart(MOZ_KnownLive(*curNode->AsElement()),
-                                           HTMLEditor::ChangeMargin::Increase);
+      nsresult rv = ChangeMarginStart(MOZ_KnownLive(*curNode->AsElement()),
+                                      ChangeMargin::Increase);
       if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
         return NS_ERROR_EDITOR_DESTROYED;
       }
-      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to inrease indentation");
+      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+                           "ChangeMarginStart() failed, but ignored");
       curQuote = nullptr;
       continue;
     }
 
     if (!curQuote) {
       // First, check that our element can contain a div.
-      if (!HTMLEditorRef().CanContainTag(*atCurNode.GetContainer(),
-                                         *nsGkAtoms::div)) {
+      if (!CanContainTag(*atCurNode.GetContainer(), *nsGkAtoms::div)) {
         return NS_OK;  // cancelled
       }
 
       SplitNodeResult splitNodeResult =
-          MOZ_KnownLive(HTMLEditorRef())
-              .MaybeSplitAncestorsForInsertWithTransaction(*nsGkAtoms::div,
-                                                           atCurNode);
+          MaybeSplitAncestorsForInsertWithTransaction(*nsGkAtoms::div,
+                                                      atCurNode);
       if (NS_WARN_IF(splitNodeResult.Failed())) {
         return splitNodeResult.Rv();
       }
-      curQuote = MOZ_KnownLive(HTMLEditorRef())
-                     .CreateNodeWithTransaction(*nsGkAtoms::div,
-                                                splitNodeResult.SplitPoint());
-      if (NS_WARN_IF(!CanHandleEditAction())) {
+      curQuote = CreateNodeWithTransaction(*nsGkAtoms::div,
+                                           splitNodeResult.SplitPoint());
+      if (NS_WARN_IF(Destroyed())) {
         return NS_ERROR_EDITOR_DESTROYED;
       }
       if (NS_WARN_IF(!curQuote)) {
         return NS_ERROR_FAILURE;
       }
-      nsresult rv =
-          MOZ_KnownLive(HTMLEditorRef())
-              .ChangeMarginStart(*curQuote, HTMLEditor::ChangeMargin::Increase);
+      nsresult rv = ChangeMarginStart(*curQuote, ChangeMargin::Increase);
       if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
         return NS_ERROR_EDITOR_DESTROYED;
       }
-      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to increase indentation");
+      NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+                           "ChangeMarginStart() failed, but ignored");
       // remember our new block for postprocessing
-      HTMLEditorRef().TopLevelEditSubActionDataRef().mNewBlockElement =
-          curQuote;
+      TopLevelEditSubActionDataRef().mNewBlockElement = curQuote;
       // curQuote is now the correct thing to put curNode in
     }
 
     // tuck the node into the end of the active blockquote
-    nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                      .MoveNodeToEndWithTransaction(
-                          MOZ_KnownLive(*curNode->AsContent()), *curQuote);
-    if (NS_WARN_IF(!CanHandleEditAction())) {
+    nsresult rv = MoveNodeToEndWithTransaction(
+        MOZ_KnownLive(*curNode->AsContent()), *curQuote);
+    if (NS_WARN_IF(Destroyed())) {
       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
@@ -290,25 +290,16 @@ class HTMLEditRules : public TextEditRul
 
   /**
    * Called after handling edit action.  This may adjust Selection, remove
    * unnecessary empty nodes, create <br> elements if needed, etc.
    */
   MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE nsresult AfterEditInner();
 
   /**
-   * IndentAroundSelectionWithCSS() indents around Selection with CSS.
-   * This method creates AutoSelectionRestorer.  Therefore, each caller
-   * need to check if the editor is still available even if this returns
-   * NS_OK.
-   */
-  MOZ_CAN_RUN_SCRIPT
-  MOZ_MUST_USE nsresult IndentAroundSelectionWithCSS();
-
-  /**
    * IndentAroundSelectionWithHTML() indents around Selection with HTML.
    * This method creates AutoSelectionRestorer.  Therefore, each caller
    * need to check if the editor is still available even if this returns
    * NS_OK.
    */
   MOZ_CAN_RUN_SCRIPT
   MOZ_MUST_USE nsresult IndentAroundSelectionWithHTML();
 
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -2327,16 +2327,24 @@ class HTMLEditor final : public TextEdit
    * ChangeMarginStart() changes margin of aElement to indent or outdent.
    * If it's rtl text, margin-right will be changed.  Otherwise, margin-left.
    * XXX This is not aware of vertical writing-mode.
    */
   enum class ChangeMargin { Increase, Decrease };
   MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE nsresult
   ChangeMarginStart(Element& aElement, ChangeMargin aChangeMargin);
 
+  /**
+   * IndentAroundSelectionWithCSS() indents around Selection with CSS.
+   * This method creates AutoSelectionRestorer.  Therefore, each caller
+   * need to check if the editor is still available even if this returns
+   * NS_OK.
+   */
+  MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE nsresult IndentAroundSelectionWithCSS();
+
  protected:  // Called by helper classes.
   virtual void OnStartToHandleTopLevelEditSubAction(
       EditSubAction aEditSubAction, nsIEditor::EDirection aDirection) override;
   MOZ_CAN_RUN_SCRIPT
   virtual void OnEndHandlingTopLevelEditSubAction() override;
 
  protected:  // Shouldn't be used by friend classes
   virtual ~HTMLEditor();