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();