Index: ui/views/controls/menu/menu_controller.cc |
diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc |
index 0a5dca62ad1639ed708126188ae25607a4f01af2..2bc14aa179e78e2a5a602501b018ca21911b9df7 100644 |
--- a/ui/views/controls/menu/menu_controller.cc |
+++ b/ui/views/controls/menu/menu_controller.cc |
@@ -53,16 +53,6 @@ using base::Time; |
using base::TimeDelta; |
using ui::OSExchangeData; |
-// Period of the scroll timer (in milliseconds). |
-static const int kScrollTimerMS = 30; |
- |
-// Amount of time from when the drop exits the menu and the menu is hidden. |
-static const int kCloseOnExitTime = 1200; |
- |
-// If a context menu is invoked by touch, we shift the menu by this offset so |
-// that the finger does not obscure the menu. |
-static const int kCenteredContextMenuYOffset = -15; |
- |
namespace views { |
namespace { |
@@ -70,7 +60,17 @@ namespace { |
// When showing context menu on mouse down, the user might accidentally select |
// the menu item on the subsequent mouse up. To prevent this, we add the |
// following delay before the user is able to select an item. |
-static int menu_selection_hold_time_ms = kMinimumMsPressedToActivate; |
+int menu_selection_hold_time_ms = kMinimumMsPressedToActivate; |
+ |
+// Period of the scroll timer (in milliseconds). |
+const int kScrollTimerMS = 30; |
+ |
+// Amount of time from when the drop exits the menu and the menu is hidden. |
+const int kCloseOnExitTime = 1200; |
+ |
+// If a context menu is invoked by touch, we shift the menu by this offset so |
+// that the finger does not obscure the menu. |
+const int kCenteredContextMenuYOffset = -15; |
// The spacing offset for the bubble tip. |
const int kBubbleTipSizeLeftRight = 12; |
@@ -97,7 +97,7 @@ bool TitleMatchesMnemonic(MenuItemView* menu, base::char16 key) { |
} |
// Returns the first descendant of |view| that is hot tracked. |
-static CustomButton* GetFirstHotTrackedView(View* view) { |
+CustomButton* GetFirstHotTrackedView(View* view) { |
if (!view) |
return NULL; |
CustomButton* button = CustomButton::AsCustomButton(view); |
@@ -114,12 +114,32 @@ static CustomButton* GetFirstHotTrackedView(View* view) { |
return NULL; |
} |
+// Returns a CustomButton descendant of |view| that is (a) hot-tracked and |
+// (b) not equal to |hot_button|. Returns null if such descendant is not found. |
+// This allows a caller to detect when a button is set to be hot-tracked by a |
+// mouse event and its |hot_button_| indicator becomes stale. |
+CustomButton* GetFirstHotTrackedViewDifferingFrom(View* view, |
+ CustomButton* hot_button) { |
+ if (!view) |
+ return nullptr; |
+ CustomButton* button = CustomButton::AsCustomButton(view); |
+ if (button && button != hot_button && button->IsHotTracked()) |
+ return button; |
+ |
+ for (int i = 0; i < view->child_count(); ++i) { |
+ button = GetFirstHotTrackedViewDifferingFrom(view->child_at(i), hot_button); |
+ if (button) |
+ return button; |
+ } |
+ return nullptr; |
+} |
+ |
// Recurses through the child views of |view| returning the first view starting |
// at |start| that is focusable. A value of -1 for |start| indicates to start at |
// the first view (if |forward| is false, iterating starts at the last view). If |
// |forward| is true the children are considered first to last, otherwise last |
// to first. |
-static View* GetFirstFocusableView(View* view, int start, bool forward) { |
+View* GetFirstFocusableView(View* view, int start, bool forward) { |
if (forward) { |
for (int i = start == -1 ? 0 : start; i < view->child_count(); ++i) { |
View* deepest = GetFirstFocusableView(view->child_at(i), -1, forward); |
@@ -137,15 +157,13 @@ static View* GetFirstFocusableView(View* view, int start, bool forward) { |
} |
// Returns the first child of |start| that is focusable. |
-static View* GetInitialFocusableView(View* start, bool forward) { |
+View* GetInitialFocusableView(View* start, bool forward) { |
return GetFirstFocusableView(start, -1, forward); |
} |
// Returns the next view after |start_at| that is focusable. Returns NULL if |
// there are no focusable children of |ancestor| after |start_at|. |
-static View* GetNextFocusableView(View* ancestor, |
- View* start_at, |
- bool forward) { |
+View* GetNextFocusableView(View* ancestor, View* start_at, bool forward) { |
DCHECK(ancestor->Contains(start_at)); |
View* parent = start_at; |
do { |
@@ -800,10 +818,15 @@ View* MenuController::GetTooltipHandlerForPoint(SubmenuView* source, |
void MenuController::ViewHierarchyChanged( |
SubmenuView* source, |
const View::ViewHierarchyChangedDetails& details) { |
- // If the current mouse handler is removed, remove it as the handler. |
- if (!details.is_add && details.child == current_mouse_event_target_) { |
- current_mouse_event_target_ = nullptr; |
- current_mouse_pressed_state_ = 0; |
+ if (!details.is_add) { |
+ // If the current mouse handler is removed, remove it as the handler. |
+ if (details.child == current_mouse_event_target_) { |
+ current_mouse_event_target_ = nullptr; |
+ current_mouse_pressed_state_ = 0; |
+ } |
+ // Update |hot_button_| if it gets deleted while a menu is up. |
+ if (details.child == hot_button_) |
+ hot_button_ = nullptr; |
} |
} |
@@ -1034,7 +1057,20 @@ void MenuController::SetSelection(MenuItemView* menu_item, |
if (pending_item_changed && pending_state_.item) { |
CustomButton* button = GetFirstHotTrackedView(pending_state_.item); |
if (button) |
- button->SetHotTracked(false); |
+ SetHotTrackedButton(button, false); |
+ } |
+ // A button could become hot-tracked when a mouse is moved without changing |
sky
2016/02/23 20:11:13
I still don't get what case you are hitting that i
varkha
2016/02/23 21:01:39
This scenario:
1. Open app menu with extension but
|
+ // which MenuItem is selected. When a menu item selection is updated, presence |
+ // of a hot-tracked button that is not |hot_button_| can mean only that a new |
+ // hot-tracked button was set. In this case update |hot_button_| to match. |
+ // This allows correct hot-tracked button to be used when alternating between |
+ // keyboard and mouse navigation in the menu. |
+ CustomButton* new_hot_button = |
+ GetFirstHotTrackedViewDifferingFrom(menu_item, hot_button_); |
+ if (new_hot_button) { |
+ if (hot_button_) |
+ SetHotTrackedButton(hot_button_, false); |
+ SetHotTrackedButton(new_hot_button, true); |
} |
// Notify the old path it isn't selected. |
@@ -1286,6 +1322,7 @@ MenuController::MenuController(bool blocking, |
last_drop_operation_(MenuDelegate::DROP_UNKNOWN), |
showing_submenu_(false), |
active_mouse_view_id_(ViewStorage::GetInstance()->CreateStorageID()), |
+ hot_button_(nullptr), |
delegate_(delegate), |
message_loop_depth_(0), |
closing_event_time_(base::TimeDelta()), |
@@ -1321,8 +1358,7 @@ bool MenuController::SendAcceleratorToHotTrackedView() { |
ui::Accelerator accelerator(ui::VKEY_RETURN, ui::EF_NONE); |
hot_view->AcceleratorPressed(accelerator); |
- CustomButton* button = static_cast<CustomButton*>(hot_view); |
- button->SetHotTracked(true); |
+ SetHotTrackedButton(hot_view, true); |
return true; |
} |
@@ -2084,8 +2120,7 @@ void MenuController::IncrementSelection( |
// select the first menu item that is visible and enabled. |
if (item->GetSubmenu()->GetMenuItemCount()) { |
MenuItemView* to_select = FindInitialSelectableMenuItem(item, direction); |
- if (to_select) |
- SetSelection(to_select, SELECTION_DEFAULT); |
+ SetInitialHotTrackedView(to_select, direction); |
return; |
} |
} |
@@ -2093,20 +2128,20 @@ void MenuController::IncrementSelection( |
if (item->has_children()) { |
CustomButton* button = GetFirstHotTrackedView(item); |
if (button) { |
- button->SetHotTracked(false); |
- View* to_make_hot = GetNextFocusableView( |
+ SetHotTrackedButton(button, false); |
+ View* hot_view = GetNextFocusableView( |
item, button, direction == INCREMENT_SELECTION_DOWN); |
- CustomButton* button_hot = CustomButton::AsCustomButton(to_make_hot); |
- if (button_hot) { |
- button_hot->SetHotTracked(true); |
+ CustomButton* hot_button = CustomButton::AsCustomButton(hot_view); |
+ if (hot_button) { |
+ SetHotTrackedButton(hot_button, true); |
return; |
} |
} else { |
- View* to_make_hot = |
+ View* hot_view = |
GetInitialFocusableView(item, direction == INCREMENT_SELECTION_DOWN); |
- CustomButton* button_hot = CustomButton::AsCustomButton(to_make_hot); |
- if (button_hot) { |
- button_hot->SetHotTracked(true); |
+ CustomButton* hot_button = CustomButton::AsCustomButton(hot_view); |
+ if (hot_button) { |
+ SetHotTrackedButton(hot_button, true); |
return; |
} |
} |
@@ -2120,14 +2155,7 @@ void MenuController::IncrementSelection( |
if (parent->GetSubmenu()->GetMenuItemAt(i) == item) { |
MenuItemView* to_select = |
FindNextSelectableMenuItem(parent, i, direction); |
- if (!to_select) |
- break; |
- SetSelection(to_select, SELECTION_DEFAULT); |
- View* to_make_hot = GetInitialFocusableView( |
- to_select, direction == INCREMENT_SELECTION_DOWN); |
- CustomButton* button_hot = CustomButton::AsCustomButton(to_make_hot); |
- if (button_hot) |
- button_hot->SetHotTracked(true); |
+ SetInitialHotTrackedView(to_select, direction); |
break; |
} |
} |
@@ -2609,4 +2637,25 @@ void MenuController::HandleMouseLocation(SubmenuView* source, |
} |
} |
+void MenuController::SetHotTrackedButton(CustomButton* hot_button, |
+ bool is_hot_tracked) { |
+ DCHECK(hot_button); |
sky
2016/02/23 20:11:13
The constraints on calling this function are not o
varkha
2016/02/23 21:01:39
Yes, will try that.
varkha
2016/02/24 01:42:21
Done.
|
+ hot_button->SetHotTracked(is_hot_tracked); |
+ DCHECK(is_hot_tracked || (hot_button_ == hot_button)); |
+ hot_button_ = is_hot_tracked ? hot_button : nullptr; |
+} |
+ |
+void MenuController::SetInitialHotTrackedView( |
+ MenuItemView* item, |
+ SelectionIncrementDirectionType direction) { |
+ if (!item) |
+ return; |
+ SetSelection(item, SELECTION_DEFAULT); |
+ View* hot_view = |
+ GetInitialFocusableView(item, direction == INCREMENT_SELECTION_DOWN); |
+ CustomButton* hot_button = CustomButton::AsCustomButton(hot_view); |
+ if (hot_button) |
+ SetHotTrackedButton(hot_button, true); |
+} |
+ |
} // namespace views |