fix know issues in RenderText

views/controls/textfield/textfield_views_model_unittest.cc

Index: views/controls/textfield/textfield_views_model_unittest.cc
===================================================================
--- views/controls/textfield/textfield_views_model_unittest.cc	(revision 100008)
+++ views/controls/textfield/textfield_views_model_unittest.cc	(working copy)
@@ -17,6 +17,16 @@
 #include "views/test/views_test_base.h"
 #include "views/views_delegate.h"
 
+namespace {
+
+struct Selection {
+  size_t cursor;
+  size_t caret;
+  gfx::SelectionModel::CaretPlacement placement;
+};
+
+}  // namespace
+
 namespace views {
 
 class TextfieldViewsModelTest : public ViewsTestBase,
@@ -83,8 +93,306 @@
   // but backspace should work.
   EXPECT_TRUE(model.Backspace());
   EXPECT_STR_EQ("HELLORL", model.GetText());
+
+  model.MoveCursorTo(gfx::SelectionModel(5));
+  model.ReplaceText(ASCIIToUTF16(" WOR"));
+  EXPECT_STR_EQ("HELLO WORL", model.GetText());
 }
 
+TEST_F(TextfieldViewsModelTest, EditString_SimpleRTL) {
+  TextfieldViewsModel model(NULL);
+  // Append two strings.
+  model.Append(WideToUTF16(L"\x05d0\x05d1\x05d2"));
+  EXPECT_EQ(WideToUTF16(L"\x05d0\x05d1\x05d2"), model.GetText());
+  model.Append(WideToUTF16(L"\x05e0\x05e1\x05e2"));

[msw, 2011/09/09 23:03:24]

Perhaps make the second string different than the first so we can verify that it
was appended and not pre-pended?

[xji, 2011/09/12 21:31:48]

they are different.

[msw, 2011/09/14 02:43:48]

doh! you are right.

+  EXPECT_EQ(WideToUTF16(L"\x05d0\x05d1\x05d2\x05e0\x05e1\x05e2"),
+            model.GetText());
+
+  // Insert 0x05f0.
+  model.MoveCursorTo(gfx::SelectionModel(1U));
+  model.InsertChar(0x05f0);
+  EXPECT_EQ(WideToUTF16(L"\x05d0\x05f0\x05d1\x05d2\x05e0\x05e1\x05e2"),
+            model.GetText());
+
+  // Replace "\x05d1" with "\x05f1".
+  model.ReplaceChar(0x05f1);
+  EXPECT_EQ(WideToUTF16(L"\x05d0\x05f0\x5f1\x05d2\x05e0\x05e1\x05e2"),
+            model.GetText());
+
+  // Delete and backspace.
+  EXPECT_EQ(3U, model.GetCursorPosition());
+  EXPECT_TRUE(model.Delete());
+  EXPECT_EQ(WideToUTF16(L"\x05d0\x05f0\x5f1\x05e0\x05e1\x05e2"),
+            model.GetText());
+  EXPECT_TRUE(model.Backspace());
+  EXPECT_EQ(2U, model.GetCursorPosition());
+  EXPECT_EQ(WideToUTF16(L"\x05d0\x05f0\x05e0\x05e1\x05e2"), model.GetText());
+}
+
+TEST_F(TextfieldViewsModelTest, EditString_ComplexScript) {
+  TextfieldViewsModel model(NULL);
+  // Append two Hindi strings.
+  model.Append(WideToUTF16(L"\x0915\x093f\x0915\x094d\x0915"));
+  EXPECT_EQ(WideToUTF16(L"\x0915\x093f\x0915\x094d\x0915"),
+            model.GetText());
+  model.Append(WideToUTF16(L"\x0915\x094d\x092e\x094d"));
+  EXPECT_EQ(WideToUTF16(
+      L"\x0915\x093f\x0915\x094d\x0915\x0915\x094d\x092e\x094d"),
+      model.GetText());
+
+  // Check it is not able to place cursor in middle of a grapheme.
+  model.MoveCursorTo(gfx::SelectionModel(1U));
+  EXPECT_EQ(0U, model.GetCursorPosition());
+
+  model.MoveCursorTo(gfx::SelectionModel(2U));
+  EXPECT_EQ(2U, model.GetCursorPosition());
+  model.InsertChar('a');
+  EXPECT_EQ(WideToUTF16(
+      L"\x0915\x093f\x0061\x0915\x094d\x0915\x0915\x094d\x092e\x094d"),
+      model.GetText());
+
+  // ReplaceChar will replace the whole grapheme.
+  model.ReplaceChar('b');
+  EXPECT_EQ(WideToUTF16(
+      L"\x0915\x093f\x0061\x0062\x0915\x0915\x094d\x092e\x094d"),
+      model.GetText());
+  EXPECT_EQ(4U, model.GetCursorPosition());
+
+  // DELETE should delte the whole grapheme.

[msw, 2011/09/09 23:03:24]

"Delete" isn't all-caps and "del*e*te".

[xji, 2011/09/12 21:31:48]

Done.

+  model.MoveCursorTo(gfx::SelectionModel(0U));
+  EXPECT_TRUE(model.Delete());
+  EXPECT_EQ(WideToUTF16(L"\x0061\x0062\x0915\x0915\x094d\x092e\x094d"),
+            model.GetText());
+  model.MoveCursorTo(gfx::SelectionModel(model.GetText().length()));
+  EXPECT_TRUE(model.Backspace());
+  EXPECT_EQ(WideToUTF16(L"\x0061\x0062\x0915\x0915\x094d\x092e"),
+            model.GetText());
+
+  // Test cursor position and deletion for Hindi VIRAMA.

[msw, 2011/09/09 23:03:24]

Virama isn't all-caps.

[xji, 2011/09/12 21:31:48]

Done.

+  model.SetText(WideToUTF16(L"\x0D38\x0D4D\x0D15\x0D16\x0D2E"));
+  model.MoveCursorTo(gfx::SelectionModel(0));
+  EXPECT_EQ(0U, model.GetCursorPosition());
+
+  model.MoveCursorTo(gfx::SelectionModel(1));
+  EXPECT_EQ(0U, model.GetCursorPosition());
+
+  model.MoveCursorTo(gfx::SelectionModel(2));
+  EXPECT_EQ(2U, model.GetCursorPosition());
+
+  model.MoveCursorTo(gfx::SelectionModel(3));
+  EXPECT_EQ(3U, model.GetCursorPosition());
+
+  model.MoveCursorTo(gfx::SelectionModel(2));
+
+  EXPECT_TRUE(model.Backspace());
+  EXPECT_EQ(WideToUTF16(L"\x0D38\x0D15\x0D16\x0D2E"), model.GetText());
+
+  // Test DELETE/BACKSPACE on Hebrew with non-spacing marks.

[msw, 2011/09/09 23:03:24]

Delete and Backspace are not all-caps.

[xji, 2011/09/12 21:31:48]

Done.

+  model.SetText(WideToUTF16(L"\x05d5\x05b7\x05D9\x05B0\x05D4\x05B4\x05D9"));
+  model.MoveCursorTo(gfx::SelectionModel(0));
+  EXPECT_TRUE(model.Delete());
+  EXPECT_TRUE(model.Delete());
+  EXPECT_TRUE(model.Delete());
+  EXPECT_TRUE(model.Delete());
+  EXPECT_EQ(WideToUTF16(L""), model.GetText());
+
+  model.SetText(WideToUTF16(L"\x002C\x0020\x05D1\x05BC\x05B7\x05E9\x05BC"));
+  model.MoveCursorLeft(gfx::LINE_BREAK, false);

[msw, 2011/09/09 23:03:24]

Won't this fail on windows if the UI mode isn't RTL?

[xji, 2011/09/12 21:31:48]

add #if defined and use move to right line end for Win.

+  EXPECT_TRUE(model.Backspace());
+  EXPECT_EQ(WideToUTF16(L"\x002C\x0020\x05D1\x05BC\x05B7\x05E9"),
+            model.GetText());
+}
+
+void RunMoveCursorLeftRightTest(TextfieldViewsModel* model,
+                                struct Selection* expected,
+                                int num_of_selection_entry,
+                                bool move_right) {
+  for (int i = 0; i <= num_of_selection_entry + 1; ++i) {

[msw, 2011/09/09 23:03:24]

Why are you looping i from 0 to n+1 but then clamping index to n-1? Why not just
loop i to n-1?

[xji, 2011/09/12 21:31:48]

It is to test when cursor is at the HOME/END, press right/left again, cursor
should stay at the same place.
Actually, the condition should be "i <= num_of_selection_entry", offset by 1
should be enough.

+    int index = std::min(i, num_of_selection_entry - 1);
+    struct Selection sel = expected[index];

[msw, 2011/09/09 23:03:24]

I don't believe 'struct' is needed here and elsewhere (I didn't mark each use).
But hopefully you'll use std::vector<gfx::SelectionModel> instead anyway.

[xji, 2011/09/12 21:31:48]

removed struct.

+    EXPECT_TRUE(model->render_text()->selection_model().Equals(
+        gfx::SelectionModel(sel.cursor, sel.caret, sel.placement)));
+
+    if (move_right)
+      model->MoveCursorRight(gfx::CHARACTER_BREAK, false);
+    else
+      model->MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  }
+
+  struct Selection sel = expected[num_of_selection_entry - 1];
+  if (move_right)
+    model->MoveCursorRight(gfx::LINE_BREAK, false);
+  else
+    model->MoveCursorLeft(gfx::LINE_BREAK, false);
+  EXPECT_TRUE(model->render_text()->selection_model().Equals(
+      gfx::SelectionModel(sel.cursor, sel.caret, sel.placement)));
+}
+
+TEST_F(TextfieldViewsModelTest, MoveCursorLeftRight) {
+  TextfieldViewsModel model(NULL);
+
+  // Pure LTR.
+  model.SetText(ASCIIToUTF16("abc"));
+  // left_to_right saves the expected (cursor, caret, caret_placement) triplet
+  // when moving cursor from left to right.
+  struct Selection left_to_right[] = {

[msw, 2011/09/09 23:03:24]

Use std::vector<gfx::SelectionModel> instead here and elsewhere. Example:
std::vector<gfx::SelectionModel> ltr;
ltr.append(gfx::SelectionModel(0, 0, gfx::SelectionModel::TRAILING));
ltr.append(gfx::SelectionModel(1, 0, gfx::SelectionModel::LEADING));
...

[xji, 2011/09/12 21:31:48]

Done.

+    {0, 0, gfx::SelectionModel::LEADING},
+    {1, 0, gfx::SelectionModel::TRAILING},
+    {2, 1, gfx::SelectionModel::TRAILING},
+    {3, 2, gfx::SelectionModel::TRAILING},
+  };
+  RunMoveCursorLeftRightTest(&model, left_to_right,
+      sizeof(left_to_right)/sizeof(struct Selection), true);
+
+  struct Selection right_to_left[] = {
+    {3, 2, gfx::SelectionModel::TRAILING},
+    {2, 2, gfx::SelectionModel::LEADING},
+    {1, 1, gfx::SelectionModel::LEADING},
+    {0, 0, gfx::SelectionModel::LEADING},
+  };
+  RunMoveCursorLeftRightTest(&model, right_to_left,
+      sizeof(right_to_left)/sizeof(struct Selection), false);
+
+  // Pure RTL.
+  model.SetText(WideToUTF16(L"\x05d0\x05d1\x05d2"));
+  struct Selection right_to_left_h[] = {
+    {0, 0, gfx::SelectionModel::LEADING},
+    {1, 0, gfx::SelectionModel::TRAILING},
+    {2, 1, gfx::SelectionModel::TRAILING},
+    {3, 2, gfx::SelectionModel::TRAILING},
+  };
+  RunMoveCursorLeftRightTest(&model, right_to_left_h,
+      sizeof(right_to_left_h)/sizeof(struct Selection), false);
+
+  struct Selection left_to_right_h[] = {
+    {3, 2, gfx::SelectionModel::TRAILING},
+    {2, 2, gfx::SelectionModel::LEADING},
+    {1, 1, gfx::SelectionModel::LEADING},
+    {0, 0, gfx::SelectionModel::LEADING},
+  };
+  RunMoveCursorLeftRightTest(&model, left_to_right_h,
+      sizeof(left_to_right_h)/sizeof(struct Selection), true);
+
+  // LTR-RTL
+  model.SetText(WideToUTF16(L"abc\x05d0\x05d1\x05d2"));
+  // The last one is the expected END position.
+  struct Selection right_to_left_eh[] = {
+    {0, 0, gfx::SelectionModel::LEADING},
+    {1, 0, gfx::SelectionModel::TRAILING},
+    {2, 1, gfx::SelectionModel::TRAILING},
+    {3, 2, gfx::SelectionModel::TRAILING},
+    {5, 5, gfx::SelectionModel::LEADING},
+    {4, 4, gfx::SelectionModel::LEADING},
+    {3, 3, gfx::SelectionModel::LEADING},
+    {6, 3, gfx::SelectionModel::LEADING},
+  };
+  RunMoveCursorLeftRightTest(&model, right_to_left_eh,
+      sizeof(right_to_left_eh)/sizeof(struct Selection), true);
+
+  struct Selection left_to_right_eh[] = {
+    {6, 3, gfx::SelectionModel::LEADING},
+    {4, 3, gfx::SelectionModel::TRAILING},
+    {5, 4, gfx::SelectionModel::TRAILING},
+    {6, 5, gfx::SelectionModel::TRAILING},
+    {2, 2, gfx::SelectionModel::LEADING},
+    {1, 1, gfx::SelectionModel::LEADING},
+    {0, 0, gfx::SelectionModel::LEADING},
+  };
+  RunMoveCursorLeftRightTest(&model, left_to_right_eh,
+      sizeof(left_to_right_eh)/sizeof(struct Selection), false);
+
+  // RTL-LTR
+  model.SetText(WideToUTF16(L"\x05d0\x05d1\x05d2"L"abc"));
+  struct Selection right_to_left_he[] = {
+    {0, 0, gfx::SelectionModel::LEADING},
+    {1, 0, gfx::SelectionModel::TRAILING},
+    {2, 1, gfx::SelectionModel::TRAILING},
+    {3, 2, gfx::SelectionModel::TRAILING},
+    {5, 5, gfx::SelectionModel::LEADING},
+    {4, 4, gfx::SelectionModel::LEADING},
+    {3, 3, gfx::SelectionModel::LEADING},
+    {6, 3, gfx::SelectionModel::LEADING},
+  };
+  RunMoveCursorLeftRightTest(&model, right_to_left_he,
+      sizeof(right_to_left_he)/sizeof(struct Selection), false);
+
+  struct Selection left_to_right_he[] = {
+    {6, 3, gfx::SelectionModel::LEADING},
+    {4, 3, gfx::SelectionModel::TRAILING},
+    {5, 4, gfx::SelectionModel::TRAILING},
+    {6, 5, gfx::SelectionModel::TRAILING},
+    {2, 2, gfx::SelectionModel::LEADING},
+    {1, 1, gfx::SelectionModel::LEADING},
+    {0, 0, gfx::SelectionModel::LEADING},
+  };
+  RunMoveCursorLeftRightTest(&model, left_to_right_he,
+      sizeof(left_to_right_he)/sizeof(struct Selection), true);
+
+  // LTR-RTL-LTR.
+  model.SetText(WideToUTF16(L"a"L"\x05d1"L"b"));
+  struct Selection left_to_right_ehe[] = {
+    {0, 0, gfx::SelectionModel::LEADING},
+    {1, 0, gfx::SelectionModel::TRAILING},
+    {1, 1, gfx::SelectionModel::LEADING},
+    {3, 2, gfx::SelectionModel::TRAILING},
+  };
+  RunMoveCursorLeftRightTest(&model, left_to_right_ehe,
+      sizeof(left_to_right_ehe)/sizeof(struct Selection), true);
+
+  struct Selection right_to_left_ehe[] = {
+    {3, 2, gfx::SelectionModel::TRAILING},
+    {2, 2, gfx::SelectionModel::LEADING},
+    {2, 1, gfx::SelectionModel::TRAILING},
+    {0, 0, gfx::SelectionModel::LEADING},
+  };
+  RunMoveCursorLeftRightTest(&model, right_to_left_ehe,
+      sizeof(right_to_left_ehe)/sizeof(struct Selection), false);
+
+  // RTL-LTR-RTL.
+  model.SetText(WideToUTF16(L"\x05d0"L"a"L"\x05d1"));
+  struct Selection right_to_left_heh[] = {
+    {0, 0, gfx::SelectionModel::LEADING},
+    {1, 0, gfx::SelectionModel::TRAILING},
+    {1, 1, gfx::SelectionModel::LEADING},
+    {3, 2, gfx::SelectionModel::TRAILING},
+  };
+  RunMoveCursorLeftRightTest(&model, right_to_left_heh,
+      sizeof(right_to_left_heh)/sizeof(struct Selection), false);
+
+  struct Selection left_to_right_heh[] = {
+    {3, 2, gfx::SelectionModel::TRAILING},
+    {2, 2, gfx::SelectionModel::LEADING},
+    {2, 1, gfx::SelectionModel::TRAILING},
+    {0, 0, gfx::SelectionModel::LEADING},
+  };
+  RunMoveCursorLeftRightTest(&model, left_to_right_heh,
+      sizeof(left_to_right_heh)/sizeof(struct Selection), true);
+}
+
+TEST_F(TextfieldViewsModelTest, MoveCursorLeftRight_ComplexScript) {
+  TextfieldViewsModel model(NULL);
+
+  model.Append(WideToUTF16(L"\x0915\x093f\x0915\x094d\x0915"));

[msw, 2011/09/09 23:03:24]

Any reason you chose not to use RunMoveCursorLeftRightTest and also verify the
caret positions here?

[xji, 2011/09/12 21:31:48]

since this is uni-directional text, I am not testing caret_pos here, only test
cursor logical position, which is simpler.

+  EXPECT_EQ(0U, model.GetCursorPosition());
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(2U, model.GetCursorPosition());
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(4U, model.GetCursorPosition());
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(5U, model.GetCursorPosition());
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(5U, model.GetCursorPosition());
+
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(4U, model.GetCursorPosition());
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(2U, model.GetCursorPosition());
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(0U, model.GetCursorPosition());
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(0U, model.GetCursorPosition());
+}
+
 TEST_F(TextfieldViewsModelTest, EmptyString) {
   TextfieldViewsModel model(NULL);
   EXPECT_EQ(string16(), model.GetText());
@@ -143,6 +451,100 @@
   EXPECT_EQ(5U, model.GetCursorPosition());
 }
 
