GTK Themes: Theme the bookmark bubble. (And first run bubble).

GTK Themes: Theme the bookmark bubble. (And first run bubble).

http://crbug.com/16783

[Elliot Glaysher, 2009-07-23 23:26:59]

ping?

[Evan Martin, 2009-07-23 23:36:34]

http://codereview.chromium.org/160025/diff/28/1001
File chrome/browser/gtk/bookmark_bubble_gtk.cc (right):

http://codereview.chromium.org/160025/diff/28/1001#newcode129
Line 129: gtk_widget_modify_fg(*it, GTK_STATE_NORMAL, &gfx::kGdkBlack);
Does the theme provider not specify this?

http://codereview.chromium.org/160025/diff/28/1001#newcode154
Line 154: labels_.push_back(label);
Does we get the initial color correct?  It seems you only listen for theme
changes now.

http://codereview.chromium.org/160025/diff/28/1007
File chrome/browser/gtk/info_bubble_gtk.cc (right):

http://codereview.chromium.org/160025/diff/28/1007#newcode232
Line 232: if (type.value == NotificationType::ACTIVE_WINDOW_CHANGED) {
should this be a switch statement?  seems more clear to me

[Elliot Glaysher, 2009-07-23 23:44:54]

http://codereview.chromium.org/160025/diff/28/1001
File chrome/browser/gtk/bookmark_bubble_gtk.cc (right):

http://codereview.chromium.org/160025/diff/28/1001#newcode129
Line 129: gtk_widget_modify_fg(*it, GTK_STATE_NORMAL, &gfx::kGdkBlack);
On 2009/07/23 23:36:34, Evan Martin wrote:
> Does the theme provider not specify this?

Nope. InfoBubbles are hard coded to black on white text.

http://codereview.chromium.org/160025/diff/28/1001#newcode154
Line 154: labels_.push_back(label);
On 2009/07/23 23:36:34, Evan Martin wrote:
> Does we get the initial color correct?  It seems you only listen for theme
> changes now.

I call "theme_provider_->InitThemesFor(this);" below. It simulates a
BROWSER_THEME_CHANGE message.

[Evan Stade, 2009-07-23 23:46:55]

lgtm

I too wonder why the info bubble text is not a theme value but Elliot's comment
in the first run bubble makes me think that that is on his TODO list.

http://codereview.chromium.org/160025/diff/28/1001
File chrome/browser/gtk/bookmark_bubble_gtk.cc (right):

http://codereview.chromium.org/160025/diff/28/1001#newcode154
Line 154: labels_.push_back(label);
I've noticed that black is not quite right (at least, doesn't match windows).
There, the "Bookmark" label rgb is 0, 44, 115 and is 10 pixels tall (while the
other labels in the dialog are 8px tall and are black). This doesn't have to
block this patch but should be noted and fixed sometime.

http://codereview.chromium.org/160025/diff/28/1012
File chrome/common/gtk_util.cc (right):

http://codereview.chromium.org/160025/diff/28/1012#newcode144
Line 144: const GdkColor* text_color,
seems we might not need text_color any more?