Commit d4dda5f7 authored by Edin Kadric's avatar Edin Kadric Committed by Commit Bot

Open EditableCombobox menu on mouse release event (and gesture).

This makes it such that when the EditableCombobox is focused but the
menu isn't open, a click on the textfield but not the arrow still opens
the menu (as would happen with a click on the textfield if the menu
weren't focused).
We can get in this situation in two ways:
1) After selecting an item or closing the menu with something like Esc,
the menu is still in focus.
2) When first opening a View hosting the EditableCombobox, it could
have been set not to show the menu on initial focus (as is the case
in the password bubble for example).

See associated bug for screenshot.

We look at mouse release instead of press so that the user can still
select the by clicking and dragging the mouse. If we look at mouse
press events then the menu will open and the drag will not select
text.

Note that we added a call to TextfieldController::HandleMouseEvent in
Textfield::OnMouseReleased, so we also had to add a check in other
users of TextfieldController::HandleMouseEvent.

Bug: 960317
Change-Id: Iaba4834be5e0f68ad3191ca572f2ab011825678e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1608805Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Commit-Queue: Edin Kadric <edinkadric@google.com>
Cr-Commit-Position: refs/heads/master@{#661238}
parent 27138bd6
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "ui/base/models/menu_separator_types.h" #include "ui/base/models/menu_separator_types.h"
#include "ui/base/ui_base_types.h" #include "ui/base/ui_base_types.h"
#include "ui/events/event.h" #include "ui/events/event.h"
#include "ui/events/event_constants.h"
#include "ui/gfx/canvas.h" #include "ui/gfx/canvas.h"
#include "ui/gfx/font_list.h" #include "ui/gfx/font_list.h"
#include "ui/gfx/geometry/insets.h" #include "ui/gfx/geometry/insets.h"
...@@ -369,6 +370,32 @@ void EditableCombobox::ContentsChanged(Textfield* sender, ...@@ -369,6 +370,32 @@ void EditableCombobox::ContentsChanged(Textfield* sender,
ShowDropDownMenu(ui::MENU_SOURCE_KEYBOARD); ShowDropDownMenu(ui::MENU_SOURCE_KEYBOARD);
} }
bool EditableCombobox::HandleMouseEvent(Textfield* sender,
const ui::MouseEvent& mouse_event) {
// We show the menu on mouse release instead of mouse press so that the menu
// showing up doesn't interrupt a potential text selection operation by the
// user.
// If we don't already have focus when the mouse is pressed, we set
// |show_menu_on_next_focus_| to false so that the focus event that will
// follow the press event and precede the release event does not end up
// showing the menu and interrupting a text selection operation.
if (mouse_event.type() == ui::ET_MOUSE_PRESSED && !textfield_->HasFocus())
show_menu_on_next_focus_ = false;
else if (mouse_event.type() == ui::ET_MOUSE_RELEASED)
ShowDropDownMenu(ui::MENU_SOURCE_MOUSE);
return false;
}
bool EditableCombobox::HandleGestureEvent(
Textfield* sender,
const ui::GestureEvent& gesture_event) {
ShowDropDownMenu(ui::MENU_SOURCE_TOUCH);
return false;
}
////////////////////////////////////////////////////////////////////////////////
// EditableCombobox, View overrides:
void EditableCombobox::OnViewBlurred(View* observed_view) { void EditableCombobox::OnViewBlurred(View* observed_view) {
CloseMenu(); CloseMenu();
} }
......
...@@ -128,6 +128,10 @@ class VIEWS_EXPORT EditableCombobox : public View, ...@@ -128,6 +128,10 @@ class VIEWS_EXPORT EditableCombobox : public View,
// Overridden from TextfieldController: // Overridden from TextfieldController:
void ContentsChanged(Textfield* sender, void ContentsChanged(Textfield* sender,
const base::string16& new_contents) override; const base::string16& new_contents) override;
bool HandleMouseEvent(Textfield* sender,
const ui::MouseEvent& mouse_event) override;
bool HandleGestureEvent(Textfield* sender,
const ui::GestureEvent& gesture_event) override;
// Overridden from ViewObserver: // Overridden from ViewObserver:
void OnViewFocused(View* observed_view) override; void OnViewFocused(View* observed_view) override;
......
...@@ -80,6 +80,7 @@ class EditableComboboxTest : public ViewsTestBase { ...@@ -80,6 +80,7 @@ class EditableComboboxTest : public ViewsTestBase {
protected: protected:
void ClickArrow(); void ClickArrow();
void ClickMenuItem(int index); void ClickMenuItem(int index);
void ClickTextfield();
bool IsMenuOpen(); bool IsMenuOpen();
void PerformMouseEvent(Widget* widget, void PerformMouseEvent(Widget* widget,
const gfx::Point& point, const gfx::Point& point,
...@@ -141,6 +142,7 @@ void EditableComboboxTest::InitEditableCombobox( ...@@ -141,6 +142,7 @@ void EditableComboboxTest::InitEditableCombobox(
dummy_focusable_view_->SetID(2); dummy_focusable_view_->SetID(2);
InitWidget(); InitWidget();
combobox_->GetTextfieldForTest()->RequestFocus();
combobox_->RequestFocus(); combobox_->RequestFocus();
combobox_->SizeToPreferredSize(); combobox_->SizeToPreferredSize();
} }
...@@ -183,6 +185,11 @@ void EditableComboboxTest::ClickMenuItem(const int index) { ...@@ -183,6 +185,11 @@ void EditableComboboxTest::ClickMenuItem(const int index) {
PerformClick(*child_widgets.begin(), middle_of_item); PerformClick(*child_widgets.begin(), middle_of_item);
} }
void EditableComboboxTest::ClickTextfield() {
const gfx::Point textfield(combobox_->x() + 1, combobox_->y() + 1);
PerformClick(widget_, textfield);
}
bool EditableComboboxTest::IsMenuOpen() { bool EditableComboboxTest::IsMenuOpen() {
return combobox_->GetMenuRunnerForTest() && return combobox_->GetMenuRunnerForTest() &&
combobox_->GetMenuRunnerForTest()->IsRunning(); combobox_->GetMenuRunnerForTest()->IsRunning();
...@@ -224,6 +231,7 @@ void EditableComboboxTest::SendKeyEvent(ui::KeyboardCode key_code, ...@@ -224,6 +231,7 @@ void EditableComboboxTest::SendKeyEvent(ui::KeyboardCode key_code,
TEST_F(EditableComboboxTest, FocusOnTextfieldOpensMenu) { TEST_F(EditableComboboxTest, FocusOnTextfieldOpensMenu) {
InitEditableCombobox(); InitEditableCombobox();
dummy_focusable_view_->RequestFocus();
EXPECT_FALSE(IsMenuOpen()); EXPECT_FALSE(IsMenuOpen());
combobox_->GetTextfieldForTest()->RequestFocus(); combobox_->GetTextfieldForTest()->RequestFocus();
EXPECT_TRUE(IsMenuOpen()); EXPECT_TRUE(IsMenuOpen());
...@@ -627,6 +635,7 @@ TEST_F(EditableComboboxTest, PasswordCanBeHiddenAndRevealed) { ...@@ -627,6 +635,7 @@ TEST_F(EditableComboboxTest, PasswordCanBeHiddenAndRevealed) {
TEST_F(EditableComboboxTest, ArrowButtonOpensAndClosesMenu) { TEST_F(EditableComboboxTest, ArrowButtonOpensAndClosesMenu) {
InitEditableCombobox(); InitEditableCombobox();
dummy_focusable_view_->RequestFocus();
EXPECT_FALSE(IsMenuOpen()); EXPECT_FALSE(IsMenuOpen());
ClickArrow(); ClickArrow();
...@@ -656,5 +665,20 @@ TEST_F(EditableComboboxTest, ShowMenuOnNextFocusBehavior) { ...@@ -656,5 +665,20 @@ TEST_F(EditableComboboxTest, ShowMenuOnNextFocusBehavior) {
EXPECT_TRUE(IsMenuOpen()); EXPECT_TRUE(IsMenuOpen());
} }
TEST_F(EditableComboboxTest, ShowMenuOnClickWhenAlreadyFocused) {
std::vector<base::string16> items = {ASCIIToUTF16("item0"),
ASCIIToUTF16("item1")};
InitEditableCombobox(items, /*filter_on_edit=*/false,
/*show_on_empty=*/true);
dummy_focusable_view_->RequestFocus();
combobox_->set_show_menu_on_next_focus(false);
combobox_->GetTextfieldForTest()->RequestFocus();
EXPECT_FALSE(IsMenuOpen());
ClickTextfield();
EXPECT_TRUE(IsMenuOpen());
}
} // namespace } // namespace
} // namespace views } // namespace views
...@@ -682,6 +682,8 @@ bool Textfield::OnMouseDragged(const ui::MouseEvent& event) { ...@@ -682,6 +682,8 @@ bool Textfield::OnMouseDragged(const ui::MouseEvent& event) {
} }
void Textfield::OnMouseReleased(const ui::MouseEvent& event) { void Textfield::OnMouseReleased(const ui::MouseEvent& event) {
if (controller_)
controller_->HandleMouseEvent(this, event);
selection_controller_.OnMouseReleased(event); selection_controller_.OnMouseReleased(event);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment