diff options
author | suzhe@chromium.org <suzhe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-23 01:15:33 +0000 |
---|---|---|
committer | suzhe@chromium.org <suzhe@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-23 01:15:33 +0000 |
commit | ed8bf13bccd28196fa2e1ddd30dc98a100167919 (patch) | |
tree | e721a87554d87716db83adcbdcc6c1cdeb109f60 /chrome | |
parent | 9febdb951b2a75ae69c2b2153c94ee5be86bb3ed (diff) | |
download | chromium_src-ed8bf13bccd28196fa2e1ddd30dc98a100167919.zip chromium_src-ed8bf13bccd28196fa2e1ddd30dc98a100167919.tar.gz chromium_src-ed8bf13bccd28196fa2e1ddd30dc98a100167919.tar.bz2 |
Fix conflicts between accelerator keys and HTML DOM accesskeys.
This CL fixes conflicts between accelerator keys and HTML DOM accesskeys by suppressing Char events if corresponding RawKeyDown event was handled by the browser after returning from the renderer unhandled.
This CL not only fixes this conflict issue, but also makes the behavior of handling accelerator keys similar than IE, which also suppresses a key press event if the key down event was handled as an accelerator key.
BUG=21624 accesskey attributes conflict with browser shortcuts (like tab-switching)
TEST=Open http://djmitche.github.com/buildbot/docs/0.7.11/ in one of opened multiple tabs, switch to another tab by pressing an alt-# key binding, then switch back to the original page to see if it's just as you left it before switching tabs.
Review URL: http://codereview.chromium.org/235039
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@29857 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
22 files changed, 553 insertions, 74 deletions
diff --git a/chrome/browser/chromeos/main_menu.h b/chrome/browser/chromeos/main_menu.h index 11e9942..c432c4b 100644 --- a/chrome/browser/chromeos/main_menu.h +++ b/chrome/browser/chromeos/main_menu.h @@ -162,7 +162,9 @@ class MainMenu : public RenderViewHostDelegate, virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event) { return false; } - virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) {} + virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { + return false; + } virtual void HandleMouseEvent() {} virtual void HandleMouseLeave() {} virtual void UpdatePreferredSize(const gfx::Size& pref_size) {} diff --git a/chrome/browser/cocoa/chrome_event_processing_window.h b/chrome/browser/cocoa/chrome_event_processing_window.h index 59241b7..4f13c27 100644 --- a/chrome/browser/cocoa/chrome_event_processing_window.h +++ b/chrome/browser/cocoa/chrome_event_processing_window.h @@ -15,6 +15,7 @@ @interface ChromeEventProcessingWindow : NSWindow { @private BOOL redispatchingEvent_; + BOOL eventHandled_; } // Returns |YES| if |event| has been shortcircuited and should not be processed @@ -25,7 +26,8 @@ // short-circuited to the RWHV. This is used to send keyboard events to the menu // and the cmd-` handler if a keyboard event comes back unhandled from the // renderer. -- (void)redispatchEvent:(NSEvent*)event; +// Returns |YES| if |event| has been handled. +- (BOOL)redispatchEvent:(NSEvent*)event; // See global_keyboard_shortcuts_mac.h for details on the next two functions. diff --git a/chrome/browser/cocoa/chrome_event_processing_window.mm b/chrome/browser/cocoa/chrome_event_processing_window.mm index e5c569a..3170012 100644 --- a/chrome/browser/cocoa/chrome_event_processing_window.mm +++ b/chrome/browser/cocoa/chrome_event_processing_window.mm @@ -87,17 +87,25 @@ typedef int (*KeyToCommandMapper)(bool, bool, bool, bool, int); return [super performKeyEquivalent:event]; } -- (void)redispatchEvent:(NSEvent*)event { +- (BOOL)redispatchEvent:(NSEvent*)event { DCHECK(event); DCHECK([event window] == self); + eventHandled_ = YES; redispatchingEvent_ = YES; [NSApp sendEvent:event]; redispatchingEvent_ = NO; + + // If the event was not handled by [NSApp sendEvent:], the sendEvent: + // method below will be called, and because |redispatchingEvent_| is YES, + // |eventHandled_| will be set to NO. + return eventHandled_; } - (void)sendEvent:(NSEvent*)event { if (!redispatchingEvent_) [super sendEvent:event]; + else + eventHandled_ = NO; } @end // ChromeEventProcessingWindow diff --git a/chrome/browser/extensions/extension_host.cc b/chrome/browser/extensions/extension_host.cc index 2f76e41..2f94612 100644 --- a/chrome/browser/extensions/extension_host.cc +++ b/chrome/browser/extensions/extension_host.cc @@ -434,7 +434,8 @@ bool ExtensionHost::IsReservedAccelerator(const NativeWebKeyboardEvent& event) { return false; } -void ExtensionHost::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { +bool ExtensionHost::HandleKeyboardEvent(const NativeWebKeyboardEvent& event) { + return false; } void ExtensionHost::HandleMouseEvent() { diff --git a/chrome/browser/extensions/extension_host.h b/chrome/browser/extensions/extension_host.h index 0c43067..ea9660d 100644 --- a/chrome/browser/extensions/extension_host.h +++ b/chrome/browser/extensions/extension_host.h @@ -131,7 +131,7 @@ class ExtensionHost : public RenderViewHostDelegate, virtual void GotFocus(); virtual void TakeFocus(bool reverse); virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event); - virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void HandleMouseEvent(); virtual void HandleMouseLeave(); virtual void UpdatePreferredSize(const gfx::Size& new_size); diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc index d4e0ffe..9549788 100644 --- a/chrome/browser/gtk/browser_window_gtk.cc +++ b/chrome/browser/gtk/browser_window_gtk.cc @@ -593,7 +593,7 @@ BrowserWindowGtk::~BrowserWindowGtk() { } } -void BrowserWindowGtk::HandleKeyboardEvent(GdkEventKey* event) { +bool BrowserWindowGtk::HandleKeyboardEvent(GdkEventKey* event) { // Handles a key event in following sequence: // 1. Our special key accelerators, such as ctrl-tab, etc. // 2. Gtk mnemonics and accelerators. @@ -603,8 +603,9 @@ void BrowserWindowGtk::HandleKeyboardEvent(GdkEventKey* event) { // gtk_window_activate_key() takes care of it automatically. if (!HandleCustomAccelerator(event->keyval, GdkModifierType(event->state), browser_.get())) { - gtk_window_activate_key(window_, event); + return gtk_window_activate_key(window_, event); } + return true; } gboolean BrowserWindowGtk::OnCustomFrameExpose(GtkWidget* widget, diff --git a/chrome/browser/gtk/browser_window_gtk.h b/chrome/browser/gtk/browser_window_gtk.h index 762af0a..ee48c9b 100644 --- a/chrome/browser/gtk/browser_window_gtk.h +++ b/chrome/browser/gtk/browser_window_gtk.h @@ -57,7 +57,7 @@ class BrowserWindowGtk : public BrowserWindow, Browser* browser() const { return browser_.get(); } // Process a keyboard event which was not handled by webkit. - void HandleKeyboardEvent(GdkEventKey* event); + bool HandleKeyboardEvent(GdkEventKey* event); // Overridden from BrowserWindow virtual void Show(); diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index 5fe0b05..b59b0ce 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -1442,12 +1442,13 @@ bool RenderViewHost::ShouldSendToRenderer(const NativeWebKeyboardEvent& event) { return !view->IsReservedAccelerator(event); } -void RenderViewHost::UnhandledKeyboardEvent( +bool RenderViewHost::UnhandledKeyboardEvent( const NativeWebKeyboardEvent& event) { RenderViewHostDelegate::View* view = delegate_->GetViewDelegate(); if (view) { - view->HandleKeyboardEvent(event); + return view->HandleKeyboardEvent(event); } + return false; } void RenderViewHost::OnUserGesture() { diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h index 8bfb591..aa7fd49 100644 --- a/chrome/browser/renderer_host/render_view_host.h +++ b/chrome/browser/renderer_host/render_view_host.h @@ -444,7 +444,7 @@ class RenderViewHost : public RenderWidgetHost, protected: // RenderWidgetHost protected overrides. virtual bool ShouldSendToRenderer(const NativeWebKeyboardEvent& event); - virtual void UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void OnUserGesture(); virtual void NotifyRendererUnresponsive(); virtual void NotifyRendererResponsive(); diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h index 5db400d..c3dd529 100644 --- a/chrome/browser/renderer_host/render_view_host_delegate.h +++ b/chrome/browser/renderer_host/render_view_host_delegate.h @@ -133,7 +133,8 @@ class RenderViewHostDelegate { // Callback to inform the browser that the renderer did not process the // specified events. This gives an opportunity to the browser to process the // event (used for keyboard shortcuts). - virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event) = 0; + // Returns true if the event was handled. + virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event) = 0; // Notifications about mouse events in this view. This is useful for // implementing global 'on hover' features external to the view. diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc index 6d2b4bd..d436975 100644 --- a/chrome/browser/renderer_host/render_widget_host.cc +++ b/chrome/browser/renderer_host/render_widget_host.cc @@ -71,7 +71,10 @@ RenderWidgetHost::RenderWidgetHost(RenderProcessHost* process, view_being_painted_(false), text_direction_updated_(false), text_direction_(WebKit::WebTextDirectionLeftToRight), - text_direction_canceled_(false) { + text_direction_canceled_(false), + pending_key_events_(false), + suppress_next_char_events_(false), + death_flag_(NULL) { if (routing_id_ == MSG_ROUTING_NONE) routing_id_ = process_->GetNextRoutingID(); @@ -82,6 +85,10 @@ RenderWidgetHost::RenderWidgetHost(RenderProcessHost* process, } RenderWidgetHost::~RenderWidgetHost() { + // Force the method that destroys this object to exit immediately. + if (death_flag_) + *death_flag_ = true; + // Clear our current or cached backing store if either remains. BackingStoreManager::RemoveBackingStore(this); @@ -395,22 +402,95 @@ void RenderWidgetHost::ForwardKeyboardEvent( if (!process_->HasConnection()) return; + // To help understand following logic, please refer to the comments + // explaining |pending_key_events_| and |suppress_next_char_events_| + // members. And the comments in OnMsgInputEventAck() method, which contains + // the bottom half of the logic. + // + // There are to different situations for us to handle key events: + // 1. After sending a key event to the renderer, we receive its ACK message + // from the renderer before sending the next key event. + // In this case, there is always no more than 1 key event in |key_queue_|, + // and we send the key event one by one in this method, except that a + // sequence of Char events may be suppressed directly if the preceding + // RawKeyDown event was handled as an accelerator key in the browser. + // + // 2. We get the next key event before receving the previous one's ACK + // message from the renderer. + // In this case, when we get a key event, the previous key event is still in + // |keq_queue_| waiting for being handled in OnMsgInputEventAck() method. + // Then we need handle following cases differently: + // 1) If we get a Char event, then the previous key event in |key_queue_| + // waiting for ACK must be a RawKeyDown event. Then we need keep this Char + // event in |key_queue_| rather than sending it immediately, because we + // can't determine if we should send it to the renderer or just suppress + // it, until we receive the preceding RawKeyDown event's ACK message. + // We increase the count of |pending_key_events_| to help remember this + // Char event is not sent to the renderer yet. + // 2) If there is already one or more key event pending in |key_queue_| + // (|pending_key_events_| > 0), we can not send a new key event + // immediately no matter what type it is. Otherwise the key events will be + // in wrong order. In this case we just keep them in |key_queue_| and + // increase |pending_key_events_| to remember them. + // + // Then all pending key events in |key_queue_| will be handled properly in + // OnMsgInputEventAck() method. Please refer to that method for details. + // Tab switching/closing accelerators aren't sent to the renderer to avoid a // hung/malicious renderer from interfering. if (!ShouldSendToRenderer(key_event)) { UnhandledKeyboardEvent(key_event); + if (key_event.type == WebKeyboardEvent::RawKeyDown) + suppress_next_char_events_ = true; return; } + bool is_char = (key_event.type == WebKeyboardEvent::Char); + + // Handle the first situation mentioned above. + if (suppress_next_char_events_) { + // If preceding RawKeyDown event was handled by the browser, then we need + // suppress all Char events generated by it. Please note that, one + // RawKeyDown event may generate multiple Char events, so we can't reset + // |suppress_next_char_events_| until we get a KeyUp or a RawKeyDown. + if (is_char) + return; + // We get a KeyUp or a RawKeyDown event. + suppress_next_char_events_ = false; + } + // Put all WebKeyboardEvent objects in a queue since we can't trust the // renderer and we need to give something to the UnhandledInputEvent // handler. - key_queue_.push(key_event); + key_queue_.push_back(key_event); HISTOGRAM_COUNTS_100("Renderer.KeyboardQueueSize", key_queue_.size()); - } - // Only forward the non-native portions of our event. - ForwardInputEvent(key_event, sizeof(WebKeyboardEvent)); + // Handle the second situation mentioned above. + size_t key_queue_size = key_queue_.size(); + if ((is_char && key_queue_size > 1) || pending_key_events_) { + // The event is not send to the renderer, increase |pending_key_events_| + // to remember it. + ++pending_key_events_; + + // Some sanity checks. + // At least one key event in the front of |key_queue_| has been sent to + // the renderer, otherwise we won't be able to get any ACK back from the + // renderer. + DCHECK(key_queue_size > pending_key_events_); + // Char events must be preceded by RawKeyDown events. + // TODO(suzhe): verify it on all platforms. + DCHECK(key_queue_[key_queue_size - pending_key_events_ - 1].type == + WebKeyboardEvent::RawKeyDown); + // The first pending key event must be a Char event. Other key events must + // already be sent to the renderer at this point. + DCHECK(key_queue_[key_queue_size - pending_key_events_].type == + WebKeyboardEvent::Char); + } else { + // Fall into the first situation, so we can send the event immediately. + // Only forward the non-native portions of our event. + ForwardInputEvent(key_event, sizeof(WebKeyboardEvent)); + } + } } void RenderWidgetHost::ForwardInputEvent(const WebInputEvent& input_event, @@ -721,6 +801,10 @@ void RenderWidgetHost::OnMsgScrollRect( } void RenderWidgetHost::OnMsgInputEventAck(const IPC::Message& message) { + // Track if |this| is destroyed or not. + bool is_dead = false; + death_flag_ = &is_dead; + // Log the time delta for processing an input event. TimeDelta delta = TimeTicks::Now() - input_event_start_time_; UMA_HISTOGRAM_TIMES("MPArch.RWH_InputEventDelta", delta); @@ -751,23 +835,86 @@ void RenderWidgetHost::OnMsgInputEventAck(const IPC::Message& message) { LOG(ERROR) << "We seem to have a different key type sent from " << "the renderer. (" << key_queue_.front().type << " vs. " << type << "). Ignoring event."; + + // Something must be wrong. |key_queue_| must be cleared here to make sure + // the feedback loop for sending upcoming keyboard events can be resumed + // correctly. + key_queue_.clear(); + pending_key_events_ = 0; + suppress_next_char_events_ = false; } else { bool processed = false; if (!message.ReadBool(&iter, &processed)) process()->ReceivedBadMessage(message.type()); NativeWebKeyboardEvent front_item = key_queue_.front(); - key_queue_.pop(); + key_queue_.pop_front(); + bool processed_by_browser = false; if (!processed) { - UnhandledKeyboardEvent(front_item); + processed_by_browser = UnhandledKeyboardEvent(front_item); // WARNING: This RenderWidgetHost can be deallocated at this point // (i.e. in the case of Ctrl+W, where the call to // UnhandledKeyboardEvent destroys this RenderWidgetHost). } + + // This RenderWidgetHost was already deallocated, we can't do anything + // from now on, including resetting |death_flag_|. So just return. + if (is_dead) + return; + + // Suppress the following Char events if the RawKeyDown event was handled + // by the browser rather than the renderer. + if (front_item.type == WebKeyboardEvent::RawKeyDown) + suppress_next_char_events_ = processed_by_browser; + + // If more than one key events in |key_queue_| were already sent to the + // renderer but haven't got ACK messages yet, we must wait for ACK + // messages of these key events before sending more key events to the + // renderer. + if (pending_key_events_ && key_queue_.size() == pending_key_events_) { + size_t i = 0; + if (suppress_next_char_events_) { + // Suppress the sequence of pending Char events if preceding + // RawKeyDown event was handled by the browser. + while (pending_key_events_ && + key_queue_[0].type == WebKeyboardEvent::Char) { + --pending_key_events_; + key_queue_.pop_front(); + } + } else { + // Otherwise, send these pending Char events to the renderer. + // Note: we can't remove them from |key_queue_|, as we still need to + // wait for their ACK messages from the renderer. + while (pending_key_events_ && + key_queue_[i].type == WebKeyboardEvent::Char) { + --pending_key_events_; + ForwardInputEvent(key_queue_[i++], sizeof(WebKeyboardEvent)); + } + } + + // Reset |suppress_next_char_events_| if there is still one or more + // pending KeyUp or RawKeyDown events in the queue. + // We can't reset it if there is no pending event anymore, because we + // don't know if the following event is a Char event or not. + if (pending_key_events_) + suppress_next_char_events_ = false; + + // We can safely send following pending KeyUp and RawKeyDown events to + // the renderer, until we meet another Char event. + while (pending_key_events_ && + key_queue_[i].type != WebKeyboardEvent::Char) { + --pending_key_events_; + ForwardInputEvent(key_queue_[i++], sizeof(WebKeyboardEvent)); + } + } } } + + // Reset |death_flag_| to NULL, otherwise it'll point to an invalid memory + // address after returning from this method. + death_flag_ = NULL; } void RenderWidgetHost::OnMsgFocus() { diff --git a/chrome/browser/renderer_host/render_widget_host.h b/chrome/browser/renderer_host/render_widget_host.h index 01fce6c..39b3e07 100644 --- a/chrome/browser/renderer_host/render_widget_host.h +++ b/chrome/browser/renderer_host/render_widget_host.h @@ -5,7 +5,7 @@ #ifndef CHROME_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_H_ #define CHROME_BROWSER_RENDERER_HOST_RENDER_WIDGET_HOST_H_ -#include <queue> +#include <deque> #include "app/gfx/native_widget_types.h" #include "base/process.h" @@ -365,7 +365,10 @@ class RenderWidgetHost : public IPC::Channel::Listener, // Called when we an InputEvent was not processed by the renderer. This is // overridden by RenderView to send upwards to its delegate. - virtual void UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) {} + // Returns true if the event was handled by its delegate. + virtual bool UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) { + return false; + } // Notification that the user has made some kind of input that could // perform an action. The render view host overrides this to forward the @@ -528,11 +531,12 @@ class RenderWidgetHost : public IPC::Channel::Listener, base::TimeTicks repaint_start_time_; // Queue of keyboard events that we need to track. - typedef std::queue<NativeWebKeyboardEvent> KeyQueue; + typedef std::deque<NativeWebKeyboardEvent> KeyQueue; // A queue of keyboard events. We can't trust data from the renderer so we // stuff key events into a queue and pop them out on ACK, feeding our copy // back to whatever unhandled handler instead of the returned version. + // See the description of |pending_key_events_| below for more details. KeyQueue key_queue_; // Set when we update the text direction of the selected input element. @@ -544,6 +548,42 @@ class RenderWidgetHost : public IPC::Channel::Listener, // NotifyTextDirection(). bool text_direction_canceled_; + // How many key events in |key_queue_| are not sent to the renderer yet, + // counted from the back of |key_queue_|. + // In order to suppress a Char event when necessary (see the description of + // |suppress_next_char_events_| below), we can't just send it to the + // renderer as soon as we get it. Instead, we need wait for the result of + // preceding RawKeyDown event back from the renderer, and then decide how to + // process the pending Char events according to the result. + // So if we get one or more Char events before receiving the result of + // preceding RawKeyDown event, we need keep them in |key_queue_|. And in + // order to keep the order the key events, all following key events must be + // pending until the pending Char events got processed. + size_t pending_key_events_; + + // Indicates if the next sequence of Char events should be suppressed or not. + // System may translate a RawKeyDown event into zero or more Char events, + // usually we send them to the renderer directly in sequence. However, If a + // RawKeyDown event was not handled by the renderer but was handled by + // our UnhandledKeyboardEvent() method, eg. as an accelerator key, then we + // shall not send the following sequence of Char events, which was generated + // by this RawKeyDown event, to the renderer. Otherwise the renderer may + // handle the Char events and cause unexpected behavior. + // For example, pressing alt-2 may let the browser switch to the second tab, + // but the Char event generated by alt-2 may also activate a HTML element + // if its accesskey happens to be "2", then the user may get confused when + // switching back to the original tab, because the content may already be + // changed. + bool suppress_next_char_events_; + + // During the call to some methods, eg. OnMsgInputEventAck, this + // RenderWidgetHost object may be destroyed before executing some code that + // still want to access this object. To avoid this situation, |death_flag_| + // shall be pointed to a local variable in the method, and then |*death_flag_| + // will be set to true when destroying this RenderWidgetHost object, then the + // method shall exit immediately when |*death_flag_| becomes true. + bool* death_flag_; + DISALLOW_COPY_AND_ASSIGN(RenderWidgetHost); }; diff --git a/chrome/browser/renderer_host/render_widget_host_unittest.cc b/chrome/browser/renderer_host/render_widget_host_unittest.cc index 4f16e8d..95c3624 100644 --- a/chrome/browser/renderer_host/render_widget_host_unittest.cc +++ b/chrome/browser/renderer_host/render_widget_host_unittest.cc @@ -123,7 +123,9 @@ class MockRenderWidgetHost : public RenderWidgetHost { public: MockRenderWidgetHost(RenderProcessHost* process, int routing_id) : RenderWidgetHost(process, routing_id), - unhandled_keyboard_event_called_(false) { + unhandled_keyboard_event_called_(false), + handle_unhandled_keyboard_event_(false), + unhandled_keyboard_event_type_(WebInputEvent::Undefined) { } // Tests that make sure we ignore keyboard event acknowledgments to events we @@ -132,13 +134,25 @@ class MockRenderWidgetHost : public RenderWidgetHost { return unhandled_keyboard_event_called_; } + WebInputEvent::Type unhandled_keyboard_event_type() const { + return unhandled_keyboard_event_type_; + } + + void set_handle_unhandled_keyboard_event(bool handle) { + handle_unhandled_keyboard_event_ = handle; + } + protected: - virtual void UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) { + virtual bool UnhandledKeyboardEvent(const NativeWebKeyboardEvent& event) { + unhandled_keyboard_event_type_ = event.type; unhandled_keyboard_event_called_ = true; + return handle_unhandled_keyboard_event_; } private: bool unhandled_keyboard_event_called_; + bool handle_unhandled_keyboard_event_; + WebInputEvent::Type unhandled_keyboard_event_type_; }; // RenderWidgetHostTest -------------------------------------------------------- @@ -170,6 +184,21 @@ class RenderWidgetHostTest : public testing::Test { MessageLoop::current()->RunAllPending(); } + void SendInputEventACK(WebInputEvent::Type type, bool processed) { + scoped_ptr<IPC::Message> response( + new ViewHostMsg_HandleInputEvent_ACK(0)); + response->WriteInt(type); + response->WriteBool(processed); + host_->OnMessageReceived(*response); + } + + void SimulateKeyboardEvent(WebInputEvent::Type type) { + NativeWebKeyboardEvent key_event; + key_event.type = type; + key_event.windowsKeyCode = base::VKEY_L; // non-null made up value. + host_->ForwardKeyboardEvent(key_event); + } + MessageLoopForUI message_loop_; scoped_ptr<TestingProfile> profile_; @@ -398,12 +427,8 @@ TEST_F(RenderWidgetHostTest, HiddenPaint) { } TEST_F(RenderWidgetHostTest, HandleKeyEventsWeSent) { - NativeWebKeyboardEvent key_event; - key_event.type = WebInputEvent::KeyDown; - key_event.modifiers = WebInputEvent::ControlKey; - key_event.windowsKeyCode = base::VKEY_L; // non-null made up value. - - host_->ForwardKeyboardEvent(key_event); + // Simulate a keyboard event. + SimulateKeyboardEvent(WebInputEvent::RawKeyDown); // Make sure we sent the input event to the renderer. EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( @@ -411,22 +436,265 @@ TEST_F(RenderWidgetHostTest, HandleKeyEventsWeSent) { process_->sink().ClearMessages(); // Send the simulated response from the renderer back. - scoped_ptr<IPC::Message> response( - new ViewHostMsg_HandleInputEvent_ACK(0)); - response->WriteInt(key_event.type); - response->WriteBool(false); - host_->OnMessageReceived(*response); + SendInputEventACK(WebInputEvent::RawKeyDown, false); EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); } TEST_F(RenderWidgetHostTest, IgnoreKeyEventsWeDidntSend) { // Send a simulated, unrequested key response. We should ignore this. - scoped_ptr<IPC::Message> response( - new ViewHostMsg_HandleInputEvent_ACK(0)); - response->WriteInt(WebInputEvent::KeyDown); - response->WriteBool(false); - host_->OnMessageReceived(*response); + SendInputEventACK(WebInputEvent::RawKeyDown, false); + + EXPECT_FALSE(host_->unhandled_keyboard_event_called()); +} + +TEST_F(RenderWidgetHostTest, IgnoreKeyEventsHandledByRenderer) { + // Simulate a keyboard event. + SimulateKeyboardEvent(WebInputEvent::RawKeyDown); + + // Make sure we sent the input event to the renderer. + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::RawKeyDown, true); EXPECT_FALSE(host_->unhandled_keyboard_event_called()); } + +TEST_F(RenderWidgetHostTest, DontSuppressNextCharEventsNoPending) { + // Simulate a keyboard event. + SimulateKeyboardEvent(WebInputEvent::RawKeyDown); + + // Make sure we sent the input event to the renderer. + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::RawKeyDown, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); + + // Forward the Char event. + SimulateKeyboardEvent(WebInputEvent::Char); + + // Make sure we sent the input event to the renderer. + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::Char, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::Char, host_->unhandled_keyboard_event_type()); + + // Forward the KeyUp event. + SimulateKeyboardEvent(WebInputEvent::KeyUp); + + // Make sure we sent the input event to the renderer. + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::KeyUp, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); +} + +TEST_F(RenderWidgetHostTest, SuppressNextCharEventsNoPending) { + // Simulate a keyboard event. + SimulateKeyboardEvent(WebInputEvent::RawKeyDown); + + // Make sure we sent the input event to the renderer. + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + + // Simluate the situation that the browser handled the key down event. + host_->set_handle_unhandled_keyboard_event(true); + + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::RawKeyDown, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); + + // Forward the Char event. + SimulateKeyboardEvent(WebInputEvent::Char); + + // Make sure the Char event is suppressed. + EXPECT_EQ(0U, process_->sink().message_count()); + + // Forward another Char event. + SimulateKeyboardEvent(WebInputEvent::Char); + + // Make sure the Char event is suppressed. + EXPECT_EQ(0U, process_->sink().message_count()); + + // Forward the KeyUp event. + SimulateKeyboardEvent(WebInputEvent::KeyUp); + + // Make sure we sent the input event to the renderer. + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + + // The browser does not handle KeyUp events. + host_->set_handle_unhandled_keyboard_event(false); + + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::KeyUp, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); +} + +TEST_F(RenderWidgetHostTest, DontSuppressNextCharEventsPending) { + SimulateKeyboardEvent(WebInputEvent::RawKeyDown); + + // Make sure we sent the input event to the renderer. + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + + // Forward the Char event before receiving the ACK of previous KeyDown event. + SimulateKeyboardEvent(WebInputEvent::Char); + + // Make sure the Char event is pending. + EXPECT_EQ(0U, process_->sink().message_count()); + + // Forward the KeyUp event before receiving the ACK of previous KeyDown event. + SimulateKeyboardEvent(WebInputEvent::KeyUp); + + // Make sure the KeyUp event is pending. + EXPECT_EQ(0U, process_->sink().message_count()); + + // Send the simulated response of the KeyDown event from the renderer back. + SendInputEventACK(WebInputEvent::RawKeyDown, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); + + // Make sure both pending Char and KeyUp were sent to the renderer. + EXPECT_EQ(2U, process_->sink().message_count()); + EXPECT_EQ(ViewMsg_HandleInputEvent::ID, + process_->sink().GetMessageAt(0)->type()); + EXPECT_EQ(ViewMsg_HandleInputEvent::ID, + process_->sink().GetMessageAt(1)->type()); + process_->sink().ClearMessages(); + + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::Char, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::Char, host_->unhandled_keyboard_event_type()); + + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::KeyUp, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); +} + +TEST_F(RenderWidgetHostTest, SuppressNextCharEventsPending) { + SimulateKeyboardEvent(WebInputEvent::RawKeyDown); + + // Make sure we sent the KeyDown event to the renderer. + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + + // Forward the Char event before receiving the ACK of previous KeyDown event. + SimulateKeyboardEvent(WebInputEvent::Char); + + // Make sure the Char event is pending. + EXPECT_EQ(0U, process_->sink().message_count()); + + // Forward another Char event before receiving the ACK of previous KeyDown + // event. + SimulateKeyboardEvent(WebInputEvent::Char); + + // Make sure the Char event is pending. + EXPECT_EQ(0U, process_->sink().message_count()); + + // Forward the KeyUp event before receiving the ACK of previous KeyDown event. + SimulateKeyboardEvent(WebInputEvent::KeyUp); + + // Make sure the KeyUp event is pending. + EXPECT_EQ(0U, process_->sink().message_count()); + + // Simluate the situation that the browser handled the key down event. + host_->set_handle_unhandled_keyboard_event(true); + + // Send the simulated response of the KeyDown event from the renderer back. + SendInputEventACK(WebInputEvent::RawKeyDown, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::RawKeyDown, host_->unhandled_keyboard_event_type()); + + // Make sure only pending KeyUp was sent to the renderer. + EXPECT_EQ(1U, process_->sink().message_count()); + EXPECT_EQ(ViewMsg_HandleInputEvent::ID, + process_->sink().GetMessageAt(0)->type()); + process_->sink().ClearMessages(); + + // Send the simulated response from the renderer back. + SendInputEventACK(WebInputEvent::KeyUp, false); + + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); +} + +TEST_F(RenderWidgetHostTest, ManyKeyEventsPending) { + process_->sink().ClearMessages(); + + for (int i = 0; i < 10; ++i) { + // Forward a KeyDown event. + SimulateKeyboardEvent(WebInputEvent::RawKeyDown); + + // Forward a Char event before receiving the ACK of previous KeyDown event. + SimulateKeyboardEvent(WebInputEvent::Char); + + // Forward a KeyUp event before receiving the ACK of previous KeyDown event. + SimulateKeyboardEvent(WebInputEvent::KeyUp); + } + + // Make sure only the first KeyDown event was sent to the renderer. All others + // are pending. + EXPECT_EQ(1U, process_->sink().message_count()); + EXPECT_TRUE(process_->sink().GetUniqueMessageMatching( + ViewMsg_HandleInputEvent::ID)); + process_->sink().ClearMessages(); + + for (int i = 0; i < 10; ++i) { + // Send the simulated response of the KeyDown event from the renderer back. + SendInputEventACK(WebInputEvent::RawKeyDown, false); + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::RawKeyDown, + host_->unhandled_keyboard_event_type()); + + // Make sure the following pending Char, KeyUp and KeyDown event were sent + // to the renderer. + if (i < 9) + EXPECT_EQ(3U, process_->sink().message_count()); + else + EXPECT_EQ(2U, process_->sink().message_count()); + process_->sink().ClearMessages(); + + // Send the simulated response of the Char event from the renderer back. + SendInputEventACK(WebInputEvent::Char, false); + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::Char, host_->unhandled_keyboard_event_type()); + + // Send the simulated response of the KeyUp event from the renderer back. + SendInputEventACK(WebInputEvent::KeyUp, false); + EXPECT_TRUE(host_->unhandled_keyboard_event_called()); + EXPECT_EQ(WebInputEvent::KeyUp, host_->unhandled_keyboard_event_type()); + } +} diff --git a/chrome/browser/tab_contents/interstitial_page.cc b/chrome/browser/tab_contents/interstitial_page.cc index cb6648a..024f250 100644 --- a/chrome/browser/tab_contents/interstitial_page.cc +++ b/chrome/browser/tab_contents/interstitial_page.cc @@ -97,7 +97,7 @@ class InterstitialPage::InterstitialPageRVHViewDelegate virtual void GotFocus(); virtual void TakeFocus(bool reverse); virtual bool IsReservedAccelerator(const NativeWebKeyboardEvent& event); - virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); virtual void HandleMouseEvent(); virtual void HandleMouseLeave(); virtual void OnFindReply(int request_id, @@ -584,10 +584,12 @@ bool InterstitialPage::InterstitialPageRVHViewDelegate::IsReservedAccelerator( return false; } -void InterstitialPage::InterstitialPageRVHViewDelegate::HandleKeyboardEvent( +bool InterstitialPage::InterstitialPageRVHViewDelegate::HandleKeyboardEvent( const NativeWebKeyboardEvent& event) { if (interstitial_page_->tab() && interstitial_page_->tab()->GetViewDelegate()) - interstitial_page_->tab()->GetViewDelegate()->HandleKeyboardEvent(event); + return interstitial_page_->tab()->GetViewDelegate()-> + HandleKeyboardEvent(event); + return false; } void InterstitialPage::InterstitialPageRVHViewDelegate::HandleMouseEvent() { diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/tab_contents/tab_contents_view_gtk.cc index b738b8a..a51aa10 100644 --- a/chrome/browser/tab_contents/tab_contents_view_gtk.cc +++ b/chrome/browser/tab_contents/tab_contents_view_gtk.cc @@ -320,14 +320,14 @@ void TabContentsViewGtk::TakeFocus(bool reverse) { } } -void TabContentsViewGtk::HandleKeyboardEvent( +bool TabContentsViewGtk::HandleKeyboardEvent( const NativeWebKeyboardEvent& event) { // This may be an accelerator. Try to pass it on to our browser window // to handle. GtkWindow* window = GetTopLevelNativeWindow(); if (!window) { NOTREACHED(); - return; + return false; } // Filter out pseudo key events created by GtkIMContext signal handlers. @@ -339,12 +339,12 @@ void TabContentsViewGtk::HandleKeyboardEvent( // as an accelerator. if (event.type == WebKit::WebInputEvent::Char || event.type == WebKit::WebInputEvent::KeyUp) - return; + return false; BrowserWindowGtk* browser_window = BrowserWindowGtk::GetBrowserWindowForNativeWindow(window); DCHECK(browser_window); - browser_window->HandleKeyboardEvent(event.os_event); + return browser_window->HandleKeyboardEvent(event.os_event); } void TabContentsViewGtk::Observe(NotificationType type, diff --git a/chrome/browser/tab_contents/tab_contents_view_gtk.h b/chrome/browser/tab_contents/tab_contents_view_gtk.h index d2da996..d4bc0cf 100644 --- a/chrome/browser/tab_contents/tab_contents_view_gtk.h +++ b/chrome/browser/tab_contents/tab_contents_view_gtk.h @@ -70,7 +70,7 @@ class TabContentsViewGtk : public TabContentsView, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); // NotificationObserver implementation --------------------------------------- diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.h b/chrome/browser/tab_contents/tab_contents_view_mac.h index 79c7fe4..99e9f22 100644 --- a/chrome/browser/tab_contents/tab_contents_view_mac.h +++ b/chrome/browser/tab_contents/tab_contents_view_mac.h @@ -74,7 +74,7 @@ class TabContentsViewMac : public TabContentsView, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); // NotificationObserver implementation --------------------------------------- diff --git a/chrome/browser/tab_contents/tab_contents_view_mac.mm b/chrome/browser/tab_contents/tab_contents_view_mac.mm index 908b411..a8933be 100644 --- a/chrome/browser/tab_contents/tab_contents_view_mac.mm +++ b/chrome/browser/tab_contents/tab_contents_view_mac.mm @@ -46,7 +46,7 @@ COMPILE_ASSERT_MATCHING_ENUM(DragOperationEvery); @interface TabContentsViewCocoa (Private) - (id)initWithTabContentsViewMac:(TabContentsViewMac*)w; -- (void)processKeyboardEvent:(NativeWebKeyboardEvent*)event; +- (BOOL)processKeyboardEvent:(NativeWebKeyboardEvent*)event; - (void)registerDragTypes; - (void)setCurrentDragOperation:(NSDragOperation)operation; - (void)startDragWithDropData:(const WebDropData&)dropData @@ -212,10 +212,10 @@ void TabContentsViewMac::TakeFocus(bool reverse) { } } -void TabContentsViewMac::HandleKeyboardEvent( +bool TabContentsViewMac::HandleKeyboardEvent( const NativeWebKeyboardEvent& event) { - [cocoa_view_.get() processKeyboardEvent: - const_cast<NativeWebKeyboardEvent*>(&event)]; + return [cocoa_view_.get() processKeyboardEvent: + const_cast<NativeWebKeyboardEvent*>(&event)] == YES; } void TabContentsViewMac::ShowContextMenu(const ContextMenuParams& params) { @@ -310,9 +310,9 @@ void TabContentsViewMac::Observe(NotificationType type, return tabContentsView_->tab_contents(); } -- (void)processKeyboardEvent:(NativeWebKeyboardEvent*)wkEvent { +- (BOOL)processKeyboardEvent:(NativeWebKeyboardEvent*)wkEvent { if (wkEvent->skip_in_browser) - return; + return NO; NSEvent* event = wkEvent->os_event; @@ -320,13 +320,13 @@ void TabContentsViewMac::Observe(NotificationType type, // Char events are synthesized and do not contain a real event. We are not // interested in them anyway. DCHECK(wkEvent->type == WebKit::WebInputEvent::Char); - return; + return NO; } // If this tab is no longer active, its window will be |nil|. In that case, // best ignore the event. if (![self window]) - return; + return NO; ChromeEventProcessingWindow* window = (ChromeEventProcessingWindow*)[self window]; DCHECK([window isKindOfClass:[ChromeEventProcessingWindow class]]); @@ -334,9 +334,9 @@ void TabContentsViewMac::Observe(NotificationType type, // Do not fire shortcuts on key up. if ([event type] == NSKeyDown) { if ([window handleExtraBrowserKeyboardShortcut:event]) - return; + return YES; if ([window handleExtraWindowKeyboardShortcut:event]) - return; + return YES; } // We need to re-dispatch the event, so that it is sent to the menu or other @@ -345,8 +345,9 @@ void TabContentsViewMac::Observe(NotificationType type, tabContentsView_->GetContentNativeView()); DCHECK([rwhv isKindOfClass:[RenderWidgetHostViewCocoa class]]); [rwhv setIgnoreKeyEvents:YES]; - [window redispatchEvent:event]; + BOOL eventHandled = [window redispatchEvent:event]; [rwhv setIgnoreKeyEvents:NO]; + return eventHandled; } - (void)mouseEvent:(NSEvent *)theEvent { diff --git a/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc b/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc index f466290..fce6c9c 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc +++ b/chrome/browser/views/tab_contents/tab_contents_view_gtk.cc @@ -307,19 +307,19 @@ void TabContentsViewGtk::TakeFocus(bool reverse) { reverse ? GTK_DIR_TAB_BACKWARD : GTK_DIR_TAB_FORWARD); } -void TabContentsViewGtk::HandleKeyboardEvent( +bool TabContentsViewGtk::HandleKeyboardEvent( const NativeWebKeyboardEvent& event) { // The renderer returned a keyboard event it did not process. This may be // a keyboard shortcut that we have to process. if (event.type != WebInputEvent::RawKeyDown) - return; + return false; views::FocusManager* focus_manager = views::FocusManager::GetFocusManagerForNativeView(GetNativeView()); // We may not have a focus_manager at this point (if the tab has been switched // by the time this message returned). if (!focus_manager) - return; + return false; bool shift_pressed = (event.modifiers & WebInputEvent::ShiftKey) == WebInputEvent::ShiftKey; @@ -328,7 +328,7 @@ void TabContentsViewGtk::HandleKeyboardEvent( bool alt_pressed = (event.modifiers & WebInputEvent::AltKey) == WebInputEvent::AltKey; - focus_manager->ProcessAccelerator( + return focus_manager->ProcessAccelerator( views::Accelerator(static_cast<base::KeyboardCode>(event.windowsKeyCode), shift_pressed, ctrl_pressed, alt_pressed)); // DANGER: |this| could be deleted now! diff --git a/chrome/browser/views/tab_contents/tab_contents_view_gtk.h b/chrome/browser/views/tab_contents/tab_contents_view_gtk.h index 7177737..f9377a2 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_gtk.h +++ b/chrome/browser/views/tab_contents/tab_contents_view_gtk.h @@ -63,7 +63,7 @@ class TabContentsViewGtk : public TabContentsView, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); private: // Signal handlers ----------------------------------------------------------- diff --git a/chrome/browser/views/tab_contents/tab_contents_view_win.cc b/chrome/browser/views/tab_contents/tab_contents_view_win.cc index e1aa377..65937e0 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_win.cc +++ b/chrome/browser/views/tab_contents/tab_contents_view_win.cc @@ -353,14 +353,14 @@ void TabContentsViewWin::TakeFocus(bool reverse) { } } -void TabContentsViewWin::HandleKeyboardEvent( +bool TabContentsViewWin::HandleKeyboardEvent( const NativeWebKeyboardEvent& event) { // Previous calls to TranslateMessage can generate CHAR events as well as // RAW_KEY_DOWN events, even if the latter triggered an accelerator. In these // cases, we discard the CHAR events. if (event.type == WebInputEvent::Char && ignore_next_char_event_) { ignore_next_char_event_ = false; - return; + return true; } ignore_next_char_event_ = false; @@ -388,7 +388,7 @@ void TabContentsViewWin::HandleKeyboardEvent( ignore_next_char_event_ = true; if (focus_manager->ProcessAccelerator(accelerator)) { // DANGER: |this| could be deleted now! - return; + return true; } else { // ProcessAccelerator didn't handle the accelerator, so we know both // that |this| is still valid, and that we didn't want to set the flag. @@ -399,15 +399,20 @@ void TabContentsViewWin::HandleKeyboardEvent( if (tab_contents()->delegate() && tab_contents()->delegate()->HandleKeyboardEvent(event)) { - return; + return true; } // Any unhandled keyboard/character messages should be defproced. // This allows stuff like Alt+F4, etc to work correctly. - DefWindowProc(event.os_event.hwnd, - event.os_event.message, - event.os_event.wParam, - event.os_event.lParam); + DefWindowProc(event.os_event.hwnd, event.os_event.message, + event.os_event.wParam, event.os_event.lParam); + + // DefWindowProc() always returns 0, which means it handled the event. + // But actually DefWindowProc() will only handle very few system key strokes, + // such as F10, Alt+Tab, Alt+F4, Alt+Esc, etc. + // So returning false here is just ok for most cases. + // Reference: http://msdn.microsoft.com/en-us/library/ms646267(VS.85).aspx + return false; } views::FocusManager* TabContentsViewWin::GetFocusManager() { diff --git a/chrome/browser/views/tab_contents/tab_contents_view_win.h b/chrome/browser/views/tab_contents/tab_contents_view_win.h index b7ed158..e728449 100644 --- a/chrome/browser/views/tab_contents/tab_contents_view_win.h +++ b/chrome/browser/views/tab_contents/tab_contents_view_win.h @@ -58,7 +58,7 @@ class TabContentsViewWin : public TabContentsView, virtual void UpdateDragCursor(WebKit::WebDragOperation operation); virtual void GotFocus(); virtual void TakeFocus(bool reverse); - virtual void HandleKeyboardEvent(const NativeWebKeyboardEvent& event); + virtual bool HandleKeyboardEvent(const NativeWebKeyboardEvent& event); // WidgetWin overridde. virtual views::FocusManager* GetFocusManager(); |