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)
There's some discussion on this at https://bugzilla.redhat.com/677427
Created attachment 536039 [details] [diff] [review] patch fixing the bug this patch fixes the bug for me on Gnome 3
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.
Created attachment 536221 [details] [diff] [review] updated patch Here is an updated patch preserving the Windows options.
Comment on attachment 536221 [details] [diff] [review] updated patch I can't officially review this, but it looks good to me.
thanks Margaret, I'll wait for Gavin's review then :)
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 ? :)
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.
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.