137523 - cmd+M to minimize the window doesn't minimize n Mozilla App Suite

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Kathleen :Brade]

Carryover from bug 75898:
"cmd-m to minimize the window doesn't work at all."

Comment 1

[sairuh (rarely reading bugmail)]

nominating, and setting milestone (with permission from kathy).

Comment 2

[Prachi Gauriar]

Perhaps adding a Minimize Window to the Window menu could solve the problem with
binding cmd-m to the minimize. Mail has this, as do a few other apps. Also, just
to keep things straightforward, you could also have a Zoom window and a Close
Window (command-w), like Mail, Fire, and Terminal. Using the same order that all
of those apps use, the Window menu could look something like this: 

Window
  Close Window               cmd-w
  Zoom Window
  Minimize Window            cmd-m
             <Separator>
  Navigator                  cmd-1
  Mail & Newsgroups          cmd-2
  IRC Chat                   cmd-3
  Composer                   cmd-4
  Address Book               cmd-5
             <Separator>
         <Open Window List>

This would only apply to Mac or Mac OS X. Sound good?

Comment 3

[Ken Harris]

Sounds like Apple's own guidelines, in fact:

http://developer.apple.com/techpubs/macosx/Essentials/AquaHIGuidelines/AHIGMenus/The_Window_Menu.html

(Yeah, the order is different, but I think the order isn't terribly important
if you're doing this primarily for cmd-M.)

Comment 4

[Jesse Ruderman]

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

Comment 5

[Boris Zbarsky [:bzbarsky]]

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

Comment 6

[Kathleen :Brade]

pushing this off; I don't have time to work on this right now (removing rtm
nomination, making nsbeta1+ nsbeta1-)

Comment 7

[Prachi Gauriar]

To implement the changes in comment 2, what files would I need to look at?

Comment 8

[Kathleen :Brade]

http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/tasksOverlay.xul#42

note that you'll probably need to do a platform overlay so that the additional
items are added only to the Macintosh platform.

Comment 9

[sairuh (rarely reading bugmail)]

nominating...

Comment 10

[Greg K.]

Note that FizzillaMach still assigns command+m to File/New Message.

Comment 11

[sairuh (rarely reading bugmail)]

greg, thanks for pointing that out --i've filed bug 185268 to cover that.

Comment 12

[Samir Gehani]

adt: nsbeta1-

Comment 13

[Jo Hermans]

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

Comment 14

[Daniel Küstner]

Is this somehow affected by the fix for bug 186789? (see bug 186789#c70)

Comment 15

[Jo Hermans]

(In reply to comment #14)
> Is this somehow affected by the fix for bug 186789? (see bug 186789#c70)

cmd-shift-M still works as before (opens new mail), cmd-M still doesn't work

Comment 16

[Jo Hermans]

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

Comment 17

[Asaf Romano (gone)]

see backend patch on bug 204636.

Comment 18

[Asaf Romano (gone)]

Attachment: fix mac window menu

(the backend part was fixed in bug 204636)

I hope we don't over-avoid #ifdefs inside xpfe/

Comment 19

[Aaron Leventhal]

Comment on attachment 171553 [details] [diff] [review]
fix mac window menu

Sorry, I'm not an appropriate r= for this bug.

Comment 20

[Asaf Romano (gone)]

Attachment: better patch

Comment 21

[[e-mail]]

Comment on attachment 171739 [details] [diff] [review]
better patch

I'd need a Mac to be able to review this; as for superreview:

>+  // also hide the window list separator
>+  broadcaster = document.getElementById("sep-window-list");
>+  if (broadcaster)
>+    broadcaster.setAttribute("hidden", "true");
You should use a new variable name (e.g. separator) for this.

>+function ZoomCurrentWindow()
A comment that this is used by the mac platform overlay would be helpful.

Comment 22

[Asaf Romano (gone)]

Attachment: final patch

Comment 23

[jhp (no longer active)]

Comment on attachment 171746 [details] [diff] [review]
final patch

Looks good.  Is there anyway to make the two menu items disable even if there
is a window, but it has been minimized?

Comment 24

[Asaf Romano (gone)]

Attachment: update to tip

Comment 25

[Asaf Romano (gone)]

(In reply to comment #23)
> (From update of attachment 171746 [details] [diff] [review] [edit])
> Looks good.  Is there anyway to make the two menu items disable even if there
> is a window, but it has been minimized?
> 

our menus don't work when the window is minimized (this is more or less bug 21296)

Comment 26

[jhp (no longer active)]

I know they don't work; they're not supposed to.  Even more reason to disable
them.  This is the expected behaviour.

Comment 27

[Asaf Romano (gone)]

(In reply to comment #26)
> I know they don't work; they're not supposed to.  Even more reason to disable
> them.  This is the expected behaviour.

not really, File->New Navigator (for example) should work.

Comment 28

[jhp (no longer active)]

Checked in to trunk. ->FIXED

Comment 29

[Mike Pinkerton (not reading bugmail)]

(In reply to comment #26)
> I know they don't work; they're not supposed to.  Even more reason to disable
> them.  This is the expected behaviour.

a totally false statement. it's very important that there be some menu items
that work in this case.

Comment 30

[jhp (no longer active)]

Pinkerton, for what purpose?  If you have an app that has only one window open,
and that window is minimized, what should the 'Minimize' and 'Zoom' options do?
 All the other Mac apps I've seen disable both of these items when all windows
are minimized.

Comment 31

[Asaf Romano (gone)]

Javier, if it wasn't clear, i have talked about mozilla's menus in general,
they're not working when the window is minimized.

Comment 32

[Mike Pinkerton (not reading bugmail)]

right, i think we were talking about different things.

Comment 33

[Scott MacGregor]

FYI, this patch introduced a regression in Thunderbird causing all keycode
commands to trigger cmd_delete! See Bug #281401 for more details. It's
interesting that the entity errors this patch introduced in Thunderbird
manifested themselves by mapping all keyboard commands to cmd_delete. I wonder
why it reacted that way....