Bug 1739545 - part 2: Redesign TextServicesDocument::DidJoinNodes() methods r=m_kato

There is 2 overloads.  One is `TextServicesDocument`'s method, and the latter is
for `nsIEditActionListener`.  The latter could occur if `TextServicesDocument`
is added to the editor when there is no inline spellchecker, `EditorSpellCheck`
or `mozSpellChecker`.
https://searchfox.org/mozilla-central/rev/d2f8488b6a704443a5c5bfc6d2878171b5f0d393/editor/libeditor/EditorBase.cpp#2388,2391,2393,2396,2398

I don't know whether this is possible case, but unfortunately,
`nsIEditActionListener::DidJoinNodes()` is not implemented by JS and implemented
only by `TextServicesDocument`.  Therefore, we can make it "noscript" and use
non-scriptable classes as arguments.

This patch makes them get jointed point and removed content node and the joining
direction for fixing bug 1735608.  Unfortunately, there is no test framework
to check the result in `TextServicesDocument` nor the new path.  The latter
should be tested when we fix bug 1735608 later.

Differential Revision: https://phabricator.services.mozilla.com/D130458
This commit is contained in:
Masayuki Nakano 2021-11-09 01:09:20 +00:00
parent 1034b36c49
commit dfa4091f3b
4 changed files with 110 additions and 63 deletions

View file

