Bug 1574852 - part 59: Move `HTMLEditRules::JoinNearestEditableNodesWithTransaction()` to `HTMLEditor` r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 04 Sep 2019 00:18:18 +0000
changeset 491787 363362abb3e999f0014c587c6cfff309fc8ec7f7
parent 491786 576017f68d3a05950bf79718cbd69e217a295ef3
child 491788 1069ae045160d78eb4d38aeaecfe8e66f6faeed7
push id94512
push usermasayuki@d-toybox.com
push dateThu, 05 Sep 2019 00:23:50 +0000
treeherderautoland@363362abb3e9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1574852
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 59: Move `HTMLEditRules::JoinNearestEditableNodesWithTransaction()` to `HTMLEditor` r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D44198
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
editor/libeditor/HTMLEditor.h
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -2736,18 +2736,21 @@ nsresult HTMLEditRules::WillDeleteSelect
       nsCOMPtr<nsINode> stepbrother;
       if (sibling) {
         stepbrother = HTMLEditorRef().GetNextHTMLSibling(sibling);
       }
       // Are they both text nodes?  If so, join them!
       if (startPoint.GetContainer() == stepbrother &&
           startPoint.GetContainerAsText() && sibling->GetAsText()) {
         EditorDOMPoint pt;
-        nsresult rv = JoinNearestEditableNodesWithTransaction(
-            *sibling, MOZ_KnownLive(*startPoint.GetContainerAsContent()), &pt);
+        nsresult rv =
+            MOZ_KnownLive(HTMLEditorRef())
+                .JoinNearestEditableNodesWithTransaction(
+                    *sibling,
+                    MOZ_KnownLive(*startPoint.GetContainerAsContent()), &pt);
         if (NS_WARN_IF(NS_FAILED(rv))) {
           return rv;
         }
         if (NS_WARN_IF(!pt.IsSet())) {
           return NS_ERROR_FAILURE;
         }
         // Fix up selection
         ErrorResult error;
@@ -3707,18 +3710,19 @@ EditActionResult HTMLEditRules::TryToJoi
   endOfLeftBlock.SetToEndOf(leftBlock);
   RefPtr<Element> invisibleBRElement =
       HTMLEditorRef().GetInvisibleBRElementAt(endOfLeftBlock);
   EditActionResult ret(NS_OK);
   if (mergeLists ||
       leftBlock->NodeInfo()->NameAtom() == rightBlock->NodeInfo()->NameAtom()) {
     // Nodes are same type.  merge them.
     EditorDOMPoint pt;
-    nsresult rv =
-        JoinNearestEditableNodesWithTransaction(*leftBlock, *rightBlock, &pt);
+    nsresult rv = MOZ_KnownLive(HTMLEditorRef())
+                      .JoinNearestEditableNodesWithTransaction(
+                          *leftBlock, *rightBlock, &pt);
     if (NS_WARN_IF(rv == NS_ERROR_EDITOR_DESTROYED)) {
       return EditActionIgnored(NS_ERROR_EDITOR_DESTROYED);
     }
     if (pt.IsSet() && mergeLists) {
       CreateElementResult convertListTypeResult =
           MOZ_KnownLive(HTMLEditorRef())
               .ChangeListElementType(*rightBlock, MOZ_KnownLive(*existingList),
                                      *nsGkAtoms::li);
@@ -9144,97 +9148,95 @@ SplitNodeResult HTMLEditor::MaybeSplitAn
   if (NS_WARN_IF(Destroyed())) {
     return SplitNodeResult(NS_ERROR_EDITOR_DESTROYED);
   }
   NS_WARNING_ASSERTION(splitNodeResult.Succeeded(),
                        "Failed to split the node for insert the element");
   return splitNodeResult;
 }
 
-nsresult HTMLEditRules::JoinNearestEditableNodesWithTransaction(
+nsresult HTMLEditor::JoinNearestEditableNodesWithTransaction(
     nsIContent& aNodeLeft, nsIContent& aNodeRight,
     EditorDOMPoint* aNewFirstChildOfRightNode) {
-  MOZ_ASSERT(IsEditorDataAvailable());
+  MOZ_ASSERT(IsEditActionDataAvailable());
   MOZ_ASSERT(aNewFirstChildOfRightNode);
 
   // Caller responsible for left and right node being the same type
-  nsCOMPtr<nsINode> parent = aNodeLeft.GetParentNode();
-  if (NS_WARN_IF(!parent)) {
+  if (NS_WARN_IF(!aNodeLeft.GetParentNode())) {
     return NS_ERROR_FAILURE;
   }
-  nsCOMPtr<nsINode> rightParent = aNodeRight.GetParentNode();
-
   // If they don't have the same parent, first move the right node to after
   // the left one
-  if (parent != rightParent) {
-    int32_t parOffset = parent->ComputeIndexOf(&aNodeLeft);
-    nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                      .MoveNodeWithTransaction(
-                          aNodeRight, EditorDOMPoint(parent, parOffset));
-    if (NS_WARN_IF(!CanHandleEditAction())) {
+  if (aNodeLeft.GetParentNode() != aNodeRight.GetParentNode()) {
+    nsresult rv =
+        MoveNodeWithTransaction(aNodeRight, EditorDOMPoint(&aNodeLeft));
+    if (NS_WARN_IF(Destroyed())) {
       return NS_ERROR_EDITOR_DESTROYED;
     }
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return rv;
     }
   }
 
   EditorDOMPoint ret(&aNodeRight, aNodeLeft.Length());
 
   // Separate join rules for differing blocks
-  if (HTMLEditUtils::IsList(&aNodeLeft) || aNodeLeft.GetAsText()) {
+  if (HTMLEditUtils::IsList(&aNodeLeft) || aNodeLeft.IsText()) {
     // For lists, merge shallow (wouldn't want to combine list items)
-    nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                      .JoinNodesWithTransaction(aNodeLeft, aNodeRight);
-    if (NS_WARN_IF(!CanHandleEditAction())) {
+    nsresult rv = JoinNodesWithTransaction(aNodeLeft, aNodeRight);
+    if (NS_WARN_IF(Destroyed())) {
       return NS_ERROR_EDITOR_DESTROYED;
     }
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-    *aNewFirstChildOfRightNode = std::move(ret);
-    return NS_OK;
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "JoinNodesWithTransaction failed");
+    if (NS_SUCCEEDED(rv)) {
+      // XXX `ret` may point invalid point if mutation event listener changed
+      //     the previous siblings...
+      *aNewFirstChildOfRightNode = std::move(ret);
+    }
+    return rv;
   }
 
   // Remember the last left child, and first right child
-  nsCOMPtr<nsIContent> lastLeft =
-      HTMLEditorRef().GetLastEditableChild(aNodeLeft);
+  nsCOMPtr<nsIContent> lastLeft = GetLastEditableChild(aNodeLeft);
   if (NS_WARN_IF(!lastLeft)) {
     return NS_ERROR_FAILURE;
   }
 
-  nsCOMPtr<nsIContent> firstRight =
-      HTMLEditorRef().GetFirstEditableChild(aNodeRight);
+  nsCOMPtr<nsIContent> firstRight = GetFirstEditableChild(aNodeRight);
   if (NS_WARN_IF(!firstRight)) {
     return NS_ERROR_FAILURE;
   }
 
   // For list items, divs, etc., merge smart
-  nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                    .JoinNodesWithTransaction(aNodeLeft, aNodeRight);
-  if (NS_WARN_IF(!CanHandleEditAction())) {
+  nsresult rv = JoinNodesWithTransaction(aNodeLeft, aNodeRight);
+  if (NS_WARN_IF(Destroyed())) {
     return NS_ERROR_EDITOR_DESTROYED;
   }
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return rv;
   }
 
-  if (lastLeft && firstRight &&
-      HTMLEditorRef().AreNodesSameType(*lastLeft, *firstRight) &&
-      (lastLeft->GetAsText() ||
+  // XXX This condition is odd.  Despite of the name, `AreNodesSameType()`
+  //     compairs its style with `CSSEditUtils::ElementsSameStyle()` only
+  //     when `IsCSSEnabled()` returns true and `lastLeft` is a `<span>`
+  //     element.  However, calling `CSSEditUtils::ElementsSameStyle()`
+  //     without those checking again.
+  if (HTMLEditor::AreNodesSameType(*lastLeft, *firstRight) &&
+      (lastLeft->IsText() ||
        (lastLeft->IsElement() && firstRight->IsElement() &&
         CSSEditUtils::ElementsSameStyle(lastLeft->AsElement(),
                                         firstRight->AsElement())))) {
     nsresult rv = JoinNearestEditableNodesWithTransaction(
         *lastLeft, *firstRight, aNewFirstChildOfRightNode);
-    if (NS_WARN_IF(NS_FAILED(rv))) {
-      return rv;
-    }
-    return NS_OK;
-  }
+    NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
+                         "JoinNearestEditableNodesWithTransaction() failed");
+    return rv;
+  }
+  // XXX `ret` may point invalid point if mutation event listener changed
+  //     the previous siblings...
   *aNewFirstChildOfRightNode = std::move(ret);
   return NS_OK;
 }
 
 Element* HTMLEditor::GetMostAncestorMailCiteElement(nsINode& aNode) const {
   Element* mailCiteElement = nullptr;
   bool isPlaintextEditor = IsPlaintextEditor();
   for (nsINode* node = &aNode; node; node = node->GetParentNode()) {
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -520,42 +520,16 @@ class HTMLEditRules : public TextEditRul
    * MakeTransitionList() detects all the transitions in the array, where a
    * transition means that adjacent nodes in the array don't have the same
    * parent.
    */
   void MakeTransitionList(nsTArray<OwningNonNull<nsINode>>& aNodeArray,
                           nsTArray<bool>& aTransitionArray);
 
   /**
-   * JoinNearestEditableNodesWithTransaction() joins two editable nodes which
-   * are themselves or the nearest editable node of aLeftNode and aRightNode.
-   * XXX This method's behavior is odd.  For example, if user types Backspace
-   *     key at the second editable paragraph in this case:
-   *     <div contenteditable>
-   *       <p>first editable paragraph</p>
-   *       <p contenteditable="false">non-editable paragraph</p>
-   *       <p>second editable paragraph</p>
-   *     </div>
-   *     The first editable paragraph's content will be moved into the second
-   *     editable paragraph and the non-editable paragraph becomes the first
-   *     paragraph of the editor.  I don't think that it's expected behavior of
-   *     any users...
-   *
-   * @param aLeftNode   The node which will be removed.
-   * @param aRightNode  The node which will be inserted the content of
-   *                    aLeftNode.
-   * @param aNewFirstChildOfRightNode
-   *                    The point at the first child of aRightNode.
-   */
-  MOZ_CAN_RUN_SCRIPT
-  MOZ_MUST_USE nsresult JoinNearestEditableNodesWithTransaction(
-      nsIContent& aLeftNode, nsIContent& aRightNode,
-      EditorDOMPoint* aNewFirstChildOfRightNode);
-
-  /**
    * PopListItem() tries to move aListItem outside its parent.  If it's
    * in a middle of a list element, the parent list element is split before
    * aListItem.  Then, moves aListItem to before its parent list element.
    * I.e., moves aListItem between the 2 list elements if original parent
    * was split.  Then, if new parent is not a list element, the list item
    * element is removed and its contents are moved to where the list item
    * element was.
    *
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -1957,16 +1957,42 @@ class HTMLEditor final : public TextEdit
 
   /**
    * If aPoint follows invisible `<br>` element, returns the invisible `<br>`
    * element.  Otherwise, nullptr.
    */
   template <typename PT, typename CT>
   Element* GetInvisibleBRElementAt(const EditorDOMPointBase<PT, CT>& aPoint);
 
+  /**
+   * JoinNearestEditableNodesWithTransaction() joins two editable nodes which
+   * are themselves or the nearest editable node of aLeftNode and aRightNode.
+   * XXX This method's behavior is odd.  For example, if user types Backspace
+   *     key at the second editable paragraph in this case:
+   *     <div contenteditable>
+   *       <p>first editable paragraph</p>
+   *       <p contenteditable="false">non-editable paragraph</p>
+   *       <p>second editable paragraph</p>
+   *     </div>
+   *     The first editable paragraph's content will be moved into the second
+   *     editable paragraph and the non-editable paragraph becomes the first
+   *     paragraph of the editor.  I don't think that it's expected behavior of
+   *     any users...
+   *
+   * @param aLeftNode   The node which will be removed.
+   * @param aRightNode  The node which will be inserted the content of
+   *                    aLeftNode.
+   * @param aNewFirstChildOfRightNode
+   *                    [out] The point at the first child of aRightNode.
+   */
+  MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE nsresult
+  JoinNearestEditableNodesWithTransaction(
+      nsIContent& aLeftNode, nsIContent& aRightNode,
+      EditorDOMPoint* aNewFirstChildOfRightNode);
+
  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();