Bug 1388775 - extractContents should copy nodes in tree order, not backwards; r?smaug
Per spec, Range.prototype.extractNodes() should copy the nodes in tree
order:
https://dom.spec.whatwg.org/#dom-range-extractcontents
Gecko instead copies them in reverse order. This causes us to fail a
wpt MutationObserver test.
MozReview-Commit-ID: 8MYXGhDsJCd
--- a/dom/base/nsRange.cpp
+++ b/dom/base/nsRange.cpp
@@ -2128,35 +2128,40 @@ nsRange::CutContents(DocumentFragment**
// There's nothing for us to delete.
rv = CollapseRangeAfterDelete(this);
if (NS_SUCCEEDED(rv) && aFragment) {
retval.forget(aFragment);
}
return rv;
}
- // We delete backwards to avoid iterator problems!
-
- iter.Last();
+ iter.First();
bool handled = false;
// With the exception of text nodes that contain one of the range
// end points, the subtree iterator should only give us back subtrees
// that are completely contained between the range's end points.
while (!iter.IsDone())
{
nsCOMPtr<nsINode> nodeToResult;
nsCOMPtr<nsINode> node = iter.GetCurrentNode();
- // Before we delete anything, advance the iterator to the
- // next subtree.
-
- iter.Prev();
+ // Before we delete anything, advance the iterator to the next node that's
+ // not a descendant of this one. XXX It's a bit silly to iterate through
+ // the descendants only to throw them out, we should use an iterator that
+ // skips the descendants to begin with.
+
+ iter.Next();
+ nsCOMPtr<nsINode> nextNode = iter.GetCurrentNode();
+ while (nextNode && nsContentUtils::ContentIsDescendantOf(nextNode, node)) {
+ iter.Next();
+ nextNode = iter.GetCurrentNode();
+ }
handled = false;
// If it's CharacterData, make sure we might need to delete
// part of the data, instead of removing the whole node.
//
// XXX_kin: We need to also handle ProcessingInstruction
// XXX_kin: according to the spec.
@@ -2272,22 +2277,21 @@ nsRange::CutContents(DocumentFragment**
}
uint32_t parentCount = 0;
// Set the result to document fragment if we have 'retval'.
if (retval) {
nsCOMPtr<nsINode> oldCommonAncestor = commonAncestor;
if (!iter.IsDone()) {
// Setup the parameters for the next iteration of the loop.
- nsCOMPtr<nsINode> prevNode = iter.GetCurrentNode();
- NS_ENSURE_STATE(prevNode);
-
- // Get node's and prevNode's common parent. Do this before moving
+ NS_ENSURE_STATE(nextNode);
+
+ // Get node's and nextNode's common parent. Do this before moving
// nodes from original DOM to result fragment.
- commonAncestor = nsContentUtils::GetCommonAncestor(node, prevNode);
+ commonAncestor = nsContentUtils::GetCommonAncestor(node, nextNode);
NS_ENSURE_STATE(commonAncestor);
nsCOMPtr<nsINode> parentCounterNode = node;
while (parentCounterNode && parentCounterNode != commonAncestor)
{
++parentCount;
parentCounterNode = parentCounterNode->GetParentNode();
NS_ENSURE_STATE(parentCounterNode);
@@ -2296,28 +2300,36 @@ nsRange::CutContents(DocumentFragment**
// Clone the parent hierarchy between commonAncestor and node.
nsCOMPtr<nsINode> closestAncestor, farthestAncestor;
rv = CloneParentsBetween(oldCommonAncestor, node,
getter_AddRefs(closestAncestor),
getter_AddRefs(farthestAncestor));
NS_ENSURE_SUCCESS(rv, rv);
+ ErrorResult res;
if (farthestAncestor)
{
nsCOMPtr<nsINode> n = do_QueryInterface(commonCloneAncestor);
- rv = PrependChild(n, farthestAncestor);
- NS_ENSURE_SUCCESS(rv, rv);
+ n->AppendChild(*farthestAncestor, res);
+ if (NS_WARN_IF(res.Failed())) {
+ return res.StealNSResult();
+ }
}
nsMutationGuard guard;
nsCOMPtr<nsINode> parent = nodeToResult->GetParentNode();
- rv = closestAncestor ? PrependChild(closestAncestor, nodeToResult)
- : PrependChild(commonCloneAncestor, nodeToResult);
- NS_ENSURE_SUCCESS(rv, rv);
+ if (closestAncestor) {
+ closestAncestor->AppendChild(*nodeToResult, res);
+ } else {
+ commonCloneAncestor->AppendChild(*nodeToResult, res);
+ }
+ if (NS_WARN_IF(res.Failed())) {
+ return res.StealNSResult();
+ }
NS_ENSURE_STATE(!guard.Mutated(parent ? 2 : 1) ||
ValidateCurrentNode(this, iter));
} else if (nodeToResult) {
nsMutationGuard guard;
nsCOMPtr<nsINode> node = nodeToResult;
nsCOMPtr<nsINode> parent = node->GetParentNode();
if (parent) {
mozilla::ErrorResult error;
deleted file mode 100644
--- a/testing/web-platform/meta/dom/nodes/MutationObserver-childList.html.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[MutationObserver-childList.html]
- type: testharness
- [childList Range.surroundContents: children removal and addition mutation]
- expected: FAIL
-