WebCore:
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Mar 2009 02:21:26 +0000 (02:21 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Mar 2009 02:21:26 +0000 (02:21 +0000)
2009-03-29  Darin Adler  <darin@apple.com>

        Reviewed by Dan Bernstein.

        Bug 23445: Copying certain hidden text causes a crash
        https://bugs.webkit.org/show_bug.cgi?id=23445
        rdar://problem/6512520

        Test: editing/pasteboard/copy-display-none.html

        * editing/markup.cpp:
        (WebCore::createMarkup): Added a check for the case where adjusting the start node moves
        the start of the selection past the end of the range entirely. If we try to iterate we'll
        never hit the end of the range and will probably crash iterating the rest of the document.

LayoutTests:

2009-03-29  Darin Adler  <darin@apple.com>

        Reviewed by Dan Bernstein.

        Bug 23445: Copying certain hidden text causes a crash
        https://bugs.webkit.org/show_bug.cgi?id=23445
        rdar://problem/6512520

        * editing/pasteboard/copy-display-none-expected.txt: Added.
        * editing/pasteboard/copy-display-none.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/copy-display-none-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/copy-display-none.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/markup.cpp

index 24073ee..e0a5400 100644 (file)
@@ -1,5 +1,16 @@
 2009-03-29  Darin Adler  <darin@apple.com>
 
+        Reviewed by Dan Bernstein.
+
+        Bug 23445: Copying certain hidden text causes a crash
+        https://bugs.webkit.org/show_bug.cgi?id=23445
+        rdar://problem/6512520
+
+        * editing/pasteboard/copy-display-none-expected.txt: Added.
+        * editing/pasteboard/copy-display-none.html: Added.
+
+2009-03-29  Darin Adler  <darin@apple.com>
+
         Reviewed by Cameron Zwarich.
 
         * fast/forms/targeted-frame-submission.html: Fixes from review comments.
diff --git a/LayoutTests/editing/pasteboard/copy-display-none-expected.txt b/LayoutTests/editing/pasteboard/copy-display-none-expected.txt
new file mode 100644 (file)
index 0000000..3f369f0
--- /dev/null
@@ -0,0 +1,6 @@
+This tests to make sure a copy after making the selection display: none does not hit an assertion.
+
+PASSED
+
+This is text before the test paragraph.
+This is the paragraph after the hidden one.
diff --git a/LayoutTests/editing/pasteboard/copy-display-none.html b/LayoutTests/editing/pasteboard/copy-display-none.html
new file mode 100644 (file)
index 0000000..aa12778
--- /dev/null
@@ -0,0 +1,23 @@
+<head>
+<script>
+function runTest()
+{
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    var paragraph = document.getElementById("paragraph");
+    var textNode = paragraph.firstChild;
+    var range = document.createRange();
+    range.setStart(textNode, 24)
+    range.setEnd(textNode, 28)
+    getSelection().addRange(range);
+    paragraph.setAttribute("style", "display: none");
+    document.execCommand("Copy");
+    document.getElementById("message").firstChild.data = "PASSED";
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>This tests to make sure a copy after making the selection display: none does not hit an assertion.</p>
+<p id="message">TEST NOT COMPLETE</p>
+<div>This is text before the test paragraph. <p id="paragraph">This paragraph will be hidden and then copied.</p></div><p>This is the paragraph after the hidden one.</p>
+</body>
index d7ee771..46db76a 100644 (file)
@@ -2,6 +2,21 @@
 
         Reviewed by Dan Bernstein.
 
+        Bug 23445: Copying certain hidden text causes a crash
+        https://bugs.webkit.org/show_bug.cgi?id=23445
+        rdar://problem/6512520
+
+        Test: editing/pasteboard/copy-display-none.html
+
+        * editing/markup.cpp:
+        (WebCore::createMarkup): Added a check for the case where adjusting the start node moves
+        the start of the selection past the end of the range entirely. If we try to iterate we'll
+        never hit the end of the range and will probably crash iterating the rest of the document.
+
+2009-03-29  Darin Adler  <darin@apple.com>
+
+        Reviewed by Dan Bernstein.
+
         Bug 24672: ASSERTION FAILURE: !m_purgeableData in WebCore::CachedResource::data() saving a WebArchive
         https://bugs.webkit.org/show_bug.cgi?id=24672
         rdar://problem/6574263
index 7de5287..48e20f5 100644 (file)
@@ -798,14 +798,20 @@ String createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotateForInterc
 
         markups.append(interchangeNewlineString);
         startNode = visibleStart.next().deepEquivalent().node();
+
+        if (Range::compareBoundaryPoints(startNode, 0, pastEnd, 0) >= 0) {
+            if (deleteButton)
+                deleteButton->enable();
+            return interchangeNewlineString;
+        }
     }
 
     Node* next;
     for (Node* n = startNode; n != pastEnd; n = next) {
-    
-        // According to <rdar://problem/5730668>, it is possible for n to blow past pastEnd and become null here.  This 
-        // shouldn't be possible.  This null check will prevent crashes (but create too much markup) and the ASSERT will 
-        // hopefully lead us to understanding the problem.
+        // According to <rdar://problem/5730668>, it is possible for n to blow
+        // past pastEnd and become null here. This shouldn't be possible.
+        // This null check will prevent crashes (but create too much markup)
+        // and the ASSERT will hopefully lead us to understanding the problem.
         ASSERT(n);
         if (!n)
             break;