GTK: Support Ambiance/Radiance and Dust button ordering.

GTK: Support Ambiance/Radiance and Dust button ordering.

When running under GNOME, read the gconf key that defines the button placement
and lay the close, minimize and maximize buttons out. This can even handle
bizarre combinations like putting the close button on the left and the
minimize/maximize buttons on the right.

When running under anything else, use the default button ordering. 

BUG=39406
TEST=none

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

[Evan Stade, 2010-05-13 18:15:50]

does this respect Fitt's law for when some button is placed by one of the
screen's edges (sides or top)?

http://codereview.chromium.org/2095001/diff/4001/5001
File chrome/browser/gtk/browser_titlebar.cc (right):

http://codereview.chromium.org/2095001/diff/4001/5001#newcode103
chrome/browser/gtk/browser_titlebar.cc:103: class GConfKeyListener {
can we put this in its own file(s)?

http://codereview.chromium.org/2095001/diff/4001/5001#newcode200
chrome/browser/gtk/browser_titlebar.cc:200:
DISALLOW_EVIL_CONSTRUCTORS(GConfKeyListener);
EVIL is deprecated

http://codereview.chromium.org/2095001/diff/4001/5001#newcode325
chrome/browser/gtk/browser_titlebar.cc:325: // Allocate the two
the two...?

[Evan Stade, 2010-05-13 18:15:50]

does this respect Fitt's law for when some button is placed by one of the
screen's edges (sides or top)?

http://codereview.chromium.org/2095001/diff/4001/5001
File chrome/browser/gtk/browser_titlebar.cc (right):

http://codereview.chromium.org/2095001/diff/4001/5001#newcode103
chrome/browser/gtk/browser_titlebar.cc:103: class GConfKeyListener {
can we put this in its own file(s)?

http://codereview.chromium.org/2095001/diff/4001/5001#newcode200
chrome/browser/gtk/browser_titlebar.cc:200:
DISALLOW_EVIL_CONSTRUCTORS(GConfKeyListener);
EVIL is deprecated

http://codereview.chromium.org/2095001/diff/4001/5001#newcode325
chrome/browser/gtk/browser_titlebar.cc:325: // Allocate the two
the two...?

[Elliot Glaysher, 2010-05-13 19:28:00]

On 2010/05/13 18:15:50, Evan Stade wrote:
> does this respect Fitt's law for when some button is placed by one of the
> screen's edges (sides or top)?

Yes. The button hit boxes still reach to the edges of the screen.

Updated and moved the new class to its own file.

[Evan Stade, 2010-05-13 20:30:16]

Mostly LGTM. Can you post screenshots of left-aligned buttons for maximized and
not maximized windows, incognito and not?

So in RTL mode, you would have to define your buttons in reverse of the order
you actually want them?

http://codereview.chromium.org/2095001/diff/10001/11003
File chrome/browser/gtk/gconf_titlebar_listener.cc (right):

http://codereview.chromium.org/2095001/diff/10001/11003#newcode110
chrome/browser/gtk/gconf_titlebar_listener.cc:110: if (value)
nit: consider

current_value_ = value ? value : kDefaultButtonPlacement;

http://codereview.chromium.org/2095001/diff/10001/11004
File chrome/browser/gtk/gconf_titlebar_listener.h (right):

http://codereview.chromium.org/2095001/diff/10001/11004#newcode1
chrome/browser/gtk/gconf_titlebar_listener.h:1: // Copyright (c) 2009 The
Chromium Authors. All rights reserved.
nit: year

[Elliot Glaysher, 2010-05-13 21:00:19]

I have attached two images. The first shows what happens in RTL mode;
we still match what metacity does. The second is the left-aligned
button screenshots you asked for.

-- Elliot

On Thu, May 13, 2010 at 1:30 PM,  <estade@chromium.org> wrote:
> Mostly LGTM. Can you post screenshots of left-aligned buttons for maximized
> and
> not maximized windows, incognito and not?
>
> So in RTL mode, you would have to define your buttons in reverse of the
> order
> you actually want them?
>
>
> http://codereview.chromium.org/2095001/diff/10001/11003
> File chrome/browser/gtk/gconf_titlebar_listener.cc (right):
>
> http://codereview.chromium.org/2095001/diff/10001/11003#newcode110
> chrome/browser/gtk/gconf_titlebar_listener.cc:110: if (value)
> nit: consider
>
> current_value_ = value ? value : kDefaultButtonPlacement;
>
> http://codereview.chromium.org/2095001/diff/10001/11004
> File chrome/browser/gtk/gconf_titlebar_listener.h (right):
>
> http://codereview.chromium.org/2095001/diff/10001/11004#newcode1
> chrome/browser/gtk/gconf_titlebar_listener.h:1: // Copyright (c) 2009
> The Chromium Authors. All rights reserved.
> nit: year
>
> http://codereview.chromium.org/2095001/show
>

[use chromium acct., 2010-05-13 21:18:12]

great. LGTM

-- Evan Stade



On Thu, May 13, 2010 at 2:00 PM, Elliot Glaysher (Chromium)
<erg@chromium.org> wrote:
> I have attached two images. The first shows what happens in RTL mode;
> we still match what metacity does. The second is the left-aligned
> button screenshots you asked for.
>
> -- Elliot
>
> On Thu, May 13, 2010 at 1:30 PM,  <estade@chromium.org> wrote:
>> Mostly LGTM. Can you post screenshots of left-aligned buttons for maximized
>> and
>> not maximized windows, incognito and not?
>>
>> So in RTL mode, you would have to define your buttons in reverse of the
>> order
>> you actually want them?
>>
>>
>> http://codereview.chromium.org/2095001/diff/10001/11003
>> File chrome/browser/gtk/gconf_titlebar_listener.cc (right):
>>
>> http://codereview.chromium.org/2095001/diff/10001/11003#newcode110
>> chrome/browser/gtk/gconf_titlebar_listener.cc:110: if (value)
>> nit: consider
>>
>> current_value_ = value ? value : kDefaultButtonPlacement;
>>
>> http://codereview.chromium.org/2095001/diff/10001/11004
>> File chrome/browser/gtk/gconf_titlebar_listener.h (right):
>>
>> http://codereview.chromium.org/2095001/diff/10001/11004#newcode1
>> chrome/browser/gtk/gconf_titlebar_listener.h:1: // Copyright (c) 2009
>> The Chromium Authors. All rights reserved.
>> nit: year
>>
>> http://codereview.chromium.org/2095001/show
>>
>

[tony, 2010-05-14 01:02:55]

This is great.  Thanks for working on this.  Just a drive-by nit, feel free to
ignore.

Also, do you have any idea how the performance of gconf queries are?  I know we
already load gconf at startup for proxy settings, but how quick are key queries?

http://codereview.chromium.org/2095001/diff/10001/11003
File chrome/browser/gtk/gconf_titlebar_listener.cc (right):

http://codereview.chromium.org/2095001/diff/10001/11003#newcode76
chrome/browser/gtk/gconf_titlebar_listener.cc:76: return;
Nit: This early return seems pointless.