Selects last item in a menu when pressing 'up' initially

ui/views/controls/menu/menu_controller.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ui/views/controls/menu/menu_controller.h"
 
 #include "base/i18n/case_conversion.h"
 #include "base/i18n/rtl.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
@@ skipping 1886 matching lines @@
 }
 
 void MenuController::IncrementSelection(int delta) {
   MenuItemView* item = pending_state_.item;
   DCHECK(item);
   if (pending_state_.submenu_open && item->HasSubmenu() &&
       item->GetSubmenu()->IsShowing()) {
     // A menu is selected and open, but none of its children are selected,
     // select the first menu item that is visible and enabled.
     if (item->GetSubmenu()->GetMenuItemCount()) {
       MenuItemView* to_select = FindFirstSelectableMenuItem(item);

[sadrul, 2015/07/14 04:17:41]

Hm. Perhaps we could add delta to FindFirstSelectableMenuItem() too, and pass
that from here? That way, we don't have to special case delta==1 here.

FindFirstSelectableMenuItem() is essentially equivalent to
FindNextSelectableMenuItem(index=-1, delta=1), right? Would it be possible to
cleanly merge the two into one?

[varkha, 2015/07/14 16:22:54]

Done.

       if (to_select)
         SetSelection(to_select, SELECTION_DEFAULT);
-      return;
+      if (delta == 1 || !to_select)
+        return;
+      // When decrementing fall through to select a previous item.
+      item = to_select;
     }
   }
 
   if (item->has_children()) {
     CustomButton* button = GetFirstHotTrackedView(item);
     if (button) {
       button->SetHotTracked(false);
       View* to_make_hot = GetNextFocusableView(item, button, delta == 1);
       CustomButton* button_hot = CustomButton::AsCustomButton(to_make_hot);
       if (button_hot) {
@@ skipping 436 matching lines @@
   }
 }
 
 gfx::Screen* MenuController::GetScreen() {
   Widget* root = owner_ ? owner_->GetTopLevelWidget() : NULL;
   return root ? gfx::Screen::GetScreenFor(root->GetNativeView())
               : gfx::Screen::GetNativeScreen();
 }
 
 }  // namespace views