BridgedContentView: Modify IsTextRTL to use GetFirstStrongCharacterDirection.

ui/views/controls/textfield/textfield_unittest.cc

 // 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.
 
 #include "ui/views/controls/textfield/textfield.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <set>
@@ skipping 500 matching lines @@
       // For unicode characters, assume they come from IME rather than the
       // keyboard. So they are dispatched directly to the input method.
       ui::KeyEvent event(ch, ui::VKEY_UNKNOWN, ui::EF_NONE);
       input_method_->DispatchKeyEvent(&event);
     }
   }
 
   // Sends a platform-specific move (and select) to start of line.
   void SendHomeEvent(bool shift) {
     if (TestingNativeMac()) {
-      // Use Cmd+Left on native Mac. An RTL-agnostic "end" doesn't have a
-      // default key-binding on Mac.
-      SendKeyEvent(ui::VKEY_LEFT, shift /* shift */, true /* command */);
+      // Move to beginning of line doesn't have a default key binding on Mac.

[tapted, 2016/06/08 04:16:45]

This is now different to "SendHomeEvent". If the purpose of "SendHomeEvent" in
the tests is *actually* just to reset the cursor position, then that should be
reflected in the name, like MoveCursorToStart, or MoveCursorToEnd -- there might
be a test which should _actually_ be testing a home event (e.g. are there any
tests with rtl text? do we need to add one for this to spot a regression?

[karandeepb, 2016/06/08 04:56:05]

How is it different to SendHomeEvent? Based on the |shift| parameter it can
either just be a move to the beginning of line or a move and select to the
beginning of line. There are already tests testing RTL. Currently,
TextfieldTest.OverflowTest and TextfieldTest.OverflowInRTLTest both should have
failed. But the incorrect implementations of IsTextRTL and
SendHomeEvent/SendEndEvent cancelled each other out.

[tapted, 2016/06/08 05:59:03]

It sends Up. It's sorta coincidence that that goes to the beginning of the line.
Whereas Cmd+Left still sounds like it matches the comment (even if it didn't
actually). If you don't think it's necessary to change the name of
`SendHomeEvent` we should at least describe in the comment whether the "start"
means the display-direction start or the reading-direction start.

Then, I think it would be clearer to say something like

// -[NSResponder moveToBeginningOfLine:] is the correct way to do this on Mac,
but that doesn't have a default key binding on Mac. Since views::Textfield
doesn't currently support multiple lines, the same effect can be achieved by
-[NSResponder moveToBeginningOfDocument:], which is mapped to Cmd+Up.


But! Also, what is "home" mapped to? (can we now *actually* use Home now that it
[maybe] maps to beginning of line (also)?). (but we should still note that this
only works on mac because single-line)
This needs to be mentioned in the CL description.
OverflowTest/OverflowInRTLTest don't call Send{Home,End}Event (right?)

So it's still not clear to me whether changing SendHomeEvent in this way is the
right fix, or whether the failing tests are doing something wrong around rtl
characters, and we're just hiding another bug by overriding a line-navigation
(which should be RTL-aware) with a vertical-navigation (which shouldn't be).

[karandeepb, 2016/06/08 08:04:56]

Added a comment to SendHomeEvent/SendEndEvent saying that they map to the
logical beginning/end of line.
Done
It doesn't map to the moveToBeginningOfLine. You can verify that by pressing
Home in a NSTextField. Also, the tests currently depend on key bindings of the
machine on which they are run. I had an idea a while back to use a private API
called NSKeyBindingManager(iirc) which should enable us to map key strokes to
action messages for use in tests.  Let me know if you think it's worth
exploring.
Done.
Sorry, I recollected the test names from memory and gave the wrong names. The
tests are actually TextfieldTest.HitOutsideTextAreaTest and
TextfieldTest.HitOutsideTextAreaInRTLTest. You can read the test code to verify
that SendHomeEvent/SendEndEvent are meant to correspond to the logical
beginning/end of line. Also modified the CL description to show why the tests
passed up-till now.

+      // Since textfields don't support multiple lines, use Cmd+Up instead,
+      // which moves to the beginning of document.
+      SendKeyEvent(ui::VKEY_UP, shift /* shift */, true /* command */);
       return;
     }
     SendKeyEvent(ui::VKEY_HOME, shift /* shift */, false /* control */);
   }
 
   // Sends a platform-specific move (and select) to end of line.
   void SendEndEvent(bool shift) {
     if (TestingNativeMac()) {
-      SendKeyEvent(ui::VKEY_RIGHT, shift, true);  // Cmd+Right.
+      SendKeyEvent(ui::VKEY_DOWN, shift, true);  // Cmd+Down.
       return;
     }
     SendKeyEvent(ui::VKEY_END, shift, false);
   }
 
   // Sends {delete, move, select} word {forward, backward}.
   void SendWordEvent(ui::KeyboardCode key, bool shift) {
     bool alt = false;
     bool control = true;
     bool caps = false;
@@ skipping 2111 matching lines @@
 
   textfield_->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
   ui::AXViewState state_protected;
   textfield_->GetAccessibleState(&state_protected);
   EXPECT_EQ(ui::AX_ROLE_TEXT_FIELD, state_protected.role);
   EXPECT_EQ(ASCIIToUTF16("********"), state_protected.value);
   EXPECT_TRUE(state_protected.HasStateFlag(ui::AX_STATE_PROTECTED));
 }
 
 }  // namespace views