GTK Keyboard Support for New Autofill

chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.cc

Index: chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.cc
diff --git a/chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.cc b/chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.cc
index bebb0b835784ac37c4eedcfc40e509fcdfd73318..209ce0fcaaed35929e18bba24c6fec5f457b779c 100644
--- a/chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.cc
+++ b/chrome/browser/ui/gtk/autofill/autofill_popup_view_gtk.cc
@@ -4,10 +4,14 @@
 
 #include "autofill_popup_view_gtk.h"
 
+#include <gdk/gdkkeysyms.h>
+
 #include "base/logging.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/autofill/autofill_external_delegate.h"
 #include "chrome/browser/ui/gtk/gtk_util.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
 #include "ui/base/gtk/gtk_compat.h"
 #include "ui/base/gtk/gtk_hig_constants.h"
 #include "ui/base/gtk/gtk_windowing.h"
@@ -43,7 +47,8 @@ AutofillPopupViewGtk::AutofillPopupViewGtk(
     GtkWidget* parent)
     : AutofillPopupView(web_contents, external_delegate),
       parent_(parent),
-      window_(gtk_window_new(GTK_WINDOW_POPUP)) {
+      window_(gtk_window_new(GTK_WINDOW_POPUP)),
+      render_view_host_(web_contents->GetRenderViewHost()) {
   CHECK(parent != NULL);
   gtk_window_set_resizable(GTK_WINDOW(window_), FALSE);
   gtk_widget_set_app_paintable(window_, TRUE);
@@ -74,21 +79,11 @@ AutofillPopupViewGtk::~AutofillPopupViewGtk() {
 }
 
 void AutofillPopupViewGtk::ShowInternal() {
-  // Find out the maximum bounds required by the popup.
-  // TODO(csharp): Once the icon is also displayed it will affect the required
-  // size so it will need to be included in the calculation.
-  int popup_width = element_bounds().width();
-  DCHECK_EQ(autofill_values().size(), autofill_labels().size());
-  for (size_t i = 0; i < autofill_values().size(); ++i) {
-    popup_width = std::max(popup_width,
-                           font_.GetStringWidth(autofill_values()[i]) +
-                           kMiddlePadding +
-                           font_.GetStringWidth(autofill_labels()[i]));
-  }
-
   gint origin_x, origin_y;
   gdk_window_get_origin(gtk_widget_get_window(parent_), &origin_x, &origin_y);
 
+  int popup_width = GetPopupRequiredWidth();
+
   // Move the popup to appear right below the text field it is using.
   bounds_.SetRect(
       origin_x + element_bounds().x(),
@@ -103,6 +98,8 @@ void AutofillPopupViewGtk::ShowInternal() {
       popup_width,
       row_height_ * autofill_values().size());
 
+  render_view_host_->AddKeyboardListener(this);
+
   gtk_widget_show(window_);
 
   GtkWidget* toplevel = gtk_widget_get_toplevel(parent_);
@@ -111,6 +108,8 @@ void AutofillPopupViewGtk::ShowInternal() {
 }
 
 void AutofillPopupViewGtk::HideInternal() {
+  render_view_host_->RemoveKeyboardListener(this);
+
   gtk_widget_hide(window_);
 }
 
@@ -127,13 +126,9 @@ gboolean AutofillPopupViewGtk::HandleButtonRelease(GtkWidget* widget,
   if (event->button != 1)
     return FALSE;
 
-  size_t line = LineFromY(event->y);
-  DCHECK_LT(line, autofill_values().size());
+  DCHECK_EQ(selected_line(), LineFromY(event->y));
 
-  external_delegate()->DidAcceptAutofillSuggestions(
-      autofill_values()[line],
-      autofill_unique_ids()[line],
-      line);
+  AcceptSelectedLine();
 
   return TRUE;
 }
@@ -224,6 +219,35 @@ gboolean AutofillPopupViewGtk::HandleMotion(GtkWidget* widget,
   return TRUE;
 }
 
+bool AutofillPopupViewGtk::HandleKeyPressEvent(GdkEventKey* event) {
+  switch (event->keyval) {
+    case GDK_Up:
+      SelectPreviousLine();
+      return true;
+    case GDK_Down:
+      SelectNextLine();
+      return true;
+    case GDK_Page_Up:
+      SetSelectedLine(0);
+      return true;
+    case GDK_Page_Down:
+      SetSelectedLine(autofill_values().size() - 1);
+      return true;
+    case GDK_Escape:
+      Hide();
+      return true;
+    case GDK_Delete:
+    case GDK_KP_Delete:
+      if (event->state == GDK_SHIFT_MASK)
+        return RemoveSelectedLine();

[Ilya Sherman, 2012/03/27 17:52:40]

This should be "return (event->state == GDK_SHIFT_MASK) &&
RemoveSelectedLine();" -- otherwise you incorrectly fall through to the cases
below...

[csharp, 2012/03/27 18:06:20]

D'oh. Done.

+    case GDK_Return:
+    case GDK_KP_Enter:
+      return AcceptSelectedLine();
+  }
+
+  return false;
+}
+
 void AutofillPopupViewGtk::SetupLayout(const gfx::Rect& window_rect,
                                        const GdkColor& text_color) {
   int allocated_content_width = window_rect.width();
@@ -242,6 +266,21 @@ void AutofillPopupViewGtk::SetupLayout(const gfx::Rect& window_rect,
   pango_attr_list_unref(attrs);
 }
 
+int AutofillPopupViewGtk::GetPopupRequiredWidth() {
+  // TODO(csharp): Once the icon is also displayed it will affect the required
+  // size so it will need to be included in the calculation.
+  int popup_width = element_bounds().width();
+  DCHECK_EQ(autofill_values().size(), autofill_labels().size());
+  for (size_t i = 0; i < autofill_values().size(); ++i) {
+    popup_width = std::max(popup_width,
+                           font_.GetStringWidth(autofill_values()[i]) +
+                           kMiddlePadding +
+                           font_.GetStringWidth(autofill_labels()[i]));
+  }
+
+  return popup_width;
+}
+
 int AutofillPopupViewGtk::LineFromY(int y) {
   return y / row_height_;
 }