2009-01-12 Eric Roman <eroman@chromium.org>
authordglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Jan 2009 21:07:19 +0000 (21:07 +0000)
committerdglazkov@chromium.org <dglazkov@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Jan 2009 21:07:19 +0000 (21:07 +0000)
        Reviewed by Darin Adler.

        Fix some bugs with Selection::appendTrailingWhitespace().
        https://bugs.webkit.org/show_bug.cgi?id=23232

        Test: editing/selection/doubleclick-whitespace-crash.html

        * editing/Selection.cpp:
        (WebCore::makeSearchRange):
        (WebCore::Selection::appendTrailingWhitespace):

2009-01-12  Eric Roman  <eroman@chromium.org>

        Reviewed by Darin Adler.
        https://bugs.webkit.org/show_bug.cgi?id=23232

        * editing/selection/doubleclick-whitespace-crash-expected.txt: Added.
        * editing/selection/doubleclick-whitespace-crash.html: Added.
        * editing/selection/doubleclick-whitespace-expected.txt:
        * editing/selection/doubleclick-whitespace.html:

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/doubleclick-whitespace-crash-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/doubleclick-whitespace-crash.html [new file with mode: 0644]
LayoutTests/editing/selection/doubleclick-whitespace-expected.txt
LayoutTests/editing/selection/doubleclick-whitespace.html
WebCore/ChangeLog
WebCore/editing/Selection.cpp

index e67d4f6..e6599dc 100644 (file)
@@ -1,3 +1,13 @@
+2009-01-12  Eric Roman  <eroman@chromium.org>
+
+        Reviewed by Darin Adler.
+        https://bugs.webkit.org/show_bug.cgi?id=23232
+
+        * editing/selection/doubleclick-whitespace-crash-expected.txt: Added.
+        * editing/selection/doubleclick-whitespace-crash.html: Added.
+        * editing/selection/doubleclick-whitespace-expected.txt:
+        * editing/selection/doubleclick-whitespace.html:
+
 2009-01-12  Alexey Proskuryakov  <ap@webkit.org>
 
         Update test results.
diff --git a/LayoutTests/editing/selection/doubleclick-whitespace-crash-expected.txt b/LayoutTests/editing/selection/doubleclick-whitespace-crash-expected.txt
new file mode 100644 (file)
index 0000000..1f712bc
--- /dev/null
@@ -0,0 +1,3 @@
+Double-click in the white space below this text block -- should not crash.
+BUG 23232.
+PASS
diff --git a/LayoutTests/editing/selection/doubleclick-whitespace-crash.html b/LayoutTests/editing/selection/doubleclick-whitespace-crash.html
new file mode 100644 (file)
index 0000000..31a60fb
--- /dev/null
@@ -0,0 +1,30 @@
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+     layoutTestController.dumpAsText();
+     layoutTestController.setSmartInsertDeleteEnabled(false);
+     layoutTestController.setSelectTrailingWhitespaceEnabled(true);
+}
+</script>
+</head>
+<body>
+<pre>
+Double-click in the white space below this text block -- should not crash.
+<a href="https://bugs.webkit.org/show_bug.cgi?id=23232">BUG 23232</a>.
+</pre> 
+<script type="text/javascript">
+    if (window.layoutTestController) {
+        // Double click at the end of the body.
+        eventSender.mouseMoveTo(10, 100);
+        eventSender.mouseDown();
+        eventSender.mouseUp();
+        eventSender.mouseDown();
+        eventSender.mouseUp();
+
+        // As long as didn't crash, we passed.
+        document.write("PASS");
+    }
+</script>
+</body>
+</html>
index 96ff98b..ac92296 100644 (file)
@@ -1,6 +1,17 @@
-This tests that double-clicking a word on the Windows platform selects the whitespace after the word.  You should see the word "PASS" below.
-
-PASS
+This tests that double-clicking a word on the Windows platform selects the whitespace after the word.
 
+Doubleclickme     |END|
+Doubleclickme|END|
+Doubleclickme    |END|
+Doubleclickme
+   |(Should not have been extended into this line)|
+Doubleclickme      
+Doubleclickme                          |END|
 
+Passed test1
+Passed test2
+Passed test3
+Passed test4
+Passed test5
+Passed test6
 
index 0d4dcdc..a6ac152 100644 (file)
@@ -6,33 +6,109 @@ if (window.layoutTestController) {
      layoutTestController.setSmartInsertDeleteEnabled(false);
      layoutTestController.setSelectTrailingWhitespaceEnabled(true);
 }
+
+function getPositionOfNode(id)
+{
+    var n = document.getElementById(id);
+    var pos = {x: 0, y: 0};
+
+    while (n) {
+        pos.x += n.offsetLeft + n.clientLeft;
+        pos.y += n.offsetTop + n.clientTop;
+        n = n.offsetParent;
+    }
+    return pos;
+}
+
+function doubleClickNode(id)
+{
+    var pos = getPositionOfNode(id);
+    eventSender.mouseMoveTo(pos.x + 2, pos.y + 2);
+    eventSender.mouseDown();
+    eventSender.leapForward(1);
+    eventSender.mouseUp();
+    eventSender.leapForward(100);
+    eventSender.mouseDown();
+    eventSender.leapForward(1);
+    eventSender.mouseUp();
+}
+
+function doTest(testId, expectedText)
+{
+    // Simulate a double click.
+    doubleClickNode(testId);
+
+    // Get the text of the current selection.
+    var sel = window.getSelection();
+    var actualText = sel.getRangeAt(0).toString();
+
+    if (expectedText == actualText) {
+        log("Passed " + testId);
+    } else {
+        log("Failed " + testId);
+        log("  Expected: " + expectedText);
+        log("  Actual: " + actualText);
+    }
+
+}
+
+function runTests()
+{
+    if (window.layoutTestController) {
+        doTest("test1", "Doubleclickme \u00a0\u00a0\u00a0 ");
+        doTest("test2", "Doubleclickme");
+        doTest("test3", "Doubleclickme \u00a0\u00a0 ");
+        doTest("test4", "Doubleclickme");
+        doTest("test5", "Doubleclickme  \u00a0\u00a0");
+        doTest("test6", "Doubleclickme  \u00a0\t\t\t\u00a0");
+    }
+}
+
+function log(msg)
+{
+    var l = document.getElementById('log');
+    l.appendChild(document.createTextNode(msg));
+    l.appendChild(document.createElement('br'));
+}
+
 </script>
