MacViews: Implement move and move*AndModifySelection commands.

ui/views/cocoa/bridged_native_widget_unittest.mm

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "ui/views/cocoa/bridged_native_widget.h"
 
 #import <Cocoa/Cocoa.h>
 
 #include <memory>
 
 #import "base/mac/foundation_util.h"
 #import "base/mac/mac_util.h"
 #import "base/mac/sdk_forward_declarations.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #import "testing/gtest_mac.h"
 #import "ui/base/cocoa/window_size_constants.h"
 #include "ui/base/ime/input_method.h"
 #import "ui/gfx/mac/coordinate_conversion.h"
 #import "ui/gfx/test/ui_cocoa_test_helper.h"
 #import "ui/views/cocoa/bridged_content_view.h"
 #import "ui/views/cocoa/native_widget_mac_nswindow.h"
 #import "ui/views/cocoa/views_nswindow_delegate.h"
@@ skipping 179 matching lines @@
   Widget::InitParams init_params_;
 };
 
 class BridgedNativeWidgetTest : public BridgedNativeWidgetTestBase {
  public:
   BridgedNativeWidgetTest();
   ~BridgedNativeWidgetTest() override;
 
   // Install a textfield with input type |text_input_type| in the view hierarchy
   // and make it the text input client. Also initializes |dummy_text_view_|.
-  void InstallTextField(const std::string& text,
+  void InstallTextField(const base::string16& text,
                         ui::TextInputType text_input_type);
 
   // Install a textfield with input type ui::TEXT_INPUT_TYPE_TEXT in the view
   // hierarchy and make it the text input client. Also initializes
   // |dummy_text_view_|.
+  void InstallTextField(const base::string16& text);
+
   void InstallTextField(const std::string& text);
 
   // Returns the actual current text for |ns_view_|.
   NSString* GetActualText();
 
   // Returns the expected current text from |dummy_text_view_|.
   NSString* GetExpectedText();
 
   // Returns the actual selection range for |ns_view_|.
   NSRange GetActualSelectionRange();
 
   // Returns the expected selection range from |dummy_text_view_|.
   NSRange GetExpectedSelectionRange();
 
   // Set the selection range for the installed textfield and |dummy_text_view_|.
   void SetSelectionRange(NSRange range);
 
   // Perform command |sel| on |ns_view_| and |dummy_text_view_|.
   void PerformCommand(SEL sel);
 
+  // Make selection from |start| to |end| on installed textfield and

[tapted, 2016/05/16 05:34:41]

textfield -> views::Textfield

[karandeepb, 2016/05/16 11:18:06]

Done.

+  // |dummy_text_view_|. If |start| > |end|, extend selection to left from
+  // |start|.
+  void MakeSelection(int start, int end);
+
   // testing::Test:
   void SetUp() override;
   void TearDown() override;
 
  protected:
   // Test delete to beginning of line or paragraph based on |sel|. |sel| can be
   // either deleteToBeginningOfLine: or deleteToBeginningOfParagraph:.
   void TestDeleteBeginning(SEL sel);
 
   // Test delete to end of line or paragraph based on |sel|. |sel| can be
   // either deleteToEndOfLine: or deleteToEndOfParagraph:.
   void TestDeleteEnd(SEL sel);
 
+  // Test editing commands in array |selectors| against the expectations set by

[tapted, 2016/05/16 05:34:40]

needs to say |selectors| is an NSArray of NSStrings.

(you could also declare a array literal of SEL (~ function pointers), but
they're harder to iterate over).

[karandeepb, 2016/05/16 11:18:06]

Done.

+  // |dummy_text_view_|.
+  void TestEditingCommands(NSArray* selectors);
+
   std::unique_ptr<views::View> view_;
 
   // Weak. Owned by bridge().
   BridgedContentView* ns_view_;
 
   // An NSTextView which helps set the expectations for our tests.
   base::scoped_nsobject<NSTextView> dummy_text_view_;
 
   base::MessageLoopForUI message_loop_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(BridgedNativeWidgetTest);
 };
 
 BridgedNativeWidgetTest::BridgedNativeWidgetTest() {
 }
 
 BridgedNativeWidgetTest::~BridgedNativeWidgetTest() {
 }
 
 void BridgedNativeWidgetTest::InstallTextField(
-    const std::string& text,
+    const base::string16& text,
     ui::TextInputType text_input_type) {
   Textfield* textfield = new Textfield();
-  textfield->SetText(ASCIIToUTF16(text));
+  textfield->SetText(text);
   textfield->SetTextInputType(text_input_type);
   view_->RemoveAllChildViews(true);
   view_->AddChildView(textfield);
   textfield->SetBoundsRect(init_params_.bounds);
 
   // Request focus so the InputMethod can dispatch events to the RootView, and
   // have them delivered to the textfield. Note that focusing a textfield
   // schedules a task to flash the cursor, so this requires |message_loop_|.
   textfield->RequestFocus();
 
   [ns_view_ setTextInputClient:textfield];
 
   // Initialize the dummy text view.
   dummy_text_view_.reset([[NSTextView alloc] initWithFrame:NSZeroRect]);
-  [dummy_text_view_ setString:SysUTF8ToNSString(text)];
+  [dummy_text_view_ setString:SysUTF16ToNSString(text)];
+}
+
+void BridgedNativeWidgetTest::InstallTextField(const base::string16& text) {
+  InstallTextField(text, ui::TEXT_INPUT_TYPE_TEXT);
 }
 
 void BridgedNativeWidgetTest::InstallTextField(const std::string& text) {
-  InstallTextField(text, ui::TEXT_INPUT_TYPE_TEXT);
+  InstallTextField(base::ASCIIToUTF16(text), ui::TEXT_INPUT_TYPE_TEXT);
 }
 
 NSString* BridgedNativeWidgetTest::GetActualText() {
   NSRange range = NSMakeRange(0, NSUIntegerMax);
   return [[ns_view_ attributedSubstringForProposedRange:range
                                             actualRange:nullptr] string];
 }
 
 NSString* BridgedNativeWidgetTest::GetExpectedText() {
   NSRange range = NSMakeRange(0, NSUIntegerMax);
@@ skipping 15 matching lines @@
   client->SetSelectionRange(gfx::Range(range));
 
   [dummy_text_view_ setSelectedRange:range];
 }
 
 void BridgedNativeWidgetTest::PerformCommand(SEL sel) {
   [ns_view_ doCommandBySelector:sel];
   [dummy_text_view_ doCommandBySelector:sel];
 }
 
+void BridgedNativeWidgetTest::MakeSelection(int start, int end) {
+  ui::TextInputClient* client = [ns_view_ textInputClient];
+  client->SetSelectionRange(gfx::Range(start, end));
+
+  // NSTextView does not support specifying the selection "direction" i.e. the

[tapted, 2016/05/16 05:34:41]

It does :)

Perhaps try [dummy_text_view_ setSelectedRange:range
affinity:(NSSelectionAffinityUpstream <or> NSSelectionAffinityDownstream)
stillSelecting:NO];

[karandeepb, 2016/05/16 11:18:06]

Can't get it to work. For example consider this test-
TEST_F(BridgedNativeWidgetTest, XYZ) {
  NSTextView* dummy_text_view_ = [[NSTextView alloc] init];
  [dummy_text_view_ setString: @"12345"];

  // Select "23" from left to right.
  [dummy_text_view_ setSelectedRange: NSMakeRange(1,2)
affinity:NSSelectionAffinityDownstream stillSelecting:NO];

  [dummy_text_view_
doCommandBySelector:@selector(moveWordRightAndModifySelection:)];

  // State should be "1|2345|".
  EXPECT_EQ_RANGE(NSMakeRange(1, 4) ,[dummy_text_view_ selectedRange]);

  // Select "23" from right to left.
  [dummy_text_view_ setSelectedRange: NSMakeRange(1,2)
affinity:NSSelectionAffinityUpstream stillSelecting:NO];

  [dummy_text_view_
doCommandBySelector:@selector(moveWordRightAndModifySelection:)];

  // State should be "123|45".
  EXPECT_EQ_RANGE(NSMakeRange(3, 0) ,[dummy_text_view_ selectedRange]);
}

The last test fails, since "1|2345|" is selected. However, it passes with the
current MakeSelection implementation.

Also see
http://stackoverflow.com/questions/20116963/how-can-i-get-the-selection-direc...

[tapted, 2016/05/17 04:02:57]

Ah, can you update the comment then? i.e. we shouldn't say NSTextView doesn't
support it, but something to the effect that TSTextView can accept an
upstream/downstream affinity, but it doesn't work because of the reasons you
specified.

[karandeepb, 2016/05/18 01:33:18]

Done.

+  // leading edge of selection. Hence we extend the selection from |start| to
+  // |end|.
+  [dummy_text_view_ setSelectedRange:NSMakeRange(start, 0)];
+  SEL sel = start > end ? @selector(moveBackwardAndModifySelection:)
+                        : @selector(moveForwardAndModifySelection:);
+  size_t delta = std::abs(end - start);
+
+  for (size_t i = 0; i < delta; i++)
+    [dummy_text_view_ doCommandBySelector:sel];
+}
+
 void BridgedNativeWidgetTest::SetUp() {
   BridgedNativeWidgetTestBase::SetUp();
 
   view_.reset(new views::internal::RootView(widget_.get()));
   base::scoped_nsobject<NSWindow> window([test_window() retain]);
 
   // BridgedNativeWidget expects to be initialized with a hidden (deferred)
   // window.
   [window orderOut:nil];
   EXPECT_FALSE([window delegate]);
@@ skipping 67 matching lines @@
 
   // Make a selection as- "|foo |bar".
   SetSelectionRange(NSMakeRange(0, 4));
   PerformCommand(sel);
   // Verify only the selection is deleted so that the state is "|bar".
   EXPECT_NSEQ_3(@"bar", GetExpectedText(), GetActualText());
   EXPECT_EQ_RANGE_3(NSMakeRange(0, 0), GetExpectedSelectionRange(),
                     GetActualSelectionRange());
 }
 
+void BridgedNativeWidgetTest::TestEditingCommands(NSArray* selectors) {

[tapted, 2016/05/16 05:34:40]

nit: comment

[karandeepb, 2016/05/16 11:18:07]

Doesn't the comment in the class declaration suffice? It seems we don't
generally comment on function definitions.

[tapted, 2016/05/17 04:02:57]

Ah, I thought this was in an anon namespace for some reason. But I added that
note here because this method has some extra logic that isn't captured in any
comment. Specifically the range stuff. So either the comment on the earlier
declaration should be expanded with a sentence like ~"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 NSTextField and the BridgedContentView
hosting a focused views::TextField to ensure the resulting cursor and selection
ranges match."

or something equivalent inside the function here.

[karandeepb, 2016/05/18 01:33:18]

Done.

+  struct {
+    const std::wstring test_string;

[tapted, 2016/05/16 05:34:41]

string16? wstring doesn't get used much. You'll probably need to have
base::WideToUTF16 in the aggregate initializer list

[karandeepb, 2016/05/16 11:18:06]

Done. Also a question. From what I saw, we use wstring for hardcoding non-ascii
strings generally. Is it the case that we don't use plain std::strings for
these, since the encoding will be implementation defined? 

[tapted, 2016/05/17 04:02:56]

yeah - wchar_t has different meanings on different platforms - I think it's even
UCS4 (i.e. 32 bit) on some (maybe that's Windows in fact).

+    bool is_rtl;
+  } cases[] = {{L"abc def", false},

[tapted, 2016/05/16 05:34:41]

Can we get equivalent coverage by making these shorter? I see we would want a
space, but "ab cd" should be sufficient. perhaps even "ab c"?

[karandeepb, 2016/05/16 11:18:06]

Done.

+               {L"\x0634\x0632\x0630 \x064A\x062B\x0628", true}};
+
+  for (auto test_case : cases) {

[tapted, 2016/05/16 05:34:40]

nit: const auto&

[karandeepb, 2016/05/16 11:18:06]

Done.

+    for (NSString* selector_string in selectors) {
+      SEL sel = NSSelectorFromString(selector_string);
+      const int len = test_case.test_string.length();
+      for (int i = 0; i <= len; i++) {
+        for (int j = 0; j <= len; j++) {
+          SCOPED_TRACE(base::StringPrintf(
+              "Testing range [%d-%d] for case %s and selector %s\n", i, j,
+              base::WideToUTF8(test_case.test_string).c_str(),
+              base::SysNSStringToUTF8(selector_string).c_str()));
+
+          InstallTextField(base::WideToUTF16(test_case.test_string));
+          MakeSelection(i, j);
+          EXPECT_EQ_RANGE_3(NSMakeRange(std::min(i, j), std::abs(i - j)),
+                            GetExpectedSelectionRange(),
+                            GetActualSelectionRange());
+
+          PerformCommand(sel);
+          EXPECT_NSEQ(GetExpectedText(), GetActualText());
+          EXPECT_EQ_RANGE(GetExpectedSelectionRange(),
+                          GetActualSelectionRange());
+        }
+      }
+    }
+  }
+}
+
 // The TEST_VIEW macro expects the view it's testing to have a superview. In
 // these tests, the NSView bridge is a contentView, at the root. These mimic
 // what TEST_VIEW usually does.
 TEST_F(BridgedNativeWidgetTest, BridgedNativeWidgetTest_TestViewAddRemove) {
   base::scoped_nsobject<BridgedContentView> view([bridge()->ns_view() retain]);
   EXPECT_NSEQ([test_window() contentView], view);
   EXPECT_NSEQ(test_window(), [view window]);
 
   // The superview of a contentView is an NSNextStepFrame.
   EXPECT_TRUE([view superview]);
@@ skipping 113 matching lines @@
   PerformInit();
   EXPECT_TRUE([window_ hasShadow]);  // Preserves shadow.
 
   window_.reset();
   widget_.reset();
 }
 
 // Ensure a nil NSTextInputContext is returned when the ui::TextInputClient is
 // not editable, a password field, or unset.
 TEST_F(BridgedNativeWidgetTest, InputContext) {
-  const std::string test_string = "test_str";
+  const base::string16 test_string = base::ASCIIToUTF16("test_str");
   InstallTextField(test_string, ui::TEXT_INPUT_TYPE_PASSWORD);
   EXPECT_FALSE([ns_view_ inputContext]);
   InstallTextField(test_string, ui::TEXT_INPUT_TYPE_TEXT);
   EXPECT_TRUE([ns_view_ inputContext]);
   [ns_view_ setTextInputClient:nil];
   EXPECT_FALSE([ns_view_ inputContext]);
   InstallTextField(test_string, ui::TEXT_INPUT_TYPE_NONE);
   EXPECT_FALSE([ns_view_ inputContext]);
 }
 
@@ skipping 302 matching lines @@
 }
 
 TEST_F(BridgedNativeWidgetTest, TextInput_DeleteToBeginningOfParagraph) {
   TestDeleteBeginning(@selector(deleteToBeginningOfParagraph:));
 }
 
 TEST_F(BridgedNativeWidgetTest, TextInput_DeleteToEndOfParagraph) {
   TestDeleteEnd(@selector(deleteToEndOfParagraph:));
 }
 
+// Test move commands against expectations set by |dummy_text_view_|.
+TEST_F(BridgedNativeWidgetTest, TextInput_MoveEditingCommands) {
+  NSArray* selectors = @[
+    @"moveForward:",
+    @"moveRight:",
+    @"moveBackward:",
+    @"moveLeft:",
+    @"moveUp:",
+    @"moveDown:",
+    @"moveWordForward:",
+    @"moveWordBackward:",
+    @"moveToBeginningOfLine:",
+    @"moveToEndOfLine:",
+    @"moveToBeginningOfParagraph:",
+    @"moveToEndOfParagraph:",
+    @"moveToEndOfDocument:",
+    @"moveToBeginningOfDocument:",
+    @"pageDown:",
+    @"pageUp:",
+    @"moveWordRight:",
+    @"moveWordLeft:",
+    @"moveToLeftEndOfLine:",
+    @"moveToRightEndOfLine:"
+  ];
+  TestEditingCommands(selectors);
+}
+
+// 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) {
+  NSArray* selectors = @[
+    @"moveBackwardAndModifySelection:",
+    @"moveForwardAndModifySelection:",
+    @"moveWordForwardAndModifySelection:",
+    @"moveWordBackwardAndModifySelection:",
+    @"moveUpAndModifySelection:",
+    @"moveDownAndModifySelection:",
+    @"moveToBeginningOfLineAndModifySelection:",
+    @"moveToEndOfLineAndModifySelection:",
+    @"moveToBeginningOfParagraphAndModifySelection:",
+    @"moveToEndOfParagraphAndModifySelection:",
+    @"moveToEndOfDocumentAndModifySelection:",
+    @"moveToBeginningOfDocumentAndModifySelection:",
+    @"pageDownAndModifySelection:",
+    @"pageUpAndModifySelection:",
+    @"moveParagraphForwardAndModifySelection:",
+    @"moveParagraphBackwardAndModifySelection:",
+    @"moveRightAndModifySelection:",
+    @"moveLeftAndModifySelection:",
+    @"moveWordRightAndModifySelection:",
+    @"moveWordLeftAndModifySelection:",
+    @"moveToLeftEndOfLineAndModifySelection:",
+    @"moveToRightEndOfLineAndModifySelection:"
+  ];
+  TestEditingCommands(selectors);
+}
+
+// Test delete commands against expectations set by |dummy_text_view_|.
+TEST_F(BridgedNativeWidgetTest, TextInput_DeleteCommands) {
+  NSArray* selectors = @[
+    @"deleteForward:", @"deleteBackward:", @"deleteWordForward:",
+    @"deleteWordBackward:", @"deleteToBeginningOfLine:", @"deleteToEndOfLine:",
+    @"deleteToBeginningOfParagraph:", @"deleteToEndOfParagraph:"
+  ];
+  TestEditingCommands(selectors);
+}
+
 // Test firstRectForCharacterRange:actualRange for cases where query range is
 // empty or outside composition range.
 TEST_F(BridgedNativeWidgetTest, TextInput_FirstRectForCharacterRange_Caret) {
   InstallTextField("");
   ui::TextInputClient* client = [ns_view_ textInputClient];
 
   // No composition. Ensure bounds and range corresponding to the current caret
   // position are returned.
   // Initially selection range will be [0, 0].
   NSRange caret_range = NSMakeRange(0, 0);
@@ skipping 144 matching lines @@
   [center postNotificationName:NSWindowDidExitFullScreenNotification
                         object:window];
   EXPECT_EQ(1, [window ignoredToggleFullScreenCount]);  // No change.
   EXPECT_FALSE(bridge()->target_fullscreen_state());
 
   widget_->CloseNow();
 }
 
 }  // namespace test
 }  // namespace views