Bug 1574852 - part 61: Move `HTMLEditRules::MoveBlock()` to `HTMLEditor` r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 04 Sep 2019 05:00:11 +0000
changeset 491798 cd2b83d5dda739873625d7d3345a3a163443ae1f
parent 491797 5a173762d443586a76a2b6495b4655a84691db6d
child 491799 d1181cbf7840362b625c2f26203a1e52d761f41b
push id36537
push userccoroiu@mozilla.com
push dateThu, 05 Sep 2019 16:14:31 +0000
treeherdermozilla-central@3d7b9bf00bdb [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 61: Move `HTMLEditRules::MoveBlock()` to `HTMLEditor` r=m_kato Differential Revision: https://phabricator.services.mozilla.com/D44200
editor/libeditor/EditorUtils.h
editor/libeditor/HTMLEditRules.cpp
editor/libeditor/HTMLEditRules.h
editor/libeditor/HTMLEditor.h
--- a/editor/libeditor/EditorUtils.h
+++ b/editor/libeditor/EditorUtils.h
@@ -190,16 +190,18 @@ class MOZ_STACK_CLASS MoveNodeResult fin
   bool Ignored() const { return !mHandled; }
   nsresult Rv() const { return mRv; }
   bool EditorDestroyed() const { return mRv == NS_ERROR_EDITOR_DESTROYED; }
   const EditorDOMPoint& NextInsertionPointRef() const {
     return mNextInsertionPoint;
   }
   EditorDOMPoint NextInsertionPoint() const { return mNextInsertionPoint; }
 
+  void MarkAsHandled() { mHandled = true; }
+
   MoveNodeResult() : mRv(NS_ERROR_NOT_INITIALIZED), mHandled(false) {}
 
   explicit MoveNodeResult(nsresult aRv) : mRv(aRv), mHandled(false) {
     MOZ_DIAGNOSTIC_ASSERT(NS_FAILED(mRv));
   }
 
   MoveNodeResult(const MoveNodeResult& aOther) = delete;
   MoveNodeResult& operator=(const MoveNodeResult& aOther) = delete;
--- a/editor/libeditor/HTMLEditRules.cpp
+++ b/editor/libeditor/HTMLEditRules.cpp
@@ -3509,27 +3509,32 @@ EditActionResult HTMLEditRules::TryToJoi
       // remove this comment if we don't need to do it.  Otherwise, please
       // move children of the right list node to the end of the left list node.
       MOZ_DIAGNOSTIC_ASSERT(!atChildInBlock.IsSet());
 
       // XXX Although, we don't do nothing here, but for keeping traditional
       //     behavior, we should mark as handled.
       ret.MarkAsHandled();
     } else {
-      // XXX Why do we ignore the result of MoveBlock()?
-      EditActionResult retMoveBlock =
-          MoveBlock(*leftBlock, *rightBlock, -1, atRightBlockChild.Offset());
-      if (NS_WARN_IF(retMoveBlock.Rv() == NS_ERROR_EDITOR_DESTROYED)) {
-        return ret;
-      }
-      NS_WARNING_ASSERTION(
-          retMoveBlock.Succeeded(),
-          "Failed to move contents of the right block to the left block");
-      if (retMoveBlock.Handled()) {
-        ret.MarkAsHandled();
+      // XXX Why do we ignore the result of MoveOneHardLineContents()?
+      NS_WARNING_ASSERTION(rightBlock == atRightBlockChild.GetContainer(),
+                           "The relation is not guaranteed but assumed");
+      MoveNodeResult moveNodeResult =
+          MOZ_KnownLive(HTMLEditorRef())
+              .MoveOneHardLineContents(
+                  EditorDOMPoint(rightBlock, atRightBlockChild.Offset()),
+                  EditorDOMPoint(leftBlock, 0),
+                  HTMLEditor::MoveToEndOfContainer::Yes);
+      if (NS_WARN_IF(moveNodeResult.EditorDestroyed())) {
+        return ret.SetResult(NS_ERROR_EDITOR_DESTROYED);
+      }
+      NS_WARNING_ASSERTION(moveNodeResult.Succeeded(),
+                           "MoveOneHardLineContents() failed, but ignored");
+      if (moveNodeResult.Succeeded()) {
+        ret |= moveNodeResult;
       }
       // Now, all children of rightBlock were moved to leftBlock.  So,
       // atRightBlockChild is now invalid.
       atRightBlockChild.Clear();
     }
     if (invisibleBRElement) {
       nsresult rv = MOZ_KnownLive(HTMLEditorRef())
                         .DeleteNodeWithTransaction(*invisibleBRElement);
@@ -3667,18 +3672,19 @@ EditActionResult HTMLEditRules::TryToJoi
           previousContent.Set(previousContentParent, previousContentOffset);
         }
       }
 
       if (NS_WARN_IF(!previousContent.IsSet())) {
         return EditActionIgnored(NS_ERROR_NULL_POINTER);
       }
 