+TEST_F(TextfieldViewsModelTest, Selection_BidiWithNonSpacingMarks) {
+  TextfieldViewsModel model(NULL);
+  model.Append(WideToUTF16(
+      L"abc\x05E9\x05BC\x05C1\x05B8\x05E0\x05B8"L"def"));
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"c"), model.GetSelectedText());
+
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"c\x05E9\x05BC\x05C1\x05B8"),
+            model.GetSelectedText());
+
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"c"), model.GetSelectedText());

[msw, 2011/09/09 23:03:24]

This is the spec'ed behavior, but it might confuse someone, can you add a
comment?

[xji, 2011/09/12 21:31:48]

added comments and selection_start/end expected results. hope it explains.

+
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"c\x05E9\x05BC\x05C1\x05B8\x05E0\x05B8"L"d"),
+            model.GetSelectedText());
+
+  model.ClearSelection();
+  EXPECT_EQ(string16(), model.GetSelectedText());
+  model.SelectAll();
+  EXPECT_EQ(WideToUTF16(L"abc\x05E9\x05BC\x05C1\x05B8\x05E0\x05B8"L"def"),
+            model.GetSelectedText());
+
+  // In case of "aBc", this test shows how to select "aB" or "Bc", assume 'B' is
+  // an RTL character.
+  model.SetText(WideToUTF16(L"a\x05E9"L"b"));
+  model.MoveCursorTo(gfx::SelectionModel(0));
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"a"), model.GetSelectedText());
+
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"a"), model.GetSelectedText());
+
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"a\x05E9"L"b"), model.GetSelectedText());
+
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(3U, model.GetCursorPosition());
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"b"), model.GetSelectedText());
+
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"b"), model.GetSelectedText());
+
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"a\x05E9"L"b"), model.GetSelectedText());
+
+  model.MoveCursorLeft(gfx::LINE_BREAK, false);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"a\x05E9"), model.GetSelectedText());
+
+  model.MoveCursorRight(gfx::LINE_BREAK, false);
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, true);
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, true);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  EXPECT_EQ(WideToUTF16(L"\x05E9"L"b"), model.GetSelectedText());
+
+  model.ClearSelection();
+  EXPECT_EQ(string16(), model.GetSelectedText());
+  model.SelectAll();
+  EXPECT_EQ(WideToUTF16(L"a\x05E9"L"b"), model.GetSelectedText());
+}
+
+TEST_F(TextfieldViewsModelTest, MoveCursorLeftRightWithSelection) {
+  TextfieldViewsModel model(NULL);
+  model.SetText(WideToUTF16(L"abc\x05d0\x05d1\x05d2"));

[msw, 2011/09/09 23:03:24]

Please add EXPECT_EQs for model.GetCursorPosition for each step of this test, or
add comments to explain what this is doing.

[xji, 2011/09/12 21:31:48]

Done.

+  model.MoveCursorRight(gfx::LINE_BREAK, false);
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, true);
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(6U, model.GetCursorPosition());
+
+  model.MoveCursorLeft(gfx::LINE_BREAK, false);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+

