Closed Bug 608934 Opened 13 years ago Closed 13 years ago

About window needs a close button

Categories

(Firefox :: General, defect)

All
Linux
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 7
Tracking Status
firefox5 --- affected
firefox6 --- affected

People

(Reporter: micmon, Assigned: pascalc)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Attached file Screenshot (obsolete) —
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)
Attachment #487535 - Attachment is obsolete: true
Depends on: 633812
Attached patch patch fixing the bug (obsolete) — Splinter Review
this patch fixes the bug for me on Gnome 3
Attachment #536039 - Flags: review?(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?
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 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.
Attachment #536039 - Flags: feedback+
Attached patch updated patchSplinter Review
Here is an updated patch preserving the Windows options.
Assignee: nobody → pascalc
Attachment #536039 - Attachment is obsolete: true
Attachment #536221 - Flags: review?(margaret.leibovic)
Attachment #536039 - Flags: review?(margaret.leibovic)
Comment on attachment 536221 [details] [diff] [review]
updated patch

I can't officially review this, but it looks good to me.
Attachment #536221 - Flags: review?(margaret.leibovic)
Attachment #536221 - Flags: review?(gavin.sharp)
Attachment #536221 - Flags: feedback+
thanks Margaret, I'll wait for Gavin's review then :)
Attachment #536221 - Flags: review?(gavin.sharp) → review+
Keywords: checkin-needed
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 ? :)
Hardware: x86 → All
Keywords: checkin-needed
Whiteboard: [fixed in cedar]
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.
Pushed:
http://hg.mozilla.org/mozilla-central/rev/082845fb7841
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed in cedar]
Target Milestone: --- → Firefox 7
Blocks: 633812
No longer depends on: 633812
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.
Status: RESOLVED → VERIFIED
See Also: → 1653510
You need to log in before you can comment on or make changes to this bug.