<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 3c8ce8fa999ba26903959f1abb07511fb9048de0..1b17de2f3064c86187dc31a2f1ce8ecb8f749126 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 843e4587fa86f22c4371963c22ff9bbed81ae20e..e5042250ec48a2c3a62e6d460b362e3eb672b69d 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 1f15f26efd5ca834b25fbab8b33e9814007366e0..a41fc49a11643724f9b17ddcdd63a79279bd0f9f 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)