Fix non-Latin key event handling for menu mnemonics

BUG=564255,582555

Review URL: https://codereview.chromium.org/1652133003

Cr-Commit-Position: refs/heads/master@{#372987}

2 files changed, 63 insertions, 6 deletions

diff --git a/ui/views/controls/menu/menu_key_event_handler.cc b/ui/views/controls/menu/menu_key_event_handler.cc
index c751c4c..af0566e 100644
--- a/ui/views/controls/menu/menu_key_event_handler.cc
+++ b/ui/views/controls/menu/menu_key_event_handler.cc
@@ -57,7 +57,7 @@ void MenuKeyEventHandler::OnKeyEvent(ui::KeyEvent* event) {
     const int flags = event->flags();
     if (menu_controller->exit_type() == MenuController::EXIT_NONE &&
         (flags & kKeyFlagsMask) == 0) {
-      char c = event->GetCharacter();
+      base::char16 c = event->GetCharacter();
       menu_controller->SelectByChar(c);
       // Menu controller might have been deleted.
       if (!MenuController::GetActiveInstance())
diff --git a/ui/views/controls/menu/menu_runner_unittest.cc b/ui/views/controls/menu/menu_runner_unittest.cc
index 246322c..dab7de9 100644
--- a/ui/views/controls/menu/menu_runner_unittest.cc
+++ b/ui/views/controls/menu/menu_runner_unittest.cc
@@ -8,11 +8,13 @@
 
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/strings/utf_string_conversions.h"
 #include "ui/base/ui_base_types.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/views/controls/menu/menu_delegate.h"
 #include "ui/views/controls/menu/menu_item_view.h"
 #include "ui/views/controls/menu/menu_types.h"
+#include "ui/views/controls/menu/submenu_view.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget.h"
 
@@ -20,22 +22,28 @@ namespace views {
 namespace test {
 
 // Implementation of MenuDelegate that only reports the values of calls to
-// OnMenuClosed.
+// OnMenuClosed and ExecuteCommand.
 class TestMenuDelegate : public MenuDelegate {
  public:
   TestMenuDelegate();
   ~TestMenuDelegate() override;
 
-  int on_menu_closed_called() { return on_menu_closed_called_; }
-  MenuItemView* on_menu_closed_menu() { return on_menu_closed_menu_; }
-  MenuRunner::RunResult on_menu_closed_run_result() {
+  int execute_command_id() const { return execute_command_id_; }
+
+  int on_menu_closed_called() const { return on_menu_closed_called_; }
+  MenuItemView* on_menu_closed_menu() const { return on_menu_closed_menu_; }
+  MenuRunner::RunResult on_menu_closed_run_result() const {
     return on_menu_closed_run_result_;
   }
 
   // MenuDelegate:
+  void ExecuteCommand(int id) override;
   void OnMenuClosed(MenuItemView* menu, MenuRunner::RunResult result) override;
 
  private:
+  // ID of last executed command.
+  int execute_command_id_;
+
   // The number of times OnMenuClosed was called.
   int on_menu_closed_called_;
 
@@ -47,12 +55,17 @@ class TestMenuDelegate : public MenuDelegate {
 };
 
 TestMenuDelegate::TestMenuDelegate()
-    : on_menu_closed_called_(0),
+    : execute_command_id_(0),
+      on_menu_closed_called_(0),
       on_menu_closed_menu_(nullptr),
       on_menu_closed_run_result_(MenuRunner::MENU_DELETED) {}
 
 TestMenuDelegate::~TestMenuDelegate() {}
 
+void TestMenuDelegate::ExecuteCommand(int id) {
+  execute_command_id_ = id;
+}
+
 void TestMenuDelegate::OnMenuClosed(MenuItemView* menu,
                                     MenuRunner::RunResult result) {
   on_menu_closed_called_++;
@@ -101,6 +114,9 @@ void MenuRunnerTest::SetUp() {
   ViewsTestBase::SetUp();
   menu_delegate_.reset(new TestMenuDelegate);
   menu_item_view_ = new MenuItemView(menu_delegate_.get());
+  menu_item_view_->AppendMenuItemWithLabel(1, base::ASCIIToUTF16("One"));
+  menu_item_view_->AppendMenuItemWithLabel(2,
+                                           base::WideToUTF16(L"\x062f\x0648"));
 
   owner_.reset(new Widget);
   Widget::InitParams params = CreateParams(Widget::InitParams::TYPE_POPUP);
@@ -152,5 +168,46 @@ TEST_F(MenuRunnerTest, AsynchronousKeyEventHandling) {
   EXPECT_EQ(MenuRunner::NORMAL_EXIT, delegate->on_menu_closed_run_result());
 }
 
+// Tests that a key press on a US keyboard layout activates the correct menu
+// item.
+TEST_F(MenuRunnerTest, LatinMnemonic) {
+  InitMenuRunner(MenuRunner::ASYNC);
+  MenuRunner* runner = menu_runner();
+  MenuRunner::RunResult result = runner->RunMenuAt(
+      owner(), nullptr, gfx::Rect(), MENU_ANCHOR_TOPLEFT, ui::MENU_SOURCE_NONE);
+  EXPECT_EQ(MenuRunner::NORMAL_EXIT, result);
+  EXPECT_TRUE(runner->IsRunning());
+
+  ui::test::EventGenerator generator(GetContext(), owner()->GetNativeWindow());
+  generator.PressKey(ui::VKEY_O, 0);
+  EXPECT_FALSE(runner->IsRunning());
+  TestMenuDelegate* delegate = menu_delegate();
+  EXPECT_EQ(1, delegate->execute_command_id());
+  EXPECT_EQ(1, delegate->on_menu_closed_called());
+  EXPECT_NE(nullptr, delegate->on_menu_closed_menu());
+  EXPECT_EQ(MenuRunner::NORMAL_EXIT, delegate->on_menu_closed_run_result());
+}
+
+// Tests that a key press on a non-US keyboard layout activates the correct menu
+// item.
+TEST_F(MenuRunnerTest, NonLatinMnemonic) {
+  InitMenuRunner(MenuRunner::ASYNC);
+  MenuRunner* runner = menu_runner();
+  MenuRunner::RunResult result = runner->RunMenuAt(
+      owner(), nullptr, gfx::Rect(), MENU_ANCHOR_TOPLEFT, ui::MENU_SOURCE_NONE);
+  EXPECT_EQ(MenuRunner::NORMAL_EXIT, result);
+  EXPECT_TRUE(runner->IsRunning());
+
+  ui::test::EventGenerator generator(GetContext(), owner()->GetNativeWindow());
+  ui::KeyEvent key_press(0x062f, ui::VKEY_N, 0);
+  generator.Dispatch(&key_press);
+  EXPECT_FALSE(runner->IsRunning());
+  TestMenuDelegate* delegate = menu_delegate();
+  EXPECT_EQ(2, delegate->execute_command_id());
+  EXPECT_EQ(1, delegate->on_menu_closed_called());
+  EXPECT_NE(nullptr, delegate->on_menu_closed_menu());
+  EXPECT_EQ(MenuRunner::NORMAL_EXIT, delegate->on_menu_closed_run_result());
+}
+
 }  // namespace test
 }  // namespace views