608934 - About window needs a close button

Status: VERIFIED FIXED

(Firefox :: General, defect)

Description

[Michael Monreal [:micmon]]

Attachment: Screenshot

The redesigned About window really needs a close button. There are system configurations (at least on Linux) which do not have a way to close this window atm:

a) some tiling window managers which do not show window bars

b) the GNOME3 default theme does not show window buttons on "about" windows (see screenshot)

Comment 1

[Christopher Aillon (sabbatical, not receiving bugmail)]

There's some discussion on this at https://bugzilla.redhat.com/677427

Comment 3

[Pascal Chevrel:pascalc]

Attachment: patch fixing the bug

this patch fixes the bug for me on Gnome 3

Comment 4

[:Margaret Leibovic]

Comment on attachment 536039 [details] [diff] [review]
patch fixing the bug

This is probably wrong for Windows, since this change would affect any non-OSX platform.

Dialog windows are supposed to have close buttons, so I think something may actually be wrong with your window manager, not the about dialog. Testing on Ubuntu 10.04, the about dialog already has a close button without this patch applied. How are dialog windows normally handled by your platform? Does esc work to close them? What do about dialogs look like for other applications?

Comment 5

[Jasper St. Pierre]

See GNOME HIG:

  http://developer.gnome.org/hig-book/3.0/windows-dialog.html.en

"Window Controls: Minimize/Roll-up"

Note the lack of "close".

GNOME designers say that dialogs shouldn't have a close button because they require the user to take a specific action, and it may be unclear what the close button does.

Comment 6

[Karl Tomlinson (:karlt)]

Comment on attachment 536039 [details] [diff] [review]
patch fixing the bug

Yes, the about window currently provided is not a dialog (because it doesn't provide any controls for user interaction), but I agree that it is safest to leave this as is for MS XP_WIN systems (unless there is a known issue there, and it has been been tested on those systems).

One side-effect here though is that the GTK port currently doesn't correctly set WM_TRANSIENT_FOR for non-dialog windows.  That means that the window manager will not keep the about window above its "parent" window.  That should be fixed, but it's not necessarily a prerequisite for this fix.  The correctness of "dependent" here is debatable (bug 611955, and I wonder whether bug 611955 comment 3 provides a reason why this window should not be presented as a dialog on NT systems).

Fortunately centerscreen seems to still have no effect, on my system at least.
(I'm not clear what positions the window over its "parent".)

(In reply to comment #4)
> Dialog windows are supposed to have close buttons, so I think something may
> actually be wrong with your window manager, not the about dialog.

Dialogs should provide controls for users to take part in the dialogue.

When we use _NET_WM_WINDOW_TYPE to tell the window manager that it is a dialog, it is appropriate for the window manager to use this "in determining the decoration".  Dialogs should already have controls, so there is no need for the window manager to provide additional controls.

> Testing on
> Ubuntu 10.04, the about dialog already has a close button without this patch
> applied. How are dialog windows normally handled by your platform? Does esc
> work to close them? What do about dialogs look like for other applications?

On my system (with kwin) evince's about window is a dialog with credits, license, and close buttons.  Similarly okular's about window is a dialog with a close button (provided by the app).  kwin chooses to provide an additional close button in its decorations even for dialog windows.

Restoring the close button to make the about window a dialog would be an acceptable solution, but Firefox's about window already looks so different from other apps' windows that I don't think there's a need to imitate them.

Esc closes all these windows.

Comment 7

[Pascal Chevrel:pascalc]

Attachment: updated patch

Here is an updated patch preserving the Windows options.

Comment 8

[:Margaret Leibovic]

Comment on attachment 536221 [details] [diff] [review]
updated patch

I can't officially review this, but it looks good to me.

Comment 9

[Pascal Chevrel:pascalc]

thanks Margaret, I'll wait for Gavin's review then :)

Comment 10

[Pascal Chevrel:pascalc]

thanks Gavin, if I understand well the terminology, should I set the flags as : 
tracking-firefox5 : ?
status-firefox5   : affected
tracking-firefox6 : ?
status-firefox6   : affected
tracking-firefox7 : ?
status-firefox7   : affected

Or should I target one release at a time? Or maybe I shouldn't touch any of these flags ? :)

Comment 11

[:Gavin Sharp [email: [e-mail]]]

No need to touch tracking flags, those are used for bugs that need to be monitored before the branch point. You can request approval on the patch, but this doesn't seem like something critical enough to warrant landing on mozilla-aurora or mozilla-beta.

Comment 12

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/082845fb7841

Comment 15

[George Carstoiu]

Mozilla/5.0 (X11; Linux i686; rv:7.0a1) Gecko/20110627 Firefox/7.0a1

Verified this on Ubuntu 11.04 x86 that has Gnome 3 installed. Close button present in about window -> setting status as Verified Fixed.