Skip to content

Commit

Permalink
Bug 1739524 - part 1: Make RangeUpdater::SelAdjJoinNodes() handle b…
Browse files Browse the repository at this point in the history
…oth 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
  • Loading branch information
masayuki-nakano committed Nov 8, 2021
1 parent 54fe94e commit 6588e65
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 44 deletions.
7 changes: 7 additions & 0 deletions editor/libeditor/HTMLEditHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,13 @@ 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.
Expand Down
12 changes: 10 additions & 2 deletions editor/libeditor/HTMLEditor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4611,11 +4611,19 @@ nsresult HTMLEditor::JoinNodesWithTransaction(nsIContent& aLeftContent,
"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");

Expand Down
83 changes: 44 additions & 39 deletions editor/libeditor/SelectionState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@

#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
Expand Down Expand Up @@ -347,9 +349,12 @@ nsresult RangeUpdater::SelAdjSplitNode(nsIContent& aRightNode,
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;
Expand All @@ -359,44 +364,44 @@ nsresult RangeUpdater::SelAdjJoinNodes(nsINode& aLeftNode, nsINode& aRightNode,
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;
Expand Down
21 changes: 18 additions & 3 deletions editor/libeditor/SelectionState.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class Selection;
class Text;
} // namespace dom

enum class JoinNodesDirection; // Declared in HTMLEditHelpers.h

/**
* A helper struct for saving/setting ranges.
*/
Expand Down Expand Up @@ -141,9 +143,22 @@ class MOZ_STACK_CLASS RangeUpdater final {
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,
Expand Down

0 comments on commit 6588e65

Please sign in to comment.