[msw, 2011/09/09 23:03:24]

nit: The very similar block above doesn't have a blank line like this. Can you
make both of them similar one way or the other?

[xji, 2011/09/12 21:31:48]

removed blank line.

+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, true);
+  model.MoveCursorLeft(gfx::CHARACTER_BREAK, true);
+  model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+  EXPECT_EQ(4U, model.GetCursorPosition());
+}
+
 TEST_F(TextfieldViewsModelTest, SelectionAndEdit) {
   TextfieldViewsModel model(NULL);
   model.Append(ASCIIToUTF16("HELLO"));
@@ -314,8 +716,8 @@
 }
 
 void SelectWordTestVerifier(TextfieldViewsModel &model,
-    const std::string &expected_selected_string, size_t expected_cursor_pos) {
-  EXPECT_STR_EQ(expected_selected_string, model.GetSelectedText());
+    const string16 &expected_selected_string, size_t expected_cursor_pos) {
+  EXPECT_EQ(expected_selected_string, model.GetSelectedText());
   EXPECT_EQ(expected_cursor_pos, model.GetCursorPosition());
 }
 
@@ -326,40 +728,74 @@
   // Test when cursor is at the beginning.
   model.MoveCursorLeft(gfx::LINE_BREAK, false);
   model.SelectWord();
