Paint a focus indication on LinkButtonGtk buttons.

Also, rearrange helper code for centering things in hboxes so they don't expand to take up the full vertical space.

Also, don't allow focus on gtk_chrome_button buttons. This is to prevent the "Other bookmarks" button from getting focus (which matches windows) and is in line with disallowing focus on all custom chrome buttons (see CustomDrawButton).

BUG=12829
TEST=everything should look the same. Also if you tab to focus on a link button (e.g. download shelf's "show all downloads"), it should have a dotted line around the exterior.

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

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

9 files changed, 36 insertions, 48 deletions

diff --git a/chrome/browser/gtk/custom_button.cc b/chrome/browser/gtk/custom_button.cc
index e438016..00bbfda 100644
--- a/chrome/browser/gtk/custom_button.cc
+++ b/chrome/browser/gtk/custom_button.cc
@@ -93,13 +93,7 @@ gboolean CustomDrawButton::OnExpose(GtkWidget* widget,
 }
 
 // static
-CustomDrawButton* CustomDrawButton::AddBarCloseButton(GtkWidget* hbox,
-                                                      int padding) {
-  CustomDrawButton* rv = new CustomDrawButton(IDR_CLOSE_BAR, IDR_CLOSE_BAR_P,
-                                              IDR_CLOSE_BAR_H, 0);
-  GTK_WIDGET_UNSET_FLAGS(rv->widget(), GTK_CAN_FOCUS);
-  GtkWidget* centering_vbox = gtk_vbox_new(FALSE, 0);
-  gtk_box_pack_start(GTK_BOX(centering_vbox), rv->widget(), TRUE, FALSE, 0);
-  gtk_box_pack_end(GTK_BOX(hbox), centering_vbox, FALSE, FALSE, padding);
-  return rv;
+CustomDrawButton* CustomDrawButton::CloseButton() {
+  return new CustomDrawButton(IDR_CLOSE_BAR, IDR_CLOSE_BAR_P,
+                              IDR_CLOSE_BAR_H, 0);
 }
diff --git a/chrome/browser/gtk/custom_button.h b/chrome/browser/gtk/custom_button.h
index 92f9d72..a3071df 100644
--- a/chrome/browser/gtk/custom_button.h
+++ b/chrome/browser/gtk/custom_button.h
@@ -77,11 +77,8 @@ class CustomDrawButton {
   // Resume normal drawing of the widget's state.
   void UnsetPaintOverride();
 
-  // This is a convenience function for creating a widget that closes
-  // a bar (find bar, download shelf, info bars). The button will be packed in
-  // |hbox|.
-  // The caller is responsible for destroying the returned CustomDrawButton.
-  static CustomDrawButton* AddBarCloseButton(GtkWidget* hbox, int padding);
+  // Returns a standard close button.
+  static CustomDrawButton* CloseButton();
 
  private:
   // Callback for expose, used to draw the custom graphics.
diff --git a/chrome/browser/gtk/download_item_gtk.cc b/chrome/browser/gtk/download_item_gtk.cc
index 6b1ff4e..5f53f5a 100644
--- a/chrome/browser/gtk/download_item_gtk.cc
+++ b/chrome/browser/gtk/download_item_gtk.cc
@@ -310,22 +310,14 @@ DownloadItemGtk::DownloadItemGtk(DownloadShelfGtk* parent_shelf,
         l10n_util::GetStringUTF8(IDS_SAVE_DOWNLOAD).c_str());
     g_signal_connect(dangerous_accept, "clicked",
                      G_CALLBACK(OnDangerousAccept), this);
-    GtkWidget* dangerous_accept_vbox = gtk_vbox_new(FALSE, 0);
-    gtk_box_pack_start(GTK_BOX(dangerous_accept_vbox), dangerous_accept,
-                       TRUE, FALSE, 0);
-    gtk_box_pack_start(GTK_BOX(dangerous_hbox_), dangerous_accept_vbox,
-                       FALSE, FALSE, 0);
+    gtk_util::CenterWidgetInHBox(dangerous_hbox_, dangerous_accept, false, 0);
 
     // Create the nevermind button.
     GtkWidget* dangerous_decline = gtk_button_new_with_label(
         l10n_util::GetStringUTF8(IDS_DISCARD_DOWNLOAD).c_str());
     g_signal_connect(dangerous_decline, "clicked",
                      G_CALLBACK(OnDangerousDecline), this);
-    GtkWidget* dangerous_decline_vbox = gtk_vbox_new(FALSE, 0);
-    gtk_box_pack_start(GTK_BOX(dangerous_decline_vbox), dangerous_decline,
-                       TRUE, FALSE, 0);
-    gtk_box_pack_start(GTK_BOX(dangerous_hbox_), dangerous_decline_vbox,
-                       FALSE, FALSE, 0);
+    gtk_util::CenterWidgetInHBox(dangerous_hbox_, dangerous_decline, false, 0);
 
     // Put it in an alignment so that padding will be added on the left and
     // right, and an event box so that drawing will be clipped correctly.
diff --git a/chrome/browser/gtk/download_shelf_gtk.cc b/chrome/browser/gtk/download_shelf_gtk.cc
index 999ab27..97a142a 100644
--- a/chrome/browser/gtk/download_shelf_gtk.cc
+++ b/chrome/browser/gtk/download_shelf_gtk.cc
@@ -87,7 +87,8 @@ DownloadShelfGtk::DownloadShelfGtk(TabContents* tab_contents)
   gtk_box_pack_start(GTK_BOX(shelf_.get()), padding_bg, FALSE, FALSE, 0);
 
   // Create and pack the close button.
-  close_button_.reset(CustomDrawButton::AddBarCloseButton(hbox_, 0));
+  close_button_.reset(CustomDrawButton::CloseButton());
+  gtk_util::CenterWidgetInHBox(hbox_, close_button_->widget(), true, 0);
   g_signal_connect(close_button_->widget(), "clicked",
                    G_CALLBACK(OnButtonClick), this);
 
@@ -108,9 +109,8 @@ DownloadShelfGtk::DownloadShelfGtk(TabContents* tab_contents)
 
   // Pack the link and the icon in an hbox.
   link_hbox_ = gtk_hbox_new(FALSE, 5);
-  gtk_box_pack_start(GTK_BOX(link_hbox_), download_image, FALSE, FALSE, 0);
-  gtk_box_pack_start(GTK_BOX(link_hbox_), link_button_->widget(),
-                     FALSE, FALSE, 0);
+  gtk_util::CenterWidgetInHBox(link_hbox_, download_image, false, 0);
+  gtk_util::CenterWidgetInHBox(link_hbox_, link_button_->widget(), false, 0);
   gtk_box_pack_end(GTK_BOX(hbox_), link_hbox_, FALSE, FALSE, 0);
 
   slide_widget_.reset(new SlideAnimatorGtk(shelf_.get(),
diff --git a/chrome/browser/gtk/find_bar_gtk.cc b/chrome/browser/gtk/find_bar_gtk.cc
index 7270b2a..85b251d 100644
--- a/chrome/browser/gtk/find_bar_gtk.cc
+++ b/chrome/browser/gtk/find_bar_gtk.cc
@@ -129,8 +129,9 @@ void FindBarGtk::InitWidgets() {
   gtk_fixed_put(GTK_FIXED(widget()), slide_widget(), 0, 0);
   gtk_widget_set_size_request(widget(), -1, 0);
 
-  close_button_.reset(
-      CustomDrawButton::AddBarCloseButton(hbox, kCloseButtonPaddingLeft));
+  close_button_.reset(CustomDrawButton::CloseButton());
+  gtk_util::CenterWidgetInHBox(hbox, close_button_->widget(), false,
+                               kCloseButtonPaddingLeft);
   g_signal_connect(G_OBJECT(close_button_->widget()), "clicked",
                    G_CALLBACK(OnClicked), this);
   gtk_widget_set_tooltip_text(close_button_->widget(),
@@ -181,9 +182,7 @@ void FindBarGtk::InitWidgets() {
   GtkWidget* border_bin_aa = gtk_util::CreateGtkBorderBin(border_bin,
                                                           &kTextBorderColorAA,
                                                           1, 1, 1, 0);
-  GtkWidget* centering_vbox = gtk_vbox_new(FALSE, 0);
-  gtk_box_pack_start(GTK_BOX(centering_vbox), border_bin_aa, TRUE, FALSE, 0);
-  gtk_box_pack_end(GTK_BOX(hbox), centering_vbox, FALSE, FALSE, 0);
+  gtk_util::CenterWidgetInHBox(hbox, border_bin_aa, true, 0);
 
   // We take care to avoid showing the slide animator widget.
   gtk_widget_show_all(container_);
diff --git a/chrome/browser/gtk/gtk_chrome_button.cc b/chrome/browser/gtk/gtk_chrome_button.cc
index 9d344b9..8c076c1 100644
--- a/chrome/browser/gtk/gtk_chrome_button.cc
+++ b/chrome/browser/gtk/gtk_chrome_button.cc
@@ -69,6 +69,8 @@ static void gtk_chrome_button_init(GtkChromeButton* button) {
   priv->paint_state = -1;
 
   gtk_widget_set_app_paintable(GTK_WIDGET(button), TRUE);
+
+  GTK_WIDGET_UNSET_FLAGS(button, GTK_CAN_FOCUS);
 }
 
 static gboolean gtk_chrome_button_expose(GtkWidget* widget,
diff --git a/chrome/browser/gtk/infobar_gtk.cc b/chrome/browser/gtk/infobar_gtk.cc
index 10d6838..6ec2686 100644
--- a/chrome/browser/gtk/infobar_gtk.cc
+++ b/chrome/browser/gtk/infobar_gtk.cc
@@ -63,7 +63,8 @@ InfoBar::InfoBar(InfoBarDelegate* delegate)
     gtk_box_pack_start(GTK_BOX(hbox_), image, FALSE, FALSE, 0);
   }
 
-  close_button_.reset(CustomDrawButton::AddBarCloseButton(hbox_, 0));
+  close_button_.reset(CustomDrawButton::CloseButton());
+  gtk_util::CenterWidgetInHBox(hbox_, close_button_->widget(), true, 0);
   g_signal_connect(close_button_->widget(), "clicked",
                    G_CALLBACK(OnCloseButton), this);
 
@@ -171,8 +172,7 @@ class LinkInfoBar : public InfoBar {
       // this hbox that doesn't use kElementPadding.
       GtkWidget* hbox = gtk_hbox_new(FALSE, 0);
       gtk_box_pack_start(GTK_BOX(hbox), initial_label, FALSE, FALSE, 0);
-      gtk_box_pack_start(GTK_BOX(hbox), link_button_->widget(),
-                         FALSE, FALSE, 0);
+      gtk_util::CenterWidgetInHBox(hbox, link_button_->widget(), false, 0);
       gtk_box_pack_start(GTK_BOX(hbox), trailing_label, FALSE, FALSE, 0);
       gtk_box_pack_start(GTK_BOX(hbox_), hbox, FALSE, FALSE, 0);
     }
@@ -212,9 +212,7 @@ class ConfirmInfoBar : public AlertInfoBar {
     if (delegate_->AsConfirmInfoBarDelegate()->GetButtons() & type) {
       GtkWidget* button = gtk_button_new_with_label(WideToUTF8(
           delegate_->AsConfirmInfoBarDelegate()->GetButtonLabel(type)).c_str());
-      GtkWidget* centering_vbox = gtk_vbox_new(FALSE, 0);
-      gtk_box_pack_end(GTK_BOX(centering_vbox), button, TRUE, FALSE, 0);
-      gtk_box_pack_end(GTK_BOX(hbox_), centering_vbox, FALSE, FALSE, 0);
+      gtk_util::CenterWidgetInHBox(hbox_, button, true, 0);
       g_signal_connect(button, "clicked",
                        G_CALLBACK(type == ConfirmInfoBarDelegate::BUTTON_OK ?
                                   OnOkButton : OnCancelButton),
diff --git a/chrome/browser/gtk/link_button_gtk.cc b/chrome/browser/gtk/link_button_gtk.cc
index 73eb309..66c85bd 100644
--- a/chrome/browser/gtk/link_button_gtk.cc
+++ b/chrome/browser/gtk/link_button_gtk.cc
@@ -79,26 +79,33 @@ gboolean LinkButtonGtk::OnLeave(GtkWidget* widget,
   return FALSE;
 }
 
-// TODO(estade): we need some visual indication when this widget is focused.
 // static
 gboolean LinkButtonGtk::OnExpose(GtkWidget* widget,
                                  GdkEventExpose* event,
                                  LinkButtonGtk* link_button) {
+  GtkWidget* label = link_button->label_;
+
   if (GTK_WIDGET_STATE(widget) == GTK_STATE_ACTIVE && link_button->is_blue_) {
-    gtk_label_set_markup(GTK_LABEL(link_button->label_),
-                         link_button->red_markup);
+    gtk_label_set_markup(GTK_LABEL(label), link_button->red_markup);
     link_button->is_blue_ = false;
   } else if (GTK_WIDGET_STATE(widget) != GTK_STATE_ACTIVE &&
              !link_button->is_blue_) {
-    gtk_label_set_markup(GTK_LABEL(link_button->label_),
-                         link_button->blue_markup);
+    gtk_label_set_markup(GTK_LABEL(label), link_button->blue_markup);
     link_button->is_blue_ = true;
   }
 
   // Draw the link inside the button.
-  gtk_container_propagate_expose(GTK_CONTAINER(widget),
-                                 gtk_bin_get_child(GTK_BIN(widget)),
-                                 event);
+  gtk_container_propagate_expose(GTK_CONTAINER(widget), label, event);
+
+  // Draw the focus rectangle.
+  if (GTK_WIDGET_HAS_FOCUS(widget)) {
+    gtk_paint_focus(widget->style, widget->window,
+                    static_cast<GtkStateType>(GTK_WIDGET_STATE(widget)),
+                    &event->area, widget, NULL,
+                    widget->allocation.x, widget->allocation.y,
+                    widget->allocation.width, widget->allocation.height);
+  }
+
   // Don't let the button draw itself, ever.
   return TRUE;
 }
diff --git a/chrome/browser/gtk/link_button_gtk.h b/chrome/browser/gtk/link_button_gtk.h
index 3a37fed..dc4b809 100644
--- a/chrome/browser/gtk/link_button_gtk.h
+++ b/chrome/browser/gtk/link_button_gtk.h
@@ -9,7 +9,6 @@
 
 // Creates a link button that shows |text| in blue and underlined. The cursor
 // changes to a hand when over the link.
-// TODO(estade): the link should turn red during the user's click.
 class LinkButtonGtk {
  public:
   explicit LinkButtonGtk(const char* text);