GTK Themes: Make the omnibox area look more native.

GTK Themes: Make the location bar area look native.

Replace the star and go buttons with a native looking thing, where
we manually draw the theme's button image for the full omnibox area
and then position the star/go buttons on top of that.

Also adds stand in versions of star_noborder and starred_noborder
until Glen can make real icons.

http://crbug.com/16227
http://crbug.com/13967
http://crbug.com/16915

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=20914

[Evan Stade, 2009-07-15 22:33:27]

overall looks good, can you post a screenshot of it?

http://codereview.chromium.org/149681/diff/1024/1029
File chrome/browser/gtk/browser_toolbar_gtk.cc (right):

http://codereview.chromium.org/149681/diff/1024/1029#newcode452
Line 452: gint y = star_allocation.y;
I suspect this will cause a crash in RTL because width will be negative

[Elliot Glaysher, 2009-07-15 23:27:26]

Attached is the new omnibox in five different theme engines.

On Wed, Jul 15, 2009 at 3:33 PM, <estade@chromium.org> wrote:
> http://codereview.chromium.org/149681/diff/1024/1029#newcode452
> Line 452: gint y = star_allocation.y;
> I suspect this will cause a crash in RTL because width will be negative

What would you recommend I do instead here?
std::mix(star_allocation.y, go_allocation.y)?

>
> http://codereview.chromium.org/149681
>

[Evan Stade, 2009-07-15 23:32:00]

On Wed, Jul 15, 2009 at 11:26 PM, Elliot Glaysher
(Chromium)<erg@chromium.org> wrote:
> Attached is the new omnibox in five different theme engines.

looks slick!

>
> On Wed, Jul 15, 2009 at 3:33 PM, <estade@chromium.org> wrote:
>> http://codereview.chromium.org/149681/diff/1024/1029#newcode452
>> Line 452: gint y = star_allocation.y;
>> I suspect this will cause a crash in RTL because width will be negative
>
> What would you recommend I do instead here?
> std::mix(star_allocation.y, go_allocation.y)?

See what I did in http://codereview.chromium.org/149647/diff/8/1014

>
>>
>> http://codereview.chromium.org/149681
>>
>

[Dean McNamee, 2009-07-16 12:41:42]

I only looked at the location bar part.

There is a frame around the star in the last screenshot, something funny going
on?

http://codereview.chromium.org/149681/diff/19/1047
File chrome/browser/gtk/location_bar_view_gtk.cc (right):

http://codereview.chromium.org/149681/diff/19/1047#newcode392
Line 392: if (!profile_ ||
Please add a comment, just something like:

// If we're not using GTK theming, draw our own border.

[Dean McNamee, 2009-07-16 12:45:48]

Also, both the star and go icons are awkwardly placed, they aren't really
centered (either horizontally or vertically).

The code changes to location_bar_view_gtk seem ok to me.

You are confusing the names between omnibox and location bar a bit, omnibox is
just the text entry widget, the location bar is the rest (which contains the
omnibox).  It is an important distinction in some cases...

On 2009/07/16 12:41:42, Dean McNamee wrote:
> I only looked at the location bar part.
> 
> There is a frame around the star in the last screenshot, something funny going
> on?
> 
> http://codereview.chromium.org/149681/diff/19/1047
> File chrome/browser/gtk/location_bar_view_gtk.cc (right):
> 
> http://codereview.chromium.org/149681/diff/19/1047#newcode392
> Line 392: if (!profile_ ||
> Please add a comment, just something like:
> 
> // If we're not using GTK theming, draw our own border.