views: obey PlatformStyle::kReturnClicksFocusedControl in MenuButton

Bug: 1006022
Change-Id: I54a875d1a4d59a0e4208f5ae76e7dca7addaf048
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1815701
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#698614}

ui/views/controls/button/menu_button_controller.cc

@@ -13,6 +13,7 @@
 #include "ui/views/controls/button/menu_button.h"
 #include "ui/views/controls/button/menu_button_listener.h"
 #include "ui/views/mouse_constants.h"
+#include "ui/views/style/platform_style.h"
 #include "ui/views/widget/root_view.h"
 #include "ui/views/widget/widget.h"
 
@@ -125,12 +126,18 @@ void MenuButtonController::OnMouseExited(const ui::MouseEvent& event) {
 }
 
 bool MenuButtonController::OnKeyPressed(const ui::KeyEvent& event) {
+  // Alt-space on windows should show the window menu.
+  if (event.key_code() == ui::VKEY_SPACE && event.IsAltDown())
+    return false;
+
+  // If Return doesn't normally click buttons, don't do it here either.
+  if (event.key_code() == ui::VKEY_RETURN &&
+      !PlatformStyle::kReturnClicksFocusedControl) {
+    return false;
+  }
+
   switch (event.key_code()) {
     case ui::VKEY_SPACE:
-      // Alt-space on windows should show the window menu.
-      if (event.IsAltDown())
-        break;
-      FALLTHROUGH;
     case ui::VKEY_RETURN:
     case ui::VKEY_UP:
     case ui::VKEY_DOWN: {

ui/views/controls/button/menu_button_unittest.cc

@@ -12,12 +12,14 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
+#include "ui/events/keycodes/dom/dom_code.h"
 #include "ui/events/test/event_generator.h"
 #include "ui/views/animation/test/ink_drop_host_view_test_api.h"
 #include "ui/views/animation/test/test_ink_drop.h"
 #include "ui/views/controls/button/menu_button_controller.h"
 #include "ui/views/controls/button/menu_button_listener.h"
 #include "ui/views/drag_controller.h"
+#include "ui/views/style/platform_style.h"
 #include "ui/views/test/views_test_base.h"
 #include "ui/views/widget/widget_utils.h"
 
@@ -168,6 +170,11 @@ class TestMenuButtonListener : public MenuButtonListener {
   View* last_source() { return last_source_; }
   Button::ButtonState last_source_state() { return last_source_state_; }
 
+  void Reset() {
+    last_source_ = nullptr;
+    last_source_state_ = Button::STATE_NORMAL;
+  }
+
  private:
   View* last_source_ = nullptr;
   Button::ButtonState last_source_state_ = Button::STATE_NORMAL;
@@ -343,6 +350,28 @@ TEST_F(MenuButtonTest, ActivateDropDownOnMouseClick) {
   EXPECT_EQ(Button::STATE_HOVERED, menu_button_listener.last_source_state());
 }
 
+TEST_F(MenuButtonTest, ActivateOnKeyPress) {
+  TestMenuButtonListener menu_button_listener;
+  CreateMenuButtonWithMenuButtonListener(&menu_button_listener);
+
+  EXPECT_EQ(nullptr, menu_button_listener.last_source());
+  button()->OnKeyPressed(ui::KeyEvent(
+      ui::ET_KEY_PRESSED, ui::KeyboardCode::VKEY_SPACE, ui::DomCode::SPACE, 0));
+  EXPECT_EQ(button(), menu_button_listener.last_source());
+
+  menu_button_listener.Reset();
+  EXPECT_EQ(nullptr, menu_button_listener.last_source());
+
+  button()->OnKeyPressed(ui::KeyEvent(ui::ET_KEY_PRESSED,
+                                      ui::KeyboardCode::VKEY_RETURN,
+                                      ui::DomCode::ENTER, 0));
+  if (PlatformStyle::kReturnClicksFocusedControl) {
+    EXPECT_EQ(button(), menu_button_listener.last_source());
+  } else {
+    EXPECT_EQ(nullptr, menu_button_listener.last_source());
+  }
+}
+
 // Tests that the ink drop center point is set from the mouse click point.
 TEST_F(MenuButtonTest, InkDropCenterSetFromClick) {
   TestMenuButtonListener menu_button_listener;