Keyboard shortcuts are ignored in the menu.
Keyboard Shortcuts: https://support.google.com/chrome/answer/157179
Modifiers are mistakenly ignored for all menu hotkeys. Keyboard
shortcuts need to be handled in the app menu, when any keyboard
shortcut is pressed, close the app menu, and after that, it should
perform the supposed task.
This CL will handle keyboard shortcuts in the app menu. Currently we
are handling Shift-Esc only. Adding Ctrl-J shortcut with this CL, as
it is supposed to open the downloads page. In follow-up patches,
other shortcuts will be included.
Bug: None
Change-Id: I906818922206d2d48dd22a9a67212fabe571de46
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5584445
Reviewed-by: Shubham Gupta <shubham.gupta@chromium.org>
Reviewed-by: Keren Zhu <kerenzhu@chromium.org>
Commit-Queue: Sakib Shabir <s1.tantray@samsung.com>
Cr-Commit-Position: refs/heads/main@{#1308472}
diff --git a/ui/views/controls/menu/menu_controller.cc b/ui/views/controls/menu/menu_controller.cc
index a46bffb..49e475d 100644
--- a/ui/views/controls/menu/menu_controller.cc
+++ b/ui/views/controls/menu/menu_controller.cc
@@ -1354,14 +1354,11 @@
// Key events can lead to this being deleted.
if (!this_ref) {
- // Don't stop event propagation for Shift-Esc because it is
- // supposed to open the Task Manager on Windows and Linux.
- if (event->IsShiftDown() && event->key_code() == ui::VKEY_ESCAPE) {
- return ui::POST_DISPATCH_PERFORM_DEFAULT;
- } else {
+ if (ShouldStopEventPropagation(*event)) {
event->StopPropagation();
return ui::POST_DISPATCH_NONE;
}
+ return ui::POST_DISPATCH_PERFORM_DEFAULT;
}
if (!IsEditableCombobox() && !event->stopped_propagation()) {
@@ -1800,12 +1797,44 @@
break;
#endif
+ case ui::VKEY_J:
+ // Fully exit the menu when Ctrl+J is pressed because it is supposed
+ // to open the downloads page on Windows and Linux.
+ if (event.IsControlDown()) {
+ Cancel(ExitType::kAll);
+ break;
+ }
+ break;
default:
break;
}
return handled_key_code;
}
+bool MenuController::ShouldStopEventPropagation(const ui::KeyEvent& event) {
+ DCHECK_EQ(event.type(), ui::ET_KEY_PRESSED);
+ const ui::KeyboardCode key_code = event.key_code();
+ switch (key_code) {
+ case ui::VKEY_ESCAPE:
+ // Don't stop event propagation for Shift-Esc because it is
+ // supposed to open the Task Manager on Windows and Linux.
+ if (event.IsShiftDown()) {
+ return false;
+ }
+ break;
+ case ui::VKEY_J:
+ // Don't stop event propagation for Ctrl-J because it is
+ // supposed to open downloads page.
+ if (event.IsControlDown()) {
+ return false;
+ }
+ break;
+ default:
+ break;
+ }
+ return true;
+}
+
MenuController::MenuController(bool for_drop,
internal::MenuControllerDelegate* delegate)
: for_drop_(for_drop),
diff --git a/ui/views/controls/menu/menu_controller.h b/ui/views/controls/menu/menu_controller.h
index a051195..b70d80c 100644
--- a/ui/views/controls/menu/menu_controller.h
+++ b/ui/views/controls/menu/menu_controller.h
@@ -391,6 +391,9 @@
// Returns true if OnKeyPressed handled the key |event|.
bool OnKeyPressed(const ui::KeyEvent& event);
+ // Returns false if we don't want to stop event propagation here.
+ bool ShouldStopEventPropagation(const ui::KeyEvent& event);
+
// Creates a MenuController. See |for_drop_| member for details on |for_drop|.
MenuController(bool for_drop, internal::MenuControllerDelegate* delegate);
