Keyboard accessibility for the page and app menus.

Works on Windows, and on Linux with toolkit_views.

The goal is to make Chrome behave more like a standard Windows
application, for users who rely on the keyboard and expect standard
keyboard accelerators to work.

Pressing F10, or pressing and releasing Alt, will set focus to the
Page menu, as if it was the first item in a menu bar.

Pressing enter, space, up arrow, or down arrow will open the focused menu.
Once a menu is opened, pressing left and right arrows will switch between
the two menus. Pressing escape will return focus to the title of the
previously open menu.

A new UI test attempts to select something from the menus using only the
keyboard. It works on Linux (with toolkit_views) and on Windows.

BUG=none
TEST=New keyboard accessibility ui test.

Review URL: http://codereview.chromium.org/660323

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@42217 0039d316-1c4b-4281-b951-d872f2087c98

13 files changed, 214 insertions, 18 deletions

diff --git a/views/controls/menu/menu_2.cc b/views/controls/menu/menu_2.cc
index 2228b72..2fe090a 100644
--- a/views/controls/menu/menu_2.cc
+++ b/views/controls/menu/menu_2.cc
@@ -42,4 +42,8 @@ void Menu2::UpdateStates() {
   wrapper_->UpdateStates();
 }
 
+MenuWrapper::MenuAction Menu2::GetMenuAction() const {
+  return wrapper_->GetMenuAction();
+}
+
 }  // namespace
diff --git a/views/controls/menu/menu_2.h b/views/controls/menu/menu_2.h
index 77ff3d0..adb1c0f 100644
--- a/views/controls/menu/menu_2.h
+++ b/views/controls/menu/menu_2.h
@@ -1,6 +1,6 @@
-// Copyright (c) 2010 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.
+// Copyright (c) 2010 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.
 
 #ifndef VIEWS_CONTROLS_MENU_MENU_2_H_
 #define VIEWS_CONTROLS_MENU_MENU_2_H_
