2011-01-20 Levi Weintraub <leviw@chromium.org>
authorleviw@chromium.org <leviw@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jan 2011 01:21:11 +0000 (01:21 +0000)
committerleviw@chromium.org <leviw@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jan 2011 01:21:11 +0000 (01:21 +0000)
        Reviewed by Ryosuke Niwa.

        RTL: Caret goes to the opposite direction when pressing an arrow key after selection is made
        https://bugs.webkit.org/show_bug.cgi?id=49511

        Test: editing/selection/rtl-move-selection-right-left.html

        * editing/SelectionController.cpp:
        (WebCore::SelectionController::willBeModified):
        Respecting the direction of the containing block when switching selection base and extent in
        RTL content.

        (WebCore::SelectionController::modifyMovingRight):
        (WebCore::SelectionController::modifyMovingLeft):
        Using directionOfEnclosingBlock when deciding to use the selection start or end to do the
        correct thing for RTL.
2011-01-20  Levi Weintraub  <leviw@chromium.org>

        Reviewed by Ryosuke Niwa.

        RTL: Caret goes to the opposite direction when pressing an arrow key after selection is made
        https://bugs.webkit.org/show_bug.cgi?id=49511

        * editing/editing.js:
        (execMoveSelectionLeftByCharacterCommand):
        (moveSelectionLeftByCharacterCommand):
        (execMoveSelectionRightByCharacterCommand):
        (moveSelectionRightByCharacterCommand):
        (execExtendSelectionLeftByCharacterCommand):
        (extendSelectionLeftByCharacterCommand):
        (execExtendSelectionRightByCharacterCommand):
        (extendSelectionRightByCharacterCommand):
        Adding some left/right selection convenience methods for use with RTL tests.

        * editing/selection/rtl-move-selection-right-left-expected.txt: Added.
        * editing/selection/rtl-move-selection-right-left.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/editing.js
LayoutTests/editing/selection/rtl-move-selection-right-left-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/rtl-move-selection-right-left.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/SelectionController.cpp

index abeee51..a82f1b1 100644 (file)
@@ -1,3 +1,24 @@
+2011-01-20  Levi Weintraub  <leviw@chromium.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        RTL: Caret goes to the opposite direction when pressing an arrow key after selection is made
+        https://bugs.webkit.org/show_bug.cgi?id=49511
+
+        * editing/editing.js:
+        (execMoveSelectionLeftByCharacterCommand):
+        (moveSelectionLeftByCharacterCommand):
+        (execMoveSelectionRightByCharacterCommand):
+        (moveSelectionRightByCharacterCommand):
+        (execExtendSelectionLeftByCharacterCommand):
+        (extendSelectionLeftByCharacterCommand):
+        (execExtendSelectionRightByCharacterCommand):
+        (extendSelectionRightByCharacterCommand):
+        Adding some left/right selection convenience methods for use with RTL tests.
+
+        * editing/selection/rtl-move-selection-right-left-expected.txt: Added.
+        * editing/selection/rtl-move-selection-right-left.html: Added.
+
 2011-01-20  Tony Chang  <tony@chromium.org>
 
         Unreviewed, mark some rtl scrolling tests as failing to green the chromium bots.
