diff options
author | dmazzoni@chromium.org <dmazzoni@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-22 17:39:35 +0000 |
---|---|---|
committer | dmazzoni@chromium.org <dmazzoni@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-03-22 17:39:35 +0000 |
commit | 9201662e1c9ce2c9afae43c2e6c74c673f7aed17 (patch) | |
tree | cc73c6141a81f9202ef14e83605d1b2f614e7217 /chrome | |
parent | c6cd2dbcec52a90b3a97e931a36757151f23b6ed (diff) | |
download | chromium_src-9201662e1c9ce2c9afae43c2e6c74c673f7aed17.zip chromium_src-9201662e1c9ce2c9afae43c2e6c74c673f7aed17.tar.gz chromium_src-9201662e1c9ce2c9afae43c2e6c74c673f7aed17.tar.bz2 |
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@42234 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
27 files changed, 593 insertions, 34 deletions
diff --git a/chrome/app/chrome_dll.rc b/chrome/app/chrome_dll.rc index 7ed9edf..14cee6d 100644 --- a/chrome/app/chrome_dll.rc +++ b/chrome/app/chrome_dll.rc @@ -48,6 +48,8 @@ BEGIN VK_F6, IDC_FOCUS_LOCATION, VIRTKEY "D", IDC_FOCUS_LOCATION, VIRTKEY, ALT "L", IDC_FOCUS_LOCATION, VIRTKEY, CONTROL + VK_F10, IDC_FOCUS_MENU_BAR, VIRTKEY + VK_MENU, IDC_FOCUS_MENU_BAR, VIRTKEY "K", IDC_FOCUS_SEARCH, VIRTKEY, CONTROL "E", IDC_FOCUS_SEARCH, VIRTKEY, CONTROL "T", IDC_FOCUS_TOOLBAR, VIRTKEY, SHIFT, ALT diff --git a/chrome/app/chrome_dll_resource.h b/chrome/app/chrome_dll_resource.h index 1d5caff..9a3f282 100644 --- a/chrome/app/chrome_dll_resource.h +++ b/chrome/app/chrome_dll_resource.h @@ -189,6 +189,7 @@ #define IDC_FOCUS_TOOLBAR 39000 #define IDC_FOCUS_LOCATION 39001 #define IDC_FOCUS_SEARCH 39002 +#define IDC_FOCUS_MENU_BAR 39003 // Show various bits of UI #define IDC_OPEN_FILE 40000 diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc index a2b7d8d..d8bfa76 100644 --- a/chrome/browser/automation/automation_provider.cc +++ b/chrome/browser/automation/automation_provider.cc @@ -86,6 +86,11 @@ #include "chrome/browser/automation/ui_controls.h" #endif // !defined(OS_MACOSX) +#if defined(TOOLKIT_VIEWS) +#include "views/focus/focus_manager.h" +#include "views/view.h" +#endif // defined(TOOLKIT_VIEWS) + using base::Time; class AutomationInterstitialPage : public InterstitialPage { @@ -345,7 +350,11 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) { IPC_MESSAGE_HANDLER(AutomationMsg_ConstrainedWindowCount, GetConstrainedWindowCount) IPC_MESSAGE_HANDLER(AutomationMsg_FindInPage, HandleFindInPageRequest) +#if defined(TOOLKIT_VIEWS) IPC_MESSAGE_HANDLER(AutomationMsg_GetFocusedViewID, GetFocusedViewID) + IPC_MESSAGE_HANDLER_DELAY_REPLY(AutomationMsg_WaitForFocusedViewIDToChange, + WaitForFocusedViewIDToChange) +#endif IPC_MESSAGE_HANDLER_DELAY_REPLY(AutomationMsg_InspectElement, HandleInspectElementRequest) IPC_MESSAGE_HANDLER(AutomationMsg_DownloadDirectory, GetDownloadDirectory) @@ -497,6 +506,7 @@ void AutomationProvider::OnMessageReceived(const IPC::Message& message) { IPC_MESSAGE_HANDLER_DELAY_REPLY(AutomationMsg_LoginWithUserAndPass, LoginWithUserAndPass) #endif + IPC_MESSAGE_HANDLER(AutomationMsg_IsPopUpMenuOpen, IsPopUpMenuOpen) IPC_END_MESSAGE_MAP() } @@ -2528,3 +2538,99 @@ void AutomationProvider::SetContentSetting( *success = true; } } + +#if defined(TOOLKIT_VIEWS) +void AutomationProvider::GetFocusedViewID(int handle, int* view_id) { + *view_id = -1; + if (window_tracker_->ContainsHandle(handle)) { + gfx::NativeWindow window = window_tracker_->GetResource(handle); + views::FocusManager* focus_manager = + views::FocusManager::GetFocusManagerForNativeWindow(window); + DCHECK(focus_manager); + views::View* focused_view = focus_manager->GetFocusedView(); + if (focused_view) + *view_id = focused_view->GetID(); + } +} + +// Helper class that waits until the focus has changed to a view other +// than the one with the provided view id. +class ViewFocusChangeWaiter : public views::FocusChangeListener { + public: + ViewFocusChangeWaiter(views::FocusManager* focus_manager, + int previous_view_id, + AutomationProvider* automation, + IPC::Message* reply_message) + : focus_manager_(focus_manager), + previous_view_id_(previous_view_id), + automation_(automation), + reply_message_(reply_message), + ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) { + focus_manager_->AddFocusChangeListener(this); + // Call the focus change notification once in case the focus has + // already changed. + FocusWillChange(NULL, focus_manager_->GetFocusedView()); + } + + ~ViewFocusChangeWaiter() { + focus_manager_->RemoveFocusChangeListener(this); + } + + // Inherited from FocusChangeListener + virtual void FocusWillChange(views::View* focused_before, + views::View* focused_now) { + // This listener is called before focus actually changes. Post a task + // that will get run after focus changes. + MessageLoop::current()->PostTask( + FROM_HERE, + method_factory_.NewRunnableMethod( + &ViewFocusChangeWaiter::FocusChanged, + focused_before, + focused_now)); + } + + private: + void FocusChanged(views::View* focused_before, + views::View* focused_now) { + if (focused_now && focused_now->GetID() != previous_view_id_) { + AutomationMsg_WaitForFocusedViewIDToChange::WriteReplyParams( + reply_message_, true, focused_now->GetID()); + + automation_->Send(reply_message_); + delete this; + } + } + + views::FocusManager* focus_manager_; + int previous_view_id_; + AutomationProvider* automation_; + IPC::Message* reply_message_; + ScopedRunnableMethodFactory<ViewFocusChangeWaiter> method_factory_; + + DISALLOW_COPY_AND_ASSIGN(ViewFocusChangeWaiter); +}; + +void AutomationProvider::WaitForFocusedViewIDToChange( + int handle, int previous_view_id, IPC::Message* reply_message) { + if (!window_tracker_->ContainsHandle(handle)) + return; + gfx::NativeWindow window = window_tracker_->GetResource(handle); + views::FocusManager* focus_manager = + views::FocusManager::GetFocusManagerForNativeWindow(window); + + // The waiter will respond to the IPC and delete itself when done. + new ViewFocusChangeWaiter(focus_manager, + previous_view_id, + this, + reply_message); +} +#else +void AutomationProvider::GetFocusedViewID(int handle, int* view_id) { + NOTIMPLEMENTED(); +}; + +void AutomationProvider::WaitForFocusedViewIDToChange( + int handle, int previous_view_id, IPC::Message* reply_message) { + NOTIMPLEMENTED(); +} +#endif // defined(TOOLKIT_VIEWS) diff --git a/chrome/browser/automation/automation_provider.h b/chrome/browser/automation/automation_provider.h index e5b36bf..29aba68 100644 --- a/chrome/browser/automation/automation_provider.h +++ b/chrome/browser/automation/automation_provider.h @@ -247,6 +247,9 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>, #endif // defined(OS_WIN) void GetFocusedViewID(int handle, int* view_id); + void WaitForFocusedViewIDToChange(int handle, + int previous_view_id, + IPC::Message* reply_message); // Helper function to find the browser window that contains a given // NavigationController and activate that tab. @@ -579,6 +582,10 @@ class AutomationProvider : public base::RefCounted<AutomationProvider>, // Returns NULL on failure. RenderViewHost* GetViewForTab(int tab_handle); + // Returns true if any popup menu is open. Currently only fully implemented + // on Windows and doesn't support bookmark menus. + void IsPopUpMenuOpen(int handle, bool* success, bool* is_open); + typedef ObserverList<NotificationObserver> NotificationObserverList; typedef std::map<NavigationController*, LoginHandler*> LoginHandlerMap; typedef std::map<int, ExtensionPortContainer*> PortContainerMap; diff --git a/chrome/browser/automation/automation_provider_gtk.cc b/chrome/browser/automation/automation_provider_gtk.cc index 601e81a..c617f3f 100644 --- a/chrome/browser/automation/automation_provider_gtk.cc +++ b/chrome/browser/automation/automation_provider_gtk.cc @@ -79,10 +79,6 @@ void AutomationProvider::IsWindowMaximized(int handle, bool* is_maximized, NOTIMPLEMENTED(); } -void AutomationProvider::GetFocusedViewID(int handle, int* view_id) { - NOTIMPLEMENTED(); -} - void AutomationProvider::PrintAsync(int tab_handle) { NOTIMPLEMENTED(); } @@ -228,3 +224,11 @@ void AutomationProvider::GetWindowTitle(int handle, string16* text) { text->assign(UTF8ToUTF16(title)); } +void AutomationProvider::IsPopUpMenuOpen( + int handle, bool* success, bool* is_open) { + *success = true; + *is_open = true; + // A GTK implementation of this method is not currently needed; GTK menus + // pop up immediately so there's no need to wait until they appear. + NOTIMPLEMENTED(); +} diff --git a/chrome/browser/automation/automation_provider_mac.mm b/chrome/browser/automation/automation_provider_mac.mm index f58b7a8..08a9575 100644 --- a/chrome/browser/automation/automation_provider_mac.mm +++ b/chrome/browser/automation/automation_provider_mac.mm @@ -66,10 +66,6 @@ void AutomationProvider::IsWindowMaximized(int handle, bool* is_maximized, NOTIMPLEMENTED(); } -void AutomationProvider::GetFocusedViewID(int handle, int* view_id) { - NOTIMPLEMENTED(); -} - void AutomationProvider::PrintAsync(int tab_handle) { NOTIMPLEMENTED(); } @@ -159,3 +155,9 @@ void AutomationProvider::GetWindowTitle(int handle, string16* text) { text->assign(base::SysNSStringToUTF16(title)); } +void AutomationProvider::IsPopUpMenuOpen( + int handle, bool* success, bool* is_open) { + *success = false; + *is_open = false; + NOTIMPLEMENTED(); +} diff --git a/chrome/browser/automation/automation_provider_views.cc b/chrome/browser/automation/automation_provider_views.cc index f161105..a827b06 100644 --- a/chrome/browser/automation/automation_provider_views.cc +++ b/chrome/browser/automation/automation_provider_views.cc @@ -33,4 +33,3 @@ void AutomationProvider::WindowGetViewBounds(int handle, int view_id, } } } - diff --git a/chrome/browser/automation/automation_provider_win.cc b/chrome/browser/automation/automation_provider_win.cc index 9539498..9474e90 100644 --- a/chrome/browser/automation/automation_provider_win.cc +++ b/chrome/browser/automation/automation_provider_win.cc @@ -231,19 +231,6 @@ void AutomationProvider::WindowSimulateDrag(int handle, } } -void AutomationProvider::GetFocusedViewID(int handle, int* view_id) { - *view_id = -1; - if (window_tracker_->ContainsHandle(handle)) { - HWND hwnd = window_tracker_->GetResource(handle); - views::FocusManager* focus_manager = - views::FocusManager::GetFocusManagerForNativeView(hwnd); - DCHECK(focus_manager); - views::View* focused_view = focus_manager->GetFocusedView(); - if (focused_view) - *view_id = focused_view->GetID(); - } -} - void AutomationProvider::GetWindowBounds(int handle, gfx::Rect* bounds, bool* success) { *success = false; @@ -506,3 +493,13 @@ void AutomationProvider::GetWindowTitle(int handle, string16* text) { ::GetWindowText(window, WriteInto(&result, length), length); text->assign(WideToUTF16(result)); } + +void AutomationProvider::IsPopUpMenuOpen( + int handle, bool* success, bool* is_open) { + *success = true; + + // Check for the existence of a pop-up menu using its + // window class (#32768). Note that this won't cover + // bookmark menus. + *is_open = (::FindWindow(L"#32768", 0) != NULL); +} diff --git a/chrome/browser/automation/ui_controls_win.cc b/chrome/browser/automation/ui_controls_win.cc index 3ff1a5c..aba7248 100644 --- a/chrome/browser/automation/ui_controls_win.cc +++ b/chrome/browser/automation/ui_controls_win.cc @@ -166,6 +166,21 @@ bool SendKeyPressImpl(base::KeyboardCode key, scoped_refptr<InputDispatcher> dispatcher( task ? new InputDispatcher(task, WM_KEYUP) : NULL); + // If a pop-up menu is open, it won't receive events sent using SendInput. + // Check for a pop-up menu using its window class (#32768) and if one + // exists, send the key event directly there. + HWND popup_menu = ::FindWindow(L"#32768", 0); + if (popup_menu != NULL) { + WPARAM w_param = win_util::KeyboardCodeToWin(key); + LPARAM l_param = 0; + ::SendMessage(popup_menu, WM_KEYDOWN, w_param, l_param); + ::SendMessage(popup_menu, WM_KEYUP, w_param, l_param); + + if (dispatcher.get()) + dispatcher->AddRef(); + return true; + } + INPUT input[8] = { 0 }; // 8, assuming all the modifiers are activated int i = 0; diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index f2779af..3ddc3a8 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -873,6 +873,9 @@ void Browser::UpdateCommandsForFullscreenMode(bool is_fullscreen) { command_updater_.UpdateCommandEnabled(IDC_FOCUS_TOOLBAR, show_main_ui); command_updater_.UpdateCommandEnabled(IDC_FOCUS_LOCATION, show_main_ui); command_updater_.UpdateCommandEnabled(IDC_FOCUS_SEARCH, show_main_ui); + command_updater_.UpdateCommandEnabled( + IDC_FOCUS_MENU_BAR, + show_main_ui && !is_fullscreen && (type() & TYPE_POPUP) == 0); // Show various bits of UI command_updater_.UpdateCommandEnabled(IDC_DEVELOPER_MENU, show_main_ui); @@ -1306,6 +1309,11 @@ void Browser::FocusToolbar() { window_->FocusToolbar(); } +void Browser::FocusPageAndAppMenus() { + UserMetrics::RecordAction("EnterMenuBarEmulationMode", profile_); + window_->FocusPageAndAppMenus(); +} + void Browser::FocusLocationBar() { UserMetrics::RecordAction("FocusLocation", profile_); window_->SetFocusToLocationBar(); @@ -1729,6 +1737,7 @@ void Browser::ExecuteCommandWithDisposition( case IDC_FOCUS_TOOLBAR: FocusToolbar(); break; case IDC_FOCUS_LOCATION: FocusLocationBar(); break; case IDC_FOCUS_SEARCH: FocusSearch(); break; + case IDC_FOCUS_MENU_BAR: FocusPageAndAppMenus(); break; // Show various bits of UI case IDC_OPEN_FILE: OpenFile(); break; diff --git a/chrome/browser/browser.h b/chrome/browser/browser.h index 51470f7..fad8295 100644 --- a/chrome/browser/browser.h +++ b/chrome/browser/browser.h @@ -460,6 +460,7 @@ class Browser : public TabStripModelDelegate, void FocusToolbar(); void FocusLocationBar(); void FocusSearch(); + void FocusPageAndAppMenus(); // Show various bits of UI void OpenFile(); diff --git a/chrome/browser/browser_window.h b/chrome/browser/browser_window.h index a4ff572..c2e3911 100644 --- a/chrome/browser/browser_window.h +++ b/chrome/browser/browser_window.h @@ -150,6 +150,11 @@ class BrowserWindow { // Focuses the toolbar (for accessibility). virtual void FocusToolbar() = 0; + // Focuses the page and app menus like they were a menu bar. + // + // Not used on the Mac, which has a "normal" menu bar. + virtual void FocusPageAndAppMenus() = 0; + // Returns whether the bookmark bar is visible or not. virtual bool IsBookmarkBarVisible() const = 0; diff --git a/chrome/browser/cocoa/browser_window_cocoa.h b/chrome/browser/cocoa/browser_window_cocoa.h index b0ace80..4de884a 100644 --- a/chrome/browser/cocoa/browser_window_cocoa.h +++ b/chrome/browser/cocoa/browser_window_cocoa.h @@ -57,6 +57,7 @@ class BrowserWindowCocoa : public BrowserWindow, virtual void UpdateToolbar(TabContents* contents, bool should_restore_state); virtual void FocusToolbar(); + virtual void FocusPageAndAppMenus(); virtual bool IsBookmarkBarVisible() const; virtual bool IsBookmarkBarAnimating() const; virtual bool IsToolbarVisible() const; diff --git a/chrome/browser/cocoa/browser_window_cocoa.mm b/chrome/browser/cocoa/browser_window_cocoa.mm index 01d592e..3b9ac25 100644 --- a/chrome/browser/cocoa/browser_window_cocoa.mm +++ b/chrome/browser/cocoa/browser_window_cocoa.mm @@ -237,6 +237,10 @@ void BrowserWindowCocoa::FocusToolbar() { NOTIMPLEMENTED(); } +void BrowserWindowCocoa::FocusPageAndAppMenus(); + // Chrome uses the standard Mac OS X menu bar, so this isn't needed. +} + bool BrowserWindowCocoa::IsBookmarkBarVisible() const { return browser_->profile()->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar); } diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc index e0d60adf..fa63d58 100644 --- a/chrome/browser/gtk/browser_window_gtk.cc +++ b/chrome/browser/gtk/browser_window_gtk.cc @@ -783,6 +783,10 @@ void BrowserWindowGtk::FocusToolbar() { NOTIMPLEMENTED(); } +void BrowserWindowGtk::FocusPageAndAppMenus() { + NOTIMPLEMENTED(); +} + bool BrowserWindowGtk::IsBookmarkBarVisible() const { return browser_->SupportsWindowFeature(Browser::FEATURE_BOOKMARKBAR) && bookmark_bar_.get(); diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h index f24e094..67613b6 100644 --- a/chrome/browser/gtk/browser_window_gtk.h +++ b/chrome/browser/gtk/browser_window_gtk.h @@ -75,6 +75,7 @@ class BrowserWindowGtk : public BrowserWindow, virtual void UpdateToolbar(TabContents* contents, bool should_restore_state); virtual void FocusToolbar(); + virtual void FocusPageAndAppMenus(); virtual bool IsBookmarkBarVisible() const; virtual bool IsBookmarkBarAnimating() const; virtual bool IsToolbarVisible() const; diff --git a/chrome/browser/views/accelerator_table_gtk.cc b/chrome/browser/views/accelerator_table_gtk.cc index 28a630b..f3a7a54 100644 --- a/chrome/browser/views/accelerator_table_gtk.cc +++ b/chrome/browser/views/accelerator_table_gtk.cc @@ -20,6 +20,8 @@ const AcceleratorMapping kAcceleratorMap[] = { { base::VKEY_L, false, true, false, IDC_FOCUS_LOCATION }, { base::VKEY_D, false, false, true, IDC_FOCUS_LOCATION }, { base::VKEY_F6, false, false, false, IDC_FOCUS_LOCATION }, + { base::VKEY_F10, false, false, false, IDC_FOCUS_MENU_BAR }, + { base::VKEY_MENU, false, false, false, IDC_FOCUS_MENU_BAR }, // Tab/window controls. { base::VKEY_T, false, true, false, IDC_NEW_TAB }, diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc index d484765..328b954 100644 --- a/chrome/browser/views/frame/browser_view.cc +++ b/chrome/browser/views/frame/browser_view.cc @@ -638,10 +638,10 @@ void BrowserView::TraverseNextAccessibleToolbar(bool forward) { // TODO(mohamed) This needs to be smart, that applies to all toolbars. // Currently it just traverses between bookmarks and toolbar. if (!forward && toolbar_->IsVisible() && toolbar_->IsEnabled()) { - toolbar_->InitiateTraversal(last_focused_view_storage_id_); + toolbar_->RequestFocus(); } else if (forward && bookmark_bar_view_->IsVisible() && bookmark_bar_view_->IsEnabled()) { - bookmark_bar_view_->InitiateTraversal(last_focused_view_storage_id_); + bookmark_bar_view_->RequestFocus(); } } @@ -863,17 +863,31 @@ void BrowserView::UpdateToolbar(TabContents* contents, } void BrowserView::FocusToolbar() { - // Remove existing views in the storage, traversal should be restarted. + // Start the traversal within the main toolbar, passing it the storage id + // of the view where focus should be returned if the user exits the toolbar. + SaveFocusedView(); + toolbar_->InitiateTraversal(last_focused_view_storage_id_); +} + +void BrowserView::FocusPageAndAppMenus() { + // Chrome doesn't have a traditional menu bar, but it has menu buttons in + // the main toolbar that play the same role. If the user presses a key + // that would typically focus the menu bar, tell the toolbar to focus + // the first menu button. Pass it the storage id of the view where + // focus should be returned if the user presses escape. + // + // Not used on the Mac, which has a normal menu bar. + SaveFocusedView(); + toolbar_->EnterMenuBarEmulationMode(last_focused_view_storage_id_, NULL); +} + +void BrowserView::SaveFocusedView() { views::ViewStorage* view_storage = views::ViewStorage::GetSharedInstance(); if (view_storage->RetrieveView(last_focused_view_storage_id_)) view_storage->RemoveView(last_focused_view_storage_id_); - - // Store the last focused view into the storage, to handle existing traversal. - view_storage->StoreView(last_focused_view_storage_id_, - GetRootView()->GetFocusedView()); - - // Start the traversal within the main toolbar. - toolbar_->InitiateTraversal(last_focused_view_storage_id_); + views::View* focused_view = GetRootView()->GetFocusedView(); + if (focused_view) + view_storage->StoreView(last_focused_view_storage_id_, focused_view); } void BrowserView::DestroyBrowser() { diff --git a/chrome/browser/views/frame/browser_view.h b/chrome/browser/views/frame/browser_view.h index bf753e9..026fa53 100644 --- a/chrome/browser/views/frame/browser_view.h +++ b/chrome/browser/views/frame/browser_view.h @@ -270,6 +270,7 @@ class BrowserView : public BrowserBubbleHost, virtual void UpdateStopGoState(bool is_loading, bool force); virtual void UpdateToolbar(TabContents* contents, bool should_restore_state); virtual void FocusToolbar(); + virtual void FocusPageAndAppMenus(); virtual void DestroyBrowser(); virtual bool IsBookmarkBarVisible() const; virtual bool IsBookmarkBarAnimating() const; @@ -472,6 +473,9 @@ class BrowserView : public BrowserBubbleHost, // Initialize the hung plugin detector. void InitHangMonitor(); + // Save the current focused view to view storage + void SaveFocusedView(); + // Initialize class statics. static void InitClass(); diff --git a/chrome/browser/views/toolbar_view.cc b/chrome/browser/views/toolbar_view.cc index 9670c24..0153012 100644 --- a/chrome/browser/views/toolbar_view.cc +++ b/chrome/browser/views/toolbar_view.cc @@ -35,6 +35,7 @@ #include "chrome/browser/views/bookmark_menu_button.h" #include "chrome/browser/views/browser_actions_container.h" #include "chrome/browser/views/event_utils.h" +#include "chrome/browser/views/frame/browser_view.h" #include "chrome/browser/views/go_button.h" #include "chrome/browser/views/location_bar_view.h" #include "chrome/browser/views/toolbar_star_toggle.h" @@ -52,6 +53,8 @@ #include "views/controls/label.h" #include "views/controls/menu/menu_2.h" #include "views/drag_utils.h" +#include "views/focus/view_storage.h" +#include "views/widget/tooltip_manager.h" #include "views/window/non_client_view.h" #include "views/window/window.h" @@ -90,7 +93,9 @@ ToolbarView::ToolbarView(Browser* browser) bookmark_menu_(NULL), profile_(NULL), browser_(browser), - profiles_menu_contents_(NULL) { + profiles_menu_contents_(NULL), + last_focused_view_storage_id_(-1), + ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) { SetID(VIEW_ID_TOOLBAR); browser_->command_updater()->AddCommandObserver(IDC_BACK, this); browser_->command_updater()->AddCommandObserver(IDC_FORWARD, this); @@ -108,6 +113,14 @@ ToolbarView::ToolbarView(Browser* browser) } } +ToolbarView::~ToolbarView() { + if (page_menu_->HasFocus() || app_menu_->HasFocus()) { + views::FocusManager* focus_manager = GetFocusManager(); + focus_manager->UnregisterAccelerators(this); + focus_manager->RemoveFocusChangeListener(this); + } +} + void ToolbarView::Init(Profile* profile) { back_menu_model_.reset(new BackForwardMenuModel( browser_, BackForwardMenuModel::BACKWARD_MENU)); @@ -148,6 +161,66 @@ void ToolbarView::SetAppMenuModel(AppMenuModel* model) { app_menu_menu_.reset(new views::Menu2(app_menu_model_.get())); } +void ToolbarView::EnterMenuBarEmulationMode(int last_focused_view_storage_id, + views::MenuButton* menu_to_focus) { + last_focused_view_storage_id_ = last_focused_view_storage_id; + if (!menu_to_focus) + menu_to_focus = page_menu_; + + // If we're already in the menu bar emulation mode, just set the focus. + if (page_menu_->IsFocusable() && app_menu_->IsFocusable()) { + menu_to_focus->RequestFocus(); + return; + } + + // Make the menus focusable and set focus to the initial menu. + page_menu_->SetFocusable(true); + app_menu_->SetFocusable(true); + menu_to_focus->RequestFocus(); + + // Listen so we know when focus has moved to something other than one + // of these menus. + views::FocusManager* focus_manager = GetFocusManager(); + focus_manager->AddFocusChangeListener(this); + + // Add accelerators so that the usual keys used to interact with a + // menu bar work as expected. + views::Accelerator return_key(base::VKEY_RETURN, false, false, false); + focus_manager->RegisterAccelerator(return_key, this); + views::Accelerator space(base::VKEY_SPACE, false, false, false); + focus_manager->RegisterAccelerator(space, this); + views::Accelerator escape(base::VKEY_ESCAPE, false, false, false); + focus_manager->RegisterAccelerator(escape, this); + views::Accelerator down(base::VKEY_DOWN, false, false, false); + focus_manager->RegisterAccelerator(down, this); + views::Accelerator up(base::VKEY_UP, false, false, false); + focus_manager->RegisterAccelerator(up, this); + views::Accelerator left(base::VKEY_LEFT, false, false, false); + focus_manager->RegisterAccelerator(left, this); + views::Accelerator right(base::VKEY_RIGHT, false, false, false); + focus_manager->RegisterAccelerator(right, this); +} + +//////////////////////////////////////////////////////////////////////////////// +// ToolbarView, FocusChangeListener overrides: + +void ToolbarView::FocusWillChange(views::View* focused_before, + views::View* focused_now) { + // If the focus is switching to something outside the menu bar, + // take it out of the focus traversal. + if (focused_now != NULL && + focused_now != page_menu_ && + focused_now != app_menu_) { + // Post ExitMenuBarEmulationMode to the queue rather than running it + // right away, because otherwise we'll remove ourselves from the + // list of listeners while FocusManager is in the middle of iterating + // over that list. + MessageLoop::current()->PostTask( + FROM_HERE, method_factory_.NewRunnableMethod( + &ToolbarView::ExitMenuBarEmulationMode)); + } +} + //////////////////////////////////////////////////////////////////////////////// // ToolbarView, AccessibleToolbarView overrides: @@ -324,6 +397,43 @@ void ToolbarView::ExecuteCommand(int command_id) { //////////////////////////////////////////////////////////////////////////////// // ToolbarView, views::View overrides: +bool ToolbarView::AcceleratorPressed( + const views::Accelerator& accelerator) { + // The only accelerators we handle here are if the menus are focused. + views::View* focused_view = GetFocusManager()->GetFocusedView(); + if (focused_view != page_menu_ && focused_view != app_menu_) { + ExitMenuBarEmulationMode(); + return false; + } + + // Safe to cast, given the check above. + views::MenuButton* menu = static_cast<views::MenuButton*>(focused_view); + switch (accelerator.GetKeyCode()) { + case base::VKEY_ESCAPE: + RestoreLastFocusedView(); + return true; + case base::VKEY_LEFT: + case base::VKEY_RIGHT: + if (menu == app_menu_) + page_menu_->RequestFocus(); + else + app_menu_->RequestFocus(); + return true; + case base::VKEY_UP: + case base::VKEY_DOWN: + case base::VKEY_RETURN: + case base::VKEY_SPACE: + // Hide the tooltip before activating a menu button. + if (GetWidget()->GetTooltipManager()) + GetWidget()->GetTooltipManager()->HideKeyboardTooltip(); + + ActivateMenuButton(menu); + return true; + default: + return false; + } +} + gfx::Size ToolbarView::GetPreferredSize() { if (IsDisplayModeNormal()) { int min_width = kControlIndent + back_->GetPreferredSize().width() + @@ -709,10 +819,86 @@ void ToolbarView::RunPageMenu(const gfx::Point& pt) { page_menu_model_.reset(new PageMenuModel(this, browser_)); page_menu_menu_.reset(new views::Menu2(page_menu_model_.get())); page_menu_menu_->RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT); + SwitchToOtherMenuIfNeeded(page_menu_menu_.get(), app_menu_); } void ToolbarView::RunAppMenu(const gfx::Point& pt) { if (app_menu_model_->BuildProfileSubMenu()) app_menu_menu_->Rebuild(); app_menu_menu_->RunMenuAt(pt, views::Menu2::ALIGN_TOPRIGHT); + SwitchToOtherMenuIfNeeded(app_menu_menu_.get(), page_menu_); +} + +void ToolbarView::SwitchToOtherMenuIfNeeded( + views::Menu2* previous_menu, views::MenuButton* next_menu_button) { + // If the user tried to move to the right or left, switch from the + // app menu to the page menu. Switching to the next menu is delayed + // until the next event loop so that the call stack that initiated + // activating the first menu can return. (If we didn't do this, the + // call stack would grow each time the user switches menus, and + // the actions taken after the user finally exits a menu would cause + // flicker.) + views::MenuWrapper::MenuAction action = previous_menu->GetMenuAction(); + if (action == views::MenuWrapper::MENU_ACTION_NEXT || + action == views::MenuWrapper::MENU_ACTION_PREVIOUS) { + MessageLoop::current()->PostTask( + FROM_HERE, method_factory_.NewRunnableMethod( + &ToolbarView::ActivateMenuButton, + next_menu_button)); + } +} + +void ToolbarView::ActivateMenuButton(views::MenuButton* menu_button) { +#if defined(OS_LINUX) + // Under GTK, opening a pop-up menu causes the main window to lose focus. + // Focus is automatically returned when the menu closes. + // + // Make sure that the menu button being activated has focus, so that + // when the user escapes from the menu without selecting anything, focus + // will be returned here. + if (!menu_button->HasFocus()) { + menu_button->RequestFocus(); + GetFocusManager()->StoreFocusedView(); + } +#endif + +#if defined(OS_WIN) + // On Windows, we have to explicitly clear the focus before opening + // the pop-up menu, then set the focus again when it closes. + GetFocusManager()->ClearFocus(); +#endif + + // Tell the menu button to activate, opening its pop-up menu. + menu_button->Activate(); + +#if defined(OS_WIN) + EnterMenuBarEmulationMode(last_focused_view_storage_id_, menu_button); +#endif +} + +void ToolbarView::ExitMenuBarEmulationMode() { + if (page_menu_->HasFocus() || app_menu_->HasFocus()) + RestoreLastFocusedView(); + + views::FocusManager* focus_manager = GetFocusManager(); + focus_manager->UnregisterAccelerators(this); + focus_manager->RemoveFocusChangeListener(this); + page_menu_->SetFocusable(false); + app_menu_->SetFocusable(false); +} + +void ToolbarView::RestoreLastFocusedView() { + views::ViewStorage* view_storage = views::ViewStorage::GetSharedInstance(); + views::View* last_focused_view = + view_storage->RetrieveView(last_focused_view_storage_id_); + if (last_focused_view) { + last_focused_view->RequestFocus(); + } else { + // Focus the location bar + views::View* view = GetAncestorWithClassName(BrowserView::kViewClassName); + if (view) { + BrowserView* browser_view = static_cast<BrowserView*>(view); + browser_view->SetFocusToLocationBar(); + } + } } diff --git a/chrome/browser/views/toolbar_view.h b/chrome/browser/views/toolbar_view.h index 23f5232..9425b56 100644 --- a/chrome/browser/views/toolbar_view.h +++ b/chrome/browser/views/toolbar_view.h @@ -36,6 +36,7 @@ class Menu2; class ToolbarView : public AccessibleToolbarView, public views::ViewMenuDelegate, public views::DragController, + public views::FocusChangeListener, public menus::SimpleMenuModel::Delegate, public LocationBarView::Delegate, public NotificationObserver, @@ -44,7 +45,7 @@ class ToolbarView : public AccessibleToolbarView, public BubblePositioner { public: explicit ToolbarView(Browser* browser); - virtual ~ToolbarView() { } + virtual ~ToolbarView(); // Create the contents of the Browser Toolbar void Init(Profile* profile); @@ -62,6 +63,19 @@ class ToolbarView : public AccessibleToolbarView, // Sets the app menu model. void SetAppMenuModel(AppMenuModel* model); + // Focuses the page menu and enters a special mode where the page + // and app menus are focusable and allow for keyboard navigation just + // like a normal menu bar. As soon as focus leaves one of the menus, + // the special mode is exited. + // + // Pass it the storage id of the view where focus should be returned + // if the user escapes, and the menu button to focus initially. If + // |menu_to_focus| is NULL, it will focus the page menu by default. + // + // Not used on the Mac, which has a "normal" menu bar. + void EnterMenuBarEmulationMode(int last_focused_view_storage_id, + views::MenuButton* menu_to_focus); + // Accessors... Browser* browser() const { return browser_; } BrowserActionsContainer* browser_actions() const { return browser_actions_; } @@ -71,6 +85,10 @@ class ToolbarView : public AccessibleToolbarView, views::MenuButton* page_menu() const { return page_menu_; } views::MenuButton* app_menu() const { return app_menu_; } + // Overridden from views::FocusChangeListener: + virtual void FocusWillChange(views::View* focused_before, + views::View* focused_now); + // Overridden from AccessibleToolbarView: virtual bool IsAccessibleViewTraversable(views::View* view); @@ -106,6 +124,7 @@ class ToolbarView : public AccessibleToolbarView, virtual void ExecuteCommand(int command_id); // Overridden from views::View: + virtual bool AcceleratorPressed(const views::Accelerator& accelerator); virtual gfx::Size GetPreferredSize(); virtual void Layout(); virtual void Paint(gfx::Canvas* canvas); @@ -138,6 +157,13 @@ class ToolbarView : public AccessibleToolbarView, void RunPageMenu(const gfx::Point& pt); void RunAppMenu(const gfx::Point& pt); + // Check if the menu exited with a code indicating the user wants to + // switch to the other menu, and then switch to that other menu. + void SwitchToOtherMenuIfNeeded(views::Menu2* previous_menu, + views::MenuButton* next_menu_button); + + void ActivateMenuButton(views::MenuButton* menu_button); + // Types of display mode this toolbar can have. enum DisplayMode { DISPLAYMODE_NORMAL, // Normal toolbar with buttons, etc. @@ -148,6 +174,14 @@ class ToolbarView : public AccessibleToolbarView, return display_mode_ == DISPLAYMODE_NORMAL; } + // Take the menus out of the focus traversal, unregister accelerators, + // and stop listening to focus change events. + void ExitMenuBarEmulationMode(); + + // Restore the view that was focused before EnterMenuBarEmulationMode + // was called. + void RestoreLastFocusedView(); + scoped_ptr<BackForwardMenuModel> back_menu_model_; scoped_ptr<BackForwardMenuModel> forward_menu_model_; @@ -186,6 +220,13 @@ class ToolbarView : public AccessibleToolbarView, // TODO(beng): build these into MenuButton. scoped_ptr<views::Menu2> page_menu_menu_; scoped_ptr<views::Menu2> app_menu_menu_; + + // Storage id for the last view that was focused before focus + // was given to one of the toolbar views. + int last_focused_view_storage_id_; + + // Used to post tasks to switch to the next/previous menu. + ScopedRunnableMethodFactory<ToolbarView> method_factory_; }; #endif // CHROME_BROWSER_VIEWS_TOOLBAR_VIEW_H_ diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi index 410946b..6ca4beb 100644 --- a/chrome/chrome_tests.gypi +++ b/chrome/chrome_tests.gypi @@ -230,6 +230,7 @@ 'test/automated_ui_tests/automated_ui_test_interactive_test.cc', 'test/automated_ui_tests/automated_ui_tests.cc', 'test/automated_ui_tests/automated_ui_tests.h', + 'test/automated_ui_tests/keyboard_access_uitest.cc', ], 'conditions': [ ['OS=="linux"', { diff --git a/chrome/test/automated_ui_tests/keyboard_access_uitest.cc b/chrome/test/automated_ui_tests/keyboard_access_uitest.cc new file mode 100644 index 0000000..fd7cdda --- /dev/null +++ b/chrome/test/automated_ui_tests/keyboard_access_uitest.cc @@ -0,0 +1,99 @@ +// 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. + +#include "base/keyboard_codes.h" +#include "chrome/test/automated_ui_tests/automated_ui_test_base.h" +#include "chrome/test/automation/browser_proxy.h" +#include "chrome/test/automation/tab_proxy.h" +#include "chrome/test/automation/window_proxy.h" +#include "googleurl/src/gurl.h" + +// Menu keyboard access currently works on Windows and on Linux when +// toolkit_views is defined (i.e. for Chrome OS). It's not needed +// on the Mac, and it's not yet implemented on Linux. +#if defined(TOOLKIT_VIEWS) + +namespace { + +class KeyboardAccessTest : public AutomatedUITestBase { + public: + KeyboardAccessTest() { + dom_automation_enabled_ = true; + show_window_ = true; + } + + // Use the keyboard to select "New Tab" from the app menu. + // This test depends on the fact that there are two menus and that + // New Tab is the first item in the app menu. If the menus change, + // this test will need to be changed to reflect that. + // + // If alternate_key_sequence is true, use "Alt" instead of "F10" to + // open the menu bar, and "Down" instead of "Enter" to open a menu. + void TestMenuKeyboardAccess(bool alternate_key_sequence); + + DISALLOW_COPY_AND_ASSIGN(KeyboardAccessTest); +}; + +void KeyboardAccessTest::TestMenuKeyboardAccess(bool alternate_key_sequence) { + scoped_refptr<BrowserProxy> browser = automation()->GetBrowserWindow(0); + scoped_refptr<WindowProxy> window = browser->GetWindow(); + + // Navigate to a page in the first tab, which makes sure that focus is + // set to the browser window. + scoped_refptr<TabProxy> tab(GetActiveTab()); + ASSERT_TRUE(tab.get()); + ASSERT_TRUE(tab->NavigateToURL(GURL("about:"))); + + // The initial tab index should be 0. + int tab_index = -1; + ASSERT_TRUE(browser->GetActiveTabIndex(&tab_index)); + ASSERT_EQ(0, tab_index); + + // Get the focused view ID, then press a key to activate the + // page menu, then wait until the focused view changes. + int original_view_id = -1; + ASSERT_TRUE(window->GetFocusedViewID(&original_view_id)); + + if (alternate_key_sequence) + ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_MENU, 0)); + else + ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_F10, 0)); + + int new_view_id = -1; + ASSERT_TRUE(window->WaitForFocusedViewIDToChange( + original_view_id, &new_view_id)); + + // Press RIGHT to focus the app menu, then RETURN or DOWN to open it. + ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_RIGHT, 0)); + if (alternate_key_sequence) + ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_DOWN, 0)); + else + ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_RETURN, 0)); + + // Wait until the popup menu actually opens. + ASSERT_TRUE(window->WaitForPopupMenuOpen(sleep_timeout_ms())); + + // Press DOWN to select the first item, then RETURN to select it. + ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_DOWN, 0)); + ASSERT_TRUE(window->SimulateOSKeyPress(base::VKEY_RETURN, 0)); + + // Wait for the new tab to appear. + ASSERT_TRUE(browser->WaitForTabCountToBecome(2, sleep_timeout_ms())); + + // Make sure that the new tab index is 1. + ASSERT_TRUE(browser->GetActiveTabIndex(&tab_index)); + ASSERT_EQ(1, tab_index); +} + +TEST_F(KeyboardAccessTest, TestMenuKeyboardAccess) { + TestMenuKeyboardAccess(false); +} + +TEST_F(KeyboardAccessTest, TestAltMenuKeyboardAccess) { + TestMenuKeyboardAccess(true); +} + +} // namespace + +#endif // defined(TOOLKIT_VIEWS) diff --git a/chrome/test/automation/automation_messages_internal.h b/chrome/test/automation/automation_messages_internal.h index 7447020..b73ef4b 100644 --- a/chrome/test/automation/automation_messages_internal.h +++ b/chrome/test/automation/automation_messages_internal.h @@ -348,6 +348,14 @@ IPC_BEGIN_MESSAGES(Automation) int /* view_handle */, int /* focused_view_id */) + // Block until the focused view id changes to something other than + // |previous_view_id|. + IPC_SYNC_MESSAGE_ROUTED2_2(AutomationMsg_WaitForFocusedViewIDToChange, + int /* window handle */, + int /* previous_view_id */, + bool /* success */, + int /* new_view_id */) + // This message shows/hides the window. IPC_SYNC_MESSAGE_ROUTED2_1(AutomationMsg_SetWindowVisible, int /* view_handle */, @@ -1288,4 +1296,10 @@ IPC_BEGIN_MESSAGES(Automation) int64 /* id */, bool /* success */) + // Determine if a pop-up menu is open. + IPC_SYNC_MESSAGE_ROUTED1_2(AutomationMsg_IsPopUpMenuOpen, + int /* window handle */, + bool /* success */, + bool /* is_open */) + IPC_END_MESSAGES(Automation) diff --git a/chrome/test/automation/window_proxy.cc b/chrome/test/automation/window_proxy.cc index 295f05b..5f18f6b 100644 --- a/chrome/test/automation/window_proxy.cc +++ b/chrome/test/automation/window_proxy.cc @@ -8,6 +8,7 @@ #include <algorithm> #include "base/logging.h" +#include "base/time.h" #include "chrome/test/automation/automation_constants.h" #include "chrome/test/automation/automation_messages.h" #include "chrome/test/automation/automation_proxy.h" @@ -16,6 +17,9 @@ #include "gfx/rect.h" #include "googleurl/src/gurl.h" +using base::TimeDelta; +using base::TimeTicks; + bool WindowProxy::SimulateOSClick(const gfx::Point& click, int flags) { if (!is_valid()) return false; @@ -123,6 +127,14 @@ bool WindowProxy::GetFocusedViewID(int* view_id) { view_id)); } +bool WindowProxy::WaitForFocusedViewIDToChange( + int old_view_id, int* new_view_id) { + bool result = false; + sender_->Send(new AutomationMsg_WaitForFocusedViewIDToChange + (0, handle_, old_view_id, &result, new_view_id)); + return result; +} + scoped_refptr<BrowserProxy> WindowProxy::GetBrowser() { return GetBrowserWithTimeout(base::kNoTimeout, NULL); } @@ -163,3 +175,22 @@ bool WindowProxy::IsMaximized(bool* maximized) { &result)); return result; } + +bool WindowProxy::WaitForPopupMenuOpen(uint32 timeout_ms) { + const TimeTicks start = TimeTicks::Now(); + const TimeDelta timeout = TimeDelta::FromMilliseconds(timeout_ms); + while (TimeTicks::Now() - start < timeout) { + PlatformThread::Sleep(automation::kSleepTime); + + bool is_open = false; + bool success = false; + if (!sender_->Send(new AutomationMsg_IsPopUpMenuOpen( + 0, handle_, &success, &is_open))) + return false; + if (!success) + return false; + if (is_open) + return true; + } + return false; +} diff --git a/chrome/test/automation/window_proxy.h b/chrome/test/automation/window_proxy.h index 3c5deca..be7ed64 100644 --- a/chrome/test/automation/window_proxy.h +++ b/chrome/test/automation/window_proxy.h @@ -90,6 +90,14 @@ class WindowProxy : public AutomationResourceProxy { // was retrieved. bool GetFocusedViewID(int* view_id); + // Waits until the focused view ID changes to something other than + // |old_view_id|. Returns true if the focused view ID did change. + bool WaitForFocusedViewIDToChange(int old_view_id, int* new_view_id); + + // Waits until a pop-up menu is opened. Returns true on success, or false + // if a pop-up menu is not opened within the timeout period. + bool WaitForPopupMenuOpen(uint32 timeout_ms); + // Returns the browser this window corresponds to, or NULL if this window // is not a browser. The caller owns the returned BrowserProxy. scoped_refptr<BrowserProxy> GetBrowser(); diff --git a/chrome/test/test_browser_window.h b/chrome/test/test_browser_window.h index 169afdb..1390dd8 100644 --- a/chrome/test/test_browser_window.h +++ b/chrome/test/test_browser_window.h @@ -48,6 +48,7 @@ class TestBrowserWindow : public BrowserWindow { virtual void UpdateToolbar(TabContents* contents, bool should_restore_state) {} virtual void FocusToolbar() {} + virtual void FocusPageAndAppMenus() {} virtual void ShowPageMenu() {} virtual void ShowAppMenu() {} virtual bool PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, |