Implementing context menu for the new views textfield.

views/controls/textfield/native_textfield_views.cc

Index: views/controls/textfield/native_textfield_views.cc
diff --git a/views/controls/textfield/native_textfield_views.cc b/views/controls/textfield/native_textfield_views.cc
index 923de0c4e4265f75952dec8a7b711523a374eec9..9293600cc17c344996baaa26d25f615043a547ba 100644
--- a/views/controls/textfield/native_textfield_views.cc
+++ b/views/controls/textfield/native_textfield_views.cc
@@ -13,12 +13,16 @@
 #include "gfx/canvas.h"
 #include "gfx/canvas_skia.h"
 #include "gfx/insets.h"
+#include "grit/app_strings.h"
+#include "ui/base/clipboard/clipboard.h"
 #include "views/background.h"
 #include "views/border.h"
+#include "views/controls/menu/menu_2.h"
 #include "views/controls/textfield/native_textfield_gtk.h"
 #include "views/controls/textfield/textfield.h"
 #include "views/controls/textfield/textfield_views_model.h"
 #include "views/event.h"
+#include "views/views_delegate.h"
 
 namespace {
 
@@ -63,6 +67,8 @@ NativeTextfieldViews::NativeTextfieldViews(Textfield* parent)
   DCHECK_NE(parent->style(), Textfield::STYLE_MULTILINE);
   // Lowercase is not supported.
   DCHECK_NE(parent->style(), Textfield::STYLE_LOWERCASE);
+
+  SetContextMenuController(this);
 }
 
 NativeTextfieldViews::~NativeTextfieldViews() {
@@ -73,10 +79,12 @@ NativeTextfieldViews::~NativeTextfieldViews() {
 
 bool NativeTextfieldViews::OnMousePressed(const views::MouseEvent& e) {
   textfield_->RequestFocus();
-  size_t pos = FindCursorPosition(e.location());
-  if (model_->MoveCursorTo(pos, false)) {
-    UpdateCursorBoundsAndTextOffset();
-    SchedulePaint();
+  if (e.IsLeftMouseButton()) {

[oshima, 2011/01/12 18:46:25]

q: should this be "Only"?

[varunjain, 2011/01/12 19:59:05]

the behaviour of the current textfield in chrome's external build suggests not.
If left click is combined with any other button, only left click seems to be
effective.

+    size_t pos = FindCursorPosition(e.location());
+    if (model_->MoveCursorTo(pos, false)) {
+      UpdateCursorBoundsAndTextOffset();
+      SchedulePaint();
+    }

[sadrul, 2011/01/12 19:09:16]

Are there plans for middle-click-to-paste? Or is that only if X selection is
used?

[oshima, 2011/01/12 19:25:31]

It's X selection and different from cut&paste. I've heard that we don't want
x-selection, but not 100% sure.

   }
   return true;
 }
@@ -133,6 +141,16 @@ void NativeTextfieldViews::WillLoseFocus() {
 }
 
 /////////////////////////////////////////////////////////////////
+// NativeTextfieldViews, views::ContextMenuController overrides:
+void NativeTextfieldViews::ShowContextMenu(View* source,
+                                           const gfx::Point& p,
+                                           bool is_mouse_gesture) {
+  if (!context_menu_menu_.get())
+    CreateContextMenu();
+  context_menu_menu_->RunContextMenuAt(p);
+}
+
+/////////////////////////////////////////////////////////////////
 // NativeTextfieldViews, NativeTextifieldWrapper overrides:
 
 string16 NativeTextfieldViews::GetText() const {
@@ -293,6 +311,64 @@ void NativeTextfieldViews::HandleWillLoseFocus() {
   }
 }
 
+/////////////////////////////////////////////////////////////////
+// NativeTextfieldViews, menus::SimpleMenuModel::Delegate overrides:
+
+bool NativeTextfieldViews::IsCommandIdChecked(int command_id) const {
+  return true;
+}
+
+bool NativeTextfieldViews::IsCommandIdEnabled(int command_id) const {
+  string16 result;
+  switch (command_id) {
+    case IDS_APP_CUT:
+      return 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();
+    case IDS_APP_DELETE:
+      return model_->HasSelection();
+    case IDS_APP_SELECT_ALL:
+      return true;
+    default:
+      return false;
+  }
+  return false;
+}
+
+bool NativeTextfieldViews::GetAcceleratorForCommandId(int command_id,
+    menus::Accelerator* accelerator) {
+  return false;
+}
+
+void NativeTextfieldViews::ExecuteCommand(int command_id) {
+  bool text_changed = false;
+  switch (command_id) {
+    case IDS_APP_CUT:
+      text_changed = model_->Cut();
+      break;
+    case IDS_APP_COPY:
+      model_->Copy();
+      break;
+    case IDS_APP_PASTE:
+      text_changed = model_->Paste();
+      break;
+    case IDS_APP_DELETE:
+      text_changed = model_->Delete();
+      break;
+    case IDS_APP_SELECT_ALL:
+      SelectAll();
+      break;
+    default:

[oshima, 2011/01/12 18:46:25]

NOTREACHED() << "unkonwn command:" << command_id;

[varunjain, 2011/01/12 19:59:05]

Done.

+      break;
+  }
+  if (text_changed)
+    PropagateTextChange();
+}
+
 // static
 bool NativeTextfieldViews::IsTextfieldViewsEnabled() {
 #if defined(TOUCH_UI)
@@ -525,12 +601,8 @@ bool NativeTextfieldViews::HandleKeyEvent(const KeyEvent& key_event) {
         model_->Replace(print_char);
       text_changed = true;
     }
-    if (text_changed) {
-      textfield_->SyncText();
-      Textfield::Controller* controller = textfield_->GetController();
-      if (controller)
-        controller->ContentsChanged(textfield_, GetText());
-    }
+    if (text_changed)
+      PropagateTextChange();
     if (cursor_changed) {
       is_cursor_visible_ = true;
       RepaintCursor();
@@ -693,6 +765,25 @@ size_t NativeTextfieldViews::FindCursorPosition(const gfx::Point& point) const {
   return left_pos;
 }
 
+void NativeTextfieldViews::PropagateTextChange() {
+  textfield_->SyncText();
+  Textfield::Controller* controller = textfield_->GetController();
+  if (controller)
+    controller->ContentsChanged(textfield_, GetText());
+}
+
+void NativeTextfieldViews::CreateContextMenu() {
+  context_menu_contents_.reset(new menus::SimpleMenuModel(this));
+  context_menu_contents_->AddItemWithStringId(IDS_APP_CUT, IDS_APP_CUT);

[sadrul, 2011/01/12 19:09:16]

IDC_xxx should be used for command-id, and IDS_xxx should be used for string-id.
I notice that this is incorrect in native_textfield_win too. Perhaps fix both?

[varunjain, 2011/01/12 19:59:05]

I was confused by this... IDCs are defined in chrome tree so I didnot want to
include a dependency here in views. The IDSs I have used here are from app/

[sadrul, 2011/01/12 20:24:59]

I fixed up some cases of IDS_/IDC_ mixups in chrome/ a few weeks back, but the
case for views/ do seem more difficult enough to belong in a separate fix. I
wouldn't hold this against this CL.

+  context_menu_contents_->AddItemWithStringId(IDS_APP_COPY, IDS_APP_COPY);
+  context_menu_contents_->AddItemWithStringId(IDS_APP_PASTE, IDS_APP_PASTE);
+  context_menu_contents_->AddItemWithStringId(IDS_APP_DELETE, IDS_APP_DELETE);
+  context_menu_contents_->AddSeparator();
+  context_menu_contents_->AddItemWithStringId(IDS_APP_SELECT_ALL,
+                                              IDS_APP_SELECT_ALL);
+  context_menu_menu_.reset(new Menu2(context_menu_contents_.get()));
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 // NativeTextfieldWrapper: