Last Comment Bug 608934 - About window needs a close button
: About window needs a close button
Status: VERIFIED FIXED
:
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: All Linux
: -- normal (vote)
: Firefox 7
Assigned To: Pascal Chevrel:pascalc
:
Mentors:
: 656491 661346 667208 677029 (view as bug list)
Depends on:
Blocks: 633812
  Show dependency treegraph
 
Reported: 2010-11-02 02:47 PDT by Michael Monreal [:monreal]
Modified: 2011-08-06 09:46 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
affected


Attachments
Screenshot (198 bytes, application/octet-stream)
2010-11-02 02:47 PDT, Michael Monreal [:monreal]
no flags Details
patch fixing the bug (600 bytes, patch)
2011-05-30 02:39 PDT, Pascal Chevrel:pascalc
karlt: feedback+
Details | Diff | Splinter Review
updated patch (778 bytes, patch)
2011-05-30 20:56 PDT, Pascal Chevrel:pascalc
gavin.sharp: review+
margaret.leibovic: feedback+
Details | Diff | Splinter Review

Description Michael Monreal [:monreal] 2010-11-02 02:47:00 PDT
Created attachment 487535 [details]
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) 2011-02-18 10:28:21 PST
There's some discussion on this at https://bugzilla.redhat.com/677427
Comment 2 Pascal Chevrel:pascalc 2011-05-27 07:24:01 PDT
*** Bug 656491 has been marked as a duplicate of this bug. ***
Comment 3 Pascal Chevrel:pascalc 2011-05-30 02:39:24 PDT
Created attachment 536039 [details] [diff] [review]
patch fixing the bug

this patch fixes the bug for me on Gnome 3
Comment 4 :Margaret Leibovic 2011-05-30 08:44:30 PDT
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 2011-05-30 11:31:39 PDT
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) 2011-05-30 16:39:55 PDT
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 2011-05-30 20:56:53 PDT
Created attachment 536221 [details] [diff] [review]
updated patch

Here is an updated patch preserving the Windows options.
Comment 8 :Margaret Leibovic 2011-05-31 03:42:35 PDT
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 2011-05-31 04:20:51 PDT
thanks Margaret, I'll wait for Gavin's review then :)
Comment 10 Pascal Chevrel:pascalc 2011-05-31 05:16:28 PDT
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: gavin@gavinsharp.com] 2011-05-31 10:59:48 PDT
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) 2011-06-01 01:30:24 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/082845fb7841
Comment 13 Justin Dolske [:Dolske] 2011-06-01 14:33:23 PDT
*** Bug 661346 has been marked as a duplicate of this bug. ***
Comment 14 :Margaret Leibovic 2011-06-27 09:05:57 PDT
*** Bug 667208 has been marked as a duplicate of this bug. ***
Comment 15 George Carstoiu 2011-06-28 08:31:29 PDT
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.
Comment 16 Bill Gianopoulos [:WG9s] 2011-08-06 09:46:19 PDT
*** Bug 677029 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.