@ -4614,6 +4614,10 @@ nsresult HTMLEditor::JoinNodesWithTransaction(nsIContent& aLeftContent,
return NS_ERROR_EDITOR_UNEXPECTED_DOM_TREE;
}
// Be aware, the joined point should be created for each call because
// they may refer the child node, but some of them may change the DOM tree
// after that, thus we need to avoid invalid point (Although it shouldn't
// occur).
// XXX Some other transactions manage range updater by themselves.
// Why doesn't JoinNodeTransaction do it?
DebugOnly<nsresult> rvIgnored = RangeUpdaterRef().SelAdjJoinNodes(
@ -4625,18 +4629,23 @@ nsresult HTMLEditor::JoinNodesWithTransaction(nsIContent& aLeftContent,
TopLevelEditSubActionDataRef().DidJoinContents(
*this, EditorRawDOMPoint(&aRightContent, oldLeftNodeLen));
if (mTextServicesDocument && NS_SUCCEEDED(rv)) {
RefPtr<TextServicesDocument> textServicesDocument = mTextServicesDocument;
textServicesDocument->DidJoinNodes(aLeftContent, aRightContent);
if (NS_SUCCEEDED(rv)) {
if (RefPtr<TextServicesDocument> textServicesDocument =
mTextServicesDocument) {
textServicesDocument->DidJoinContents(
EditorRawDOMPoint(&aRightContent, oldLeftNodeLen), aLeftContent,
JoinNodesDirection::LeftNodeIntoRightNode);
}
}
if (!mActionListeners.IsEmpty()) {
for (auto& listener : mActionListeners.Clone()) {
DebugOnly<nsresult> rvIgnored = listener->DidJoinNodes(
&aLeftContent, &aRightContent, atRightContent.GetContainer(), rv);
DebugOnly<nsresult> rvIgnored = listener->DidJoinContents(
EditorRawDOMPoint(&aRightContent, oldLeftNodeLen), &aLeftContent,
true);
NS_WARNING_ASSERTION(
NS_SUCCEEDED(rvIgnored),
"nsIEditActionListener::DidJoinNodes() failed, but ignored");
"nsIEditActionListener::DidJoinContents() failed, but ignored");
}
}

View file

@ -10,6 +10,17 @@ webidl CharacterData;
webidl Node;
webidl Range;
%{C++
#include "mozilla/EditorDOMPoint.h"
class nsINode;
class nsIContent;
namespace mozilla {
template class EditorDOMPointBase<nsINode*, nsIContent*>;
} // namespace mozilla
%}
[ref] native EditorRawDOMPointRef(mozilla::EditorDOMPointBase<nsINode*, nsIContent*>);
/*
Editor Action Listener interface to outside world
*/
@ -37,17 +48,21 @@ interface nsIEditActionListener : nsISupports
/**
* Called after the editor joins 2 nodes.
* @param aLeftNode This node will be merged into the right node
* @param aRightNode The node that will be merged into.
* There is no requirement that the two nodes be of
* the same type.
* @param aParent The parent of aRightNode
* @param aResult The result of the join operation.
* @param aJoinedPoint The joined point. If aLeftNodeWasRemoved is true,
* it points after inserted left node content in the
* right node. Otherwise, it points start of inserted
* right node content in the left node.
* @param aRemovedNode The removed node.
* @param aLeftNodeWasRemoved
* true if left node is removed and its contents were
* moved into start of the right node.
* false if right node is removed and its contents were
* moved into end of the left node.
*/
void DidJoinNodes(in Node aLeftNode,
in Node aRightNode,
in Node aParent,
in nsresult aResult);
[noscript]
void DidJoinContents([const] in EditorRawDOMPointRef aJoinedPoint,
[const] in Node aRemovedNode,
in bool aLeftNodeWasRemoved);
/**
* Called after the editor inserts text.

View file

@ -9,6 +9,7 @@
#include "mozilla/Assertions.h" // for MOZ_ASSERT, etc
#include "mozilla/EditorBase.h" // for EditorBase
#include "mozilla/EditorUtils.h" // for AutoTransactionBatchExternal
#include "mozilla/HTMLEditHelpers.h" // for JoinNodesDirection
#include "mozilla/HTMLEditUtils.h" // for HTMLEditUtils
#include "mozilla/mozalloc.h" // for operator new, etc
#include "mozilla/OwningNonNull.h"
@ -1332,75 +1333,95 @@ void TextServicesDocument::DidDeleteContent(const nsIContent& aChildContent) {
}
}
void TextServicesDocument::DidJoinNodes(const nsIContent& aLeftContent,
const nsIContent& aRightContent) {
void TextServicesDocument::DidJoinContents(
const EditorRawDOMPoint& aJoinedPoint, const nsIContent& aRemovedContent,
JoinNodesDirection aJoinNodesDirection) {
// Make sure that both nodes are text nodes -- otherwise we don't care.
if (!aLeftContent.IsText() || !aRightContent.IsText()) {
if (!aJoinedPoint.IsInTextNode() || !aRemovedContent.IsText()) {
return;
}
// Note: The editor merges the contents of the left node into the
// contents of the right.
Maybe<size_t> maybeLeftIndex =
mOffsetTable.FirstIndexOf(*aLeftContent.AsText());
if (maybeLeftIndex.isNothing()) {
Maybe<size_t> maybeRemovedIndex =
mOffsetTable.FirstIndexOf(*aRemovedContent.AsText());
if (maybeRemovedIndex.isNothing()) {
// It's okay if the node isn't in the offset table, the
// editor could be cleaning house.
return;
}
Maybe<size_t> maybeRightIndex =
mOffsetTable.FirstIndexOf(*aRightContent.AsText());
if (maybeRightIndex.isNothing()) {
Maybe<size_t> maybeJoinedIndex =
mOffsetTable.FirstIndexOf(*aJoinedPoint.ContainerAsText());
if (maybeJoinedIndex.isNothing()) {
// It's okay if the node isn't in the offset table, the
// editor could be cleaning house.
return;
}
const size_t leftIndex = *maybeLeftIndex;
const size_t rightIndex = *maybeRightIndex;
NS_ASSERTION(leftIndex < rightIndex, "Indexes out of order.");
const size_t removedIndex = *maybeRemovedIndex;
const size_t joinedIndex = *maybeJoinedIndex;
if (leftIndex > rightIndex) {
// Don't know how to handle this situation.
return;
if (aJoinNodesDirection == JoinNodesDirection::LeftNodeIntoRightNode) {
if (MOZ_UNLIKELY(removedIndex > joinedIndex)) {
NS_ASSERTION(removedIndex < joinedIndex, "Indexes out of order.");
return;
}
NS_ASSERTION(mOffsetTable[joinedIndex]->mOffsetInTextNode == 0,
"Unexpected offset value for joinedIndex.");
} else {
if (MOZ_UNLIKELY(joinedIndex > removedIndex)) {
NS_ASSERTION(joinedIndex < removedIndex, "Indexes out of order.");
return;
}
NS_ASSERTION(mOffsetTable[removedIndex]->mOffsetInTextNode == 0,
"Unexpected offset value for rightIndex.");
}
NS_ASSERTION(mOffsetTable[rightIndex]->mOffsetInTextNode == 0,
"Unexpected offset value for rightIndex.");
// Run through the table and change all entries referring to
// the left node so that they now refer to the right node:
uint32_t nodeLength = aLeftContent.AsText()->Length();
for (uint32_t i = leftIndex; i < rightIndex; i++) {
// the removed node so that they now refer to the joined node,
// and adjust offsets if necessary.
const uint32_t movedTextDataLength =
aJoinNodesDirection == JoinNodesDirection::LeftNodeIntoRightNode
? aJoinedPoint.Offset()
: aJoinedPoint.ContainerAsText()->TextDataLength() -
aJoinedPoint.Offset();
for (uint32_t i = removedIndex; i < mOffsetTable.Length(); i++) {
const UniquePtr<OffsetEntry>& entry = mOffsetTable[i];
LockOffsetEntryArrayLengthInDebugBuild(observer, mOffsetTable);
if (entry->mTextNode != aLeftContent.AsText()) {
if (entry->mTextNode != aRemovedContent.AsText()) {
break;
}
if (entry->mIsValid) {
entry->mTextNode = const_cast<Text*>(aRightContent.AsText());
entry->mTextNode = aJoinedPoint.ContainerAsText();
if (aJoinNodesDirection == JoinNodesDirection::RightNodeIntoLeftNode) {
// The text was moved from aRemovedContent to end of the container of
// aJoinedPoint.
entry->mOffsetInTextNode += movedTextDataLength;
}
}
}
// Run through the table and adjust the node offsets
// for all entries referring to the right node.
for (uint32_t i = rightIndex; i < mOffsetTable.Length(); i++) {
const UniquePtr<OffsetEntry>& entry = mOffsetTable[i];
LockOffsetEntryArrayLengthInDebugBuild(observer, mOffsetTable);
if (entry->mTextNode != aRightContent.AsText()) {
break;
}
if (entry->mIsValid) {
entry->mOffsetInTextNode += nodeLength;
if (aJoinNodesDirection == JoinNodesDirection::LeftNodeIntoRightNode) {
// The text was moved from aRemovedContent to start of the container of
// aJoinedPoint.
for (uint32_t i = joinedIndex; i < mOffsetTable.Length(); i++) {
const UniquePtr<OffsetEntry>& entry = mOffsetTable[i];
LockOffsetEntryArrayLengthInDebugBuild(observer, mOffsetTable);
if (entry->mTextNode != aJoinedPoint.ContainerAsText()) {
break;
}
if (entry->mIsValid) {
entry->mOffsetInTextNode += movedTextDataLength;
}
}
}
// Now check to see if the iterator is pointing to the
// left node. If it is, make it point to the right node!
if (mFilteredIter->GetCurrentNode() == aLeftContent.AsText()) {
mFilteredIter->PositionAt(const_cast<Text*>(aRightContent.AsText()));
// left node. If it is, make it point to the joined node!
if (mFilteredIter->GetCurrentNode() == aRemovedContent.AsText()) {
mFilteredIter->PositionAt(aJoinedPoint.ContainerAsText());
}
}
@ -2751,17 +2772,17 @@ TextServicesDocument::DidDeleteNode(nsINode* aChild, nsresult aResult) {
return NS_OK;
}
NS_IMETHODIMP
TextServicesDocument::DidJoinNodes(nsINode* aLeftNode, nsINode* aRightNode,
nsINode* aParent, nsresult aResult) {
if (NS_WARN_IF(NS_FAILED(aResult))) {
NS_IMETHODIMP TextServicesDocument::DidJoinContents(
const EditorRawDOMPoint& aJoinedPoint, const nsINode* aRemovedNode,
bool aLeftNodeWasRemoved) {
if (MOZ_UNLIKELY(NS_WARN_IF(!aJoinedPoint.IsSetAndValid()) ||
NS_WARN_IF(!aRemovedNode->IsContent()))) {
return NS_OK;
}
if (NS_WARN_IF(!aLeftNode) || !aLeftNode->IsContent() ||
NS_WARN_IF(!aRightNode) || !aRightNode->IsContent()) {
return NS_OK;
}
DidJoinNodes(*aLeftNode->AsContent(), *aRightNode->AsContent());
DidJoinContents(aJoinedPoint, *aRemovedNode->AsContent(),
aLeftNodeWasRemoved
? JoinNodesDirection::LeftNodeIntoRightNode
: JoinNodesDirection::RightNodeIntoLeftNode);
return NS_OK;
}

View file

@ -28,6 +28,7 @@ namespace mozilla {
class EditorBase;
class FilteredContentIterator;
class OffsetEntry;
enum class JoinNodesDirection; // Declared in HTMLEditHelpers.h
namespace dom {
class AbstractRange;
@ -373,8 +374,9 @@ class TextServicesDocument final : public nsIEditActionListener {
* nsIEditActionListener (slow path, though).
*/
void DidDeleteContent(const nsIContent& aChildContent);
void DidJoinNodes(const nsIContent& aLeftContent,
const nsIContent& aRightContent);
void DidJoinContents(const EditorRawDOMPoint& aJoinedPoint,
const nsIContent& aRemovedContent,
JoinNodesDirection aJoinNodesDirection);
private:
// TODO: We should get rid of this method since `aAbstractRange` has