Bug 1739524 - part 1: Make `RangeUpdater::SelAdjJoinNodes()` handle both cases that right/left node is joined into the other r=m_kato
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 08 Nov 2021 09:49:33 +0000
changeset 672270 f7cfac8d4a177e5e4ee684e3eceabce8b7784e74
parent 672269 ff1b3692fe81b58fe1149fa43d0d196536297b90
child 672271 747fde49e671fc04ca10f1fad128fd7c8e97176e
push id2724
push userffxbld-merge
push dateMon, 03 Jan 2022 22:10:59 +0000
treeherdermozilla-release@ef2a8189e3dc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato
bugs1739524, 1735608
milestone96.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 1739524 - part 1: Make `RangeUpdater::SelAdjJoinNodes()` handle both cases that right/left node is joined into the other r=m_kato Currently, it assumes that all children or text data of `aRightContent` is moved into `aLeftContent`. However, we need the opposite case when we fix bug 1735608. Unfortunately, we cannot test the new path until we fix bug 1735608. Therefore, the new path may contain some bugs. Differential Revision: https://phabricator.services.mozilla.com/D130426
editor/libeditor/HTMLEditHelpers.h
editor/libeditor/HTMLEditor.cpp
editor/libeditor/SelectionState.cpp
editor/libeditor/SelectionState.h
--- a/editor/libeditor/HTMLEditHelpers.h
+++ b/editor/libeditor/HTMLEditHelpers.h
@@ -26,16 +26,23 @@
 #include "nsString.h"
 
 class nsISimpleEnumerator;
 
 namespace mozilla {
 template <class T>
 class OwningNonNull;
 
+// JoinNodesDirection is also affected to which one is new node at splitting
+// a node because a couple of undo/redo.
+enum class JoinNodesDirection {
+  LeftNodeIntoRightNode,
+  RightNodeIntoLeftNode,
+};
+
 /*****************************************************************************
  * EditResult returns nsresult and preferred point where selection should be
  * collapsed or the range where selection should select.
  *
  * NOTE: If we stop modifying selection at every DOM tree change, perhaps,
  *       the following classes need to inherit this class.
  *****************************************************************************/
 class MOZ_STACK_CLASS EditResult final {
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -4606,21 +4606,29 @@ nsresult HTMLEditor::JoinNodesWithTransa
     rv = DoTransactionInternal(transaction);
     if (MOZ_UNLIKELY(Destroyed())) {
       rv = NS_ERROR_EDITOR_DESTROYED;
     }
     NS_WARNING_ASSERTION(NS_SUCCEEDED(rv),
                          "EditorBase::DoTransactionInternal() failed");
   }
 
+  // If joined node is moved to different place, offset may not have any
+  // meaning.  In this case, the web app modified the DOM tree takes on the
+  // responsibility for the remaning things.
+  if (MOZ_UNLIKELY(NS_WARN_IF(aRightContent.GetParentNode() !=
+                              atRightContent.GetContainer()))) {
+    return NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE;
+  }
+
   // XXX Some other transactions manage range updater by themselves.
   //     Why doesn't JoinNodeTransaction do it?
   DebugOnly<nsresult> rvIgnored = RangeUpdaterRef().SelAdjJoinNodes(
-      aLeftContent, aRightContent, *atRightContent.GetContainer(),
-      atRightContent.Offset(), oldLeftNodeLen);
+      EditorRawDOMPoint(&aRightContent, oldLeftNodeLen), aLeftContent,
+      atRightContent.Offset(), JoinNodesDirection::LeftNodeIntoRightNode);
   NS_WARNING_ASSERTION(NS_SUCCEEDED(rvIgnored),
                        "RangeUpdater::SelAdjJoinNodes() failed, but ignored");
 
   TopLevelEditSubActionDataRef().DidJoinContents(
       *this, EditorRawDOMPoint(&aRightContent, oldLeftNodeLen));
 
   if (mInlineSpellChecker) {
     RefPtr<mozInlineSpellChecker> spellChecker = mInlineSpellChecker;
--- a/editor/libeditor/SelectionState.cpp
+++ b/editor/libeditor/SelectionState.cpp
@@ -1,17 +1,19 @@
 /* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "SelectionState.h"
 
-#include "mozilla/Assertions.h"   // for MOZ_ASSERT, etc.
-#include "mozilla/EditorUtils.h"  // for EditorUtils
+#include "EditorUtils.h"      // for EditorUtils
+#include "HTMLEditHelpers.h"  // for JoinNodesDirection
+
+#include "mozilla/Assertions.h"  // for MOZ_ASSERT, etc.
 #include "mozilla/dom/RangeBinding.h"
 #include "mozilla/dom/Selection.h"  // for Selection
 #include "nsAString.h"              // for nsAString::Length
 #include "nsCycleCollectionParticipant.h"
 #include "nsDebug.h"          // for NS_WARNING, etc.
 #include "nsError.h"          // for NS_OK, etc.
 #include "nsIContent.h"       // for nsIContent
 #include "nsISupportsImpl.h"  // for nsRange::Release
@@ -342,66 +344,69 @@ nsresult RangeUpdater::SelAdjSplitNode(n
       } else {
         rangeItem->mEndContainer = &aNewLeftNode;
       }
     }
   }
   return NS_OK;
 }
 
-nsresult RangeUpdater::SelAdjJoinNodes(nsINode& aLeftNode, nsINode& aRightNode,
-                                       nsINode& aParent, uint32_t aOffset,
-                                       uint32_t aOldLeftNodeLength) {
+nsresult RangeUpdater::SelAdjJoinNodes(
+    const EditorRawDOMPoint& aStartOfRightContent,
+    const nsIContent& aRemovedContent, uint32_t aOffsetOfRemovedContent,
+    JoinNodesDirection aJoinNodesDirection) {
+  MOZ_ASSERT(aStartOfRightContent.IsSetAndValid());
+
   if (mLocked) {
     // lock set by Will/DidReplaceParent, etc...
     return NS_OK;
   }
 
   if (mArray.IsEmpty()) {
     return NS_OK;
   }
 
+  auto AdjustDOMPoint = [&](nsCOMPtr<nsINode>& aContainer,
+                            uint32_t& aOffset) -> void {
+    if (aContainer == aStartOfRightContent.GetContainerParent()) {
+      // If the point is in common parent of joined content nodes and the
+      // point is after the removed point, decrease the offset.
+      if (aOffset > aOffsetOfRemovedContent) {
+        aOffset--;
+      }
+      // If it pointed the removed content node, move to start of right content
+      // which was moved from the removed content.
+      else if (aOffset == aOffsetOfRemovedContent) {
+        aContainer = aStartOfRightContent.GetContainer();
+        aOffset = aStartOfRightContent.Offset();
+      }
+    } else if (aContainer == aStartOfRightContent.GetContainer()) {
+      // If the point is in joined node, and removed content is moved to
+      // start of the joined node, we need to adjust the offset.
+      if (aJoinNodesDirection == JoinNodesDirection::LeftNodeIntoRightNode) {
+        aOffset += aStartOfRightContent.Offset();
+      }
+    } else if (aContainer == &aRemovedContent) {
+      // If the point is in the removed content, move the point to the new
+      // point in the joined node.  If left node content is moved into
+      // right node, the offset should be same.  Otherwise, we need to advance
+      // the offset to length of the removed content.
+      aContainer = aStartOfRightContent.GetContainer();
+      if (aJoinNodesDirection == JoinNodesDirection::RightNodeIntoLeftNode) {
+        aOffset += aStartOfRightContent.Offset();
+      }
+    }
+  };
+
   for (RefPtr<RangeItem>& rangeItem : mArray) {
     if (NS_WARN_IF(!rangeItem)) {
       return NS_ERROR_FAILURE;
     }
-
-    if (rangeItem->mStartContainer == &aParent) {
-      // adjust start point in aParent
-      if (rangeItem->mStartOffset > aOffset) {
-        rangeItem->mStartOffset--;
-      } else if (rangeItem->mStartOffset == aOffset) {
-        // join keeps right hand node
-        rangeItem->mStartContainer = &aRightNode;
-        rangeItem->mStartOffset = aOldLeftNodeLength;
-      }
-    } else if (rangeItem->mStartContainer == &aRightNode) {
-      // adjust start point in aRightNode
-      rangeItem->mStartOffset += aOldLeftNodeLength;
-    } else if (rangeItem->mStartContainer == &aLeftNode) {
-      // adjust start point in aLeftNode
-      rangeItem->mStartContainer = &aRightNode;
-    }
-
-    if (rangeItem->mEndContainer == &aParent) {
-      // adjust end point in aParent
-      if (rangeItem->mEndOffset > aOffset) {
-        rangeItem->mEndOffset--;
-      } else if (rangeItem->mEndOffset == aOffset) {
-        // join keeps right hand node
-        rangeItem->mEndContainer = &aRightNode;
-        rangeItem->mEndOffset = aOldLeftNodeLength;
-      }
-    } else if (rangeItem->mEndContainer == &aRightNode) {
-      // adjust end point in aRightNode
-      rangeItem->mEndOffset += aOldLeftNodeLength;
-    } else if (rangeItem->mEndContainer == &aLeftNode) {
-      // adjust end point in aLeftNode
-      rangeItem->mEndContainer = &aRightNode;
-    }
+    AdjustDOMPoint(rangeItem->mStartContainer, rangeItem->mStartOffset);
+    AdjustDOMPoint(rangeItem->mEndContainer, rangeItem->mEndOffset);
   }
 
   return NS_OK;
 }
 
 void RangeUpdater::SelAdjReplaceText(const Text& aTextNode, uint32_t aOffset,
                                      uint32_t aReplacedLength,
                                      uint32_t aInsertedLength) {
--- a/editor/libeditor/SelectionState.h
+++ b/editor/libeditor/SelectionState.h
@@ -21,16 +21,18 @@ class nsRange;
 namespace mozilla {
 class RangeUpdater;
 namespace dom {
 class Element;
 class Selection;
 class Text;
 }  // namespace dom
 
+enum class JoinNodesDirection;  // Declared in HTMLEditHelpers.h
+
 /**
  * A helper struct for saving/setting ranges.
  */
 struct RangeItem final {
   RangeItem() : mStartOffset(0), mEndOffset(0) {}
 
  private:
   // Private destructor, to discourage deletion outside of Release():
@@ -136,19 +138,32 @@ class MOZ_STACK_CLASS RangeUpdater final
   // node on deletion, which is not what you want if you know you are
   // reinserting it.
   template <typename PT, typename CT>
   nsresult SelAdjCreateNode(const EditorDOMPointBase<PT, CT>& aPoint);
   template <typename PT, typename CT>
   nsresult SelAdjInsertNode(const EditorDOMPointBase<PT, CT>& aPoint);
   void SelAdjDeleteNode(nsINode& aNode);
   nsresult SelAdjSplitNode(nsIContent& aRightNode, nsIContent& aNewLeftNode);
-  nsresult SelAdjJoinNodes(nsINode& aLeftNode, nsINode& aRightNode,
-                           nsINode& aParent, uint32_t aOffset,
-                           uint32_t aOldLeftNodeLength);
+  /**
+   * SelAdjJoinNodes() is called immediately after joining aRemovedContent and
+   * the container of aStartOfRightContent.
+   *
+   * @param aStartOfRightContent    The container is joined content node which
+   *                                now has all children or text data which were
+   *                                in aRemovedContent.  And this points where
+   *                                the joined position.
+   * @param aRemovedContent         The removed content.
+   * @param aOffsetOfRemovedContent The offset which aRemovedContent was in
+   *                                its ex-parent.
+   */
+  nsresult SelAdjJoinNodes(const EditorRawDOMPoint& aStartOfRightContent,
+                           const nsIContent& aRemovedContent,
+                           uint32_t aOffsetOfRemovedContent,
+                           JoinNodesDirection aJoinNodesDirection);
   void SelAdjInsertText(const dom::Text& aTextNode, uint32_t aOffset,
                         uint32_t aInsertedLength);
   void SelAdjDeleteText(const dom::Text& aTextNode, uint32_t aOffset,
                         uint32_t aDeletedLength);
   void SelAdjReplaceText(const dom::Text& aTextNode, uint32_t aOffset,
                          uint32_t aReplacedLength, uint32_t aInsertedLength);
   // the following gravity routines need will/did sandwiches, because the other
   // gravity routines will be called inside of these sandwiches, but should be