Make uneditable when readonly.

 This change makes the textfield uneditable. The text can
still be updated via API (which is necessary to show URL
in popup).

BUG=none
TEST=uadded new ReadOnlyTest

Review URL: http://codereview.chromium.org/6314012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@72022 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 105 insertions, 20 deletions

diff --git a/views/controls/textfield/native_textfield_views.cc b/views/controls/textfield/native_textfield_views.cc
index 3cffeb8..a4324be2 100644
--- a/views/controls/textfield/native_textfield_views.cc
+++ b/views/controls/textfield/native_textfield_views.cc
@@ -329,18 +329,19 @@ bool NativeTextfieldViews::IsCommandIdChecked(int command_id) const {
 }
 
 bool NativeTextfieldViews::IsCommandIdEnabled(int command_id) const {
+  bool editable = !textfield_->read_only();
   string16 result;
   switch (command_id) {
     case IDS_APP_CUT:
-      return model_->HasSelection();
+      return editable && model_->HasSelection();
     case IDS_APP_COPY:
       return model_->HasSelection();
     case IDS_APP_PASTE:
       views::ViewsDelegate::views_delegate->GetClipboard()
           ->ReadText(ui::Clipboard::BUFFER_STANDARD, &result);
-      return !result.empty();
+      return editable && !result.empty();
     case IDS_APP_DELETE:
-      return model_->HasSelection();
+      return editable && model_->HasSelection();
     case IDS_APP_SELECT_ALL:
       return true;
     default:
@@ -356,18 +357,22 @@ bool NativeTextfieldViews::GetAcceleratorForCommandId(int command_id,
 
 void NativeTextfieldViews::ExecuteCommand(int command_id) {
   bool text_changed = false;
+  bool editable = !textfield_->read_only();
   switch (command_id) {
     case IDS_APP_CUT:
-      text_changed = model_->Cut();
+      if (editable)
+        text_changed = model_->Cut();
       break;
     case IDS_APP_COPY:
       model_->Copy();
       break;
     case IDS_APP_PASTE:
-      text_changed = model_->Paste();
+      if (editable)
+        text_changed = model_->Paste();
       break;
     case IDS_APP_DELETE:
-      text_changed = model_->Delete();
+      if (editable)
+        text_changed = model_->Delete();
       break;
     case IDS_APP_SELECT_ALL:
       SelectAll();
@@ -519,6 +524,7 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
     // TODO(oshima): shift-tab does not work. Figure out why and fix.
     if (key_code == ui::VKEY_TAB)
       return false;
+    bool editable = !textfield_->read_only();
     bool selection = key_event.IsShiftDown();
     bool control = key_event.IsControlDown();
     bool text_changed = false;
@@ -531,7 +537,7 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
         }
         break;
       case ui::VKEY_X:
-        if (control)
+        if (control && editable)
           text_changed = model_->Cut();
         break;
       case ui::VKEY_C:
@@ -539,7 +545,7 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
           model_->Copy();
         break;
       case ui::VKEY_V:
-        if (control)
+        if (control && editable)
           text_changed = model_->Paste();
         break;
       case ui::VKEY_RIGHT:
@@ -561,6 +567,8 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
         cursor_changed = true;
         break;
       case ui::VKEY_BACK:
+        if (!editable)
+          break;
         if (!model_->HasSelection()) {
           if (selection && control) {
             // If both shift and control are pressed, then erase upto the
@@ -579,6 +587,8 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
         cursor_changed = true;
         break;
       case ui::VKEY_DELETE:
+        if (!editable)
+          break;
         if (!model_->HasSelection()) {
           if (selection && control) {
             // If both shift and control are pressed, then erase upto the
@@ -603,7 +613,7 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
         break;
     }
     char16 print_char = GetPrintableChar(key_event);
-    if (!control && print_char) {
+    if (!control && print_char && editable) {
       if (insert_)
         model_->Insert(print_char);
       else
diff --git a/views/controls/textfield/native_textfield_views_unittest.cc b/views/controls/textfield/native_textfield_views_unittest.cc
index 9e946ab..d8bffbe 100644
--- a/views/controls/textfield/native_textfield_views_unittest.cc
+++ b/views/controls/textfield/native_textfield_views_unittest.cc
@@ -7,6 +7,7 @@
 #include "base/utf_string_conversions.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/clipboard/clipboard.h"
+#include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/keycodes/keyboard_codes.h"
 #include "views/controls/menu/menu_2.h"
 #include "views/controls/textfield/native_textfield_views.h"
@@ -21,8 +22,12 @@
 
 namespace views {
 
+// Convert to Wide so that the printed string will be readable when
+// check fails.
 #define EXPECT_STR_EQ(ascii, utf16) \
   EXPECT_EQ(ASCIIToWide(ascii), UTF16ToWide(utf16))
+#define EXPECT_STR_NE(ascii, utf16) \
+  EXPECT_NE(ASCIIToWide(ascii), UTF16ToWide(utf16))
 
 // TODO(oshima): Move tests that are independent of TextfieldViews to
 // textfield_unittests.cc once we move the test utility functions
@@ -404,8 +409,8 @@ void VerifyTextfieldContextMenuContents(bool textfield_has_selection,
   EXPECT_EQ(textfield_has_selection, menu_model->IsEnabledAt(1 /* COPY */));
   EXPECT_EQ(textfield_has_selection, menu_model->IsEnabledAt(3 /* DELETE */));
   string16 str;
-  views::ViewsDelegate::views_delegate->GetClipboard()
-      ->ReadText(ui::Clipboard::BUFFER_STANDARD, &str);
+  views::ViewsDelegate::views_delegate->GetClipboard()->
+      ReadText(ui::Clipboard::BUFFER_STANDARD, &str);
   EXPECT_NE(str.empty(), menu_model->IsEnabledAt(2 /* PASTE */));
 }
 
@@ -423,4 +428,65 @@ TEST_F(NativeTextfieldViewsTest, ContextMenuDisplayTest) {
   VerifyTextfieldContextMenuContents(true, GetContextMenu()->model());
 }
 
+TEST_F(NativeTextfieldViewsTest, ReadOnlyTest) {
+  scoped_ptr<TestViewsDelegate> test_views_delegate(new TestViewsDelegate());
+  AutoReset<views::ViewsDelegate*> auto_reset(
+      &views::ViewsDelegate::views_delegate, test_views_delegate.get());
+
+  InitTextfield(Textfield::STYLE_DEFAULT);
+  textfield_->SetText(ASCIIToUTF16(" one two three "));
+  textfield_->SetReadOnly(true);
+  SendKeyEventToTextfieldViews(ui::VKEY_HOME);
+  EXPECT_EQ(0U, textfield_->GetCursorPosition());
+
+  SendKeyEventToTextfieldViews(ui::VKEY_END);
+  EXPECT_EQ(15U, textfield_->GetCursorPosition());
+
+  SendKeyEventToTextfieldViews(ui::VKEY_LEFT, false, false);
+  EXPECT_EQ(14U, textfield_->GetCursorPosition());
+
+  SendKeyEventToTextfieldViews(ui::VKEY_LEFT, false, true);
+  EXPECT_EQ(9U, textfield_->GetCursorPosition());
+
+  SendKeyEventToTextfieldViews(ui::VKEY_LEFT, true, true);
+  EXPECT_EQ(5U, textfield_->GetCursorPosition());
+  EXPECT_STR_EQ("two ", textfield_->GetSelectedText());
+
+  textfield_->SelectAll();
+  EXPECT_STR_EQ(" one two three ", textfield_->GetSelectedText());
+
+  // CUT&PASTE does not work, but COPY works
+  SendKeyEventToTextfieldViews(ui::VKEY_X, false, true);
+  EXPECT_STR_EQ(" one two three ", textfield_->GetSelectedText());
+  string16 str;
+  views::ViewsDelegate::views_delegate->GetClipboard()->
+      ReadText(ui::Clipboard::BUFFER_STANDARD, &str);
+  EXPECT_STR_NE(" one two three ", str);
+
+  SendKeyEventToTextfieldViews(ui::VKEY_C, false, true);
+  views::ViewsDelegate::views_delegate->GetClipboard()->
+      ReadText(ui::Clipboard::BUFFER_STANDARD, &str);
+  EXPECT_STR_EQ(" one two three ", str);
+
+  // SetText should work even in read only mode.
+  textfield_->SetText(ASCIIToUTF16(" four five six "));
+  EXPECT_STR_EQ(" four five six ", textfield_->text());
+
+  // Paste shouldn't work.
+  SendKeyEventToTextfieldViews(ui::VKEY_V, false, true);
+  EXPECT_STR_EQ(" four five six ", textfield_->text());
+  EXPECT_TRUE(textfield_->GetSelectedText().empty());
+
+  textfield_->SelectAll();
+  EXPECT_STR_EQ(" four five six ", textfield_->GetSelectedText());
+
+  // Text field is unmodifiable and selection shouldn't change.
+  SendKeyEventToTextfieldViews(ui::VKEY_DELETE);
+  EXPECT_STR_EQ(" four five six ", textfield_->GetSelectedText());
+  SendKeyEventToTextfieldViews(ui::VKEY_BACK);
+  EXPECT_STR_EQ(" four five six ", textfield_->GetSelectedText());
+  SendKeyEventToTextfieldViews(ui::VKEY_T);
+  EXPECT_STR_EQ(" four five six ", textfield_->GetSelectedText());
+}
+
 }  // namespace views
diff --git a/views/controls/textfield/textfield.h b/views/controls/textfield/textfield.h
index e468423..90c8139 100644
--- a/views/controls/textfield/textfield.h
+++ b/views/controls/textfield/textfield.h
@@ -36,9 +36,11 @@ class NativeTextfieldWrapper;
 
 // TextRange specifies the range of text in the Textfield.  This is
 // used to specify selected text and will be used to change the
-// attributes of characters in the textfield. The range preserves the
-// direction, and selecting from the end to the begining is considered
-// "reverse" order.
+// attributes of characters in the textfield. When this is used for
+// selection, the end is caret position, and the start is where
+// selection started.  The range preserves the direction, and
+// selecting from the end to the begining is considered "reverse"
+// order. (that is, start > end is reverse)
 class TextRange {
  public:
   TextRange() : start_(0), end_(0) {}
diff --git a/views/controls/textfield/textfield_views_model.cc b/views/controls/textfield/textfield_views_model.cc
index 462c970..585840d 100644
--- a/views/controls/textfield/textfield_views_model.cc
+++ b/views/controls/textfield/textfield_views_model.cc
@@ -230,8 +230,9 @@ void TextfieldViewsModel::SelectRange(const TextRange& range) {
 }
 
 void TextfieldViewsModel::SelectAll() {
-  cursor_pos_ = 0;
-  selection_begin_ = text_.length();
+  // SelectAll selects towards the end.
+  cursor_pos_ = text_.length();
+  selection_begin_ = 0;
 }
 
 void TextfieldViewsModel::ClearSelection() {
diff --git a/views/controls/textfield/textfield_views_model_unittest.cc b/views/controls/textfield/textfield_views_model_unittest.cc
index f7018b8..1c45d74 100644
--- a/views/controls/textfield/textfield_views_model_unittest.cc
+++ b/views/controls/textfield/textfield_views_model_unittest.cc
@@ -104,6 +104,11 @@ TEST_F(TextfieldViewsModelTest, Selection) {
   EXPECT_EQ(string16(), model.GetSelectedText());
   model.SelectAll();
   EXPECT_STR_EQ("HELLO", model.GetSelectedText());
+  // SelectAll should select towards the end.
+  TextRange range;
+  model.GetSelectedRange(&range);
+  EXPECT_EQ(0U, range.start());
+  EXPECT_EQ(5U, range.end());
 
   // Select and move cursor
   model.MoveCursorTo(1U, false);
@@ -168,7 +173,7 @@ TEST_F(TextfieldViewsModelTest, Password) {
 
   model.SelectAll();
   EXPECT_STR_EQ("ELLO", model.GetSelectedText());
-  EXPECT_EQ(0U, model.cursor_pos());
+  EXPECT_EQ(4U, model.cursor_pos());
 
   model.Insert('X');
   EXPECT_STR_EQ("*", model.GetVisibleText());
@@ -288,10 +293,11 @@ TEST_F(TextfieldViewsModelTest, SetText) {
   EXPECT_EQ(5U, model.cursor_pos());
   model.SelectAll();
   EXPECT_STR_EQ("GOODBYE", model.GetSelectedText());
-  // Selection move the current pos to the begining.
+  // Selection move the current pos to the end.
+  EXPECT_EQ(7U, model.cursor_pos());
+  model.MoveCursorToStart(false);
   EXPECT_EQ(0U, model.cursor_pos());
   model.MoveCursorToEnd(false);
-  EXPECT_EQ(7U, model.cursor_pos());
 
   model.SetText(ASCIIToUTF16("BYE"));
   EXPECT_EQ(3U, model.cursor_pos());
@@ -342,7 +348,7 @@ TEST_F(TextfieldViewsModelTest, Clipboard) {
   clipboard->ReadText(ui::Clipboard::BUFFER_STANDARD, &clipboard_text);
   EXPECT_STR_EQ("HELLO HELLO WORLD", clipboard_text);
   EXPECT_STR_EQ("HELLO HELLO WORLD", model.text());
-  EXPECT_EQ(0U, model.cursor_pos());
+  EXPECT_EQ(17U, model.cursor_pos());
 
   // Test for paste.
   model.ClearSelection();