ash: Select omnibox text on mouse up instead of down.

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

Index: chrome/browser/ui/views/omnibox/omnibox_view_views.cc
diff --git a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
index 7aa4893269ee952fa42c2712cf1534eb5ba945c6..d04de68561eed0de20db1ed80ad3bbeb16c2301f 100644
--- a/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
+++ b/chrome/browser/ui/views/omnibox/omnibox_view_views.cc
@@ -87,7 +87,20 @@ class AutocompleteTextfield : public views::Textfield {
   }
 
   virtual bool OnMousePressed(const views::MouseEvent& event) OVERRIDE {
-    return omnibox_view_->HandleMousePressEvent(event);
+    bool result = views::Textfield::OnMousePressed(event);

[Daniel Erat, 2012/05/16 21:06:02]

Textfield doesn't implement these (yet), but I figure it's a good idea to call
them so this won't break when NativeTextfieldViews (which does implement them)
gets rolled into Textfield.

+    omnibox_view_->HandleMousePressEvent(event);

[oshima, 2012/05/16 21:28:31]

Is there reason why you should/want to ignore the result from OmniboxView? If
so, can you add comment?

[Daniel Erat, 2012/05/16 22:03:10]

It doesn't provide a result anymore; I don't think that there are any cases
where we want to override the built-in Textfield behavior.  Added a short
comment.

+    return result;
+  }
+
+  virtual bool OnMouseDragged(const views::MouseEvent& event) OVERRIDE {
+    bool result = views::Textfield::OnMouseDragged(event);
+    omnibox_view_->HandleMouseDragEvent(event);
+    return result;
+  }
+
+  virtual void OnMouseReleased(const views::MouseEvent& event) OVERRIDE {
+    views::Textfield::OnMouseReleased(event);
+    omnibox_view_->HandleMouseReleaseEvent(event);
   }
 
  private:
@@ -180,7 +193,8 @@ OmniboxViewViews::OmniboxViewViews(AutocompleteEditController* controller,
       ime_composing_before_change_(false),
       delete_at_end_pressed_(false),
       location_bar_view_(location_bar),
-      ime_candidate_window_open_(false) {
+      ime_candidate_window_open_(false),
+      select_all_on_mouse_release_(false) {
 }
 
 OmniboxViewViews::~OmniboxViewViews() {
@@ -305,14 +319,22 @@ bool OmniboxViewViews::HandleKeyReleaseEvent(const views::KeyEvent& event) {
   return false;
 }
 
-bool OmniboxViewViews::HandleMousePressEvent(const views::MouseEvent& event) {
-  if (!textfield_->HasFocus() && !textfield_->HasSelection()) {
-    textfield_->SelectAll();
-    textfield_->RequestFocus();

[Daniel Erat, 2012/05/16 21:06:02]

I'm dropping the RequestFocus() call and returning false so that
NativeTextfieldViews can handle focusing.

-    return true;
+void OmniboxViewViews::HandleMousePressEvent(const views::MouseEvent& event) {
+  if (event.IsOnlyLeftMouseButton() &&

[Peter Kasting, 2012/05/16 21:10:53]

This check isn't right, because we want click-and-release with the right button
to also select all (see Windows behavior).  We don't want the middle button to
select all, though.  (2 places)

[oshima, 2012/05/16 21:28:31]

I confirmed this on mac and win. linux doesn't but it's probably bug.

[Daniel Erat, 2012/05/16 22:03:10]

Ah, didn't know about that.  Updated.

I'm not sure of the right way to write a test for the right button case; I think
that we're stuck in a nested message loop after displaying the context menu. 
Posting the first click asynchronously and then either posting a quit or another
click might work, but it seems fragile.

This implementation of the omnibox seems counter-intuitive with regard to the
right button.  When I drag over some text with the right button, the selection
starts from the text cursor's last position instead of from the mouse-down
position.  Dragging with the left button starts at the mouse-down position.  If
this is a bug, I'll file a separate issue about it.

Oshima, see the 224 comments on
http://code.google.com/p/chromium/issues/detail?id=26140. :-P

[Peter Kasting, 2012/05/16 22:11:34]

Dragging with the right button should never select any text (or change any
existing selection).

+      !textfield_->HasFocus() &&
+      !textfield_->HasSelection()) {
+    select_all_on_mouse_release_ = true;
   }
+}
 
-  return false;
+void OmniboxViewViews::HandleMouseDragEvent(const views::MouseEvent& event) {
+  select_all_on_mouse_release_ = false;
+}
+
+void OmniboxViewViews::HandleMouseReleaseEvent(const views::MouseEvent& event) {
+  if (event.IsOnlyLeftMouseButton() && select_all_on_mouse_release_)
+    textfield_->SelectAll();
+  select_all_on_mouse_release_ = false;
 }
 
 void OmniboxViewViews::HandleFocusIn() {