Linux: Create app modal dialogs in a different way to work around a KDE 3 win...

Linux: Create app modal dialogs in a different way to work around a KDE 3 window manager bug. While we're at it, change dialog's default size to match that of Chrome on Windows.

BUG=28230
TEST=see bug

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

[Lei Zhang, 2009-11-21 01:46:54]

I'll take a LGTM from anyone with lots of GTK-fu.

[Evan Stade, 2009-11-21 01:50:36]

I don't really like this solution because it changes the way it looks for people
who aren't using kde3.

[Lei Zhang, 2009-11-21 03:19:08]

On 2009/11/21 01:50:36, Evan Stade wrote:
> I don't really like this solution because it changes the way it looks for
people
> who aren't using kde3.

Ok, we can wrap gtk_message_dialog_new(). See patch set 2.

[Evan Stade, 2009-11-23 20:36:49]

overall lg

note: there's one more (new) use of gtk_message_dialog in
common/platform_util_linux.cc

http://codereview.chromium.org/425016/diff/4001/4011
File chrome/common/gtk_util.cc (right):

http://codereview.chromium.org/425016/diff/4001/4011#newcode595
chrome/common/gtk_util.cc:595: if (message_format) {
It's kind of unfortunate to have to use va_list. Seems you could avoid all this
by using the pattern:

dialog = gtk_message_dialog_new(...);
gtk_util::ApplyMessageDialogQuirks(dialog, flags);

http://codereview.chromium.org/425016/diff/4001/4011#newcode609
chrome/common/gtk_util.cc:609: if (base::DESKTOP_ENVIRONMENT_KDE3 ==
GetDesktopEnvironment(env.get()))
is GetDesktopEnvironment expensive, or does it cache the result?

[Lei Zhang, 2009-11-23 21:23:01]

http://codereview.chromium.org/425016/diff/4001/4011
File chrome/common/gtk_util.cc (right):

http://codereview.chromium.org/425016/diff/4001/4011#newcode595
chrome/common/gtk_util.cc:595: if (message_format) {
On 2009/11/23 20:36:49, Evan Stade wrote:
> It's kind of unfortunate to have to use va_list. Seems you could avoid all
this
> by using the pattern:
> 
> dialog = gtk_message_dialog_new(...);
> gtk_util::ApplyMessageDialogQuirks(dialog, flags);

Sure, that works too.

http://codereview.chromium.org/425016/diff/4001/4011#newcode609
chrome/common/gtk_util.cc:609: if (base::DESKTOP_ENVIRONMENT_KDE3 ==
GetDesktopEnvironment(env.get()))
On 2009/11/23 20:36:49, Evan Stade wrote:
> is GetDesktopEnvironment expensive, or does it cache the result?

Getting environment variables is a cheap operation.

[Evan Stade, 2009-11-23 21:36:14]

it seems to me you should always call the ApplyQuirks method. You can use
gtk_window_get_modal() to control for non-modal cases (if it even matters?).

http://codereview.chromium.org/425016/diff/4001/4011
File chrome/common/gtk_util.cc (right):

http://codereview.chromium.org/425016/diff/4001/4011#newcode609
chrome/common/gtk_util.cc:609: if (base::DESKTOP_ENVIRONMENT_KDE3 ==
GetDesktopEnvironment(env.get()))
On 2009/11/23 21:23:01, Lei Zhang wrote:
> On 2009/11/23 20:36:49, Evan Stade wrote:
> > is GetDesktopEnvironment expensive, or does it cache the result?
> 
> Getting environment variables is a cheap operation.

well, it does involve several string comparisons, but I guess it's ok.

[Lei Zhang, 2009-11-23 21:47:47]

On 2009/11/23 21:36:14, Evan Stade wrote:
> it seems to me you should always call the ApplyQuirks method. You can use
> gtk_window_get_modal() to control for non-modal cases (if it even matters?).

It's only a problem for modal dialogs. But why should we always call ApplyQuirks
and have it call gtk_window_get_modal() at runtime, (more expensive) when it's
deterministic and we can just look at the code and decide whether to call it or
not?

[Evan Stade, 2009-11-23 22:19:21]

On 2009/11/23 21:47:47, Lei Zhang wrote:
> On 2009/11/23 21:36:14, Evan Stade wrote:
> > it seems to me you should always call the ApplyQuirks method. You can use
> > gtk_window_get_modal() to control for non-modal cases (if it even matters?).
> 
> It's only a problem for modal dialogs. But why should we always call
ApplyQuirks
> and have it call gtk_window_get_modal() at runtime, (more expensive) when it's

it's just checking the value of a bit...

> deterministic and we can just look at the code and decide whether to call it
or
> not?

future-proofing; readability

[Lei Zhang, 2009-11-23 23:09:08]

ok, give it one more whirl.

[Lei Zhang, 2009-11-23 23:21:22]

+ missing files.

[Evan Stade, 2009-11-23 23:23:44]

lgtm