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

ui/views/controls/menu/menu_controller_unittest.cc

 // Copyright 2014 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/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
 #include "ui/events/event_handler.h"
 #include "ui/events/null_event_targeter.h"
@@ skipping 16 matching lines @@
 #undef None
 #include "ui/events/devices/x11/device_data_manager_x11.h"
 #include "ui/events/test/events_test_utils_x11.h"
 #elif defined(USE_OZONE)
 #include "ui/events/event.h"
 #elif defined(OS_MACOSX)
 #include "ui/events/test/event_generator.h"
 #endif
 
 namespace views {
+namespace test {
 
 namespace {
 
 class TestMenuItemView : public MenuItemView {
  public:
   TestMenuItemView() : MenuItemView(nullptr) {}
   ~TestMenuItemView() override {}
 
  private:
   DISALLOW_COPY_AND_ASSIGN(TestMenuItemView);
 };
 
+class SubmenuViewShown : public SubmenuView {
+ public:
+  SubmenuViewShown(MenuItemView* parent) : SubmenuView(parent) {}
+  ~SubmenuViewShown() override {}
+  bool IsShowing() override { return true; }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SubmenuViewShown);
+};
+
 class TestPlatformEventSource : public ui::PlatformEventSource {
  public:
   TestPlatformEventSource() {
 #if defined(USE_X11)
     ui::DeviceDataManagerX11::CreateInstance();
 #endif
   }
   ~TestPlatformEventSource() override {}
 
   uint32_t Dispatch(const ui::PlatformEvent& event) {
@@ skipping 37 matching lines @@
   }
 
   base::MessagePumpDispatcher* dispatcher_;
 
   DISALLOW_COPY_AND_ASSIGN(TestDispatcherClient);
 };
 #endif  // USE_AURA
 
 }  // namespace
 
+class TestMenuItemViewShown : public MenuItemView {
+ public:
+  TestMenuItemViewShown() : MenuItemView(nullptr) {
+    submenu_ = new SubmenuViewShown(this);
+  }
+  ~TestMenuItemViewShown() override {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestMenuItemViewShown);
+};
+
 class MenuControllerTest : public ViewsTestBase {
  public:
   MenuControllerTest() : controller_(nullptr) {}
   ~MenuControllerTest() override { ResetMenuController(); }
 
   // Dispatches |count| number of items, each in a separate iteration of the
   // message-loop, by posting a task.
   void Step3_DispatchEvents(int count) {
     base::MessageLoopForUI* loop = base::MessageLoopForUI::current();
     base::MessageLoop::ScopedNestableTaskAllower allow(loop);
@@ skipping 47 matching lines @@
 #endif
     return widget.Pass();
   }
 
   const MenuItemView* pending_state_item() const {
     return controller_->pending_state_.item;
   }
 
   void SetPendingStateItem(MenuItemView* item) {
     controller_->pending_state_.item = item;
+    controller_->pending_state_.submenu_open = true;
   }
 
   void ResetSelection() {
     controller_->SetSelection(nullptr,
                               MenuController::SELECTION_EXIT |
                               MenuController::SELECTION_UPDATE_IMMEDIATELY);
   }
 
-  void IncrementSelection(int delta) {
-    controller_->IncrementSelection(delta);
+  void IncrementSelection() {
+    controller_->IncrementSelection(MenuController::INCREMENT_SELECTION_DOWN);
   }
 
-  MenuItemView* FindFirstSelectableMenuItem(MenuItemView* parent) {
-    return controller_->FindFirstSelectableMenuItem(parent);
+  void DecrementSelection() {
+    controller_->IncrementSelection(MenuController::INCREMENT_SELECTION_UP);
+  }
+
+  MenuItemView* FindInitialSelectableMenuItemDown(MenuItemView* parent) {
+    return controller_->FindInitialSelectableMenuItem(
+        parent, MenuController::INCREMENT_SELECTION_DOWN);
+  }
+
+  MenuItemView* FindInitialSelectableMenuItemUp(MenuItemView* parent) {
+    return controller_->FindInitialSelectableMenuItem(
+        parent, MenuController::INCREMENT_SELECTION_UP);
   }
 
   MenuItemView* FindNextSelectableMenuItem(MenuItemView* parent,
-                                           int index,
-                                           int delta) {
-    return controller_->FindNextSelectableMenuItem(parent, index, delta);
+                                           int index) {
+    return controller_->FindNextSelectableMenuItem(
+        parent, index, MenuController::INCREMENT_SELECTION_DOWN);
   }
+
+  MenuItemView* FindPreviousSelectableMenuItem(MenuItemView* parent,
+                                               int index) {
+    return controller_->FindNextSelectableMenuItem(
+        parent, index, MenuController::INCREMENT_SELECTION_UP);
+  }
+
   void SetupMenu(views::Widget* owner, views::MenuItemView* item) {
     ResetMenuController();
     controller_ = new MenuController(nullptr, true, nullptr);
     controller_->owner_ = owner;
     controller_->showing_ = true;
     controller_->SetSelection(item,
                               MenuController::SELECTION_UPDATE_IMMEDIATELY);
   }
 
   void RunMenu(views::Widget* owner) {
@@ skipping 130 matching lines @@
         break;
     }
   }
 
   int outstanding_touches() const { return outstanding_touches_; }
 
  private:
   int outstanding_touches_;
 };
 
-// Tests that touch event ids are released correctly. See
-// crbug.com/439051 for details. When the ids aren't managed
-// correctly, we get stuck down touches.
+// Tests that touch event ids are released correctly. See crbug.com/439051 for
+// details. When the ids aren't managed correctly, we get stuck down touches.
 TEST_F(MenuControllerTest, TouchIdsReleasedCorrectly) {
   scoped_ptr<Widget> owner(CreateOwnerWidget());
   TestEventHandler test_event_handler;
   owner->GetNativeWindow()->GetRootWindow()->AddPreTargetHandler(
       &test_event_handler);
 
   std::vector<int> devices;
   devices.push_back(1);
   ui::SetUpTouchDevicesForTest(devices);
 
@@ skipping 11 matching lines @@
                  base::Unretained(this), MenuController::EXIT_OUTERMOST));
 
   RunMenu(owner.get());
   EXPECT_EQ(0, test_event_handler.outstanding_touches());
 
   owner->GetNativeWindow()->GetRootWindow()->RemovePreTargetHandler(
       &test_event_handler);
 }
 #endif // defined(USE_X11)
 