@@ -61,6 +61,11 @@ class Menu2 {
   // For submenus.
   gfx::NativeMenu GetNativeMenu() const;
 
+  // Get the result of the last call to RunMenuAt to determine whether an
+  // item was selected, the user navigated to a next or previous menu, or
+  // nothing.
+  MenuWrapper::MenuAction GetMenuAction() const;
+
   // Accessors.
   menus::MenuModel* model() const { return model_; }
 
@@ -78,4 +83,3 @@ class Menu2 {
 }  // namespace views
 
 #endif  // VIEWS_CONTROLS_MENU_MENU_2_H_
-
diff --git a/views/controls/menu/menu_wrapper.h b/views/controls/menu/menu_wrapper.h
index b985387..f23bd4f 100644
--- a/views/controls/menu/menu_wrapper.h
+++ b/views/controls/menu/menu_wrapper.h
@@ -1,6 +1,6 @@
-// Copyright (c) 2009 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.
+// Copyright (c) 2010 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.
 
 #ifndef VIEWS_CONTROLS_MENU_MENU_WRAPPER_H_
 #define VIEWS_CONTROLS_MENU_MENU_WRAPPER_H_
@@ -18,6 +18,14 @@ class Menu2;
 // An interface that wraps an object that implements a menu.
 class MenuWrapper {
  public:
+  // All of the possible actions that can result from RunMenuAt.
+  enum MenuAction {
+    MENU_ACTION_NONE,      // Menu cancelled, or never opened.
+    MENU_ACTION_SELECTED,  // An item was selected.
+    MENU_ACTION_PREVIOUS,  // User wants to navigate to the previous menu.
+    MENU_ACTION_NEXT,      // User wants to navigate to the next menu.
+  };
+
   virtual ~MenuWrapper() {}
 
   // Runs the menu at the specified point. This blocks until done.
@@ -37,6 +45,11 @@ class MenuWrapper {
   // Retrieve a native menu handle.
   virtual gfx::NativeMenu GetNativeMenu() const = 0;
 
+  // Get the result of the last call to RunMenuAt to determine whether an
+  // item was selected, the user navigated to a next or previous menu, or
+  // nothing.
+  virtual MenuAction GetMenuAction() const = 0;
+
   // Creates the appropriate instance of this wrapper for the current platform.
   static MenuWrapper* CreateWrapper(Menu2* menu);
 };
diff --git a/views/controls/menu/native_menu_gtk.cc b/views/controls/menu/native_menu_gtk.cc
index 395e9a5..5ff5c00 100644
--- a/views/controls/menu/native_menu_gtk.cc
+++ b/views/controls/menu/native_menu_gtk.cc
@@ -76,7 +76,8 @@ NativeMenuGtk::NativeMenuGtk(Menu2* menu)
       activated_menu_(NULL),
       activated_index_(-1),
       activate_factory_(this),
-      host_menu_(menu) {
+      host_menu_(menu),
+      menu_action_(MENU_ACTION_NONE) {
 }
 
 NativeMenuGtk::~NativeMenuGtk() {
@@ -93,6 +94,7 @@ NativeMenuGtk::~NativeMenuGtk() {
 void NativeMenuGtk::RunMenuAt(const gfx::Point& point, int alignment) {
   activated_menu_ = NULL;
   activated_index_ = -1;
+  menu_action_ = MENU_ACTION_NONE;
 
   UpdateStates();
   Position position = { point, static_cast<Menu2::Alignment>(alignment) };
@@ -104,13 +106,18 @@ void NativeMenuGtk::RunMenuAt(const gfx::Point& point, int alignment) {
   menu_shown_ = true;
   // Listen for "hide" signal so that we know when to return from the blocking
   // RunMenuAt call.
-  gint handle_id =
+  gint hide_handle_id =
       g_signal_connect(menu_, "hide", G_CALLBACK(OnMenuHidden), this);
 
+  gint move_handle_id =
+      g_signal_connect(menu_, "move-current", G_CALLBACK(OnMenuMoveCurrent),
+                       this);
+
   // Block until menu is no longer shown by running a nested message loop.
   MessageLoopForUI::current()->Run(NULL);
 
-  g_signal_handler_disconnect(G_OBJECT(menu_), handle_id);
+  g_signal_handler_disconnect(G_OBJECT(menu_), hide_handle_id);
+  g_signal_handler_disconnect(G_OBJECT(menu_), move_handle_id);
   menu_shown_ = false;
 
   if (activated_menu_) {
@@ -174,6 +181,10 @@ gfx::NativeMenu NativeMenuGtk::GetNativeMenu() const {
   return menu_;
 }
 
+NativeMenuGtk::MenuAction NativeMenuGtk::GetMenuAction() const {
+  return menu_action_;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeMenuGtk, private:
 
@@ -188,6 +199,26 @@ void NativeMenuGtk::OnMenuHidden(GtkWidget* widget, NativeMenuGtk* menu) {
   MessageLoop::current()->Quit();
 }
 
+// static
+void NativeMenuGtk::OnMenuMoveCurrent(GtkMenu* menu_widget,
+                                      GtkMenuDirectionType focus_direction,
+                                      NativeMenuGtk* menu) {
+  GtkWidget* parent = GTK_MENU_SHELL(menu_widget)->parent_menu_shell;
+  GtkWidget* menu_item = GTK_MENU_SHELL(menu_widget)->active_menu_item;
+  GtkWidget* submenu = NULL;
+  if (menu_item) {
+    submenu = gtk_menu_item_get_submenu(GTK_MENU_ITEM(menu_item));
+  }
+
+  if (focus_direction == GTK_MENU_DIR_CHILD && submenu == NULL) {
+    menu->GetAncestor()->menu_action_ = MENU_ACTION_NEXT;
+    gtk_menu_popdown(menu_widget);
+  } else if (focus_direction == GTK_MENU_DIR_PARENT && parent == NULL) {
+    menu->GetAncestor()->menu_action_ = MENU_ACTION_PREVIOUS;
+    gtk_menu_popdown(menu_widget);
+  }
+}
+
 void NativeMenuGtk::AddSeparatorAt(int index) {
   GtkWidget* separator = gtk_separator_menu_item_new();
   gtk_widget_show(separator);
@@ -374,6 +405,7 @@ void NativeMenuGtk::OnActivate(GtkMenuItem* menu_item) {
     NativeMenuGtk* ancestor = GetAncestor();
     ancestor->activated_menu_ = this;
     activated_index_ = position;
+    ancestor->menu_action_ = MENU_ACTION_SELECTED;
   }
 }
 
diff --git a/views/controls/menu/native_menu_gtk.h b/views/controls/menu/native_menu_gtk.h
index 2c69ba3..2ea0c84c 100644
--- a/views/controls/menu/native_menu_gtk.h
+++ b/views/controls/menu/native_menu_gtk.h
@@ -1,6 +1,6 @@
-// Copyright (c) 2009 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.
+// Copyright (c) 2010 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.
 
 #ifndef VIEWS_CONTROLS_MENU_NATIVE_MENU_GTK_H_
 #define VIEWS_CONTROLS_MENU_NATIVE_MENU_GTK_H_
@@ -36,9 +36,13 @@ class NativeMenuGtk : public MenuWrapper {
   virtual void Rebuild();
   virtual void UpdateStates();
   virtual gfx::NativeMenu GetNativeMenu() const;
+  virtual MenuAction GetMenuAction() const;
 
  private:
   static void OnMenuHidden(GtkWidget* widget, NativeMenuGtk* menu);
+  static void OnMenuMoveCurrent(GtkMenu* widget,
+                                GtkMenuDirectionType focus_direction,
+                                NativeMenuGtk* menu);
 
   void AddSeparatorAt(int index);
   GtkWidget* AddMenuItemAt(int index, GtkRadioMenuItem* radio_group,
@@ -111,6 +115,9 @@ class NativeMenuGtk : public MenuWrapper {
   Menu2* host_menu_;
   gulong destroy_handler_id_;
 
+  // The action that took place during the call to RunMenuAt.
+  MenuAction menu_action_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeMenuGtk);
 };
 
diff --git a/views/controls/menu/native_menu_win.cc b/views/controls/menu/native_menu_win.cc
index 75b8d4c..d959b85 100644
--- a/views/controls/menu/native_menu_win.cc
+++ b/views/controls/menu/native_menu_win.cc
@@ -306,7 +306,8 @@ NativeMenuWin::NativeMenuWin(menus::MenuModel* model, HWND system_menu_for)
       owner_draw_(l10n_util::NeedOverrideDefaultUIFont(NULL, NULL) &&
                   !system_menu_for),
       system_menu_for_(system_menu_for),
-      first_item_index_(0) {
+      first_item_index_(0),
+      menu_action_(MENU_ACTION_NONE) {
 }
 
 NativeMenuWin::~NativeMenuWin() {
@@ -322,10 +323,24 @@ void NativeMenuWin::RunMenuAt(const gfx::Point& point, int alignment) {
   UpdateStates();
   UINT flags = TPM_LEFTBUTTON | TPM_RECURSE;
   flags |= GetAlignmentFlags(alignment);
+  menu_action_ = MENU_ACTION_NONE;
+
+  // Set a hook function so we can listen for keyboard events while the
+  // menu is open, and store a pointer to this object in a static
+  // variable so the hook has access to it (ugly, but it's the
+  // only way).
+  open_native_menu_win_ = this;
+  HHOOK hhook = SetWindowsHookEx(WH_MSGFILTER, MenuMessageHook,
+                                 GetModuleHandle(NULL), ::GetCurrentThreadId());
+
   // Command dispatch is done through WM_MENUCOMMAND, handled by the host
   // window.
+  HWND hwnd = host_window_->hwnd();
   TrackPopupMenuEx(menu_, flags, point.x(), point.y(), host_window_->hwnd(),
                    NULL);
+
+  UnhookWindowsHookEx(hhook);
+  open_native_menu_win_ = NULL;
 }
 
 void NativeMenuWin::CancelMenu() {
@@ -370,9 +385,58 @@ gfx::NativeMenu NativeMenuWin::GetNativeMenu() const {
   return menu_;
 }
 
+NativeMenuWin::MenuAction NativeMenuWin::GetMenuAction() const {
+  return menu_action_;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // NativeMenuWin, private:
 
+// static
+NativeMenuWin* NativeMenuWin::open_native_menu_win_ = NULL;
+
+// static
+bool NativeMenuWin::GetHighlightedMenuItemInfo(
+    HMENU menu, bool* has_parent, bool* has_submenu) {
+  for (int i = 0; i < ::GetMenuItemCount(menu); i++) {
+    UINT state = ::GetMenuState(menu, i, MF_BYPOSITION);
+    if (state & MF_HILITE) {
+      if (state & MF_POPUP) {
+        HMENU submenu = GetSubMenu(menu, i);
+        if (GetHighlightedMenuItemInfo(submenu, has_parent, has_submenu))
+          *has_parent = true;
+        else
+          *has_submenu = true;
+      }
+      return true;
+    }
+  }
+  return false;
+}
+
+// static
+LRESULT CALLBACK NativeMenuWin::MenuMessageHook(
+    int n_code, WPARAM w_param, LPARAM l_param) {
+  LRESULT result = CallNextHookEx(NULL, n_code, w_param, l_param);
+
+  MSG* msg = reinterpret_cast<MSG*>(l_param);
+  if (msg->message == WM_KEYDOWN) {
+    NativeMenuWin* this_ptr = open_native_menu_win_;
+    bool has_parent = false;
+    bool has_submenu = false;
+    GetHighlightedMenuItemInfo(this_ptr->menu_, &has_parent, &has_submenu);
+    if (msg->wParam == VK_LEFT && !has_parent) {
+      this_ptr->menu_action_ = MENU_ACTION_PREVIOUS;
+      ::EndMenu();
+    } else if (msg->wParam == VK_RIGHT && !has_parent && !has_submenu) {
+      this_ptr->menu_action_ = MENU_ACTION_NEXT;
+      ::EndMenu();
+    }
+  }
+
+  return result;
+}
+
 bool NativeMenuWin::IsSeparatorItemAt(int menu_index) const {
   MENUITEMINFO mii = {0};
   mii.cbSize = sizeof(mii);
diff --git a/views/controls/menu/native_menu_win.h b/views/controls/menu/native_menu_win.h
index ba0144e..5d6f621 100644
--- a/views/controls/menu/native_menu_win.h
+++ b/views/controls/menu/native_menu_win.h
@@ -1,6 +1,6 @@
-// Copyright (c) 2009 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.
+// Copyright (c) 2010 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.
 
 #ifndef VIEWS_CONTROLS_MENU_NATIVE_MENU_WIN_H_
 #define VIEWS_CONTROLS_MENU_NATIVE_MENU_WIN_H_
@@ -29,6 +29,7 @@ class NativeMenuWin : public MenuWrapper {
   virtual void Rebuild();
   virtual void UpdateStates();
   virtual gfx::NativeMenu GetNativeMenu() const;
+  virtual MenuAction GetMenuAction() const;
 
  private:
   // IMPORTANT: Note about indices.
@@ -80,6 +81,20 @@ class NativeMenuWin : public MenuWrapper {
   // Creates the host window that receives notifications from the menu.
   void CreateHostWindow();
 
+  // Given a menu that's currently popped-up, find the currently
+  // highlighted item and return whether or not that item has a parent
+  // (i.e. it's in a submenu), and whether or not that item leads to a
+  // submenu. Returns true if a highlighted item was found. This
+  // method is called to determine if the right and left arrow keys
+  // should be used to switch between menus, or to open and close
+  // submenus.
+  static bool GetHighlightedMenuItemInfo(
+      HMENU menu, bool* has_parent, bool* has_submenu);
+
+  // Hook to receive keyboard events while the menu is open.
+  static LRESULT CALLBACK MenuMessageHook(
+      int n_code, WPARAM w_param, LPARAM l_param);
+
   // Our attached model and delegate.
   menus::MenuModel* model_;
 
@@ -106,6 +121,14 @@ class NativeMenuWin : public MenuWrapper {
   // The index of the first item in the model in the menu.
   int first_item_index_;
 
+  // The action that took place during the call to RunMenuAt.
+  MenuAction menu_action_;
+
+  // Ugly: a static pointer to the instance of this class that currently
+  // has a menu open, because our hook function that receives keyboard
+  // events doesn't have a mechanism to get a user data pointer.
+  static NativeMenuWin* open_native_menu_win_;
+
   DISALLOW_COPY_AND_ASSIGN(NativeMenuWin);
 };
 
diff --git a/views/focus/accelerator_handler_gtk.cc b/views/focus/accelerator_handler_gtk.cc
index 1ff5d81..da7bd94 100644
--- a/views/focus/accelerator_handler_gtk.cc
+++ b/views/focus/accelerator_handler_gtk.cc
@@ -4,6 +4,9 @@
 
 #include <gtk/gtk.h>
 
+#include "base/keyboard_code_conversion_gtk.h"
+#include "base/keyboard_codes.h"
+#include "views/accelerator.h"
 #include "views/focus/accelerator_handler.h"
 #include "views/focus/focus_manager.h"
 #include "views/widget/widget_gtk.h"
@@ -48,6 +51,14 @@ bool AcceleratorHandler::Dispatch(GdkEvent* event) {
 
   if (event->type == GDK_KEY_PRESS) {
     KeyEvent view_key_event(key_event);
+
+    // If it's the Alt key, don't send it to the focus manager until release
+    // (to handle focusing the menu bar).
+    if (view_key_event.GetKeyCode() == base::VKEY_MENU) {
+      last_key_pressed_ = key_event->keyval;
+      return true;
+    }
+
     // FocusManager::OnKeyPressed and OnKeyReleased return false if this
     // message has been consumed and should not be propagated further.
     if (!focus_manager->OnKeyEvent(view_key_event)) {
@@ -60,6 +71,14 @@ bool AcceleratorHandler::Dispatch(GdkEvent* event) {
   // as accelerators to avoid unpaired key release.
   if (event->type == GDK_KEY_RELEASE &&
       key_event->keyval == last_key_pressed_) {
+    // Special case: the Alt key can trigger an accelerator on release
+    // rather than on press.
+    if (base::WindowsKeyCodeForGdkKeyCode(key_event->keyval) ==
+        base::VKEY_MENU) {
+      Accelerator accelerator(base::VKEY_MENU, false, false, false);
+      focus_manager->ProcessAccelerator(accelerator);
+    }
+
     last_key_pressed_ = 0;
     return true;
   }
diff --git a/views/focus/focus_manager.h b/views/focus/focus_manager.h
index bad9dba..2150560 100644
--- a/views/focus/focus_manager.h
+++ b/views/focus/focus_manager.h
@@ -278,6 +278,10 @@ class FocusManager {
   static FocusManager* GetFocusManagerForNativeView(
       gfx::NativeView native_view);
 
+  // Retrieves the FocusManager associated with the passed native view.
+  static FocusManager* GetFocusManagerForNativeWindow(
+      gfx::NativeWindow native_window);
+
  private:
   // Returns the next focusable view.
   View* GetNextFocusableView(View* starting_view, bool reverse, bool dont_loop);
diff --git a/views/focus/focus_manager_gtk.cc b/views/focus/focus_manager_gtk.cc
index 3038d93..a52a6d9 100644
--- a/views/focus/focus_manager_gtk.cc
+++ b/views/focus/focus_manager_gtk.cc
@@ -49,4 +49,10 @@ FocusManager* FocusManager::GetFocusManagerForNativeView(
   return focus_manager;
 }
 
+// static
+FocusManager* FocusManager::GetFocusManagerForNativeWindow(
+    gfx::NativeWindow native_window) {
+  return GetFocusManagerForNativeView(GTK_WIDGET(native_window));
+}
+
 }  // namespace views
diff --git a/views/focus/focus_manager_win.cc b/views/focus/focus_manager_win.cc
index fbccf7b..faf3815 100644
--- a/views/focus/focus_manager_win.cc
+++ b/views/focus/focus_manager_win.cc
@@ -27,5 +27,10 @@ FocusManager* FocusManager::GetFocusManagerForNativeView(
   return widget ? widget->GetFocusManager() : NULL;
 }
 
-}  // namespace views
+// static
+FocusManager* FocusManager::GetFocusManagerForNativeWindow(
+    gfx::NativeWindow native_window) {
+  return GetFocusManagerForNativeView(native_window);
+}
 
+}  // namespace views
diff --git a/views/view.h b/views/view.h
index f83793b..f2469f7 100644
--- a/views/view.h
+++ b/views/view.h
@@ -609,6 +609,12 @@ class View : public AcceleratorTarget {
   // accessibility focus.
   virtual View* GetAccFocusedChildView() { return NULL; }
 
+  // Try to give accessibility focus to a given child view. Returns true on
+  // success. Returns false if this view isn't already focused, if it doesn't
+  // support accessibility focus for children, or if the given view isn't a
+  // valid child view that can receive accessibility focus.
+  virtual bool SetAccFocusedChildView(View* child_view) { return false; }
+
   // Utility functions
 
   // Note that the utility coordinate conversions functions always operate on
diff --git a/views/window/window_win.cc b/views/window/window_win.cc
index b58bcd4..17a3f1b 100644
--- a/views/window/window_win.cc
+++ b/views/window/window_win.cc
@@ -1086,6 +1086,15 @@ void WindowWin::OnSysCommand(UINT notification_code, CPoint click) {
     }
   }
 
+  // Handle SC_KEYMENU, which means that the user has pressed the ALT
+  // key and released it, so we should focus the menu bar.
+  if ((notification_code & sc_mask) == SC_KEYMENU && click.x == 0) {
+    Accelerator accelerator(win_util::WinToKeyboardCode(VK_MENU),
+                            false, false, false);
+    GetFocusManager()->ProcessAccelerator(accelerator);
+    return;
+  }
+
   // First see if the delegate can handle it.
   if (window_delegate_->ExecuteWindowsCommand(notification_code))
     return;