2010-05-05 Ojan Vafai <ojan@chromium.org>
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 May 2010 16:58:12 +0000 (16:58 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 May 2010 16:58:12 +0000 (16:58 +0000)
        Reviewed by Darin Adler.

        shift+click on an existing selection doesn't work right
        https://bugs.webkit.org/show_bug.cgi?id=36542

        NSTextView behavior is to move the end of the selection
        closest to the shift-click. Win/Linux behavior is to always
        move the focus end of the selection.

        * editing/selection/script-tests/shift-click.js: Added.
        (shiftClick):
        (assertSelectionString):
        * editing/selection/shift-click-expected.txt: Added.
        * editing/selection/shift-click.html: Added.
        * platform/win/editing/selection/shift-click-expected.txt: Added.
2010-05-05  Ojan Vafai  <ojan@chromium.org>

        Reviewed by Darin Adler.

        shift+click on an existing selection doesn't work right
        https://bugs.webkit.org/show_bug.cgi?id=36542

        NSTextView behavior is to move the end of the selection
        closest to the shift-click. Win/Linux behavior is to always
        move the focus end of the selection.

        Test: editing/selection/shift-click.html

        * page/EventHandler.cpp:
        (WebCore::textDistance):
        (WebCore::EventHandler::handleMousePressEventSingleClick):

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/script-tests/shift-click.js [new file with mode: 0644]
LayoutTests/editing/selection/shift-click-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/shift-click.html [new file with mode: 0644]
LayoutTests/platform/win/editing/selection/shift-click-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/page/EventHandler.cpp

index 895b713..2698b6b 100644 (file)
@@ -1,3 +1,21 @@
+2010-05-05  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        shift+click on an existing selection doesn't work right
+        https://bugs.webkit.org/show_bug.cgi?id=36542
+
+        NSTextView behavior is to move the end of the selection
+        closest to the shift-click. Win/Linux behavior is to always
+        move the focus end of the selection.
+
+        * editing/selection/script-tests/shift-click.js: Added.
+        (shiftClick):
+        (assertSelectionString):
+        * editing/selection/shift-click-expected.txt: Added.
+        * editing/selection/shift-click.html: Added.
+        * platform/win/editing/selection/shift-click-expected.txt: Added.
+
 2010-05-05  Antonio Gomes  <tonikitoo@webkit.org>
 
         Rubber-stamped Kenneth Christiansen.
diff --git a/LayoutTests/editing/selection/script-tests/shift-click.js b/LayoutTests/editing/selection/script-tests/shift-click.js
new file mode 100644 (file)
index 0000000..cde285f
--- /dev/null
@@ -0,0 +1,79 @@
+description("Tests that shift+clicking does the platform correct behavior.");
+
+var first = document.createElement('div');
+first.innerHTML = 'one <span id="start"></span>two three';
+document.body.appendChild(first);
+
+var second = document.createElement('div');
+second.innerHTML = 'four <span id="end"></span>five six';
+document.body.appendChild(second);
+
+var start = document.getElementById('start');
+var end = document.getElementById('end');
+
+function shiftClick(x, y, macExpected, otherExpected)
+{
+    eventSender.mouseMoveTo(x, y);
+    eventSender.mouseDown(0, ['shiftKey']);
+    eventSender.mouseUp(0, ['shiftKey']);
+    assertSelectionString(macExpected, otherExpected);
+}
+
+var onMac = navigator.userAgent.search(/\bMac OS X\b/) != -1;
+
+function assertSelectionString(macExpected, otherExpected)
+{
+    var expected = onMac ? macExpected : otherExpected;
+    if (window.getSelection().toString() == expected)
+        testPassed('window.getSelection().toString() is correct');
+    else
+        testFailed('window.getSelection().toString() is "' + window.getSelection().toString() + '" and should be "' + expected + '"');
+}
+
+function assertSelectionOrder(direction)
+{
+    var expectedPosition;
+    if (direction == 'forward')
+        expectedPosition = Node.DOCUMENT_POSITION_FOLLOWING;
+    else if (direction == 'backward')
+        expectedPosition = Node.DOCUMENT_POSITION_PRECEDING;
+    
+    var sel = window.getSelection();
+    if (sel.anchorNode.compareDocumentPosition(sel.focusNode) == expectedPosition)
+        testPassed("Selection direction is correct.");
+    else
+        testFailed("Selection direction is not correct. Expected a " + direction + " selection." + selectionAsString(sel));
+}
+
+// Double-click select to get around eventSender bug where it won't select
+// text just using single-click.
+if (window.eventSender) {
+    eventSender.mouseMoveTo(start.offsetLeft, start.offsetTop);
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+    eventSender.mouseDown();
+
+    eventSender.mouseMoveTo(end.offsetLeft, end.offsetTop);
+    eventSender.mouseUp();
+
+    assertSelectionString('two three\nfour five', 'two three\nfour five')
+    assertSelectionOrder('forward');
+
+    shiftClick(second.offsetLeft + second.offsetWidth, second.offsetTop, 'two three\nfour five six', 'two three\nfour five six');
+    assertSelectionOrder('forward');
+        
+    shiftClick(end.offsetLeft, end.offsetTop, 'two three\nfour five', 'two three\nfour five');
+    assertSelectionOrder('forward');
+
+    // These two fail on Mac due to https://bugs.webkit.org/show_bug.cgi?id=36256.
+    // In the first shiftClick call, the space after five is selected and shouldn't be.
+    // In the second shiftClick call, "six" is selected and shouldn't be.
+    shiftClick(first.offsetLeft, first.offsetTop, 'one two three\nfour five', 'one two');
+    assertSelectionOrder('backward');
+
+    shiftClick(start.offsetLeft, start.offsetTop, 'two three\nfour five', 'two');
+    // FIXME: The selection direction is incorrect on Win/Linux here. It should be backward.
+    assertSelectionOrder('backward');
+}
+
+var successfullyParsed = true;
diff --git a/LayoutTests/editing/selection/shift-click-expected.txt b/LayoutTests/editing/selection/shift-click-expected.txt
new file mode 100644 (file)
index 0000000..aa14c6d
--- /dev/null
@@ -0,0 +1,24 @@
+Tests that shift+clicking does the platform correct behavior.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.getSelection().toString() is correct
+PASS Selection direction is correct.
+PASS window.getSelection().toString() is correct
+PASS Selection direction is correct.
+PASS window.getSelection().toString() is correct
+PASS Selection direction is correct.
+FAIL window.getSelection().toString() is "one two three
+four five " and should be "one two three
+four five"
+PASS Selection direction is correct.
+FAIL window.getSelection().toString() is "two three
+four five six" and should be "two three
+four five"
+PASS Selection direction is correct.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+one two three
+four five six
diff --git a/LayoutTests/editing/selection/shift-click.html b/LayoutTests/editing/selection/shift-click.html
new file mode 100644 (file)
index 0000000..4291bcc
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script src="resources/js-test-selection-shared.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="script-tests/shift-click.js"></script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/platform/win/editing/selection/shift-click-expected.txt b/LayoutTests/platform/win/editing/selection/shift-click-expected.txt
new file mode 100644 (file)
index 0000000..d1c0850
--- /dev/null
@@ -0,0 +1,20 @@
+Tests that shift+clicking does the platform correct behavior.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.getSelection().toString() is correct
+PASS Selection direction is correct.
+PASS window.getSelection().toString() is correct
+PASS Selection direction is correct.
+PASS window.getSelection().toString() is correct
+PASS Selection direction is correct.
+PASS window.getSelection().toString() is correct
+PASS Selection direction is correct.
+PASS window.getSelection().toString() is correct
+FAIL Selection direction is not correct. Expected a backward selection.[anchorNode: [object Text](two three) anchorOffset: 0 focusNode: [object Text](two three) focusOffset: 3 isCollapsed: false]
+PASS successfullyParsed is true
+
+TEST COMPLETE
+one two three
+four five six
index 73094ae..8f87f47 100644 (file)
@@ -1,3 +1,20 @@
+2010-05-05  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Darin Adler.
+
+        shift+click on an existing selection doesn't work right
+        https://bugs.webkit.org/show_bug.cgi?id=36542
+
+        NSTextView behavior is to move the end of the selection
+        closest to the shift-click. Win/Linux behavior is to always
+        move the focus end of the selection.
+
+        Test: editing/selection/shift-click.html
+
+        * page/EventHandler.cpp:
+        (WebCore::textDistance):
+        (WebCore::EventHandler::handleMousePressEventSingleClick):
+
 2010-05-06  Pavel Feldman  <pfeldman@chromium.org>
 
         Reviewed by Timothy Hatcher.
index 4acef14..f07f25b 100644 (file)
@@ -66,6 +66,7 @@
 #include "SelectionController.h"
 #include "Settings.h"
 #include "TextEvent.h"
+#include "TextIterator.h"
 #include "UserGestureIndicator.h"
 #include "WheelEvent.h"
 #include "htmlediting.h" // for comparePositions()
@@ -332,6 +333,12 @@ bool EventHandler::handleMousePressEventTripleClick(const MouseEventWithHitTestR
     return true;
 }
 
+static int textDistance(const Position& start, const Position& end)
+{
+     RefPtr<Range> range = Range::create(start.node()->document(), start, end);
+     return TextIterator::rangeLength(range.get(), true);
+}
+
 bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestResults& event)
 {
     Node* innerNode = event.targetNode();
@@ -362,14 +369,21 @@ bool EventHandler::handleMousePressEventSingleClick(const MouseEventWithHitTestR
     if (extendSelection && newSelection.isCaretOrRange()) {
         m_frame->selection()->setIsDirectional(false);
         
-        // See <rdar://problem/3668157> REGRESSION (Mail): shift-click deselects when selection 
-        // was created right-to-left
-        Position start = newSelection.start();
-        Position end = newSelection.end();
-        if (comparePositions(pos, start) <= 0)
-            newSelection = VisibleSelection(pos, end);
-        else
-            newSelection = VisibleSelection(start, pos);
+        ASSERT(m_frame->settings());
+        if (m_frame->settings()->editingBehavior() == EditingMacBehavior) {
+            // See <rdar://problem/3668157> REGRESSION (Mail): shift-click deselects when selection
+            // was created right-to-left
+            Position start = newSelection.start();
+            Position end = newSelection.end();
+            int distanceToStart = textDistance(start, pos);
+            int distanceToEnd = textDistance(pos, end);
+            if (distanceToStart <= distanceToEnd)
+                newSelection = VisibleSelection(end, pos);
+            else
+                newSelection = VisibleSelection(start, pos);
+        } else {
+            newSelection.setExtent(pos);
+        }
 
         if (m_frame->selectionGranularity() != CharacterGranularity) {
             granularity = m_frame->selectionGranularity();