Bug 1413181 - part 1: Redesign EditorBase::SplitNodeImpl() with EditorDOMPoint r?m_kato
EditorBaseSplitNodeImpl() should be clean up with EditorDOMPoint which should
be an argument to point the first point of right node (existing split node).
MozReview-Commit-ID: DN0yHm9G9yT
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -2899,151 +2899,188 @@ struct SavedRange final
{
RefPtr<Selection> mSelection;
nsCOMPtr<nsINode> mStartContainer;
nsCOMPtr<nsINode> mEndContainer;
int32_t mStartOffset;
int32_t mEndOffset;
};
-nsresult
-EditorBase::SplitNodeImpl(nsIContent& aExistingRightNode,
- int32_t aOffset,
- nsIContent& aNewLeftNode)
-{
+void
+EditorBase::SplitNodeImpl(const EditorDOMPoint& aStartOfRightNode,
+ nsIContent& aNewLeftNode,
+ ErrorResult& aError)
+{
+ if (NS_WARN_IF(aError.Failed())) {
+ return;
+ }
+
+ // XXX Perhaps, aStartOfRightNode may be invalid if this is a redo
+ // operation after modifying DOM node with JS.
+ if (NS_WARN_IF(!aStartOfRightNode.IsSet())) {
+ aError.Throw(NS_ERROR_INVALID_ARG);
+ return;
+ }
+ MOZ_ASSERT(aStartOfRightNode.IsSetAndValid());
+
// Remember all selection points.
AutoTArray<SavedRange, 10> savedRanges;
for (SelectionType selectionType : kPresentSelectionTypes) {
SavedRange range;
range.mSelection = GetSelection(selectionType);
- if (selectionType == SelectionType::eNormal) {
- NS_ENSURE_TRUE(range.mSelection, NS_ERROR_NULL_POINTER);
+ if (NS_WARN_IF(!range.mSelection &&
+ selectionType == SelectionType::eNormal)) {
+ aError.Throw(NS_ERROR_FAILURE);
+ return;
} else if (!range.mSelection) {
// For non-normal selections, skip over the non-existing ones.
continue;
}
for (uint32_t j = 0; j < range.mSelection->RangeCount(); ++j) {
RefPtr<nsRange> r = range.mSelection->GetRangeAt(j);
MOZ_ASSERT(r->IsPositioned());
+ // XXX Looks like that SavedRange should have mStart and mEnd which
+ // are RangeBoundary. Then, we can avoid to compute offset here.
range.mStartContainer = r->GetStartContainer();
range.mStartOffset = r->StartOffset();
range.mEndContainer = r->GetEndContainer();
range.mEndOffset = r->EndOffset();
savedRanges.AppendElement(range);
}
}
- nsCOMPtr<nsINode> parent = aExistingRightNode.GetParentNode();
- NS_ENSURE_TRUE(parent, NS_ERROR_NULL_POINTER);
-
- ErrorResult rv;
- nsCOMPtr<nsINode> refNode = &aExistingRightNode;
- parent->InsertBefore(aNewLeftNode, refNode, rv);
- NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult());
-
- // Split the children between the two nodes. At this point,
- // aExistingRightNode has all the children. Move all the children whose
- // index is < aOffset to aNewLeftNode.
- if (aOffset < 0) {
- // This means move no children
- return NS_OK;
- }
-
- // If it's a text node, just shuffle around some text
- if (aExistingRightNode.GetAsText() && aNewLeftNode.GetAsText()) {
- // Fix right node
- nsAutoString leftText;
- aExistingRightNode.GetAsText()->SubstringData(0, aOffset, leftText);
- aExistingRightNode.GetAsText()->DeleteData(0, aOffset);
- // Fix left node
- aNewLeftNode.GetAsText()->SetData(leftText);
- } else {
- // Otherwise it's an interior node, so shuffle around the children. Go
- // through list backwards so deletes don't interfere with the iteration.
- nsCOMPtr<nsINodeList> childNodes = aExistingRightNode.ChildNodes();
- for (int32_t i = aOffset - 1; i >= 0; i--) {
- nsCOMPtr<nsIContent> childNode = childNodes->Item(i);
- if (childNode) {
- aExistingRightNode.RemoveChild(*childNode, rv);
- if (!rv.Failed()) {
- nsCOMPtr<nsIContent> firstChild = aNewLeftNode.GetFirstChild();
- aNewLeftNode.InsertBefore(*childNode, firstChild, rv);
+ nsCOMPtr<nsINode> parent = aStartOfRightNode.Container()->GetParentNode();
+ if (NS_WARN_IF(!parent)) {
+ aError.Throw(NS_ERROR_FAILURE);
+ return;
+ }
+
+ parent->InsertBefore(aNewLeftNode, aStartOfRightNode.Container(),
+ aError);
+ if (NS_WARN_IF(aError.Failed())) {
+ return;
+ }
+
+ // At this point, the existing right node has all the children. Move all
+ // the children which are before aStartOfRightNode.
+ if (!aStartOfRightNode.IsStartOfContainer()) {
+ // If it's a text node, just shuffle around some text
+ Text* rightAsText = aStartOfRightNode.Container()->GetAsText();
+ Text* leftAsText = aNewLeftNode.GetAsText();
+ if (rightAsText && leftAsText) {
+ // Fix right node
+ nsAutoString leftText;
+ rightAsText->SubstringData(0, aStartOfRightNode.Offset(),
+ leftText);
+ rightAsText->DeleteData(0, aStartOfRightNode.Offset());
+ // Fix left node
+ leftAsText->GetAsText()->SetData(leftText);
+ } else {
+ MOZ_DIAGNOSTIC_ASSERT(!rightAsText && !leftAsText);
+ // Otherwise it's an interior node, so shuffle around the children. Go
+ // through list backwards so deletes don't interfere with the iteration.
+ // FYI: It's okay to use raw pointer for caching existing right node since
+ // it's already grabbed by aStartOfRightNode.
+ nsINode* existingRightNode = aStartOfRightNode.Container();
+ nsCOMPtr<nsINodeList> childNodes = existingRightNode->ChildNodes();
+ // XXX This is wrong loop range if some children has already gone.
+ // This will be fixed by a later patch.
+ for (int32_t i = aStartOfRightNode.Offset() - 1; i >= 0; i--) {
+ nsCOMPtr<nsIContent> childNode = childNodes->Item(i);
+ MOZ_RELEASE_ASSERT(childNode);
+ existingRightNode->RemoveChild(*childNode, aError);
+ if (NS_WARN_IF(aError.Failed())) {
+ break;
}
- }
- if (rv.Failed()) {
- break;
+ nsCOMPtr<nsIContent> firstChild = aNewLeftNode.GetFirstChild();
+ aNewLeftNode.InsertBefore(*childNode, firstChild, aError);
+ NS_WARNING_ASSERTION(!aError.Failed(),
+ "Failed to insert a child which is removed from the right node into "
+ "the left node");
}
}
}
+ // XXX Why do we ignore an error while moving nodes from the right node to
+ // the left node?
+ aError.SuppressException();
+
// Handle selection
nsCOMPtr<nsIPresShell> ps = GetPresShell();
if (ps) {
ps->FlushPendingNotifications(FlushType::Frames);
}
+ NS_WARNING_ASSERTION(!Destroyed(),
+ "The editor is destroyed during splitting a node");
bool shouldSetSelection = GetShouldTxnSetSelection();
RefPtr<Selection> previousSelection;
for (size_t i = 0; i < savedRanges.Length(); ++i) {
// Adjust the selection if needed.
SavedRange& range = savedRanges[i];
// If we have not seen the selection yet, clear all of its ranges.
if (range.mSelection != previousSelection) {
- nsresult rv = range.mSelection->RemoveAllRanges();
- NS_ENSURE_SUCCESS(rv, rv);
+ range.mSelection->RemoveAllRanges(aError);
+ if (NS_WARN_IF(aError.Failed())) {
+ return;
+ }
previousSelection = range.mSelection;
}
+ // XXX Looks like that we don't need to modify normal selection here
+ // because selection will be modified by the caller if
+ // GetShouldTxnSetSelection() will return true.
if (shouldSetSelection &&
range.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 (range.mStartContainer == &aExistingRightNode) {
- if (range.mStartOffset < aOffset) {
+ if (range.mStartContainer == aStartOfRightNode.Container()) {
+ if (static_cast<uint32_t>(range.mStartOffset) <
+ aStartOfRightNode.Offset()) {
range.mStartContainer = &aNewLeftNode;
} else {
- range.mStartOffset -= aOffset;
+ range.mStartOffset -= aStartOfRightNode.Offset();
}
}
- if (range.mEndContainer == &aExistingRightNode) {
- if (range.mEndOffset < aOffset) {
+ if (range.mEndContainer == aStartOfRightNode.Container()) {
+ if (static_cast<uint32_t>(range.mEndOffset) <
+ aStartOfRightNode.Offset()) {
range.mEndContainer = &aNewLeftNode;
} else {
- range.mEndOffset -= aOffset;
+ range.mEndOffset -= aStartOfRightNode.Offset();
}
}
RefPtr<nsRange> newRange;
nsresult rv = nsRange::CreateRange(range.mStartContainer,
range.mStartOffset,
range.mEndContainer,
range.mEndOffset,
getter_AddRefs(newRange));
- NS_ENSURE_SUCCESS(rv, rv);
- rv = range.mSelection->AddRange(newRange);
- NS_ENSURE_SUCCESS(rv, rv);
- }
-
- if (shouldSetSelection) {
- // Editor wants us to set selection at split point.
- RefPtr<Selection> selection = GetSelection();
- NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
- selection->Collapse(&aNewLeftNode, aOffset);
- }
-
- return NS_OK;
+ if (NS_WARN_IF(NS_FAILED(rv))) {
+ aError.Throw(rv);
+ return;
+ }
+ range.mSelection->AddRange(*newRange, aError);
+ if (NS_WARN_IF(aError.Failed())) {
+ return;
+ }
+ }
+
+ // We don't need to set selection here because the caller should do that
+ // in any case.
}
nsresult
EditorBase::JoinNodesImpl(nsINode* aNodeToKeep,
nsINode* aNodeToJoin,
nsINode* aParent)
{
MOZ_ASSERT(aNodeToKeep);
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -726,28 +726,37 @@ public:
* various editor actions.
*/
bool ArePreservingSelection();
void PreserveSelectionAcrossActions(Selection* aSel);
nsresult RestorePreservedSelection(Selection* aSel);
void StopPreservingSelection();
/**
- * SplitNode() creates a new node identical to an existing node, and split
- * the contents between the two nodes
- * @param aExistingRightNode The node to split. It will become the new
- * node's next sibling.
- * @param aOffset The offset of aExistingRightNode's
- * content|children to do the split at
- * @param aNewLeftNode The new node resulting from the split, becomes
- * aExistingRightNode's previous sibling.
+ * SplitNodeImpl() creates a new node (left node) identical to an existing
+ * node (right node), and split the contents between the same point in both
+ * nodes.
+ *
+ * @param aStartOfRightNode The point to split. Its container will be
+ * the right node, i.e., become the new node's
+ * next sibling. And the point will be start
+ * of the right node.
+ * @param aNewLeftNode The new node called as left node, so, this
+ * becomes the container of aPointToSplit's
+ * previous sibling.
+ * @param aError Must have not already failed.
+ * If succeed to insert aLeftNode before the
+ * right node and remove unnecessary contents
+ * (and collapse selection at end of the left
+ * node if necessary), returns no error.
+ * Otherwise, an error.
*/
- nsresult SplitNodeImpl(nsIContent& aExistingRightNode,
- int32_t aOffset,
- nsIContent& aNewLeftNode);
+ void SplitNodeImpl(const EditorDOMPoint& aStartOfRightNode,
+ nsIContent& aNewLeftNode,
+ ErrorResult& aError);
/**
* JoinNodes() takes 2 nodes and merge their content|children.
* @param aNodeToKeep The node that will remain after the join.
* @param aNodeToJoin The node that will be joined with aNodeToKeep.
* There is no requirement that the two nodes be of the
* same type.
* @param aParent The parent of aNodeToKeep
--- a/editor/libeditor/SplitNodeTransaction.cpp
+++ b/editor/libeditor/SplitNodeTransaction.cpp
@@ -42,36 +42,64 @@ NS_INTERFACE_MAP_END_INHERITING(EditTran
NS_IMETHODIMP
SplitNodeTransaction::DoTransaction()
{
if (NS_WARN_IF(!mEditorBase)) {
return NS_ERROR_NOT_INITIALIZED;
}
// Create a new node
- ErrorResult rv;
+ ErrorResult error;
// Don't use .downcast directly because AsContent has an assertion we want
- nsCOMPtr<nsINode> clone = mExistingRightNode->CloneNode(false, rv);
- NS_ASSERTION(!rv.Failed() && clone, "Could not create clone");
- NS_ENSURE_TRUE(!rv.Failed() && clone, rv.StealNSResult());
+ nsCOMPtr<nsINode> clone = mExistingRightNode->CloneNode(false, error);
+ if (NS_WARN_IF(error.Failed())) {
+ return error.StealNSResult();
+ }
+ if (NS_WARN_IF(!clone)) {
+ return NS_ERROR_UNEXPECTED;
+ }
mNewLeftNode = dont_AddRef(clone.forget().take()->AsContent());
mEditorBase->MarkNodeDirty(mExistingRightNode->AsDOMNode());
// Get the parent node
mParent = mExistingRightNode->GetParentNode();
- NS_ENSURE_TRUE(mParent, NS_ERROR_NULL_POINTER);
+ if (NS_WARN_IF(!mParent)) {
+ return NS_ERROR_FAILURE;
+ }
// Insert the new node
- rv = mEditorBase->SplitNodeImpl(*mExistingRightNode, mOffset, *mNewLeftNode);
+ int32_t offset =
+ std::min(std::max(mOffset, 0),
+ static_cast<int32_t>(mExistingRightNode->Length()));
+ mEditorBase->SplitNodeImpl(EditorDOMPoint(mExistingRightNode, offset),
+ *mNewLeftNode, error);
+ // XXX Really odd. The result of SplitNodeImpl() is respected only when
+ // we shouldn't set selection. Otherwise, it's overridden by the
+ // result of Selection.Collapse().
if (mEditorBase->GetShouldTxnSetSelection()) {
+ NS_WARNING_ASSERTION(!mEditorBase->Destroyed(),
+ "The editor has gone but SplitNodeTransaction keeps trying to modify "
+ "Selection");
RefPtr<Selection> selection = mEditorBase->GetSelection();
- NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER);
- rv = selection->Collapse(mNewLeftNode, mOffset);
+ if (NS_WARN_IF(!selection)) {
+ return NS_ERROR_FAILURE;
+ }
+ if (NS_WARN_IF(error.Failed())) {
+ // XXX This must be a bug.
+ error.SuppressException();
+ }
+ MOZ_ASSERT(offset == mNewLeftNode->Length());
+ EditorRawDOMPoint atEndOfLeftNode(mNewLeftNode, mNewLeftNode->Length());
+ selection->Collapse(atEndOfLeftNode, error);
}
- return rv.StealNSResult();
+
+ if (NS_WARN_IF(error.Failed())) {
+ return error.StealNSResult();
+ }
+ return NS_OK;
}
NS_IMETHODIMP
SplitNodeTransaction::UndoTransaction()
{
if (NS_WARN_IF(!mEditorBase) ||
NS_WARN_IF(!mNewLeftNode) ||
NS_WARN_IF(!mParent)) {