2011-02-15 Ryosuke Niwa <rniwa@webkit.org>
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Feb 2011 05:05:37 +0000 (05:05 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Feb 2011 05:05:37 +0000 (05:05 +0000)
        Reviewed by Darin Adler.

        Extract a function to process ancestor and their sibling nodes from processContents
        https://bugs.webkit.org/show_bug.cgi?id=54425

        Extracted processAncestorsAndTheirSiblings.

        * dom/Range.cpp:
        (WebCore::Range::processContents): Calls processContents.
        (WebCore::Range::processAncestorsAndTheirSiblings): Extracted from processContents.
        * dom/Range.h:

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

Source/WebCore/ChangeLog
Source/WebCore/dom/Range.cpp
Source/WebCore/dom/Range.h

index a0ebfd2..66835a5 100644 (file)
@@ -1,3 +1,17 @@
+2011-02-15  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Reviewed by Darin Adler.
+
+        Extract a function to process ancestor and their sibling nodes from processContents
+        https://bugs.webkit.org/show_bug.cgi?id=54425
+
+        Extracted processAncestorsAndTheirSiblings.
+
+        * dom/Range.cpp:
+        (WebCore::Range::processContents): Calls processContents.
+        (WebCore::Range::processAncestorsAndTheirSiblings): Extracted from processContents.
+        * dom/Range.h:
+
 2011-02-15  Kent Tamura  <tkent@chromium.org>
 
         Reviewed by Darin Fisher.
index e224843..99e54d1 100644 (file)
@@ -657,63 +657,16 @@ PassRefPtr<DocumentFragment> Range::processContents(ActionType action, Exception
     RefPtr<Node> leftContents;
     if (m_start.container() != commonRoot) {
         leftContents = processContentsBetweenOffsets(action, 0, m_start.container(), m_start.offset(), lengthOfContentsInNode(), ec);
-
-        NodeVector ancestorNodes;
-        for (ContainerNode* n = m_start.container()->parentNode(); n && n != commonRoot; n = n->parentNode())
-            ancestorNodes.append(n);
-        RefPtr<Node> n = m_start.container()->nextSibling();
-        for (NodeVector::const_iterator it = ancestorNodes.begin(); it != ancestorNodes.end(); it++) {
-            Node* leftParent = it->get();
-            if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
-                RefPtr<Node> leftContentsParent = leftParent->cloneNode(false);
-                if (leftContentsParent) { // Might have been removed already during mutation event.
-                    leftContentsParent->appendChild(leftContents, ec);
-                    leftContents = leftContentsParent;
-                }
-            }
-
-            RefPtr<Node> next;
-            for (; n; n = next) {
-                next = n->nextSibling();
-                if (action == EXTRACT_CONTENTS)
-                    leftContents->appendChild(n.get(), ec); // will remove n from leftParent
-                else if (action == CLONE_CONTENTS)
-                    leftContents->appendChild(n->cloneNode(true), ec);
-                else
-                    leftParent->removeChild(n.get(), ec);
-            }
-            n = leftParent->nextSibling();
-        }
+        leftContents = processAncestorsAndTheirSiblings(action, m_start.container(), ProcessContentsForward, leftContents, commonRoot, ec);
     }
 
     RefPtr<Node> rightContents;
     if (m_end.container() != commonRoot) {
         rightContents = processContentsBetweenOffsets(action, 0, m_end.container(), 0, m_end.offset(), ec);
-
-        ContainerNode* rightParent = m_end.container()->parentNode();
-        Node* n = m_end.container()->previousSibling();
-        for (; rightParent != commonRoot; rightParent = rightParent->parentNode()) {
-            if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
-                RefPtr<Node> rightContentsParent = rightParent->cloneNode(false);
-                rightContentsParent->appendChild(rightContents, ec);
-                rightContents = rightContentsParent;
-            }
-            Node* prev;
-            for (; n; n = prev) {
-                prev = n->previousSibling();
-                if (action == EXTRACT_CONTENTS)
-                    rightContents->insertBefore(n, rightContents->firstChild(), ec); // will remove n from its parent
-                else if (action == CLONE_CONTENTS)
-                    rightContents->insertBefore(n->cloneNode(true), rightContents->firstChild(), ec);
-                else
-                    rightParent->removeChild(n, ec);
-            }
-            n = rightParent->previousSibling();
-        }
+        rightContents = processAncestorsAndTheirSiblings(action, m_end.container(), ProcessContentsBackward, rightContents, commonRoot, ec);
     }
 
     // delete all children of commonRoot between the start and end container
