Unreviewed, rolling out r111310.

http://trac.webkit.org/changeset/111310
https://bugs.webkit.org/show_bug.cgi?id=81683

Broke layout test
dom/xhtml/level3/core/nodereplacechild30.xhtml (Requested by
aklein on #webkit).

Patch by Sheriff Bot <webkit.review.bot@gmail.com> on 2012-03-20

* dom/ContainerNode.cpp:
(WebCore::ContainerNode::replaceChild):
(WebCore::ContainerNode::appendChild):
(WebCore::ContainerNode::parserAddChild):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@111415 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 877951e3dda16ddfe09dba08b41a80901560b450..d6d5e27fecd1d0c63f1e28aa1af02ef4ec4b1566 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,18 @@
+2012-03-20  Sheriff Bot  <webkit.review.bot@gmail.com>
+
+        Unreviewed, rolling out r111310.
+        http://trac.webkit.org/changeset/111310
+        https://bugs.webkit.org/show_bug.cgi?id=81683
+
+        Broke layout test
+        dom/xhtml/level3/core/nodereplacechild30.xhtml (Requested by
+        aklein on #webkit).
+
+        * dom/ContainerNode.cpp:
+        (WebCore::ContainerNode::replaceChild):
+        (WebCore::ContainerNode::appendChild):
+        (WebCore::ContainerNode::parserAddChild):
+
 2012-03-20  Gyuyoung Kim  <gyuyoung.kim@samsung.com>
 
         Convert hasSpellingMarker to use Internals interface.

diff --git a/Source/WebCore/dom/ContainerNode.cpp b/Source/WebCore/dom/ContainerNode.cpp
index cc41216dc586e8f321276f9bed8ed8e4a4673522..14f29aef6625a46ff716096a2fbf8d2c5885fe32 100644 (file)
--- a/Source/WebCore/dom/ContainerNode.cpp
+++ b/Source/WebCore/dom/ContainerNode.cpp
@@ -284,28 +284,29 @@ bool ContainerNode::replaceChild(PassRefPtr<Node> newChild, Node* oldChild, Exce
     if (ec)
         return false;
 
-    NodeVector targets;
-    collectTargetNodes(newChild.get(), targets);
+    bool isFragment = newChild->nodeType() == DOCUMENT_FRAGMENT_NODE && !newChild->isShadowRoot();
 
     // Add the new child(ren)
-    for (NodeVector::const_iterator it = targets.begin(); it != targets.end(); ++it) {
-        Node* child = it->get();
-
+    RefPtr<Node> child = isFragment ? newChild->firstChild() : newChild;
+    while (child) {
         // If the new child is already in the right place, we're done.
         if (prev && (prev == child || prev == child->previousSibling()))
             break;
 
+        // For a fragment we have more children to do.
+        RefPtr<Node> nextChild = isFragment ? child->nextSibling() : 0;
+
         // Remove child from its old position.
         if (ContainerNode* oldParent = child->parentNode())
-            oldParent->removeChild(child, ec);
+            oldParent->removeChild(child.get(), ec);
         if (ec)
             return false;
 
         // Due to arbitrary code running in response to a DOM mutation event it's
-        // possible that "next" is no longer a child of "this".
+        // possible that "prev" is no longer a child of "this".
         // It's also possible that "child" has been inserted elsewhere.
         // In either of those cases, we'll just stop.
-        if (next && next->parentNode() != this)
+        if (prev && prev->parentNode() != this)
             break;
         if (child->parentNode())
             break;
@@ -314,22 +315,38 @@ bool ContainerNode::replaceChild(PassRefPtr<Node> newChild, Node* oldChild, Exce
         ASSERT(!child->previousSibling());
 
 #if ENABLE(INSPECTOR)
-        InspectorInstrumentation::willInsertDOMNode(document(), child, this);
+        InspectorInstrumentation::willInsertDOMNode(document(), child.get(), this);
 #endif
 
-        treeScope()->adoptIfNeeded(child);
+        treeScope()->adoptIfNeeded(child.get());
 
-        // Add child before "next".
+        // Add child after "prev".
         forbidEventDispatch();
-        if (next)
-            insertBeforeCommon(next.get(), child);
-        else
-            appendChildToContainer(child, this);
+        Node* next;
+        if (prev) {
+            next = prev->nextSibling();
+            ASSERT(m_firstChild != next);
+            prev->setNextSibling(child.get());
+        } else {
+            next = m_firstChild;
+            m_firstChild = child.get();
+        }
+        if (next) {
+            ASSERT(m_lastChild != prev);
+            ASSERT(next->previousSibling() == prev);
+            next->setPreviousSibling(child.get());
+        } else {
+            ASSERT(m_lastChild == prev);
+            m_lastChild = child.get();
+        }
+        child->setParent(this);
+        child->setPreviousSibling(prev.get());
+        child->setNextSibling(next);
         allowEventDispatch();
 
-        childrenChanged(false, prev.get(), next.get(), 1);
-        notifyChildInserted(child);
-
+        childrenChanged(false, prev.get(), next, 1);
+        notifyChildInserted(child.get());
+                
         // Add child to the rendering tree
         if (attached() && !child->attached() && child->parentNode() == this) {
             if (shouldLazyAttach)
@@ -340,8 +357,10 @@ bool ContainerNode::replaceChild(PassRefPtr<Node> newChild, Node* oldChild, Exce
 
         // Now that the child is attached to the render tree, dispatch
         // the relevant mutation events.
-        dispatchChildInsertionEvents(child);
+        dispatchChildInsertionEvents(child.get());
+
         prev = child;
+        child = nextChild.release();
     }
 
     dispatchSubtreeModifiedEvent();
@@ -620,7 +639,13 @@ bool ContainerNode::appendChild(PassRefPtr<Node> newChild, ExceptionCode& ec, bo
 
         // Append child to the end of the list
         forbidEventDispatch();
-        appendChildToContainer(child, this);
+        child->setParent(this);
+        if (m_lastChild) {
+            child->setPreviousSibling(m_lastChild);
+            m_lastChild->setNextSibling(child);
+        } else
+            m_firstChild = child;
+        m_lastChild = child;
         allowEventDispatch();
 
         // Send notification about the children change.
@@ -653,7 +678,7 @@ void ContainerNode::parserAddChild(PassRefPtr<Node> newChild)
     forbidEventDispatch();
     Node* last = m_lastChild;
     // FIXME: This method should take a PassRefPtr.
-    appendChildToContainer(newChild.get(), this);
+    appendChildToContainer<Node, ContainerNode>(newChild.get(), this);
     treeScope()->adoptIfNeeded(newChild.get());
     
     allowEventDispatch();