-  SelectWordTestVerifier(model, "  ", 2U);
+  SelectWordTestVerifier(model, ASCIIToUTF16("  "), 2U);
 
   // Test when cursor is at the beginning of a word.
   gfx::SelectionModel selection(2U);
   model.MoveCursorTo(selection);
   model.SelectWord();
-  SelectWordTestVerifier(model, "HELLO", 7U);
+  SelectWordTestVerifier(model, ASCIIToUTF16("HELLO"), 7U);
 
   // Test when cursor is at the end of a word.
   selection = gfx::SelectionModel(15U);
   model.MoveCursorTo(selection);
   model.SelectWord();
-  SelectWordTestVerifier(model, "WO", 15U);
+  SelectWordTestVerifier(model, ASCIIToUTF16("     "), 20U);
 
   // Test when cursor is somewhere in a non-alph-numeric fragment.

[msw, 2011/09/09 23:03:24]

nit: mind fixing "alph*a*"?

[xji, 2011/09/12 21:31:48]

Done.

   for (size_t cursor_pos = 8; cursor_pos < 13U; cursor_pos++) {
     selection = gfx::SelectionModel(cursor_pos);
     model.MoveCursorTo(selection);
     model.SelectWord();
-    SelectWordTestVerifier(model, "  !!  ", 13U);
+    SelectWordTestVerifier(model, ASCIIToUTF16("  !!  "), 13U);
   }
 
   // Test when cursor is somewhere in a whitespace fragment.
   selection = gfx::SelectionModel(17U);
   model.MoveCursorTo(selection);
   model.SelectWord();
