Bug 1421504 - EditorBase should move children carefully r?m_kato
While moving children of a container to another container, mutation observer
may move children before moving and/or move node immediately after the
insertion point. Therefore, EditorBase should store all children which
should be moved with a local variable. Then, move one by one carefully.
E.g., if a child before being moved is moved to different container, it
shouldn't be moved because JS already handles it as expected for the web
app. If next sibling of the insertion point has been moved, EditorBase
should stop moving the remaining children because it may move children to
odd position and it may cause dataloss.
This patch creates EditorBase::MoveChildren() and it moves children carefully
with above checks.
MozReview-Commit-ID: FJDdSqXIlqD
--- a/editor/libeditor/EditorBase.cpp
+++ b/editor/libeditor/EditorBase.cpp
@@ -1864,16 +1864,80 @@ EditorBase::MoveNode(nsIContent* aNode,
nsCOMPtr<nsINode> kungFuDeathGrip = aNode;
nsresult rv = DeleteNode(aNode);
NS_ENSURE_SUCCESS(rv, rv);
return InsertNode(*aNode, *aParent, aOffset);
}
+void
+EditorBase::MoveChildren(nsIContent& aFirstChild,
+ nsIContent& aLastChild,
+ const EditorRawDOMPoint& aPointToInsert,
+ ErrorResult& aError)
+{
+ nsCOMPtr<nsINode> oldContainer = aFirstChild.GetParentNode();
+ if (NS_WARN_IF(oldContainer != aLastChild.GetParentNode()) ||
+ NS_WARN_IF(!aPointToInsert.IsSet()) ||
+ NS_WARN_IF(!aPointToInsert.Container()->IsContainerNode())) {
+ aError.Throw(NS_ERROR_INVALID_ARG);
+ return;
+ }
+
+ // First, store all children which should be moved to the new container.
+ AutoTArray<nsCOMPtr<nsIContent>, 10> children;
+ for (nsIContent* child = &aFirstChild;
+ child;
+ child = child->GetNextSibling()) {
+ children.AppendElement(child);
+ if (child == &aLastChild) {
+ break;
+ }
+ }
+
+ if (NS_WARN_IF(children.LastElement() != &aLastChild)) {
+ aError.Throw(NS_ERROR_INVALID_ARG);
+ return;
+ }
+
+ nsCOMPtr<nsINode> newContainer = aPointToInsert.Container();
+ EditorDOMPoint pointToInsert(aPointToInsert);
+ for (size_t i = children.Length(); i > 0; --i) {
+ nsCOMPtr<nsIContent>& child = children[i - 1];
+ if (child->GetParentNode() != oldContainer) {
+ // If the child has been moved to different container, we shouldn't
+ // touch it.
+ continue;
+ }
+ oldContainer->RemoveChild(*child, aError);
+ if (NS_WARN_IF(aError.Failed())) {
+ return;
+ }
+ if (!pointToInsert.IsEndOfContainer()) {
+ // If we're not appending the children to the new container, we should
+ // check if referring child node is still in the new container.
+ pointToInsert.Set(pointToInsert.GetChildAtOffset());
+ if (NS_WARN_IF(!pointToInsert.IsSet()) ||
+ NS_WARN_IF(pointToInsert.Container() != newContainer)) {
+ // The next node of insertion point has been moved by mutation observer.
+ // Let's stop moving the remaining nodes.
+ // XXX Or should we move remaining children after the last moved child?
+ aError.Throw(NS_ERROR_FAILURE);
+ return;
+ }
+ }
+ newContainer->InsertBefore(*child, pointToInsert.GetChildAtOffset(),
+ aError);
+ if (NS_WARN_IF(aError.Failed())) {
+ return;
+ }
+ }
+}
+
NS_IMETHODIMP
EditorBase::AddEditorObserver(nsIEditorObserver* aObserver)
{
// we don't keep ownership of the observers. They must
// remove themselves as observers before they are destroyed.
NS_ENSURE_TRUE(aObserver, NS_ERROR_NULL_POINTER);
@@ -2999,34 +3063,27 @@ EditorBase::SplitNodeImpl(const EditorDO
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;
- }
- nsCOMPtr<nsIContent> firstChild = aNewLeftNode.GetFirstChild();
- aNewLeftNode.InsertBefore(*childNode, firstChild, aError);
+ nsIContent* firstChild = aStartOfRightNode.Container()->GetFirstChild();
+ nsIContent* lastChild =
+ aStartOfRightNode.GetPreviousSiblingOfChildAtOffset();
+ if (NS_WARN_IF(!firstChild) || NS_WARN_IF(!lastChild)) {
+ // Is this possible case?
+ } else {
+ MoveChildren(*firstChild, *lastChild,
+ EditorRawDOMPoint(&aNewLeftNode, 0),
+ aError);
NS_WARNING_ASSERTION(!aError.Failed(),
- "Failed to insert a child which is removed from the right node into "
- "the left node");
+ "Failed to move children from the right node to the new left node");
}
}
}
// XXX Why do we ignore an error while moving nodes from the right node to
// the left node?
aError.SuppressException();
--- a/editor/libeditor/EditorBase.h
+++ b/editor/libeditor/EditorBase.h
@@ -377,16 +377,40 @@ public:
*/
already_AddRefed<nsIContent>
SplitNode(const EditorRawDOMPoint& aStartOfRightNode,
ErrorResult& aResult);
nsresult JoinNodes(nsINode& aLeftNode, nsINode& aRightNode);
nsresult MoveNode(nsIContent* aNode, nsINode* aParent, int32_t aOffset);
+ /**
+ * MoveChildren() moves children all children between aFirstChild and
+ * aLastChild to before aPointToInsert.GetChildAtOffset().
+ * If some children are moved to different container while this method
+ * moves other children, they are just ignored.
+ * If the child node referred by aPointToInsert is moved to different
+ * container while this method moves children, returns error.
+ *
+ * @param aFirstChild The first child which should be moved to
+ * aPointToInsert.
+ * @param aLastChild The last child which should be moved. This
+ * must be a sibling of aFirstChild and it should
+ * be positioned after aFirstChild in the DOM tree
+ * order.
+ * @param aPointToInsert The insertion point. The container must not
+ * be a data node like a text node.
+ * @param aError The result. If this succeeds to move children,
+ * returns NS_OK. Otherwise, an error.
+ */
+ void MoveChildren(nsIContent& aFirstChild,
+ nsIContent& aLastChild,
+ const EditorRawDOMPoint& aPointToInsert,
+ ErrorResult& aError);
+
nsresult CloneAttribute(nsAtom* aAttribute, Element* aDestElement,
Element* aSourceElement);
nsresult RemoveAttribute(Element* aElement, nsAtom* aAttribute);
virtual nsresult RemoveAttributeOrEquivalent(Element* aElement,
nsAtom* aAttribute,
bool aSuppressTransaction) = 0;
nsresult SetAttribute(Element* aElement, nsAtom* aAttribute,
const nsAString& aValue);