From ee19cb20c9c5a0354d145a6a1ca950797b7cd3aa Mon Sep 17 00:00:00 2001 From: "suzhe@chromium.org" Date: Wed, 4 Aug 2010 19:48:19 +0000 Subject: [Linux Views] Refactor accelerator handler related code. This CL removes the accelerator handling logic in accelerator_handler_gtk.cc and implements a much simpler solution in WidgetGtk. The new approach always sends a key event to the focused View and native GtkWidget first and only sends it to the focus manager if it's not handled by any View or native GtkWidget. BUG=23383 AcceleratorHandler on Windows should not dispatch the KEYUP messages eaten by the FocusManager BUG=40966 BrowserKeyEventsTest.AccessKeys is crashy BUG=49701 [Linux Views]Some Emacs keybindings are broken in omnibox and find in page box. TEST=Press Alt key in different place (web page, omnibox, find bar, etc.) to see if menu bar can be focused correctly. Press alt-F to popup wrench menu and Escape to close it, then try alt key again. Review URL: http://codereview.chromium.org/3046041 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@54947 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/browser_keyevents_browsertest.cc | 62 +++++++++++++++++++--- .../renderer_host/gtk_im_context_wrapper.cc | 11 ++-- chrome/browser/views/frame/browser_view.cc | 7 +++ 3 files changed, 68 insertions(+), 12 deletions(-) (limited to 'chrome') diff --git a/chrome/browser/browser_keyevents_browsertest.cc b/chrome/browser/browser_keyevents_browsertest.cc index e943bef..0c4bd37db 100644 --- a/chrome/browser/browser_keyevents_browsertest.cc +++ b/chrome/browser/browser_keyevents_browsertest.cc @@ -532,14 +532,7 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, CommandKeyEvents) { } #endif -#if defined(TOOLKIT_VIEWS) && defined(OS_LINUX) -// See http://crbug.com/40037 for details. -#define MAYBE_AccessKeys DISABLED_AccessKeys -#else -#define MAYBE_AccessKeys AccessKeys -#endif - -IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, MAYBE_AccessKeys) { +IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, AccessKeys) { #if defined(OS_MACOSX) // On Mac, access keys use ctrl+alt modifiers. static const KeyEventTestData kTestAccessA = { @@ -891,4 +884,57 @@ IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, PageUpDownKeys) { EXPECT_NO_FATAL_FAILURE(CheckTextBoxValue(tab_index, L"A", L"")); } +#if defined(OS_WIN) || defined(TOOLKIT_VIEWS) +IN_PROC_BROWSER_TEST_F(BrowserKeyEventsTest, FocusMenuBarByAltKey) { + static const KeyEventTestData kTestAltKey = { + base::VKEY_MENU, false, false, false, false, + false, false, false, false, 2, + { "D 18 0 false false true false", + "U 18 0 false false true false" } + }; + + static const KeyEventTestData kTestAltKeySuppress = { + base::VKEY_MENU, false, false, false, false, + true, false, false, false, 2, + { "D 18 0 false false true false", + "U 18 0 false false true false" } + }; + + static const KeyEventTestData kTestCtrlAltKey = { + base::VKEY_MENU, true, false, false, false, + false, false, false, false, 4, + { "D 17 0 true false false false", + "D 18 0 true false true false", + "U 18 0 true false true false", + "U 17 0 true false false false" } + }; + + net::HTTPTestServer* server = StartHTTPServer(); + ASSERT_TRUE(server); + + BringBrowserWindowToFront(); + GURL url = server->TestServerPage(kTestingPage); + ui_test_utils::NavigateToURL(browser(), url); + + ASSERT_NO_FATAL_FAILURE(ClickOnView(VIEW_ID_TAB_CONTAINER)); + ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW)); + + int tab_index = browser()->selected_index(); + // Press and release Alt key to focus wrench menu button. + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(tab_index, kTestAltKey)); + EXPECT_TRUE(IsViewFocused(VIEW_ID_APP_MENU)); + + ASSERT_NO_FATAL_FAILURE(ClickOnView(VIEW_ID_TAB_CONTAINER)); + ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW)); + + // Alt key can be suppressed. + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(tab_index, kTestAltKeySuppress)); + ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW)); + + // Ctrl+Alt should have no effect. + EXPECT_NO_FATAL_FAILURE(TestKeyEvent(tab_index, kTestCtrlAltKey)); + ASSERT_TRUE(IsViewFocused(VIEW_ID_TAB_CONTAINER_FOCUS_VIEW)); +} +#endif + } // namespace diff --git a/chrome/browser/renderer_host/gtk_im_context_wrapper.cc b/chrome/browser/renderer_host/gtk_im_context_wrapper.cc index 8aa7f8c..4ac138b 100644 --- a/chrome/browser/renderer_host/gtk_im_context_wrapper.cc +++ b/chrome/browser/renderer_host/gtk_im_context_wrapper.cc @@ -132,6 +132,13 @@ void GtkIMContextWrapper::ProcessKeyEvent(GdkEventKey* event) { NativeWebKeyboardEvent wke(event); + // If the key event was handled by the input method, then we need to prevent + // RenderView::UnhandledKeyboardEvent() from processing it. + // Otherwise unexpected result may occur. For example if it's a + // Backspace key event, the browser may go back to previous page. + if (filtered) + wke.skip_in_browser = true; + // Send filtered keydown event before sending IME result. if (event->type == GDK_KEY_PRESS && filtered) ProcessFilteredKeyPressEvent(&wke); @@ -321,10 +328,6 @@ void GtkIMContextWrapper::ProcessFilteredKeyPressEvent( // keyidentifier must be updated accordingly, otherwise this key event may // still be processed by webkit. wke->setKeyIdentifierFromWindowsKeyCode(); - // Prevent RenderView::UnhandledKeyboardEvent() from processing it. - // Otherwise unexpected result may occur. For example if it's a - // Backspace key event, the browser may go back to previous page. - wke->skip_in_browser = true; } host_view_->ForwardKeyboardEvent(*wke); } diff --git a/chrome/browser/views/frame/browser_view.cc b/chrome/browser/views/frame/browser_view.cc index 0e11eaa..d8ab622 100644 --- a/chrome/browser/views/frame/browser_view.cc +++ b/chrome/browser/views/frame/browser_view.cc @@ -73,6 +73,7 @@ #elif defined(OS_LINUX) #include "chrome/browser/views/accelerator_table_gtk.h" #include "views/window/hit_test.h" +#include "views/window/window_gtk.h" #endif using base::TimeDelta; @@ -1238,8 +1239,14 @@ bool BrowserView::PreHandleKeyboardEvent(const NativeWebKeyboardEvent& event, } void BrowserView::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { +#if defined(OS_LINUX) + views::Window* window = GetWidget()->GetWindow(); + if (window && event.os_event && !event.skip_in_browser) + static_cast(window)->HandleKeyboardEvent(event.os_event); +#else unhandled_keyboard_event_handler_.HandleKeyboardEvent(event, GetFocusManager()); +#endif } // TODO(devint): http://b/issue?id=1117225 Cut, Copy, and Paste are always -- cgit v1.1