-TEST_F(MenuControllerTest, FirstSelectedItem) {
+// Tests that initial selected menu items are correct when items are enabled or
+// disabled.
+TEST_F(MenuControllerTest, InitialSelectedItem) {
   scoped_ptr<Widget> owner(CreateOwnerWidget());
   scoped_ptr<TestMenuItemView> menu_item(new TestMenuItemView);
   menu_item->AppendMenuItemWithLabel(1, base::ASCIIToUTF16("One"));
   menu_item->AppendMenuItemWithLabel(2, base::ASCIIToUTF16("Two"));
-  // Disabling the item "One" gets it skipped when a menu is first opened.
-  menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(false);
-
+  menu_item->AppendMenuItemWithLabel(3, base::ASCIIToUTF16("Three"));
   SetupMenu(owner.get(), menu_item.get());
 
-  // First selectable item should be item "Two".
-  MenuItemView* first_selectable = FindFirstSelectableMenuItem(menu_item.get());
+  // Leave items "Two" and "Three" enabled.
+  menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(false);
+  // The first selectable item should be item "Two".
+  MenuItemView* first_selectable =
+      FindInitialSelectableMenuItemDown(menu_item.get());
+  ASSERT_NE(nullptr, first_selectable);
   EXPECT_EQ(2, first_selectable->GetCommand());
+  // The last selectable item should be item "Three".
+  MenuItemView* last_selectable =
+      FindInitialSelectableMenuItemUp(menu_item.get());
+  ASSERT_NE(nullptr, last_selectable);
+  EXPECT_EQ(3, last_selectable->GetCommand());
+
+  // Leave items "One" and "Two" enabled.
+  menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(true);
+  menu_item->GetSubmenu()->GetMenuItemAt(1)->SetEnabled(true);
+  menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(false);
+  // The first selectable item should be item "One".
+  first_selectable = FindInitialSelectableMenuItemDown(menu_item.get());
+  ASSERT_NE(nullptr, first_selectable);
+  EXPECT_EQ(1, first_selectable->GetCommand());
+  // The last selectable item should be item "Two".
+  last_selectable = FindInitialSelectableMenuItemUp(menu_item.get());
+  ASSERT_NE(nullptr, last_selectable);
+  EXPECT_EQ(2, last_selectable->GetCommand());
+
+  // Leave only a single item "One" enabled.
+  menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(true);
+  menu_item->GetSubmenu()->GetMenuItemAt(1)->SetEnabled(false);
+  menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(false);
+  // The first selectable item should be item "One".
+  first_selectable = FindInitialSelectableMenuItemDown(menu_item.get());
+  ASSERT_NE(nullptr, first_selectable);
+  EXPECT_EQ(1, first_selectable->GetCommand());
+  // The last selectable item should be item "One".
+  last_selectable = FindInitialSelectableMenuItemUp(menu_item.get());
+  ASSERT_NE(nullptr, last_selectable);
+  EXPECT_EQ(1, last_selectable->GetCommand());
+
+  // Leave only a single item "Three" enabled.
+  menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(false);
+  menu_item->GetSubmenu()->GetMenuItemAt(1)->SetEnabled(false);
+  menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(true);
+  // The first selectable item should be item "Three".
+  first_selectable = FindInitialSelectableMenuItemDown(menu_item.get());
+  ASSERT_NE(nullptr, first_selectable);
+  EXPECT_EQ(3, first_selectable->GetCommand());
+  // The last selectable item should be item "Three".
+  last_selectable = FindInitialSelectableMenuItemUp(menu_item.get());
+  ASSERT_NE(nullptr, last_selectable);
+  EXPECT_EQ(3, last_selectable->GetCommand());
+
+  // Leave only a single item ("Two") selected. It should be the first and the
+  // last selectable item.
+  menu_item->GetSubmenu()->GetMenuItemAt(0)->SetEnabled(false);
+  menu_item->GetSubmenu()->GetMenuItemAt(1)->SetEnabled(true);
+  menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(false);
+  first_selectable = FindInitialSelectableMenuItemDown(menu_item.get());
+  ASSERT_NE(nullptr, first_selectable);
+  EXPECT_EQ(2, first_selectable->GetCommand());
+  last_selectable = FindInitialSelectableMenuItemUp(menu_item.get());
+  ASSERT_NE(nullptr, last_selectable);
+  EXPECT_EQ(2, last_selectable->GetCommand());
 
   // There should be no next or previous selectable item since there is only a
   // single enabled item in the menu.
-  SetPendingStateItem(first_selectable);
-  EXPECT_EQ(nullptr, FindNextSelectableMenuItem(menu_item.get(), 1, 1));
-  EXPECT_EQ(nullptr, FindNextSelectableMenuItem(menu_item.get(), 1, -1));
+  EXPECT_EQ(nullptr, FindNextSelectableMenuItem(menu_item.get(), 1));
+  EXPECT_EQ(nullptr, FindPreviousSelectableMenuItem(menu_item.get(), 1));
 
   // Clear references in menu controller to the menu item that is going away.
   ResetSelection();
 }
 
+// Tests that opening menu and pressing 'Down' and 'Up' iterates over enabled
+// items.
 TEST_F(MenuControllerTest, NextSelectedItem) {
   scoped_ptr<Widget> owner(CreateOwnerWidget());
   scoped_ptr<TestMenuItemView> menu_item(new TestMenuItemView);
   menu_item->AppendMenuItemWithLabel(1, base::ASCIIToUTF16("One"));
   menu_item->AppendMenuItemWithLabel(2, base::ASCIIToUTF16("Two"));
   menu_item->AppendMenuItemWithLabel(3, base::ASCIIToUTF16("Three"));
   menu_item->AppendMenuItemWithLabel(4, base::ASCIIToUTF16("Four"));
   // Disabling the item "Three" gets it skipped when using keyboard to navigate.
   menu_item->GetSubmenu()->GetMenuItemAt(2)->SetEnabled(false);
 
   SetupMenu(owner.get(), menu_item.get());
 
   // Fake initial hot selection.
   SetPendingStateItem(menu_item->GetSubmenu()->GetMenuItemAt(0));
   EXPECT_EQ(1, pending_state_item()->GetCommand());
 
   // Move down in the menu.
   // Select next item.
-  IncrementSelection(1);
+  IncrementSelection();
   EXPECT_EQ(2, pending_state_item()->GetCommand());
 
   // Skip disabled item.
-  IncrementSelection(1);
+  IncrementSelection();
   EXPECT_EQ(4, pending_state_item()->GetCommand());
 
   // Wrap around.
-  IncrementSelection(1);
+  IncrementSelection();
   EXPECT_EQ(1, pending_state_item()->GetCommand());
 
   // Move up in the menu.
   // Wrap around.
-  IncrementSelection(-1);
+  DecrementSelection();
   EXPECT_EQ(4, pending_state_item()->GetCommand());
 
   // Skip disabled item.
-  IncrementSelection(-1);
+  DecrementSelection();
   EXPECT_EQ(2, pending_state_item()->GetCommand());
 
   // Select previous item.
-  IncrementSelection(-1);
+  DecrementSelection();
   EXPECT_EQ(1, pending_state_item()->GetCommand());
 
   // Clear references in menu controller to the menu item that is going away.
   ResetSelection();
 }
 
+// Tests that opening menu and pressing 'Up' selects the last enabled menu item.
+TEST_F(MenuControllerTest, PreviousSelectedItem) {
+  scoped_ptr<Widget> owner(CreateOwnerWidget());
+  scoped_ptr<TestMenuItemViewShown> menu_item(new TestMenuItemViewShown);
+  menu_item->AppendMenuItemWithLabel(1, base::ASCIIToUTF16("One"));
+  menu_item->AppendMenuItemWithLabel(2, base::ASCIIToUTF16("Two"));
+  menu_item->AppendMenuItemWithLabel(3, base::ASCIIToUTF16("Three"));
+  menu_item->AppendMenuItemWithLabel(4, base::ASCIIToUTF16("Four"));
+  // Disabling the item "Four" gets it skipped when using keyboard to navigate.
+  menu_item->GetSubmenu()->GetMenuItemAt(3)->SetEnabled(false);
+
+  SetupMenu(owner.get(), menu_item.get());
+
+  // Fake initial root item selection and submenu showing.
+  SetPendingStateItem(menu_item.get());
+  EXPECT_EQ(0, pending_state_item()->GetCommand());
+
+  // Move up and select a previous (in our case the last enabled) item.
+  DecrementSelection();
+  EXPECT_EQ(3, pending_state_item()->GetCommand());
+
+  // Clear references in menu controller to the menu item that is going away.
+  ResetSelection();
+}
+
+}  // namespace test
 }  // namespace views