287162 - Mac Widget code ignores "minimizable=no" window feature

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P2)

Description

[Asaf Romano (gone)]

Apparenltly, widget:mac is ignoring eBorderStyle_minimize

Comment 1

[Phil Ringnalda (:philor)]

*** Bug 333339 has been marked as a duplicate of this bug. ***

Comment 2

[Markus Amalthea Magnuson]

Attachment: Implement the "minimize" functionality for widget:cocoa

Here's a patch to implement the minimize behaviour for cocoa windows too, including a fix which disables the minimize button for the about window on mac.

Comment 3

[Asaf Romano (gone)]

Comment on attachment 403795 [details] [diff] [review]
Implement the "minimize" functionality for widget:cocoa

r=mano.

Comment 4

[Markus Amalthea Magnuson]

Asaf, who would I ask for additional reviews/superreviews?

Comment 5

[Asaf Romano (gone)]

Comment on attachment 403795 [details] [diff] [review]
Implement the "minimize" functionality for widget:cocoa

roc

Comment 6

[Asaf Romano (gone)]

needs sr before checkin.

Comment 7

[Markus Amalthea Magnuson]

Sorry, I somehow missed the "roc" in your reply, and were told by gavin on irc that it wouldn't need a superreview. Continuing.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 403795 [details] [diff] [review]
Implement the "minimize" functionality for widget:cocoa

This actually does not need sr, but I'll mark it anyway...

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 403795 [details] [diff] [review]
Implement the "minimize" functionality for widget:cocoa

>-    window.open("chrome://browser/content/aboutDialog.xul", "About",
>-                "chrome, resizable=no, minimizable=no");
>+    window.openDialog("chrome://browser/content/aboutDialog.xul", "About",
>+                      "chrome, resizable=no, minimizable=no");

Why this change?

Comment 10

[Markus Amalthea Magnuson]

(In reply to comment #9)
> (From update of attachment 403795 [details] [diff] [review])
> >-    window.open("chrome://browser/content/aboutDialog.xul", "About",
> >-                "chrome, resizable=no, minimizable=no");
> >+    window.openDialog("chrome://browser/content/aboutDialog.xul", "About",
> >+                      "chrome, resizable=no, minimizable=no");
> 
> Why this change?

Because the minimizable feature is only regarded for dialog windows, not regular windows. See https://developer.mozilla.org/en/Window.open under the "Window functionality features" section, at the "dialog" and "minimizable" definitions.

window.openDialog does the same thing as window.open except it defaults to "dialog=yes" which would be the proper thing to do here. See https://developer.mozilla.org/en/DOM/window.openDialog#Default_behaviour for more info.

Also, I find it cleaner since the code for other platforms uses openDialog, check out http://mxr.mozilla.org/mozilla-central/source/browser/base/content/utilityOverlay.js#386

Comment 11

[Dão Gottwald [:dao]]

Alright, I didn't realize openDialog() defaults to dialog=yes while open() doesn't.

Comment 12

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/8c32e0eecbad

Comment 13

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

Comment on attachment 403795 [details] [diff] [review]
Implement the "minimize" functionality for widget:cocoa

approval1.9.2 requests aren't currently being monitored, since we're nearing RC freeze and there are too many outstanding requests, so I'm clearing this request. Feel free to re-request approval if you are confident that it's worth drivers' time to consider whether this non-blocker needs to land for 1.9.2 at this stage.

Comment 14

[Markus Amalthea Magnuson]

Gavin: Do I need to take any additional steps to nominate this fix for an upcoming (3.7a1?) Firefox release?

Comment 15

[Samuel Sidler (old account; do not CC)]

This fix has already landed on mozilla-central, which means it will be in Firefox 3.7a1. It will *not* be in Firefox 3.6.