GTK Keyboard Support for New Autofill

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

 // Copyright (c) 2012 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 "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 "ui/base/gtk/gtk_compat.h"
 #include "ui/base/gtk/gtk_hig_constants.h"
 #include "ui/base/gtk/gtk_windowing.h"
 #include "ui/gfx/native_widget_types.h"
 #include "ui/gfx/rect.h"
+#include "content/public/browser/web_contents.h"

[Ilya Sherman, 2012/03/15 20:10:56]

nit: Alpha-order

[csharp, 2012/03/21 15:32:28]

Done.

 
 namespace {
 const GdkColor kBorderColor = GDK_COLOR_RGB(0xc7, 0xca, 0xce);
 const GdkColor kHoveredBackgroundColor = GDK_COLOR_RGB(0x0CD, 0xCD, 0xCD);
 const GdkColor kTextColor = GDK_COLOR_RGB(0x00, 0x00, 0x00);
 
 // The amount of minimum padding between the Autofill value and label in pixels.
 const int kMiddlePadding = 10;
 
 // We have a 1 pixel border around the entire results popup.
@@ skipping 10 matching lines @@
 }
 
 }  // namespace
 
 AutofillPopupViewGtk::AutofillPopupViewGtk(
     content::WebContents* web_contents,
     AutofillExternalDelegate* external_delegate,
     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);
   gtk_widget_set_double_buffered(window_, TRUE);
 
   // Setup the window to ensure it receives the expose event.
   gtk_widget_add_events(window_, GDK_BUTTON_MOTION_MASK |
                                  GDK_BUTTON_RELEASE_MASK |
                                  GDK_EXPOSURE_MASK |
                                  GDK_POINTER_MOTION_MASK);
   g_signal_connect(window_, "expose-event",
                    G_CALLBACK(HandleExposeThunk), this);
 
   g_signal_connect(window_, "motion-notify-event",
                    G_CALLBACK(HandleMotionThunk), this);
   g_signal_connect(window_, "button-release-event",
                    G_CALLBACK(HandleButtonReleaseThunk), this);
 
   // Cache the layout so we don't have to create it for every expose.
   layout_ = gtk_widget_create_pango_layout(window_, NULL);
 
   row_height_ = font_.GetHeight();
 }
 
 AutofillPopupViewGtk::~AutofillPopupViewGtk() {
   g_object_unref(layout_);
   gtk_widget_destroy(window_);
 }
 
-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]));
+bool AutofillPopupViewGtk::HandleKeyPressEvent(GdkEventKey* event) {
+  switch (event->keyval) {
+    case GDK_Up: {
+      DecreaseSelectedLine();
+      return true;
+    }

[Ilya Sherman, 2012/03/15 20:10:56]

nit: No need for curly braces for these case stmts, since you are not declaring
any local variables.

[csharp, 2012/03/21 15:32:28]

Done.

+    case GDK_Down: {
+      IncreaseSelectedLine();
+      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: {
+      RemoveLine(selected_line());

[Ilya Sherman, 2012/03/15 20:10:56]

We should only remove the line if the user types shift-delete.  As with Enter
below, the Autofill popup should only consume the event if there is a line
selected.

[csharp, 2012/03/21 15:32:28]

Ok, in my playing around I see that Autofill profiles need shift-delete to be
removed, but regular Autocomplete entries just need a delete. Is this the
behaviour we want or should both actions require a shift-delete?

[Ilya Sherman, 2012/03/21 23:35:30]

I wasn't aware that Autocomplete entries behaved differently.  I'm pretty sure
we want shift-delete to consistently be the way to remove items from our popups
(e.g. including the Omnibox popup).

+      return true;
+    }
+    case GDK_Return:
+    case GDK_KP_Enter: {
+      ChooseLine(selected_line());
+      return true;

[Ilya Sherman, 2012/03/15 20:10:56]

This should only consume the event if the popup has a line selected.  Otherwise,
this should bubble normally, which will typically result in the form getting
submitted.

[csharp, 2012/03/21 15:32:28]

Done.

+    }
   }
 
+  return false;
+}
+
+void AutofillPopupViewGtk::ShowInternal() {
   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(),
       origin_y + element_bounds().y() + element_bounds().height(),
       popup_width,
       row_height_ * autofill_values().size());
 
   gtk_window_move(GTK_WINDOW(window_), bounds_.x(), bounds_.y());
 
   gtk_widget_set_size_request(
       window_,
       popup_width,
       row_height_ * autofill_values().size());
 
+  render_view_host_->AddKeyboardListener(this);
+
   gtk_widget_show(window_);
 
   GtkWidget* toplevel = gtk_widget_get_toplevel(parent_);
   CHECK(gtk_widget_is_toplevel(toplevel));
   ui::StackPopupWindow(window_, toplevel);
 }
 
 void AutofillPopupViewGtk::HideInternal() {
+  render_view_host_->RemoveKeyboardListener(this);
+
   gtk_widget_hide(window_);
 }
 
 void AutofillPopupViewGtk::InvalidateRow(size_t row) {
   GdkRectangle row_rect = GetRectForRow(
       row, bounds_.width(), row_height_).ToGdkRectangle();
   GdkWindow* gdk_window = gtk_widget_get_window(window_);
   gdk_window_invalidate_rect(gdk_window, &row_rect, FALSE);
 }
 
+void AutofillPopupViewGtk::UpdatePopupSize() {
+  gtk_widget_set_size_request(
+      window_,
+      GetPopupRequiredWidth(),
+      row_height_ * autofill_values().size());
+}
+
 gboolean AutofillPopupViewGtk::HandleButtonRelease(GtkWidget* widget,
                                                    GdkEventButton* event) {
   // We only care about the left click.
   if (event->button != 1)
     return FALSE;
 
   size_t line = LineFromY(event->y);
-  DCHECK_LT(line, autofill_values().size());
-
-  external_delegate()->DidAcceptAutofillSuggestions(
-      autofill_values()[line],
-      autofill_unique_ids()[line],
-      line);
+  ChooseLine(line);
 
   return TRUE;
 }
 
 gboolean AutofillPopupViewGtk::HandleExpose(GtkWidget* widget,
                                             GdkEventExpose* event) {
   gfx::Rect window_rect = GetWindowRect(event->window);
   gfx::Rect damage_rect = gfx::Rect(event->area);
 
   cairo_t* cr = gdk_cairo_create(GDK_DRAWABLE(gtk_widget_get_window(widget)));
@@ skipping 88 matching lines @@
   PangoAttribute* fg_attr = pango_attr_foreground_new(text_color.red,
                                                       text_color.green,
                                                       text_color.blue);
   pango_attr_list_insert(attrs, fg_attr);  // Ownership taken.
 
 
   pango_layout_set_attributes(layout_, attrs);  // Ref taken.
   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_;
 }