<http://webkit.org/b/52512> REGRESSION(r73818): range.cloneContents() ignores end...
authorddkilzer@apple.com <ddkilzer@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Jan 2011 20:44:35 +0000 (20:44 +0000)
committerddkilzer@apple.com <ddkilzer@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 15 Jan 2011 20:44:35 +0000 (20:44 +0000)
Reviewed by Adele Peterson.

WebCore:

The fix for Bug 50710 in r73799 introduced an off-by-one error
when copying nodes to a local NodeVector for processing.  A fix
was attempted for Bug 50854 in r73818, but instead of stopping
at the end offset, it iterates through all the sibling nodes
because the loop variable (i) is never incremented.  To clean
this up, revert back to the code in r73799 and fix the
off-by-one error.

Test: fast/dom/Range/range-clone-contents.html

* dom/Range.cpp:
(WebCore::Range::processContents): Fix the loop that copies
nodes to a local NodeVector by restoring the code from r73799
and fixing the off-by-one error.

LayoutTests:

* fast/dom/Range/range-clone-contents-expected.txt: Added.
* fast/dom/Range/range-clone-contents.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/dom/Range/range-clone-contents-expected.txt [new file with mode: 0644]
LayoutTests/fast/dom/Range/range-clone-contents.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Range.cpp

index 3c8ce8f..1b17de2 100644 (file)
@@ -1,3 +1,12 @@
+2011-01-15  David Kilzer  <ddkilzer@apple.com>
+
+        <http://webkit.org/b/52512> REGRESSION(r73818): range.cloneContents() ignores end offset
+
+        Reviewed by Adele Peterson.
+
+        * fast/dom/Range/range-clone-contents-expected.txt: Added.
+        * fast/dom/Range/range-clone-contents.html: Added.
+
 2011-01-15  Robert Hogan  <robert@webkit.org>
 
         Reviewed by Kenneth Rohde Christiansen.
diff --git a/LayoutTests/fast/dom/Range/range-clone-contents-expected.txt b/LayoutTests/fast/dom/Range/range-clone-contents-expected.txt
new file mode 100644 (file)
index 0000000..f0d50df
--- /dev/null
@@ -0,0 +1,36 @@
+test contents:
+| <div>
+|   <a>
+|     href="#"
+|     "link"
+| <div>
+| <div>
+|   <br>
+|   <br>
+|   <img>
+|     id="img-tag"
+|     src=""
+| <div>
+|   <br>
+|   <br>
+|   "text"
+
+PASS: Cloning right complex subtree yields:
+| <div>
+|   <a>
+|     href="#"
+|     "link"
+| <div>
+| <div>
+|   <br>
+|   <br>
+
+PASS: Cloning left complex subtree yields:
+| <div>
+| <div>
+|   <br>
+|   <br>
+|   "text"
+
+DONE
+
diff --git a/LayoutTests/fast/dom/Range/range-clone-contents.html b/LayoutTests/fast/dom/Range/range-clone-contents.html
new file mode 100644 (file)
index 0000000..2df7dfd
--- /dev/null
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../../resources/dump-as-markup.js"></script>
+</head>
+<body>
+<div id="test"><div><a href="#">link</a></div><div></div><div><br><br><img src="" id="img-tag"></div><div><br><br>text</div></div>
+<div id="expectations">
+    <div id="test_right_complex_subtree"><div><a href="#">link</a></div><div></div><div><br><br></div></div>
+    <div id="test_left_complex_subtree"><div></div><div><br><br>text</div></div>
+</div>
+<pre id="console"></pre>
+<script>
+
+function log(message) {
+    document.getElementById('console').innerHTML += message + '\n';
+}
+
+function testCloneContents(description, range, expectedContentsId) {
+    var actualContents = range.cloneContents();
+
+    var action = description + ' yields:\n' + Markup.get(actualContents).replace(/</g, '&lt;');
+    var expectedContents = document.getElementById(expectedContentsId);
+    if (Markup.get(actualContents) == Markup.get(expectedContents))
+        log('PASS: ' + action);
+    else
+        log('FAIL: ' + action + '\n but expected:\n' + Markup.get(expectedContents).replace(/</g, '&lt;'));
+
+    log('');
+}
+
+Markup.noAutoDump();
+
+var test = document.getElementById('test');
+log('test contents:\n' + Markup.get(test).replace(/</g, '&lt;') + '\n')
+
+var range = document.createRange();
+
+range.setStartBefore(test.firstChild);
+var imgTag = document.getElementById('img-tag')
+range.setEndBefore(imgTag);
+
+testCloneContents('Cloning right complex subtree', range, 'test_right_complex_subtree');
+
+range.setStartAfter(imgTag);
+range.setEndAfter(test.lastChild);
+
+testCloneContents('Cloning left complex subtree', range, 'test_left_complex_subtree');
+
+test.style.display = 'none';
+document.getElementById('expectations').style.display = 'none';
+
+log('DONE');
+
+</script>
+</body>
+</html>
index 843e458..e504225 100644 (file)
@@ -1,3 +1,24 @@
+2011-01-15  David Kilzer  <ddkilzer@apple.com>
+
+        <http://webkit.org/b/52512> REGRESSION(r73818): range.cloneContents() ignores end offset
+
+        Reviewed by Adele Peterson.
+
+        The fix for Bug 50710 in r73799 introduced an off-by-one error
+        when copying nodes to a local NodeVector for processing.  A fix
+        was attempted for Bug 50854 in r73818, but instead of stopping
+        at the end offset, it iterates through all the sibling nodes
+        because the loop variable (i) is never incremented.  To clean
+        this up, revert back to the code in r73799 and fix the
+        off-by-one error.
+
+        Test: fast/dom/Range/range-clone-contents.html
+
+        * dom/Range.cpp:
+        (WebCore::Range::processContents): Fix the loop that copies
+        nodes to a local NodeVector by restoring the code from r73799
+        and fixing the off-by-one error.
+
 2011-01-15  Adam Barth  <abarth@webkit.org>
 
         Rubber-stamped by Eric Seidel.
index 1f15f26..a41fc49 100644 (file)
@@ -794,13 +794,8 @@ PassRefPtr<DocumentFragment> Range::processContents(ActionType action, Exception
             Node* n = m_end.container()->firstChild();
             if (n && m_end.offset()) {
                 NodeVector nodes;
-                int i = 0;
-                do {
+                for (int i = 0; i < m_end.offset() && n; i++, n = n->nextSibling())
                     nodes.append(n);
-                    if (!n->nextSibling())
-                        break;
-                    n = n->nextSibling();
-                } while (i + 1 < m_end.offset());
                 for (int i = nodes.size() - 1; i >= 0; i--) {
                     n = nodes[i].get();
                     if (action == EXTRACT_CONTENTS)