-      ret |= MoveBlock(MOZ_KnownLive(*previousContent.GetContainerAsElement()),
-                       *rightBlock, previousContent.Offset(), 0);
+      ret |= MOZ_KnownLive(HTMLEditorRef())
+                 .MoveOneHardLineContents(EditorDOMPoint(rightBlock, 0),
+                                          previousContent);
       if (NS_WARN_IF(ret.Failed())) {
         return ret;
       }
     }
     if (invisibleBRElement) {
       nsresult rv = MOZ_KnownLive(HTMLEditorRef())
                         .DeleteNodeWithTransaction(*invisibleBRElement);
       if (NS_WARN_IF(!CanHandleEditAction())) {
@@ -3733,17 +3739,20 @@ EditActionResult HTMLEditRules::TryToJoi
                                      *nsGkAtoms::li);
       if (NS_WARN_IF(convertListTypeResult.Rv() == NS_ERROR_EDITOR_DESTROYED)) {
         return EditActionIgnored(NS_ERROR_EDITOR_DESTROYED);
       }
     }
     ret.MarkAsHandled();
   } else {
     // Nodes are dissimilar types.
-    ret |= MoveBlock(*leftBlock, *rightBlock, -1, 0);
+    ret |= MOZ_KnownLive(HTMLEditorRef())
+               .MoveOneHardLineContents(EditorDOMPoint(rightBlock, 0),
+                                        EditorDOMPoint(leftBlock, 0),
+                                        HTMLEditor::MoveToEndOfContainer::Yes);
     if (NS_WARN_IF(ret.Failed())) {
       return ret;
     }
   }
   if (invisibleBRElement) {
     rv = MOZ_KnownLive(HTMLEditorRef())
              .DeleteNodeWithTransaction(*invisibleBRElement);
     if (NS_WARN_IF(!CanHandleEditAction())) {
@@ -3755,91 +3764,84 @@ EditActionResult HTMLEditRules::TryToJoi
     if (NS_WARN_IF(NS_FAILED(rv))) {
       return ret.SetResult(rv);
     }
     ret.MarkAsHandled();
   }
   return ret;
 }
 
