Fix ctrl+tab/ctrl+shift+tab were not working if web contents did not have focus (e.g., location bar or find bar had focus). Refactor the special keypress handling so these accelerators work when gtk widgets have focus.

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


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

2 files changed, 43 insertions, 25 deletions

diff --git a/chrome/browser/gtk/browser_window_gtk.cc b/chrome/browser/gtk/browser_window_gtk.cc
index 70ed15d..f5655f2 100644
--- a/chrome/browser/gtk/browser_window_gtk.cc
+++ b/chrome/browser/gtk/browser_window_gtk.cc
@@ -123,6 +123,38 @@ int GetCommandFromKeyval(guint accel_key) {
   return 0;
 }
 
+// An event handler for key press events.  We need to special case key
+// combinations that are not valid gtk accelerators.  This function returns
+// TRUE if it can handle the key press.
+gboolean HandleCustomAccelerator(guint keyval, GdkModifierType modifier,
+                                 Browser* browser) {
+  // Filter modifier to only include accelerator modifiers.
+  modifier = static_cast<GdkModifierType>(
+      modifier & gtk_accelerator_get_default_mod_mask());
+  switch (keyval) {
+    // Gtk doesn't allow GDK_Tab or GDK_ISO_Left_Tab to be an accelerator (see
+    // gtk_accelerator_valid), so we need to handle these accelerators
+    // manually.
+    case GDK_Tab:
+      if (GDK_CONTROL_MASK == modifier) {
+        browser->ExecuteCommand(IDC_SELECT_NEXT_TAB);
+        return TRUE;
+      }
+      break;
+
+    case GDK_ISO_Left_Tab:
+      if ((GDK_CONTROL_MASK | GDK_SHIFT_MASK) == modifier) {
+        browser->ExecuteCommand(IDC_SELECT_PREVIOUS_TAB);
+        return TRUE;
+      }
+      break;
+
+    default:
+      break;
+  }
+  return FALSE;
+}
+
 // Usually accelerators are checked before propagating the key event, but if the
 // focus is on the render area we want to reverse the order of things to allow
 // webkit to handle key events like ctrl-l.
@@ -131,10 +163,11 @@ gboolean OnKeyPress(GtkWindow* window, GdkEventKey* event, Browser* browser) {
       browser->tabstrip_model()->GetSelectedTabContents();
   // If there is no current tab contents or it is not focused then let the
   // default GtkWindow key handler run.
-  if (!current_tab_contents)
-    return FALSE;
-  if (!gtk_widget_is_focus(current_tab_contents->GetContentNativeView()))
-    return FALSE;
+  if (!current_tab_contents ||
+      !gtk_widget_is_focus(current_tab_contents->GetContentNativeView())) {
+    return HandleCustomAccelerator(event->keyval,
+        static_cast<GdkModifierType>(event->state), browser);
+  }
 
   // If the content area is focused, let it handle the key event.
   gboolean result = gtk_window_propagate_key_event(window, event);
@@ -247,26 +280,9 @@ BrowserWindowGtk::~BrowserWindowGtk() {
 
 void BrowserWindowGtk::HandleAccelerator(guint keyval,
                                          GdkModifierType modifier) {
-  // Filter modifier to only include accelerator modifiers.
-  modifier = static_cast<GdkModifierType>(
-      modifier & gtk_accelerator_get_default_mod_mask());
-  switch (keyval) {
-    // Gtk doesn't allow GDK_Tab or GDK_ISO_Left_Tab to be an accelerator (see
-    // gtk_accelerator_valid), so we need to handle these accelerators
-    // manually.
-    case GDK_Tab:
-      if (GDK_CONTROL_MASK == modifier)
-        ExecuteBrowserCommand(IDC_SELECT_NEXT_TAB);
-      break;
-
-    case GDK_ISO_Left_Tab:
-      if ((GDK_CONTROL_MASK | GDK_SHIFT_MASK) == modifier)
-        ExecuteBrowserCommand(IDC_SELECT_PREVIOUS_TAB);
-      break;
-
-    default:
-      // Pass the accelerator on to GTK.
-      gtk_accel_groups_activate(G_OBJECT(window_), keyval, modifier);
+  if (!HandleCustomAccelerator(keyval, modifier, browser_.get())) {
+    // Pass the accelerator on to GTK.
+    gtk_accel_groups_activate(G_OBJECT(window_), keyval, modifier);
   }
 }
 
diff --git a/chrome/browser/gtk/find_bar_gtk.cc b/chrome/browser/gtk/find_bar_gtk.cc
index 67d9d07..b35a8fd 100644
--- a/chrome/browser/gtk/find_bar_gtk.cc
+++ b/chrome/browser/gtk/find_bar_gtk.cc
@@ -31,8 +31,10 @@ gboolean EntryContentsChanged(GtkWindow* window, FindBarGtk* find_bar) {
 
 gboolean KeyPressEvent(GtkWindow* window, GdkEventKey* event,
                        FindBarGtk* find_bar) {
-  if (GDK_Escape == event->keyval)
+  if (GDK_Escape == event->keyval) {
     find_bar->EscapePressed();
+    return TRUE;
+  }
   return FALSE;
 }