Fix inconsistent text selection behavior with option-shift-left/right/up/down.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Jan 2012 02:51:04 +0000 (02:51 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 20 Jan 2012 02:51:04 +0000 (02:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=75652

Patch by Pablo Flouret <pablof@motorola.com> on 2012-01-19
Reviewed by Enrica Casucci.

On Mac, selecting backwards by word, line or paragraph from the middle
of some text, and then going forward leaves the caret back in the middle
with no selection, instead of directly selecting to the other end of the
word/line/paragraph (Unix/Windows behavior). Fix this by adding a new
editing behavior to control whether the selection should go across the
initial position of the caret directly or not in situations like the one
outlined above.

Source/WebCore:

Test: editing/selection/selection-extend-should-not-move-across-caret-on-mac.html

* editing/EditingBehavior.h:
(WebCore::EditingBehavior::shouldExtendSelectionByWordOrLineAcrossCaret):
* editing/FrameSelection.cpp:
(WebCore::FrameSelection::modify):

LayoutTests:

* editing/selection/selection-extend-should-not-move-across-caret-on-mac-expected.txt: Added.
* editing/selection/selection-extend-should-not-move-across-caret-on-mac.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/selection-extend-should-not-move-across-caret-on-mac-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/selection-extend-should-not-move-across-caret-on-mac.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/EditingBehavior.h
Source/WebCore/editing/FrameSelection.cpp

index fe4845e..1e0f026 100644 (file)
@@ -1,3 +1,21 @@
+2012-01-19  Pablo Flouret  <pablof@motorola.com>
+
+        Fix inconsistent text selection behavior with option-shift-left/right/up/down.
+        https://bugs.webkit.org/show_bug.cgi?id=75652
+
+        Reviewed by Enrica Casucci.
+
+        On Mac, selecting backwards by word, line or paragraph from the middle
+        of some text, and then going forward leaves the caret back in the middle
+        with no selection, instead of directly selecting to the other end of the
+        word/line/paragraph (Unix/Windows behavior). Fix this by adding a new
+        editing behavior to control whether the selection should go across the
+        initial position of the caret directly or not in situations like the one
+        outlined above.
+
+        * editing/selection/selection-extend-should-not-move-across-caret-on-mac-expected.txt: Added.
+        * editing/selection/selection-extend-should-not-move-across-caret-on-mac.html: Added.
+
 2012-01-19  Simon Fraser  <simon.fraser@apple.com>
 
         Regression (r98735): Video chat moles in Gmail render incorrectly on Mac OS
diff --git a/LayoutTests/editing/selection/selection-extend-should-not-move-across-caret-on-mac-expected.txt b/LayoutTests/editing/selection/selection-extend-should-not-move-across-caret-on-mac-expected.txt
new file mode 100644 (file)
index 0000000..6fc5dad
--- /dev/null
@@ -0,0 +1,17 @@
+On Mac when word-selecting backwards starting with the caret on the middle of a word and then word-selecting forward, the caret is left in the same place where it was, instead of directly selecting to the end of the word (which is windows/unix behavior).
+
+mac:
+Extend backward and then forward by word: PASS
+Extend forward and then backward by word: PASS
+Extend backward and then forward by line: PASS
+Extend forward and then backward by line: PASS
+Extend backward and then forward by paragraph: PASS
+Extend forward and then backward by paragraph: PASS
+win:
+Extend backward and then forward by word: PASS
+Extend forward and then backward by word: PASS
+Extend backward and then forward by line: PASS
+Extend forward and then backward by line: PASS
+Extend backward and then forward by paragraph: PASS
+Extend forward and then backward by paragraph: PASS
+
diff --git a/LayoutTests/editing/selection/selection-extend-should-not-move-across-caret-on-mac.html b/LayoutTests/editing/selection/selection-extend-should-not-move-across-caret-on-mac.html
new file mode 100644 (file)
index 0000000..45244f5
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE html>
+
+<p>
+On Mac when word-selecting backwards starting with the
+caret on the middle of a word and then word-selecting forward, the
+caret is left in the same place where it was, instead of directly selecting to the end
+of the word (which is windows/unix behavior).
+</p>
+
+<div id="test-div" contenteditable=true>
+    line 1<br>
+    line 2<br>
+    line 3
+</div>
+
+<script>
+function editingTest(behavior) {
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.setEditingBehavior(behavior);
+    }
+
+    function getSetCaretFunction(node, container, offset) {
+        return function () {
+            var selection = window.getSelection();
+            selection.empty();
+
+            var range = document.createRange();
+            range.setStart(container, offset);
+            selection.addRange(range);
+        }
+    }
+
+    function runTest(firstDirection, secondDirection, granularity, expectedText, setCaret) {
+        var selection = window.getSelection();
+        setCaret();
+        selection.modify("extend", firstDirection, granularity);
+        selection.modify("extend", secondDirection, granularity);
+        var s = selection.toString();
+        document.write("Extend " + firstDirection + " and then " + secondDirection + " by " + granularity + ": ");
+        document.write(s === expectedText ? "PASS" : 'FAIL: expected "' + escape(expectedText) + '", got "' + escape(s) + '"');
+        document.write("<br>");
+    }
+
+    var node = document.getElementById("test-div");
+    var children = node.childNodes;
+
+    var wordCaretFunction = getSetCaretFunction(node, children[2], children[2].data.search("ne 2"));
+
+    document.write(behavior + ":<br>");
+    runTest("backward", "forward", "word", behavior == "mac" ? "" : "ne", getSetCaretFunction(node, children[2], children[2].data.search("ne 2")));
+    runTest("forward", "backward", "word", behavior == "mac" ? "" : "li", getSetCaretFunction(node, children[2], children[2].data.search("ne 2")));
+    runTest("backward", "forward", "line", behavior == "mac" ? "" : "1\nline ", getSetCaretFunction(node, children[0], children[0].data.search("1")));
+    runTest("forward", "backward", "line", behavior == "mac" ? "" : "2\nline ", getSetCaretFunction(node, children[4], children[4].data.search("3")));
+    runTest("backward", "forward", "paragraph", behavior == "mac" ? "" : "1\nline ", getSetCaretFunction(node, children[0], children[0].data.search("1")));
+    runTest("forward", "backward", "paragraph", behavior == "mac" ? "" : "2\nline ", getSetCaretFunction(node, children[4], children[4].data.search("3")));
+}
+
+editingTest("mac");
+editingTest("win");
+
+var node = document.getElementById("test-div");
+node.parentNode.removeChild(node);
+</script>
index 213e10c..9a068ca 100755 (executable)
@@ -1,3 +1,25 @@
+2012-01-19  Pablo Flouret  <pablof@motorola.com>
+
+        Fix inconsistent text selection behavior with option-shift-left/right/up/down.
+        https://bugs.webkit.org/show_bug.cgi?id=75652
+
+        Reviewed by Enrica Casucci.
+
+        On Mac, selecting backwards by word, line or paragraph from the middle
+        of some text, and then going forward leaves the caret back in the middle
+        with no selection, instead of directly selecting to the other end of the
+        word/line/paragraph (Unix/Windows behavior). Fix this by adding a new
+        editing behavior to control whether the selection should go across the
+        initial position of the caret directly or not in situations like the one
+        outlined above.
+
+        Test: editing/selection/selection-extend-should-not-move-across-caret-on-mac.html
+
+        * editing/EditingBehavior.h:
+        (WebCore::EditingBehavior::shouldExtendSelectionByWordOrLineAcrossCaret):
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::modify):
+
 2012-01-19  Simon Fraser  <simon.fraser@apple.com>
 
         Regression (r98735): Video chat moles in Gmail render incorrectly on Mac OS
