MacViews: Correct behavior of move and select commands when selection direction changes.

ui/views/cocoa/bridged_native_widget_unittest.mm

Index: ui/views/cocoa/bridged_native_widget_unittest.mm
diff --git a/ui/views/cocoa/bridged_native_widget_unittest.mm b/ui/views/cocoa/bridged_native_widget_unittest.mm
index 1faee145fec4433b886cf07b9ce726cf1dc50e9d..fd014c3328a023c5f49dad80393434a93d9a21ec 100644
--- a/ui/views/cocoa/bridged_native_widget_unittest.mm
+++ b/ui/views/cocoa/bridged_native_widget_unittest.mm
@@ -261,11 +261,11 @@ class BridgedNativeWidgetTest : public BridgedNativeWidgetTestBase {
 
   // Test editing commands in |selectors| against the expectations set by
   // |dummy_text_view_|. This is done by selecting every substring within a set
-  // of test strings (both RTL and non-RTL) and performing every selector on
-  // both the NSTextView and the BridgedContentView hosting a focused
-  // views::TextField to ensure the resulting text and selection ranges match.
-  // |selectors| is an NSArray of NSStrings.
-  void TestEditingCommands(NSArray* selectors);
+  // of test strings and performing every selector on both the NSTextView and
+  // the BridgedContentView hosting a focused views::TextField to ensure the
+  // resulting text and selection ranges match. |selectors| is an NSArray of
+  // NSStrings. If |test_rtl| is false, no RTL test string is used for testing.
+  void TestEditingCommands(NSArray* selectors, bool test_rtl);
 
   std::unique_ptr<views::View> view_;
 
@@ -453,12 +453,15 @@ void BridgedNativeWidgetTest::TestDeleteEnd(SEL sel) {
                     GetActualSelectionRange());
 }
 
-void BridgedNativeWidgetTest::TestEditingCommands(NSArray* selectors) {
-  const base::string16 test_strings[] = {
+void BridgedNativeWidgetTest::TestEditingCommands(NSArray* selectors,
+                                                  bool test_rtl) {
+  std::vector<base::string16> test_strings = {
       base::WideToUTF16(L"ab c"),
-      base::WideToUTF16(L"\x0634\x0632 \x064A")  // RTL string.
   };
 
+  if (test_rtl)
+    test_strings.push_back(base::WideToUTF16(L"\x0634\x0632 \x064A"));
+
   for (const base::string16& test_string : test_strings) {
     for (NSString* selector_string in selectors) {
       SEL sel = NSSelectorFromString(selector_string);
@@ -966,14 +969,11 @@ TEST_F(BridgedNativeWidgetTest, TextInput_MoveEditingCommands) {
     @"moveToLeftEndOfLine:",
     @"moveToRightEndOfLine:"
   ];
-  TestEditingCommands(selectors);
+  TestEditingCommands(selectors, true);
 }
 
-// Todo(karandeepb): Enable this test once the behavior of all move and select
-// commands are fixed.
 // Test move and select commands against expectations set by |dummy_text_view_|.
-TEST_F(BridgedNativeWidgetTest,
-       TextInput_MoveAndSelectEditingCommands_DISABLED) {
+TEST_F(BridgedNativeWidgetTest, TextInput_MoveAndSelectEditingCommands_1) {
   NSArray* selectors = @[
     @"moveBackwardAndModifySelection:",
     @"moveForwardAndModifySelection:",
@@ -991,14 +991,23 @@ TEST_F(BridgedNativeWidgetTest,
     @"pageUpAndModifySelection:",
     @"moveParagraphForwardAndModifySelection:",
     @"moveParagraphBackwardAndModifySelection:",
-    @"moveRightAndModifySelection:",
-    @"moveLeftAndModifySelection:",
-    @"moveWordRightAndModifySelection:",
-    @"moveWordLeftAndModifySelection:",
+  ];
+  TestEditingCommands(selectors, true);
+}
+
+// Test move and select commands against expectations set by |dummy_text_view_|.
+TEST_F(BridgedNativeWidgetTest, TextInput_MoveAndSelectEditingCommands_2) {
+  NSArray* selectors = @[
+    @"moveRightAndModifySelection:", @"moveLeftAndModifySelection:",
+    @"moveWordRightAndModifySelection:", @"moveWordLeftAndModifySelection:",
     @"moveToLeftEndOfLineAndModifySelection:",
     @"moveToRightEndOfLineAndModifySelection:"
   ];
-  TestEditingCommands(selectors);
+
+  // The behavior of move and select commands with visual direction on
+  // NSTextView is a bit buggy for RTL text and is not consistent with LTR
+  // behavior  Hence don't test RTL string.

[tapted, 2016/05/20 06:47:36]

This is probably a bug.... - can you file a new one at crbug with some examples?

[karandeepb, 2016/05/24 07:47:15]

I think the bug is with the NSTextView itself. For example, In RTL consider text
"abc|". If we do moveForwardAndModifySelection, text becomes "ab|c|". Now on
doing moveLeftAndModifySelection, it should become "a|bc|". However it becomes
"ab|c". 
Probably forward/backward commands do not share selection direction with
left/right commands for RTL text, while in LTR, they do. 

+  TestEditingCommands(selectors, false);
 }
 
 // Test delete commands against expectations set by |dummy_text_view_|.
@@ -1008,7 +1017,7 @@ TEST_F(BridgedNativeWidgetTest, TextInput_DeleteCommands) {
     @"deleteWordBackward:", @"deleteToBeginningOfLine:", @"deleteToEndOfLine:",
     @"deleteToBeginningOfParagraph:", @"deleteToEndOfParagraph:"
   ];
-  TestEditingCommands(selectors);
+  TestEditingCommands(selectors, true);
 }
 
 // Test firstRectForCharacterRange:actualRange for cases where query range is