[go: up one dir, main page]

Add pre-target handler on the root view to close the menu on press
events that don't happen over the textfield (or the open menu).

(cherry picked from commit 244eedee04eaae48db685649e1ab72ced7b89dd3)

Bug: 953185
Change-Id: I84af8df0ea97da1e3bb1daebc823697523811e0d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638126
Commit-Queue: Edin Kadric <edinkadric@google.com>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#667111}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1651051
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#210}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/ui/views/controls/editable_combobox/editable_combobox.cc b/ui/views/controls/editable_combobox/editable_combobox.cc
index 56ba44e..f9777a5 100644
--- a/ui/views/controls/editable_combobox/editable_combobox.cc
+++ b/ui/views/controls/editable_combobox/editable_combobox.cc
@@ -252,6 +252,51 @@
   DISALLOW_COPY_AND_ASSIGN(EditableComboboxMenuModel);
 };
 
+// This class adds itself as the pre-target handler for the RootView of the
+// EditableCombobox. We use it to close the menu when press events happen in the
+// RootView but not inside the EditableComboobox's textfield.
+class EditableCombobox::EditableComboboxPreTargetHandler
+    : public ui::EventHandler {
+ public:
+  EditableComboboxPreTargetHandler(EditableCombobox* owner, View* root_view)
+      : owner_(owner), root_view_(root_view) {
+    root_view_->AddPreTargetHandler(this);
+  }
+
+  ~EditableComboboxPreTargetHandler() override { StopObserving(); }
+
+  // ui::EventHandler overrides.
+  void OnMouseEvent(ui::MouseEvent* event) override {
+    if (event->type() == ui::ET_MOUSE_PRESSED &&
+        event->flags() == event->changed_button_flags())
+      HandlePressEvent(event->root_location());
+  }
+  void OnTouchEvent(ui::TouchEvent* event) override {
+    if (event->type() == ui::ET_TOUCH_PRESSED)
+      HandlePressEvent(event->root_location());
+  }
+
+ private:
+  void HandlePressEvent(const gfx::Point& root_location) {
+    View* handler = root_view_->GetEventHandlerForPoint(root_location);
+    if (handler == owner_->textfield_ || handler == owner_->arrow_)
+      return;
+    owner_->CloseMenu();
+  }
+
+  void StopObserving() {
+    if (!root_view_)
+      return;
+    root_view_->RemovePreTargetHandler(this);
+    root_view_ = nullptr;
+  }
+
+  EditableCombobox* owner_;
+  View* root_view_;
+
+  DISALLOW_COPY_AND_ASSIGN(EditableComboboxPreTargetHandler);
+};
+
 ////////////////////////////////////////////////////////////////////////////////
 // EditableCombobox, public, non-overridden methods:
 EditableCombobox::EditableCombobox(
@@ -430,6 +475,7 @@
 
 void EditableCombobox::CloseMenu() {
   menu_runner_.reset();
+  pre_target_handler_.reset();
 }
 
 void EditableCombobox::OnItemSelected(int index) {
@@ -473,6 +519,15 @@
   if (!textfield_->HasFocus() || (menu_runner_ && menu_runner_->IsRunning()))
     return;
 
+  // Since we don't capture the mouse, we want to see the events that happen in
+  // the EditableCombobox's RootView to get a chance to close the menu if they
+  // happen outside |textfield_|. Events that happen over the menu belong to
+  // another Widget and they don't go through this pre-target handler.
+  // Events that happen outside both the menu and the RootView cause
+  // OnViewBlurred to be called, which also closes the menu.
+  pre_target_handler_ = std::make_unique<EditableComboboxPreTargetHandler>(
+      this, GetWidget()->GetRootView());
+
   gfx::Rect local_bounds = textfield_->GetLocalBounds();
   gfx::Point menu_position(local_bounds.origin());
   // Inset the menu's requested position so the border of the menu lines up
diff --git a/ui/views/controls/editable_combobox/editable_combobox.h b/ui/views/controls/editable_combobox/editable_combobox.h
index e1491c5..d0208e1 100644
--- a/ui/views/controls/editable_combobox/editable_combobox.h
+++ b/ui/views/controls/editable_combobox/editable_combobox.h
@@ -31,6 +31,7 @@
 namespace views {
 class EditableComboboxMenuModel;
 class EditableComboboxListener;
+class EditableComboboxPreTargetHandler;
 class MenuRunner;
 class Textfield;
 
@@ -104,6 +105,7 @@
 
  private:
   class EditableComboboxMenuModel;
+  class EditableComboboxPreTargetHandler;
 
   void CloseMenu();
 
@@ -143,6 +145,11 @@
   // The EditableComboboxMenuModel used by |menu_runner_|.
   std::unique_ptr<EditableComboboxMenuModel> menu_model_;
 
+  // Pre-target handler that closes the menu when press events happen in the
+  // root view (outside of the open menu's boundaries) but not inside the
+  // textfield.
+  std::unique_ptr<EditableComboboxPreTargetHandler> pre_target_handler_;
+
   // Typography context for the text written in the textfield and the options
   // shown in the drop-down menu.
   const int text_context_;
diff --git a/ui/views/controls/editable_combobox/editable_combobox_unittest.cc b/ui/views/controls/editable_combobox/editable_combobox_unittest.cc
index c9f42e5..396f86bb 100644
--- a/ui/views/controls/editable_combobox/editable_combobox_unittest.cc
+++ b/ui/views/controls/editable_combobox/editable_combobox_unittest.cc
@@ -100,6 +100,10 @@
   EditableCombobox* combobox_ = nullptr;
   View* dummy_focusable_view_ = nullptr;
 
+  // We make |combobox_| a child of another View to test different removal
+  // scenarios.
+  View* parent_of_combobox_ = nullptr;
+
   // Listener for our EditableCombobox.
   std::unique_ptr<DummyListener> listener_;
 
@@ -135,15 +139,17 @@
     const bool filter_on_edit,
     const bool show_on_empty,
     const EditableCombobox::Type type) {
+  parent_of_combobox_ = new View();
+  parent_of_combobox_->SetID(1);
   combobox_ =
       new EditableCombobox(std::make_unique<ui::SimpleComboboxModel>(items),
                            filter_on_edit, show_on_empty, type);
   listener_ = std::make_unique<DummyListener>();
   combobox_->set_listener(listener_.get());
-  combobox_->SetID(1);
+  combobox_->SetID(2);
   dummy_focusable_view_ = new View();
   dummy_focusable_view_->SetFocusBehavior(View::FocusBehavior::ALWAYS);
-  dummy_focusable_view_->SetID(2);
+  dummy_focusable_view_->SetID(3);
 
   InitWidget();
 }
@@ -154,11 +160,14 @@
   Widget::InitParams params =
       CreateParams(Widget::InitParams::TYPE_WINDOW_FRAMELESS);
   params.bounds = gfx::Rect(0, 0, 1000, 1000);
+  parent_of_combobox_->SetBoundsRect(gfx::Rect(0, 0, 500, 40));
+  combobox_->SetBoundsRect(gfx::Rect(0, 0, 500, 40));
+
   widget_->Init(params);
   View* container = new View();
   widget_->SetContentsView(container);
-  container->AddChildView(combobox_);
-  combobox_->SetBoundsRect(gfx::Rect(0, 0, 500, 40));
+  container->AddChildView(parent_of_combobox_);
+  parent_of_combobox_->AddChildView(combobox_);
   container->AddChildView(dummy_focusable_view_);
   widget_->Show();
 
@@ -256,6 +265,51 @@
   EXPECT_FALSE(IsMenuOpen());
 }
 
+TEST_F(EditableComboboxTest,
+       ClickOutsideEditableComboboxWithoutLosingFocusClosesMenu) {
+  InitEditableCombobox();
+  combobox_->GetTextfieldForTest()->RequestFocus();
+  EXPECT_TRUE(IsMenuOpen());
+
+  const gfx::Point outside_point(combobox_->x() + combobox_->width() + 1,
+                                 combobox_->y() + 1);
+  PerformClick(widget_, outside_point);
+
+  WaitForMenuClosureAnimation();
+  EXPECT_FALSE(IsMenuOpen());
+  EXPECT_TRUE(combobox_->GetTextfieldForTest()->HasFocus());
+}
+
+TEST_F(EditableComboboxTest, ClickTextfieldDoesntCloseMenu) {
+  InitEditableCombobox();
+  combobox_->GetTextfieldForTest()->RequestFocus();
+  EXPECT_TRUE(IsMenuOpen());
+
+  MenuRunner* menu_runner1 = combobox_->GetMenuRunnerForTest();
+  ClickTextfield();
+  MenuRunner* menu_runner2 = combobox_->GetMenuRunnerForTest();
+  EXPECT_TRUE(IsMenuOpen());
+
+  // Making sure the menu didn't close and reopen (causing a flicker).
+  EXPECT_EQ(menu_runner1, menu_runner2);
+}
+
+TEST_F(EditableComboboxTest, RemovingControlWhileMenuOpenClosesMenu) {
+  InitEditableCombobox();
+  combobox_->GetTextfieldForTest()->RequestFocus();
+  EXPECT_TRUE(IsMenuOpen());
+  parent_of_combobox_->RemoveChildView(combobox_);
+  EXPECT_EQ(nullptr, combobox_->GetMenuRunnerForTest());
+}
+
+TEST_F(EditableComboboxTest, RemovingParentOfControlWhileMenuOpenClosesMenu) {
+  InitEditableCombobox();
+  combobox_->GetTextfieldForTest()->RequestFocus();
+  EXPECT_TRUE(IsMenuOpen());
+  widget_->GetContentsView()->RemoveChildView(parent_of_combobox_);
+  EXPECT_EQ(nullptr, combobox_->GetMenuRunnerForTest());
+}
+
 TEST_F(EditableComboboxTest, LeftOrRightKeysMoveInTextfield) {
   InitEditableCombobox();
   combobox_->GetTextfieldForTest()->RequestFocus();