index 720f38a..9ed1d77 100644 (file)
@@ -67,6 +67,11 @@ public:
     // On Mac and Windows, pressing backspace (when it isn't handled otherwise) should navigate back.
     bool shouldNavigateBackOnBackspace() const { return m_type != EditingUnixBehavior; }
 
+    // On Mac, selecting backwards by word/line from the middle of a word/line, and then going
+    // forward leaves the caret back in the middle with no selection, instead of directly selecting
+    // to the other end of the line/word (Unix/Windows behavior).
+    bool shouldExtendSelectionByWordOrLineAcrossCaret() const { return m_type != EditingMacBehavior; }
+
 private:
     EditingBehaviorType m_type;
 };
index 3f2e45f..539cc5b 100644 (file)
@@ -936,6 +936,19 @@ bool FrameSelection::modify(EAlteration alter, SelectionDirection direction, Tex
         moveTo(position, userTriggered);
         break;
     case AlterationExtend:
+        if (!m_selection.isCaret()
+            && (granularity == WordGranularity || granularity == ParagraphGranularity || granularity == LineGranularity)
+            && m_frame && !m_frame->editor()->behavior().shouldExtendSelectionByWordOrLineAcrossCaret()) {
+            // Don't let the selection go across the base position directly. Needed to match mac
+            // behavior when, for instance, word-selecting backwards starting with the caret in
+            // the middle of a word and then word-selecting forward, leaving the caret in the
+            // same place where it was, instead of directly selecting to the end of the word.
+            VisibleSelection newSelection = m_selection;
+            newSelection.setExtent(position);
+            if (m_selection.isBaseFirst() != newSelection.isBaseFirst())
+                position = m_selection.base();
+        }
+
         // Standard Mac behavior when extending to a boundary is grow the selection rather than leaving the
         // base in place and moving the extent. Matches NSTextView.
         if (!m_frame || !m_frame->editor()->behavior().shouldAlwaysGrowSelectionWhenExtendingToBoundary() || m_selection.isCaret() || !isBoundary(granularity))