First run bubble polish:

- improve spacing and and sizing, both internally and externally (using windows as the guide)
- make legible for dark themes
- make show delay longer because it was too short on my machine (at least when using a Debug build).

Also explicitly set bookmark bubble labels black.

BUG=16782
TEST=none

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

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

9 files changed, 50 insertions, 16 deletions

diff --git a/chrome/browser/gtk/bookmark_bubble_gtk.cc b/chrome/browser/gtk/bookmark_bubble_gtk.cc
index 30e93d6..c08cdf9 100644
--- a/chrome/browser/gtk/bookmark_bubble_gtk.cc
+++ b/chrome/browser/gtk/bookmark_bubble_gtk.cc
@@ -9,6 +9,7 @@
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #include "base/basictypes.h"
+#include "base/gfx/gtk_util.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "chrome/browser/bookmarks/bookmark_editor.h"
@@ -130,6 +131,7 @@ BookmarkBubbleGtk::BookmarkBubbleGtk(GtkWindow* transient_toplevel,
   GtkWidget* label = gtk_label_new(l10n_util::GetStringUTF8(
       newly_bookmarked_ ? IDS_BOOMARK_BUBBLE_PAGE_BOOKMARKED :
                           IDS_BOOMARK_BUBBLE_PAGE_BOOKMARK).c_str());
+  gtk_widget_modify_fg(label, GTK_STATE_NORMAL, &gfx::kGdkBlack);
   GtkWidget* remove_button = gtk_chrome_link_button_new(
       l10n_util::GetStringUTF8(IDS_BOOMARK_BUBBLE_REMOVE_BOOKMARK).c_str());
   GtkWidget* edit_button = gtk_button_new_with_label(
@@ -164,6 +166,7 @@ BookmarkBubbleGtk::BookmarkBubbleGtk(GtkWindow* transient_toplevel,
   // We use a table to allow the labels to line up with each other, along
   // with the entry and folder combo lining up.
   GtkWidget* table = gtk_util::CreateLabeledControlsGroup(
+      &gfx::kGdkBlack,
       l10n_util::GetStringUTF8(IDS_BOOMARK_BUBBLE_TITLE_TEXT).c_str(),
       name_entry_,
       l10n_util::GetStringUTF8(IDS_BOOMARK_BUBBLE_FOLDER_TEXT).c_str(),
diff --git a/chrome/browser/gtk/bookmark_editor_gtk.cc b/chrome/browser/gtk/bookmark_editor_gtk.cc
index 9824f44..b986f20 100644
--- a/chrome/browser/gtk/bookmark_editor_gtk.cc
+++ b/chrome/browser/gtk/bookmark_editor_gtk.cc
@@ -141,7 +141,7 @@ void BookmarkEditorGtk::Init(GtkWindow* parent_window) {
                    G_CALLBACK(OnEntryChanged), this);
   gtk_entry_set_activates_default(GTK_ENTRY(url_entry_), TRUE);
 
-  GtkWidget* table = gtk_util::CreateLabeledControlsGroup(
+  GtkWidget* table = gtk_util::CreateLabeledControlsGroup(NULL,
       l10n_util::GetStringUTF8(IDS_BOOMARK_EDITOR_NAME_LABEL).c_str(),
       name_entry_,
       l10n_util::GetStringUTF8(IDS_BOOMARK_EDITOR_URL_LABEL).c_str(),
diff --git a/chrome/browser/gtk/edit_search_engine_dialog.cc b/chrome/browser/gtk/edit_search_engine_dialog.cc
index 40f3cc7..72373d1 100644
--- a/chrome/browser/gtk/edit_search_engine_dialog.cc
+++ b/chrome/browser/gtk/edit_search_engine_dialog.cc
@@ -139,7 +139,7 @@ void EditSearchEngineDialog::Init(GtkWindow* parent_window) {
         controller_->template_url()->prepopulate_id() == 0);
   }
 
-  GtkWidget* controls = gtk_util::CreateLabeledControlsGroup(
+  GtkWidget* controls = gtk_util::CreateLabeledControlsGroup(NULL,
       l10n_util::GetStringUTF8(
           IDS_SEARCH_ENGINES_EDITOR_DESCRIPTION_LABEL).c_str(),
       CreateEntryImageHBox(title_entry_, title_image_),
diff --git a/chrome/browser/gtk/first_run_bubble.cc b/chrome/browser/gtk/first_run_bubble.cc
index 05287a8..88bbc3f9 100644
--- a/chrome/browser/gtk/first_run_bubble.cc
+++ b/chrome/browser/gtk/first_run_bubble.cc
@@ -7,9 +7,12 @@
 #include <gtk/gtk.h>
 
 #include "app/l10n_util.h"
+#include "base/gfx/gtk_util.h"
 #include "chrome/browser/options_window.h"
 #include "chrome/browser/search_engines/template_url_model.h"
 #include "chrome/common/gtk_util.h"
+#include "grit/chromium_strings.h"
+#include "grit/locale_settings.h"
 #include "grit/generated_resources.h"
 #include "grit/google_chrome_strings.h"
 
@@ -60,17 +63,20 @@ FirstRunBubble::FirstRunBubble(Profile* profile,
   gtk_label_set_markup(GTK_LABEL(label1), markup);
   g_free(markup);
   gtk_misc_set_alignment(GTK_MISC(label1), 0, .5);
+  gtk_widget_modify_fg(label1, GTK_STATE_NORMAL, &gfx::kGdkBlack);
 
   GtkWidget* label2 = gtk_label_new(
       l10n_util::GetStringUTF8(IDS_FR_BUBBLE_SUBTEXT).c_str());
   gtk_misc_set_alignment(GTK_MISC(label2), 0, .5);
   gtk_label_set_line_wrap(GTK_LABEL(label2), TRUE);
+  gtk_widget_modify_fg(label2, GTK_STATE_NORMAL, &gfx::kGdkBlack);
 
   string16 search_engine = GetDefaultSearchEngineName(profile_);
   GtkWidget* label3 = gtk_label_new(
       l10n_util::GetStringFUTF8(IDS_FR_BUBBLE_QUESTION, search_engine).c_str());
   gtk_misc_set_alignment(GTK_MISC(label3), 0, .5);
   gtk_label_set_line_wrap(GTK_LABEL(label3), TRUE);
+  gtk_widget_modify_fg(label3, GTK_STATE_NORMAL, &gfx::kGdkBlack);
 
   GtkWidget* keep_button = gtk_button_new_with_label(
       l10n_util::GetStringFUTF8(IDS_FR_BUBBLE_OK, search_engine).c_str());
@@ -78,19 +84,33 @@ FirstRunBubble::FirstRunBubble(Profile* profile,
       l10n_util::GetStringUTF8(IDS_FR_BUBBLE_CHANGE).c_str());
 
   content_ = gtk_vbox_new(FALSE, 5);
-  gtk_box_pack_start(GTK_BOX(content_), label1, TRUE, TRUE, 0);
-  gtk_box_pack_start(GTK_BOX(content_), label2, TRUE, TRUE, 0);
-  gtk_box_pack_start(GTK_BOX(content_), label3, TRUE, TRUE, 0);
+  int width, height;
+  gtk_util::GetWidgetSizeFromResources(content_,
+      IDS_FIRSTRUNBUBBLE_DIALOG_WIDTH_CHARS,
+      IDS_FIRSTRUNBUBBLE_DIALOG_HEIGHT_LINES,
+      &width, &height);
+  // TODO(estade): for now, don't set the height explicitly. The bubble is way
+  // too large for the text it contains.
+  gtk_widget_set_size_request(content_, width, -1);
+  gtk_widget_set_size_request(label1, width, -1);
+  gtk_widget_set_size_request(label2, width, -1);
+  gtk_widget_set_size_request(label3, width, -1);
+
+  gtk_box_pack_start(GTK_BOX(content_), label1, FALSE, FALSE, 0);
+  gtk_box_pack_start(GTK_BOX(content_), label2, FALSE, FALSE, 0);
+  // Leave an empty line.
+  gtk_box_pack_start(GTK_BOX(content_), gtk_label_new(NULL), FALSE, FALSE, 0);
+  gtk_box_pack_start(GTK_BOX(content_), label3, FALSE, FALSE, 0);
 
   GtkWidget* bottom = gtk_hbox_new(FALSE, 0);
   // We want the buttons on the right, so just use an expanding label to fill
-  // all of the extra space on the right.
-  gtk_box_pack_start(GTK_BOX(bottom), gtk_label_new(""), TRUE, TRUE, 0);
+  // all of the extra space on the left.
+  gtk_box_pack_start(GTK_BOX(bottom), gtk_label_new(NULL), TRUE, TRUE, 0);
   gtk_box_pack_start(GTK_BOX(bottom), keep_button, FALSE, FALSE,
                      kButtonPadding);
   gtk_box_pack_start(GTK_BOX(bottom), change_button, FALSE, FALSE, 0);
 
-  gtk_box_pack_start(GTK_BOX(content_), bottom, TRUE, TRUE, 0);
+  gtk_box_pack_start(GTK_BOX(content_), bottom, FALSE, FALSE, 0);
   // We want the focus to start on the keep entry, not on the change button.
   gtk_container_set_focus_child(GTK_CONTAINER(content_), keep_button);
 
diff --git a/chrome/browser/gtk/location_bar_view_gtk.cc b/chrome/browser/gtk/location_bar_view_gtk.cc
index 38c2901..211f997 100644
--- a/chrome/browser/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/gtk/location_bar_view_gtk.cc
@@ -38,9 +38,11 @@ const int kBottomMargin = 1;
 const int kBorderThickness = 1;
 
 // Left margin of first run bubble.
-const int kFirstRunBubbleLeftMargin = 20;
+const int kFirstRunBubbleLeftMargin = 8;
+// Extra vertical spacing for first run bubble.
+const int kFirstRunBubbleTopMargin = 1;
 // Task delay (in milliseconds) to show first run bubble.
-const int kFirstRunBubbleTaskDelay = 30;
+const int kFirstRunBubbleTaskDelay = 200;
 
 // Left and right padding/margin.
 // no icon/text  : 4px url_text 4px
@@ -511,7 +513,8 @@ void LocationBarViewGtk::ShowFirstRunBubbleInternal(bool use_OEM_bubble) {
   // The bubble needs to be just below the Omnibox and slightly to the right
   // of star button, so shift x and y co-ordinates.
   x += widget()->allocation.x + kFirstRunBubbleLeftMargin;
-  y += widget()->allocation.y + widget()->allocation.height;
+  y += widget()->allocation.y + widget()->allocation.height +
+       kFirstRunBubbleTopMargin;
 
   FirstRunBubble::Show(profile_,
       GTK_WINDOW(gtk_widget_get_toplevel(widget())),
diff --git a/chrome/browser/gtk/options/advanced_contents_gtk.cc b/chrome/browser/gtk/options/advanced_contents_gtk.cc
index 84de4b0..3fa5abb 100644
--- a/chrome/browser/gtk/options/advanced_contents_gtk.cc
+++ b/chrome/browser/gtk/options/advanced_contents_gtk.cc
@@ -228,7 +228,7 @@ PrivacySection::PrivacySection(Profile* profile)
   g_signal_connect(G_OBJECT(cookie_behavior_combobox_), "changed",
                    G_CALLBACK(OnCookieBehaviorChanged), this);
 
-  GtkWidget* cookie_controls = gtk_util::CreateLabeledControlsGroup(
+  GtkWidget* cookie_controls = gtk_util::CreateLabeledControlsGroup(NULL,
       l10n_util::GetStringUTF8(IDS_OPTIONS_COOKIES_ACCEPT_LABEL).c_str(),
       cookie_behavior_combobox_,
       NULL);
diff --git a/chrome/browser/login_prompt_gtk.cc b/chrome/browser/login_prompt_gtk.cc
index 4e6bc72..ef98d26 100644
--- a/chrome/browser/login_prompt_gtk.cc
+++ b/chrome/browser/login_prompt_gtk.cc
@@ -70,7 +70,7 @@ class LoginHandlerGtk : public LoginHandler,
     gtk_entry_set_activates_default(GTK_ENTRY(password_entry_), TRUE);
     gtk_entry_set_visibility(GTK_ENTRY(password_entry_), FALSE);
 
-    GtkWidget* table = gtk_util::CreateLabeledControlsGroup(
+    GtkWidget* table = gtk_util::CreateLabeledControlsGroup(NULL,
         l10n_util::GetStringUTF8(IDS_LOGIN_DIALOG_USERNAME_FIELD).c_str(),
         username_entry_,
         l10n_util::GetStringUTF8(IDS_LOGIN_DIALOG_PASSWORD_FIELD).c_str(),
diff --git a/chrome/common/gtk_util.cc b/chrome/common/gtk_util.cc
index 8b7dfd8..1706a12 100644
--- a/chrome/common/gtk_util.cc
+++ b/chrome/common/gtk_util.cc
@@ -138,7 +138,8 @@ guint32 GetGdkEventTime(GdkEvent* event) {
 
 namespace gtk_util {
 
-GtkWidget* CreateLabeledControlsGroup(const char* text, ...) {
+GtkWidget* CreateLabeledControlsGroup(const GdkColor* text_color,
+    const char* text, ...) {
   va_list ap;
   va_start(ap, text);
   GtkWidget* table = gtk_table_new(0, 2, FALSE);
@@ -150,6 +151,9 @@ GtkWidget* CreateLabeledControlsGroup(const char* text, ...) {
     GtkWidget* control = va_arg(ap, GtkWidget*);
     GtkWidget* label = gtk_label_new(text);
     gtk_misc_set_alignment(GTK_MISC(label), 0, 0.5);
+    if (text_color)
+      gtk_widget_modify_fg(label, GTK_STATE_NORMAL, text_color);
+
     gtk_table_attach(GTK_TABLE(table), label,
                  0, 1, row, row + 1,
                  GTK_FILL, GTK_FILL,
diff --git a/chrome/common/gtk_util.h b/chrome/common/gtk_util.h
index 85bcc4b..6e1c231 100644
--- a/chrome/common/gtk_util.h
+++ b/chrome/common/gtk_util.h
@@ -51,11 +51,15 @@ const int kContentAreaSpacing = 18;
 
 // Create a table of labeled controls, using proper spacing and alignment.
 // Arguments should be pairs of const char*, GtkWidget*, concluding with a NULL.
+// The first argument is a color to force the label text to. It can be NULL to
+// get the system default.
 // For example:
-// controls = CreateLabeledControlsGroup("Name:", title_entry_,
+// controls = CreateLabeledControlsGroup(&gfx::kGdkBlack,
+//                                       "Name:", title_entry_,
 //                                       "Folder:", folder_combobox_,
 //                                       NULL);
-GtkWidget* CreateLabeledControlsGroup(const char* text, ...);
+GtkWidget* CreateLabeledControlsGroup(const GdkColor* color,
+                                      const char* text, ...);
 
 // Create a GtkBin with |child| as its child widget.  This bin will paint a
 // border of color |color| with the sizes specified in pixels.