-
     Node* processStart; // child of commonRoot
     if (m_start.container() == commonRoot) {
         processStart = m_start.container()->firstChild();
@@ -866,6 +819,53 @@ PassRefPtr<Node> Range::processContentsBetweenOffsets(ActionType action, PassRef
     return result;
 }
 
+PassRefPtr<Node> Range::processAncestorsAndTheirSiblings(ActionType action, Node* container, ContentsProcessDirection direction, PassRefPtr<Node> passedClonedContainer, Node* commonRoot, ExceptionCode& ec)
+{
+    RefPtr<Node> clonedContainer = passedClonedContainer;
+    Vector<RefPtr<Node> > ancestors;
+    for (ContainerNode* n = container->parentNode(); n && n != commonRoot; n = n->parentNode())
+        ancestors.append(n);
+
+    RefPtr<Node> firstChildInAncestorToProcess = direction == ProcessContentsForward ? container->nextSibling() : container->previousSibling();
+    for (Vector<RefPtr<Node> >::const_iterator it = ancestors.begin(); it != ancestors.end(); it++) {
+        RefPtr<Node> ancestor = *it;
+        if (action == EXTRACT_CONTENTS || action == CLONE_CONTENTS) {
+            if (RefPtr<Node> clonedAncestor = ancestor->cloneNode(false)) { // Might have been removed already during mutation event.
+                clonedAncestor->appendChild(clonedContainer, ec);
+                clonedContainer = clonedAncestor;
+            }
+        }
+
+        // Copy siblings of an ancestor of start/end containers
+        // FIXME: This assertion may fail if DOM is modified during mutation event
+        ASSERT(!firstChildInAncestorToProcess || firstChildInAncestorToProcess->parentNode() == ancestor);
+        RefPtr<Node> next;
+        for (Node* child = firstChildInAncestorToProcess.get(); child; child = next.get()) {
+            next = direction == ProcessContentsForward ? child->nextSibling() : child->previousSibling();
+            switch (action) {
+            case DELETE_CONTENTS:
+                ancestor->removeChild(child, ec);
+                break;
+            case EXTRACT_CONTENTS: // will remove child from ancestor
+                if (direction == ProcessContentsForward)
+                    clonedContainer->appendChild(child, ec);
+                else
+                    clonedContainer->insertBefore(child, clonedContainer->firstChild(), ec);
+                break;
+            case CLONE_CONTENTS:
+                if (direction == ProcessContentsForward)
+                    clonedContainer->appendChild(child->cloneNode(true), ec);
+                else
+                    clonedContainer->insertBefore(child->cloneNode(true), clonedContainer->firstChild(), ec);
+                break;
+            }
+        }
+        firstChildInAncestorToProcess = direction == ProcessContentsForward ? ancestor->nextSibling() : ancestor->previousSibling();
+    }
+
+    return clonedContainer;
+}
+
 PassRefPtr<DocumentFragment> Range::extractContents(ExceptionCode& ec)
 {
     checkDeleteExtract(ec);
index 86c1354..0b80a3a 100644 (file)
@@ -147,7 +147,9 @@ private:
 
     enum ActionType { DELETE_CONTENTS, EXTRACT_CONTENTS, CLONE_CONTENTS };
     PassRefPtr<DocumentFragment> processContents(ActionType, ExceptionCode&);
-    PassRefPtr<Node> processContentsBetweenOffsets(ActionType, PassRefPtr<DocumentFragment>, Node*, unsigned startOffset, unsigned endOffset, ExceptionCode&);
+    static PassRefPtr<Node> processContentsBetweenOffsets(ActionType, PassRefPtr<DocumentFragment>, Node*, unsigned startOffset, unsigned endOffset, ExceptionCode&);
+    enum ContentsProcessDirection { ProcessContentsForward, ProcessContentsBackward };
+    static PassRefPtr<Node> processAncestorsAndTheirSiblings(ActionType, Node* container, ContentsProcessDirection, PassRefPtr<Node> clonedContainer, Node* commonRoot, ExceptionCode&);
 
     RefPtr<Document> m_ownerDocument;
     RangeBoundaryPoint m_start;