index 53b32f6..9b12cb6 100644 (file)
@@ -39,6 +39,66 @@ function transposeCharactersCommand() {
 
 //-------------------------------------------------------------------------------------------------------
 
+function execMoveSelectionLeftByCharacterCommand() {
+    selection.modify("move", "left", "character");
+}
+function moveSelectionLeftByCharacterCommand() {
+    if (commandDelay > 0) {
+        window.setTimeout(execMoveSelectionLeftByCharacterCommand, commandCount * commandDelay);
+        commandCount++;
+    }
+    else {
+        execMoveSelectionLeftByCharacterCommand();
+    }
+}
+
+//-------------------------------------------------------------------------------------------------------
+
+function execMoveSelectionRightByCharacterCommand() {
+    selection.modify("move", "Right", "character");
+}
+function moveSelectionRightByCharacterCommand() {
+    if (commandDelay > 0) {
+        window.setTimeout(execMoveSelectionRightByCharacterCommand, commandCount * commandDelay);
+        commandCount++;
+    }
+    else {
+        execMoveSelectionRightByCharacterCommand();
+    }
+}
+
+//-------------------------------------------------------------------------------------------------------
+
+function execExtendSelectionLeftByCharacterCommand() {
+    selection.modify("extend", "left", "character");
+}
+function extendSelectionLeftByCharacterCommand() {
+    if (commandDelay > 0) {
+        window.setTimeout(execExtendSelectionLeftByCharacterCommand, commandCount * commandDelay);
+        commandCount++;
+    }
+    else {
+        execExtendSelectionLeftByCharacterCommand();
+    }
+}
+
+//-------------------------------------------------------------------------------------------------------
+
+function execExtendSelectionRightByCharacterCommand() {
+    selection.modify("extend", "Right", "character");
+}
+function extendSelectionRightByCharacterCommand() {
+    if (commandDelay > 0) {
+        window.setTimeout(execExtendSelectionRightByCharacterCommand, commandCount * commandDelay);
+        commandCount++;
+    }
+    else {
+        execExtendSelectionRightByCharacterCommand();
+    }
+}
+
+//-------------------------------------------------------------------------------------------------------
+
 function execMoveSelectionForwardByCharacterCommand() {
     selection.modify("move", "forward", "character");
 }
diff --git a/LayoutTests/editing/selection/rtl-move-selection-right-left-expected.txt b/LayoutTests/editing/selection/rtl-move-selection-right-left-expected.txt
new file mode 100644 (file)
index 0000000..0777815
--- /dev/null
@@ -0,0 +1,17 @@
+Test to make sure left and right arrows keys behave correctly in RTL content.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Move selection right
+PASS Move selection left
+PASS Extend dragged selection right (Mac)
+PASS Extend dragged selection left (Mac)
+PASS Extend dragged selection right (Win)
+PASS Extend dragged selection left (Win)
+PASS Extend dragged selection right (Unix)
+PASS Extend dragged selection left (Unix)
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/selection/rtl-move-selection-right-left.html b/LayoutTests/editing/selection/rtl-move-selection-right-left.html
new file mode 100644 (file)
index 0000000..78a30c4
--- /dev/null
@@ -0,0 +1,92 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
+<link rel="stylesheet" href="../../fast/js/resources/js-test-style.css">
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script>
+description("Test to make sure left and right arrows keys behave correctly in RTL content.")
+
+var testContainer = document.createElement("div");
+testContainer.contentEditable = true;
+testContainer.style.padding = "2px";
+testContainer.dir = "rtl";
+testContainer.innerText = 'מקור השם עברית';
+document.body.insertBefore(testContainer, document.body.firstChild);
+
+var selection = window.getSelection();
+
+function dragSelection()
+{
+    var text = testContainer.firstChild;
+    selection.setBaseAndExtent(text, 0, text, 0);
+    var middleY = testContainer.offsetTop + (testContainer.offsetHeight / 2);
+    eventSender.dragMode = false;
+    eventSender.mouseMoveTo(testContainer.offsetLeft, middleY);
+    eventSender.leapForward(200);
+    eventSender.mouseDown();
+    eventSender.leapForward(200);
+    for (var i = 0; i <= 10; i++) {
+      eventSender.mouseMoveTo(testContainer.offsetLeft + (testContainer.offsetWidth * (i /10)) - 2, middleY);
+      eventSender.leapForward(200);
+    }
+    eventSender.mouseUp();
+    eventSender.leapForward(1000);
+}
+
+function setSelection()
+{
+    var text = testContainer.firstChild;
+
+    selection.setBaseAndExtent(text, 5, text , 8);
+}
+
+function testSelectionChange(setupFunction, mod, dir, startDelta, endDelta, testName)
+{
+    setupFunction();
+    var range = window.getSelection().getRangeAt(0);
+    var initialStart = range.startOffset;
+    var initialEnd = range.endOffset;
+    
+    selection.modify(mod, dir, "character");
+    range = window.getSelection().getRangeAt(0);
+    var start = range.startOffset;
+    var end = range.endOffset;
+    
+    if (initialStart + startDelta != start)
+        testFailed(testName + ": Selection start is " + start + " but should be " + (startDelta + initialStart));
+    else if (initialEnd + endDelta != end)
+        testFailed(testName + ": Selection end is " + end + " but should be " + (endDelta + initialEnd));
+    else
+        testPassed(testName);
+}
+
+function runTests()
+{
+    testSelectionChange(setSelection, "move", "right", 0, -3, "Move selection right");
+    testSelectionChange(setSelection, "move", "left", 3, 0, "Move selection left");
+    if (window.layoutTestController) {
+        layoutTestController.dumpAsText();
+        layoutTestController.setEditingBehavior("mac");
+        testSelectionChange(dragSelection, "extend", "right", 0, 0, "Extend dragged selection right (Mac)");
+        testSelectionChange(dragSelection, "extend", "left", 0, 0, "Extend dragged selection left (Mac)");
+        layoutTestController.setEditingBehavior("win");
+        testSelectionChange(dragSelection, "extend", "right", 0, 0, "Extend dragged selection right (Win)");
+        testSelectionChange(dragSelection, "extend", "left", 1, 0, "Extend dragged selection left (Win)");
+        layoutTestController.setEditingBehavior("unix");
+        testSelectionChange(dragSelection, "extend", "right", 0, 0, "Extend dragged selection right (Unix)");
+        testSelectionChange(dragSelection, "extend", "left", 1, 0, "Extend dragged selection left (Unix)");
+    }
+}
+
+runTests();
+document.body.removeChild(testContainer);
+var successfullyParsed = true;
+</script>
+<script src="../../fast/js/resources/js-test-post.js"></script>
+</body>
+</html>
index 19a4e6e..24ce324 100644 (file)
@@ -1,3 +1,22 @@
+2011-01-20  Levi Weintraub  <leviw@chromium.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        RTL: Caret goes to the opposite direction when pressing an arrow key after selection is made
+        https://bugs.webkit.org/show_bug.cgi?id=49511
+
+        Test: editing/selection/rtl-move-selection-right-left.html
+
+        * editing/SelectionController.cpp:
+        (WebCore::SelectionController::willBeModified):
+        Respecting the direction of the containing block when switching selection base and extent in
+        RTL content.
+
+        (WebCore::SelectionController::modifyMovingRight):
+        (WebCore::SelectionController::modifyMovingLeft):
+        Using directionOfEnclosingBlock when deciding to use the selection start or end to do the
+        correct thing for RTL.
+
 2011-01-20  Nate Chapin  <japhet@chromium.org>
 
         Reviewed by Adam Barth.
index 4406b41..b8bcdda 100644 (file)
@@ -313,32 +313,45 @@ void SelectionController::willBeModified(EAlteration alter, SelectionDirection d
     Position start = m_selection.start();
     Position end = m_selection.end();
 
+    bool baseIsStart;
+
     if (m_isDirectional) {
         // Make base and extent match start and end so we extend the user-visible selection.
         // This only matters for cases where base and extend point to different positions than
         // start and end (e.g. after a double-click to select a word).
-        if (m_selection.isBaseFirst()) {
-            m_selection.setBase(start);
-            m_selection.setExtent(end);            
-        } else {
-            m_selection.setBase(end);
-            m_selection.setExtent(start);
-        }
+        if (m_selection.isBaseFirst())
+            baseIsStart = true;
+        else
+            baseIsStart = false;
     } else {
-        // FIXME: This is probably not correct for right and left when the direction is RTL.
         switch (direction) {
         case DirectionRight:
+            if (directionOfEnclosingBlock() == LTR)
+                baseIsStart = true;
+            else
+                baseIsStart = false;
+            break;
         case DirectionForward:
-            m_selection.setBase(start);
-            m_selection.setExtent(end);
+            baseIsStart = true;
             break;
         case DirectionLeft:
+            if (directionOfEnclosingBlock() == LTR)
+                baseIsStart = false;
+            else
+                baseIsStart = true;
+            break;
         case DirectionBackward:
-            m_selection.setBase(end);
-            m_selection.setExtent(start);
+            baseIsStart = false;
             break;
         }
     }
+    if (baseIsStart) {
+        m_selection.setBase(start);
+        m_selection.setExtent(end);
+    } else {
+        m_selection.setBase(end);
+        m_selection.setExtent(start);
+    }
 }
 
 TextDirection SelectionController::directionOfEnclosingBlock()
@@ -455,9 +468,12 @@ VisiblePosition SelectionController::modifyMovingRight(TextGranularity granulari
     VisiblePosition pos;
     switch (granularity) {
     case CharacterGranularity:
-        if (isRange())
-            pos = VisiblePosition(m_selection.end(), m_selection.affinity());
-        else
+        if (isRange()) {
+            if (directionOfEnclosingBlock() == LTR)
+                pos = VisiblePosition(m_selection.end(), m_selection.affinity());
+            else
+                pos = VisiblePosition(m_selection.start(), m_selection.affinity());
+        } else
             pos = VisiblePosition(m_selection.extent(), m_selection.affinity()).right(true);
         break;
     case WordGranularity:
@@ -609,7 +625,10 @@ VisiblePosition SelectionController::modifyMovingLeft(TextGranularity granularit
     switch (granularity) {
     case CharacterGranularity:
         if (isRange())
-            pos = VisiblePosition(m_selection.start(), m_selection.affinity());
+            if (directionOfEnclosingBlock() == LTR)
+                pos = VisiblePosition(m_selection.start(), m_selection.affinity());
+            else
+                pos = VisiblePosition(m_selection.end(), m_selection.affinity());
         else
             pos = VisiblePosition(m_selection.extent(), m_selection.affinity()).left(true);
         break;