-EditActionResult HTMLEditRules::MoveBlock(Element& aLeftBlock,
-                                          Element& aRightBlock,
-                                          int32_t aLeftOffset,
-                                          int32_t aRightOffset) {
-  MOZ_ASSERT(IsEditorDataAvailable());
+MoveNodeResult HTMLEditor::MoveOneHardLineContents(
+    const EditorDOMPoint& aPointInHardLine,
+    const EditorDOMPoint& aPointToInsert,
+    MoveToEndOfContainer
+        aMoveToEndOfContainer /* = MoveToEndOfContainer::No */) {
+  MOZ_ASSERT(IsEditActionDataAvailable());
 
   AutoTArray<OwningNonNull<nsINode>, 64> arrayOfNodes;
-  nsresult rv = MOZ_KnownLive(HTMLEditorRef())
-                    .SplitInlinesAndCollectEditTargetNodesInOneHardLine(
-                        EditorDOMPoint(&aRightBlock, aRightOffset),
-                        arrayOfNodes, EditSubAction::eMergeBlockContents,
-                        HTMLEditor::CollectNonEditableNodes::Yes);
+  nsresult rv = SplitInlinesAndCollectEditTargetNodesInOneHardLine(
+      aPointInHardLine, arrayOfNodes, EditSubAction::eMergeBlockContents,
+      HTMLEditor::CollectNonEditableNodes::Yes);
   if (NS_WARN_IF(NS_FAILED(rv))) {
-    return EditActionIgnored(rv);
-  }
-
-  uint32_t offset = static_cast<uint32_t>(aLeftOffset);
-  EditActionResult ret(NS_OK);
+    return MoveNodeResult(rv);
+  }
+  if (arrayOfNodes.IsEmpty()) {
+    return MoveNodeIgnored(aPointToInsert);
+  }
+
+  uint32_t offset = aPointToInsert.Offset();
+  MoveNodeResult result;
   for (auto& node : arrayOfNodes) {
-    if (aLeftOffset == -1) {
+    if (aMoveToEndOfContainer == MoveToEndOfContainer::Yes) {
       // For backward compatibility, we should move contents to end of the
-      // container if this is called with -1 for aLeftOffset.
-      offset = aLeftBlock.Length();
+      // container if this is called with MoveToEndOfContainer::Yes.
+      offset = aPointToInsert.GetContainer()->Length();
     }
     // get the node to act on
     if (HTMLEditor::NodeIsBlockStatic(node)) {
       // For block nodes, move their contents only, then delete block.
-      MoveNodeResult moveNodeResult =
-          MOZ_KnownLive(HTMLEditorRef())
-              .MoveChildren(MOZ_KnownLive(*node->AsElement()),
-                            EditorDOMPoint(&aLeftBlock, offset));
-      if (NS_WARN_IF(moveNodeResult.Failed())) {
-        return ret.SetResult(moveNodeResult.Rv());
-      }
-      offset = moveNodeResult.NextInsertionPointRef().Offset();
-      ret |= moveNodeResult;
-
-      DebugOnly<nsresult> rvIgnored =
-          MOZ_KnownLive(HTMLEditorRef()).DeleteNodeWithTransaction(*node);
-      if (NS_WARN_IF(!CanHandleEditAction())) {
-        return ret.SetResult(NS_ERROR_EDITOR_DESTROYED);
+      result |=
+          MoveChildren(MOZ_KnownLive(*node->AsElement()),
+                       EditorDOMPoint(aPointToInsert.GetContainer(), offset));
+      if (NS_WARN_IF(result.Failed())) {
+        return result;
+      }
+      offset = result.NextInsertionPointRef().Offset();
+      DebugOnly<nsresult> rvIgnored = DeleteNodeWithTransaction(*node);
+      if (NS_WARN_IF(Destroyed())) {
+        return MoveNodeResult(NS_ERROR_EDITOR_DESTROYED);
       }
       NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
                            "DeleteNodeWithTransaction() failed, but ignored");
-      ret.MarkAsHandled();
-      if (HTMLEditorRef().HasMutationEventListeners()) {
+      result.MarkAsHandled();
+      if (HasMutationEventListeners()) {
         // Mutation event listener may make `offset` value invalid with
         // removing some previous children while we call
         // `DeleteNodeWithTransaction()` so that we should adjust it here.
-        offset = std::min(offset, aLeftBlock.Length());
-      }
-    } else {
-      // Otherwise move the content as is, checking against the DTD.
-      MoveNodeResult moveNodeResult =
-          MOZ_KnownLive(HTMLEditorRef())
-              .MoveNodeOrChildren(MOZ_KnownLive(*node->AsContent()),
-                                  EditorDOMPoint(&aLeftBlock, offset));
-      if (NS_WARN_IF(moveNodeResult.EditorDestroyed())) {
-        return ret.SetResult(NS_ERROR_EDITOR_DESTROYED);
-      }
-      NS_WARNING_ASSERTION(moveNodeResult.Succeeded(),
-                           "MoveNodeOrChildren() failed, but ignored");
-      if (moveNodeResult.Succeeded()) {
-        offset = moveNodeResult.NextInsertionPointRef().Offset();
-        ret |= moveNodeResult;
-      }
-    }
-  }
-
-  // XXX We're only checking return value of the last iteration
-  if (NS_WARN_IF(ret.Failed())) {
-    return ret;
-  }
-
-  return ret;
+        offset = std::min(offset, aPointToInsert.GetContainer()->Length());
+      }
+      continue;
+    }
+    // XXX Different from the above block, we ignore error of moving nodes.
+    MoveNodeResult moveNodeResult = MoveNodeOrChildren(
+        MOZ_KnownLive(*node->AsContent()),
+        EditorDOMPoint(aPointToInsert.GetContainer(), offset));
+    if (NS_WARN_IF(moveNodeResult.EditorDestroyed())) {
+      return MoveNodeResult(NS_ERROR_EDITOR_DESTROYED);
+    }
+    NS_WARNING_ASSERTION(moveNodeResult.Succeeded(),
+                         "MoveNodeOrChildren() failed, but ignored");
+    if (moveNodeResult.Succeeded()) {
+      offset = moveNodeResult.NextInsertionPointRef().Offset();
+      result |= moveNodeResult;
+    }
+  }
+
+  NS_WARNING_ASSERTION(result.Succeeded(), "Last MoveNodeOrChildren() failed");
+  return result;
 }
 
 MoveNodeResult HTMLEditor::MoveNodeOrChildren(
     nsIContent& aContent, const EditorDOMPoint& aPointToInsert) {
   MOZ_ASSERT(IsEditActionDataAvailable());
   MOZ_ASSERT(aPointToInsert.IsSet());
 
   // Check if this node can go into the destination node
--- a/editor/libeditor/HTMLEditRules.h
+++ b/editor/libeditor/HTMLEditRules.h
@@ -170,31 +170,16 @@ class HTMLEditRules : public TextEditRul
    *                    be joined or it's impossible to join them but it's not
    *                    unexpected case, this returns true with this.
    */
   MOZ_CAN_RUN_SCRIPT
   MOZ_MUST_USE EditActionResult
   TryToJoinBlocksWithTransaction(nsIContent& aLeftNode, nsIContent& aRightNode);
 
   /**
-   * MoveBlock() moves the content from aRightBlock starting from aRightOffset
-   * into aLeftBlock at aLeftOffset. Note that the "block" can be inline nodes
-   * between <br>s, or between blocks, etc.  DTD containment rules are followed
-   * throughout.
-   *
-   * @return            Sets handled to true if this actually joins the nodes.
-   *                    canceled is always false.
-   */
-  MOZ_CAN_RUN_SCRIPT
-  MOZ_MUST_USE EditActionResult MoveBlock(Element& aLeftBlock,
-                                          Element& aRightBlock,
-                                          int32_t aLeftOffset,
-                                          int32_t aRightOffset);
-
-  /**
    * DeleteElementsExceptTableRelatedElements() removes elements except
    * table related elements (except <table> itself) and their contents
    * from the DOM tree.
    *
    * @param aNode               If this is not a table related element, this
    *                            node will be removed from the DOM tree.
    *                            Otherwise, this method calls itself recursively
    *                            with its children.
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -2009,16 +2009,40 @@ class HTMLEditor final : public TextEdit
    * @param aElement            Container element whose children should be
    *                            moved.
    * @param aPointToInsert      The point to be inserted children of aElement
    *                            or its descendants.
    */
   MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE MoveNodeResult
   MoveChildren(Element& aElement, const EditorDOMPoint& aPointToInsert);
 
+  /**
+   * MoveOneHardLineContents() moves the content in a hard line which contains
+   * aPointInHardLine to aPointToInsert or end of aPointToInsert's container.
+   *
+   * @param aPointInHardLine            A point in a hard line.  All nodes in
+   *                                    same hard line will be moved.
+   * @param aPointToInsert              Point to insert contents of the hard
+   *                                    line.
+   * @param aMoveToEndOfContainer       If `Yes`, aPointToInsert.Offset() will
+   *                                    be ignored and instead, all contents
+   *                                    will be appended to the container of
+   *                                    aPointToInsert.  The result may be
+   *                                    different from setting this to `No`
+   *                                    and aPointToInsert points end of the
+   *                                    container because mutation event
+   *                                    listeners may modify children of the
+   *                                    container while we're moving nodes.
+   */
+  enum class MoveToEndOfContainer { Yes, No };
+  MOZ_CAN_RUN_SCRIPT MOZ_MUST_USE MoveNodeResult MoveOneHardLineContents(
+      const EditorDOMPoint& aPointInHardLine,
+      const EditorDOMPoint& aPointToInsert,
+      MoveToEndOfContainer aMoveToEndOfContainer = MoveToEndOfContainer::No);
+
  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();