-<style>
-body { margin: 0; padding: 0 }
-</style>
 </head>
-<body>
-<pre>
-This tests that double-clicking a word on the Windows platform selects the whitespace after the word.  You should see the word "PASS" below.
+<body onload="runTests()">
 <p>
-<script type="text/javascript">
-    if (window.layoutTestController) {
-        eventSender.mouseMoveTo(20, 10);
-        eventSender.mouseDown();
-        eventSender.leapForward(1);
-        eventSender.mouseUp();
-        eventSender.leapForward(100);
-        eventSender.mouseDown();
-        eventSender.leapForward(1);
-        eventSender.mouseUp();
-        var sel = window.getSelection();
-        var range = sel.getRangeAt(0);
-        if (range.toString() != "This ")
-            document.write("FAIL");
-        else
-            document.write("PASS");
-    } else
-        document.write("This test is only expected to pass when run in automated mode, with access to LayoutTestController.");
-</script>
+This tests that double-clicking a word on the Windows platform selects the whitespace after the word.
+</p>
+
+<div>
+<span id=test1>Doubleclickme &nbsp;&nbsp;&nbsp; |END|</span>
+</div>
+
+<div>
+<span id=test2>Doubleclickme|END|</span>
+</div>
+
+<div>
+<span id=test3>Doubleclickme</span> &nbsp;&nbsp; 
+|END|</span>
+</div>
+
+<div>
+<div>
+<span id="test4">Doubleclickme</span>
+</div>
+&nbsp;&nbsp;&nbsp;|(Should not have been extended into this line)|
+</div>
+
+<div>
+<span id="test5">Doubleclickme </span> &nbsp;&nbsp;<img src="does-not-exist.png" />&nbsp; &nbsp;
+</div>
+
+<pre>
+<span id="test6">Doubleclickme </span> &nbsp;                  &nbsp;|END|
+</pre>
+
+<br/>
+<pre id=log>
+</pre>
+
 </body>
 </html>
index 88aa9a7..221e281 100644 (file)
@@ -1,3 +1,16 @@
+2009-01-12  Eric Roman  <eroman@chromium.org>
+        Reviewed by Darin Adler.
+        Fix some bugs with Selection::appendTrailingWhitespace().
+        https://bugs.webkit.org/show_bug.cgi?id=23232
+        Test: editing/selection/doubleclick-whitespace-crash.html
+        * editing/Selection.cpp:
+        (WebCore::makeSearchRange):
+        (WebCore::Selection::appendTrailingWhitespace):
+
 2009-01-12  Dimitri Glazkov  <dglazkov@chromium.org>
 
         Reviewed by Eric Seidel.
index ea30f19..4fb3a54 100644 (file)
 #include "config.h"
 #include "Selection.h"
 
+#include "CharacterNames.h"
 #include "CString.h"
 #include "Document.h"
 #include "Element.h"
 #include "htmlediting.h"
+#include "TextIterator.h"
 #include "VisiblePosition.h"
 #include "visible_units.h"
 #include "Range.h"
@@ -193,12 +195,47 @@ bool Selection::expandUsingGranularity(TextGranularity granularity)
     return true;
 }
 
+static PassRefPtr<Range> makeSearchRange(const Position& pos)
+{
+    Node* n = pos.node();
+    if (!n)
+        return 0;
+    Document* d = n->document();
+    Node* de = d->documentElement();
+    if (!de)
+        return 0;
+    Node* boundary = n->enclosingBlockFlowElement();
+    if (!boundary)
+        return 0;
+
+    RefPtr<Range> searchRange(Range::create(d));
+    ExceptionCode ec = 0;
+
+    Position start(rangeCompliantEquivalent(pos));
+    searchRange->selectNodeContents(boundary, ec);
+    searchRange->setStart(start.node(), start.offset(), ec);
+
+    ASSERT(!ec);
+    if (ec)
+        return 0;
+
+    return searchRange.release();
+}
+
 void Selection::appendTrailingWhitespace()
 {
-    VisiblePosition end = VisiblePosition(m_end, m_affinity);
-    while (end.isNotNull() && isSpaceOrNewline(end.characterAfter()))
-        end = end.next();
-    m_end = end.deepEquivalent();
+    RefPtr<Range> searchRange = makeSearchRange(m_end);
+    if (!searchRange)
+        return;
+
+    CharacterIterator charIt(searchRange.get(), true);
+
+    for (; charIt.length(); charIt.advance(1)) {
+        UChar c = charIt.characters()[0];
+        if (!isSpaceOrNewline(c) && c != noBreakSpace)
+            break;
+        m_end = charIt.range()->endPosition();
+    }
 }
 
 void Selection::validate()