Bug 1792387 - part 3-1: Make `HTMLEditor::DoSplitNode()` adjust selection for both split node directions r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 30 Sep 2022 22:20:20 +0000
changeset 636755 de9f2339aeb290e200ac33e737ae829de375d3c1
parent 636754 0df250e3eccca3fb6addf212ba186319f78b0e3a
child 636756 953647a15a7dedd3558415f3a24cf782888e80c4
push id170484
push usermasayuki@d-toybox.com
push dateFri, 30 Sep 2022 22:23:25 +0000
treeherderautoland@f4c48015c119 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1792387
milestone107.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 1792387 - part 3-1: Make `HTMLEditor::DoSplitNode()` adjust selection for both split node directions r=m_kato This patch adds new path to adjust selection for the new split node direction. So this does not change any behavior of the existing path. Differential Revision: https://phabricator.services.mozilla.com/D158100
editor/libeditor/HTMLEditor.cpp
editor/libeditor/HTMLEditor.h
editor/libeditor/JoinNodesTransaction.cpp
editor/libeditor/SelectionState.cpp
editor/libeditor/SplitNodeTransaction.cpp
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -4848,17 +4848,18 @@ SplitNodeResult HTMLEditor::SplitNodeDee
       atStartOfRightNode.Set(splittingContent);
     }
   }
 
   // Not reached because while (true) loop never breaks.
 }
 
 SplitNodeResult HTMLEditor::DoSplitNode(const EditorDOMPoint& aStartOfRightNode,
-                                        nsIContent& aNewNode) {
+                                        nsIContent& aNewNode,
+                                        SplitNodeDirection aDirection) {
   // Ensure computing the offset if it's intialized with a child content node.
   Unused << aStartOfRightNode.Offset();
 
   // XXX Perhaps, aStartOfRightNode may be invalid if this is a redo
   //     operation after modifying DOM node with JS.
   if (MOZ_UNLIKELY(NS_WARN_IF(!aStartOfRightNode.IsInContentNode()))) {
     return SplitNodeResult(NS_ERROR_INVALID_ARG);
   }
@@ -4994,40 +4995,55 @@ SplitNodeResult HTMLEditor::DoSplitNode(
     //     AllowsTransactionsToChangeSelection() will return true.
     if (allowedTransactionsToChangeSelection &&
         savedRange.mSelection->Type() == SelectionType::eNormal) {
       // If the editor should adjust the selection, don't bother restoring
       // the ranges for the normal selection here.
       continue;
     }
 
-    // Split the selection into existing node and new node.
-    if (savedRange.mStartContainer == aStartOfRightNode.GetContainer()) {
-      if (savedRange.mStartOffset < aStartOfRightNode.Offset()) {
-        savedRange.mStartContainer = &aNewNode;
-      } else if (savedRange.mStartOffset >= aStartOfRightNode.Offset()) {
-        savedRange.mStartOffset -= aStartOfRightNode.Offset();
-      } else {
-        NS_WARNING(
-            "The stored start offset was smaller than the right node offset");
-        savedRange.mStartOffset = 0u;
+    auto AdjustDOMPoint = [&](nsCOMPtr<nsINode>& aContainer,
+                              uint32_t& aOffset) {
+      if (aContainer != aStartOfRightNode.GetContainer()) {
+        return;
       }
-    }
-
-    if (savedRange.mEndContainer == aStartOfRightNode.GetContainer()) {
-      if (savedRange.mEndOffset < aStartOfRightNode.Offset()) {
-        savedRange.mEndContainer = &aNewNode;
-      } else if (savedRange.mEndOffset >= aStartOfRightNode.Offset()) {
-        savedRange.mEndOffset -= aStartOfRightNode.Offset();
-      } else {
-        NS_WARNING(
-            "The stored end offset was smaller than the right node offset");
-        savedRange.mEndOffset = 0u;
+
+      if (aDirection == SplitNodeDirection::LeftNodeIsNewOne) {
+        // If the container is the right node and offset is before the split
+        // point, the content was moved into aNewNode.  So, just changing the
+        // container will point proper position.
+        if (aOffset < aStartOfRightNode.Offset()) {
+          aContainer = &aNewNode;
+          return;
+        }
+
+        // If the container is the right node and offset equals or is larger
+        // than the split point, we need to decrease the offset since some
+        // content before the split point was moved to aNewNode.
+        if (aOffset >= aStartOfRightNode.Offset()) {
+          aOffset -= aStartOfRightNode.Offset();
+          return;
+        }
+
+        NS_WARNING("The stored offset was smaller than the right node offset");
+        aOffset = 0u;
+        return;
       }
-    }
+
+      // If the container is the left node and offset is after the split
+      // point, the content was moved from the right node to aNewNode.
+      // So, we need to change the container to aNewNode and decrease the
+      // offset.
+      if (aOffset >= aStartOfRightNode.Offset()) {
+        aContainer = &aNewNode;
+        aOffset -= aStartOfRightNode.Offset();
+      }
+    };
+    AdjustDOMPoint(savedRange.mStartContainer, savedRange.mStartOffset);
+    AdjustDOMPoint(savedRange.mEndContainer, savedRange.mEndOffset);
 
     RefPtr<nsRange> newRange =
         nsRange::Create(savedRange.mStartContainer, savedRange.mStartOffset,
                         savedRange.mEndContainer, savedRange.mEndOffset, error);
     if (MOZ_UNLIKELY(error.Failed())) {
       NS_WARNING("nsRange::Create() failed");
       return SplitNodeResult(error.StealNSResult());
     }
@@ -5061,22 +5077,22 @@ SplitNodeResult HTMLEditor::DoSplitNode(
           NS_WARN_IF(parent != aNewNode.GetParentNode()) ||
           NS_WARN_IF(aNewNode.GetNextSibling() !=
                      aStartOfRightNode.GetContainer()))) {
     return SplitNodeResult(NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE);
   }
 
   DebugOnly<nsresult> rvIgnored = RangeUpdaterRef().SelAdjSplitNode(
       *aStartOfRightNode.ContainerAs<nsIContent>(), aStartOfRightNode.Offset(),
-      aNewNode, GetSplitNodeDirection());
+      aNewNode, aDirection);
   NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
                        "RangeUpdater::SelAdjSplitNode() failed, but ignored");
 
   return SplitNodeResult(aNewNode, *aStartOfRightNode.ContainerAs<nsIContent>(),
-                         GetSplitNodeDirection());
+                         aDirection);
 }
 
 JoinNodesResult HTMLEditor::JoinNodesWithTransaction(
     nsIContent& aLeftContent, nsIContent& aRightContent) {
   MOZ_ASSERT(IsEditActionDataAvailable());
   MOZ_ASSERT(&aLeftContent != &aRightContent);
   MOZ_ASSERT(aLeftContent.GetParentNode());
   MOZ_ASSERT(aRightContent.GetParentNode());
--- a/editor/libeditor/HTMLEditor.h
+++ b/editor/libeditor/HTMLEditor.h
@@ -973,30 +973,30 @@ class HTMLEditor final : public EditorBa
   /**
    * OnModifyDocument() is called when the editor is changed.  This should
    * be called only by runnable in HTMLEditor::OnDocumentModified() to call
    * HTMLEditor::OnModifyDocument() with AutoEditActionDataSetter instance.
    */
   [[nodiscard]] MOZ_CAN_RUN_SCRIPT nsresult OnModifyDocument();
 
   /**
-   * DoSplitNode() inserts aNewNode (left node before the container of
-   * aStartOfRightNode (right node), and moves all content before
+   * DoSplitNode() inserts aNewNode and moves all content before or after
    * aStartOfRightNode to aNewNode.
    *
-   * @param aStartOfRightNode   The point to split.  Its container will be
-   *                            the right node, i.e., becomes aNewNode's
-   *                            next sibling.  And the point will be start
-   *                            of the right node.
-   * @param aNewNode            The new node called as left node, so, this
-   *                            becomes the container of all previous content
-   *                            before aPointToSplit.
+   * @param aStartOfRightNode   The point to split.  The container will keep
+   *                            having following or previous content of this.
+   * @param aNewNode            The new node called.  The previous or following
+   *                            content of aStartOfRightNode will be moved into
+   *                            this node.
+   * @param aDirection          Whether aNewNode will have previous or following
+   *                            content of aStartOfRightNode.
    */
   MOZ_CAN_RUN_SCRIPT SplitNodeResult
-  DoSplitNode(const EditorDOMPoint& aStartOfRightNode, nsIContent& aNewNode);
+  DoSplitNode(const EditorDOMPoint& aStartOfRightNode, nsIContent& aNewNode,
+              SplitNodeDirection aDirection);
 
   /**
    * DoJoinNodes() merges contents in aContentToRemove to aContentToKeep and
    * remove aContentToRemove from the DOM tree.  aContentToRemove and
    * aContentToKeep must have same parent.  Additionally, if one of
    * aContentToRemove or aContentToKeep is a text node, the other must be a
    * text node.
    *
--- a/editor/libeditor/JoinNodesTransaction.cpp
+++ b/editor/libeditor/JoinNodesTransaction.cpp
@@ -172,17 +172,17 @@ NS_IMETHODIMP JoinNodesTransaction::Undo
   }
 
   const OwningNonNull<HTMLEditor> htmlEditor = *mHTMLEditor;
   const OwningNonNull<nsIContent> removedContent = *mRemovedContent;
 
   const SplitNodeResult splitNodeResult = htmlEditor->DoSplitNode(
       EditorDOMPoint(mKeepingContent,
                      std::min(mJoinedOffset, mKeepingContent->Length())),
-      removedContent);
+      removedContent, GetSplitNodeDirection());
   NS_WARNING_ASSERTION(splitNodeResult.isOk(),
                        "HTMLEditor::DoSplitNode() failed");
   // When adding caret suggestion to SplitNodeResult, here didn't change
   // selection so that just ignore it.
   splitNodeResult.IgnoreCaretPointSuggestion();
   return splitNodeResult.unwrapErr();
 }
 
--- a/editor/libeditor/SelectionState.cpp
+++ b/editor/libeditor/SelectionState.cpp
@@ -329,17 +329,17 @@ nsresult RangeUpdater::SelAdjSplitNode(n
     if (aSplitNodeDirection == SplitNodeDirection::LeftNodeIsNewOne) {
       if (aOffset > aSplitOffset) {
         aOffset -= aSplitOffset;
       } else {
         aContainer = &aNewContent;
       }
     } else if (aOffset >= aSplitOffset) {
       aContainer = &aNewContent;
-      aOffset = aSplitOffset - aOffset;
+      aOffset -= aSplitOffset;
     }
   };
 
   for (RefPtr<RangeItem>& rangeItem : mArray) {
     if (NS_WARN_IF(!rangeItem)) {
       return NS_ERROR_FAILURE;
     }
     AdjustDOMPoint(rangeItem->mStartContainer, rangeItem->mStartOffset);
--- a/editor/libeditor/SplitNodeTransaction.cpp
+++ b/editor/libeditor/SplitNodeTransaction.cpp
@@ -149,17 +149,17 @@ SplitNodeResult SplitNodeTransaction::Do
     }
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                          "EditorBase::MarkElementDirty() failed, but ignored");
   }
 
   SplitNodeResult splitNodeResult = aHTMLEditor.DoSplitNode(
       EditorDOMPoint(&aSplittingContent,
                      std::min(aSplitOffset, aSplittingContent.Length())),
-      aNewContent);
+      aNewContent, GetSplitNodeDirection());
   NS_WARNING_ASSERTION(splitNodeResult.isOk(),
                        "HTMLEditor::DoSplitNode() failed");
   // When adding caret suggestion to SplitNodeResult, here didn't change
   // selection so that just ignore it.
   splitNodeResult.IgnoreCaretPointSuggestion();
   return splitNodeResult;
 }