-  SelectWordTestVerifier(model, "     ", 20U);
+  SelectWordTestVerifier(model, ASCIIToUTF16("     "), 20U);
 
   // Test when cursor is at the end.
   model.MoveCursorRight(gfx::LINE_BREAK, false);
   model.SelectWord();
-  SelectWordTestVerifier(model, " ", 24U);
+  SelectWordTestVerifier(model, ASCIIToUTF16(" "), 24U);
 }
 
+TEST_F(TextfieldViewsModelTest, SelectWordTest_MixScripts) {
+  TextfieldViewsModel model(NULL);
+  const struct WordAndCursor {
+    const wchar_t* word;
+    size_t cursor;
+  } words_and_cursor[] = {

[msw, 2011/09/09 23:03:24]

I suppose this works, but would you mind splitting the struct definition from
the words_and_cursor instance, just for readability? Or could you use a vector
of WordAndCursor?

[xji, 2011/09/12 21:31:48]

Done.

+    {L"a\x05d0", 2},
+    {L"a\x05d0", 2},
+    {L"\x05d1\x05d2", 5},
+    {L"\x05d1\x05d2", 5},
+    {L" ", 3},
+    {L"a\x05d0", 2},
+    {L"\x0915\x094d\x0915", 9},
+    {L"\x0915\x094d\x0915", 9},
+    {L" ", 10},
+    {L"\x4E2D\x56FD", 12},
+    {L"\x4E2D\x56FD", 12},
+    {L"\x82B1", 13},
+    {L"\x5929", 14},
+  };
+  // The text consists of Ascii, Hebrew, Hindi with  Virama sign, and Chinese.

[msw, 2011/09/09 23:03:24]

Extra space in "with * *Virama"

[xji, 2011/09/12 21:31:48]

Done.

+  model.SetText(WideToUTF16(L"a\x05d0 \x05d1\x05d2 \x0915\x094d\x0915 "
+                            L"\x4E2D\x56FD\x82B1\x5929"));
+  for (size_t i = 0; i < sizeof(words_and_cursor)/sizeof(WordAndCursor);

[msw, 2011/09/09 23:03:24]

Wouldn't it be a bit clearer to remove the WordAndCursor struct and this loop;
and to just make individual calls to SelectWordTestVerifier for each index?

[xji, 2011/09/12 21:31:48]

that is true. But the current one also tests the selected word when cursor at
different visual position but the same logical position (index). For example,
"a\x05d0", when index is 1, visual position at "a|\x05d2\x05d1 \x05d0" and
"a|\x05d2\x05d1 \x05d0|" 

+       ++i) {
+    model.MoveCursorLeft(gfx::LINE_BREAK, false);
+    for (size_t j = 0; j < i; ++j)
+      model.MoveCursorRight(gfx::CHARACTER_BREAK, false);
+    model.SelectWord();
+    SelectWordTestVerifier(model, WideToUTF16(words_and_cursor[i].word),
+                           words_and_cursor[i].cursor);
+  }
+}
+
 TEST_F(TextfieldViewsModelTest, RangeTest) {
   TextfieldViewsModel model(NULL);
   model.Append(ASCIIToUTF16("HELLO WORLD"));