355802 - [Cocoa] Missing SeaMonkey Menu Entries

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Timothy Huang]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20061006 SeaMonkey/1.5a
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20061006 SeaMonkey/1.5a

Since about Oct. 01, the "SeaMonkey" Menu does not contain several entries, such as Hide SeaMonkey. The only way to hid SeaMonkey application is to click the yellow button on the upper left corner for each window. Pain on you know where.

Reproducible: Always

Steps to Reproduce:
1. Open the latest nightly build.
2. Look at the "SeaMonkey" menu.
3.

Actual Results:  
Only three entries: About SeaMonkey, Preference, and Quit. All others are missing.

Comment 1

[Karsten Düsterloh]

Most probably caused by bug 326469. See also bug 355797.

Comment 2

[Stefan [:stefanh]]

(In reply to comment #1)
> Most probably caused by bug 326469. See also bug 355797.
> 

Those menuitems needs to be in the DOM (see the documentation in nsMenuBarX.mm line 614 and on).

Comment 3

[Stefan [:stefanh]]

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

Comment 4

[Stefan [:stefanh]]

Attachment: work in progress

Attaching a patch that I've had in my tree for some time now. It's not 100% tested, but the patch will add the missing menuitems (and fix the shortcuts). Unfortunately, "About" and "Preferences..." doesn't work with Venkman. Trying to open preferences from the menu with Venkman open also results in an assertion:

###!!! ASSERTION: Carbon event for menu hit not sent!: 'err == noErr', file /Users/Stefan/mozilla-debug/mozilla/widget/src/cocoa/nsMenuBarX.mm, line 1115

Comment 5

[Stefan [:stefanh]]

I have a patch for this. But I think the fix for this will be a lot cleaner after Josh have landed some stuff (like the patch in bug 368911).

Comment 6

[Stefan [:stefanh]]

Attachment: Fix app menuitems and shortcuts

This should get us a working app menu again. The pref menuitem will now work in all windows - even if they don't contain the menuitem. What happens is that if the window doesn't have a pref menuitem we'll use the content node from hiddenwindow. The only small issue with this is that we'll open the navigator prefs in a DOMi window/Password Manger/etc opened from Composer, MailNews etc.

Mnyromyr and I had a chat about the modifiers yesterday. In toolkit they're localized (except the Quit command). To make a long story short: Since there might be a tiny risk that some locale (that we don't know of) *might* have different modifiers in the app menu it can't be wrong to make them localizable.

Comment 8

[Karsten Düsterloh]

Comment on attachment 258717 [details] [diff] [review]
Fix app menuitems and shortcuts

>   <menupopup id="menu_FilePopup">
...
>+    <!-- These 5 menuitems (and the menu_preferences menuitem above) are added
>+      to the Application menu. They just need to be in the DOM, widget code
>+      does the rest -->

The way how this is done by the Mac widget code is really oh so hacky, but we seem to have to live with it. :-| 

>Index: suite/locales/en-US/chrome/common/mac/platformCommunicatorOverlay.dtd
>===================================================================
>+<!-- LOCALIZATION NOTE: FILE Do not translate all the Cmd.key -->

Just a nit, since you're touching this actually unrelated comment: it's still as incomprehensible as before - I had to read it a couple of times, in context, before understanding it.

I'd suggest something like:
<!-- LOCALIZATION NOTE: FILE Do not translate all the entities ending with Cmd.key -->

Comment 9

[Stefan [:stefanh]]

> I'd suggest something like:
> <!-- LOCALIZATION NOTE: FILE Do not translate all the entities ending with
> Cmd.key -->
> 

Yeah, that'll make it more clear.

Comment 10

[Stefan [:stefanh]]

Attachment: New version addressing Mnyromyr's nit

Same as previous patch, just addressing Karsten's suggestion.

Comment 11

[Stefan [:stefanh]]

Landed on trunk by ajschult (thanks!)