385966 - [10.5] Extra file menu appears to the right of the help menu (two file menus)

Status: VERIFIED FIXED

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

Description

[Colin Barrett [:cbarrett]]

STR:
1. Start Minefield.
2. Take focus from Minefield (click on The Finder)
3. Give focus to Minefield.

Expected Results:
9 Menus are shown, Minefield to Help.

Actual results:
10 menus are shown. To the right of Help is an extra File menu.

This only occurs on Mac OS X 10.5.

Comment 1

[Josh Aas]

Marking this blocking, but hopefully it'll just get fixed by Apple in a newer 10.5 seed.

Comment 3

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

Josh, did you remove blocking1.9+ on the assumption that this will get fixed in a newer seed? I still see it in 9A500n using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a8pre) Gecko/2007082204 Minefield/3.0a8pre

Are you sure enough that this is on Apple's end to make this not block 1.9?

Comment 4

[Josh Aas]

I removed the 1.9+ because in a meeting today we decided that we won't be giving blocking1.9+ to any 10.5-only bugs. They should remain 1.9? for now until Apple's builds get further along and then we will re-evaluate.

Comment 5

[Colin Barrett [:cbarrett]]

Filed as rdar://problem/5430656

Comment 6

[Josh Aas]

This is still a problem with Leopard build 9a527.

Comment 8

[Joël Kuiper]

The problem presists in 9A559

Comment 10

[Al Billings [:abillings - ex-MoCo]]

This is still present in the final Leopard release.

Comment 13

[Fred Wenzel [:wenzel]]

Confirming; problem persists in Leopard final (9A581).

Comment 15

[Fred Wenzel [:wenzel]]

Btw. the extra file menu is also not just a cosmetic issue: Clicking on it has a tendency to crash Minefield (for me, at least).

Comment 16

[Patrick]

(In reply to comment #15)
> Btw. the extra file menu is also not just a cosmetic issue: Clicking on it has
> a tendency to crash Minefield (for me, at least).
> 

Confirming, although it doesn't crash consistently. At times it only makes the menu excruciating slow, and some times it does nothing out of the ordinary. Have not discovered any particular steps to trigger each of those behaviors though.

Comment 17

[Colin Barrett [:cbarrett]]

Just an extra fun tidbit:

STR: With Minefield open, chose the Restart option from the Apple menu.
Expected results: Help menu is visible.
Actual results: Help menu *disappears*.

Whaaaa?

Comment 21

[Josh Aas]

This is marked as P1 because it is commonly reported, confusing, and destabilizing. AppKit is getting into an essentially corrupted state.

Comment 23

[Nihar Shah]

Still exists in build 2007111104 Minefield/3.0b2pre on Mac OS 10.5

Comment 25

[Josh Aas]

Attachment: test app v1.0

This test app demonstrates the OS bug. It does the same thing our app does, but in a reduced way. Much easier to play with than our actual menu code.

Comment 26

[Josh Aas]

You'll need Mac OS X 10.5 and Xcode 3 to build/run the test app.

Comment 27

[Josh Aas]

Filed Apple bug 5598550 on the issue, which includes the test app.

Comment 28

[Josh Aas]

Attachment: fix v1.0

I found a way to handle menu bar swapping that doesn't trigger the AppKit bug. This works on 10.4 and 10.5.

Comment 29

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 288616 [details] [diff] [review]
fix v1.0

>+    if ([mainMenu numberOfItems] > 0)
>+      [[mainMenu itemAtIndex:0] setSubmenu:sApplicationMenu];
>+    else
>+      NS_WARNING("Main menu does not have any items, something is terribly wrong!");
>   }

Seems like that else could be dangerous in release builds without an #ifdef or scope braces.

Comment 30

[Josh Aas]

Even if the NS_WARNING resolves to nothing, the semicolon remains as an empty statement.

Comment 31

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to comment #30)

Oh, of course. Sorry for the noise.

Comment 32

[Colin Barrett [:cbarrett]]

Comment on attachment 288616 [details] [diff] [review]
fix v1.0

>-    // an NSMenu can't have multiple supermenus, so when we paint a menu bar we unhook the
>-    // application menu from its current supermenu and hook it up to this menu bar's
>-    // application menu item. This way all changes to the application menu perist across
>-    // all instances of nsMenuBarX. We could assume 0 for indexOfItemWithSubmenu, but lets


>+  // Swap out first item into incoming menu bar.
>+  NSMenuItem* firstMenuItem = [[mainMenu itemAtIndex:0] retain];
>+  [mainMenu removeItemAtIndex:0];
>+  [mRootMenu insertItem:firstMenuItem atIndex:0];
>+  [firstMenuItem release];
>+

Bring back some of the comment you removed above, it's still relevant and explains why you're swapping the menu (I think).

r=me with that.

Comment 33

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

+    else
+      NS_WARNING("Main menu does not have any items, something is terribly wrong!");

Why don't you make this an assert and the setSubmenu call above it unconditional?

+  if ([mainMenu numberOfItems] < 1) {
+    NS_WARNING("Main menu does not have any items, something is terribly wrong!");
+    return NS_ERROR_FAILURE;

Ditto.

Comment 34

[Josh Aas]

Attachment: fix v1.1

Comment 35

[Josh Aas]

landed on trunk

Comment 36

[Rob Campbell [:rc] (:robcee)]

it's gone in today's nightly.

Comment 37

[Stephen Donner [:stephend] Not actively reading bugmail]

Pretty sure this caused bug 403908.

Comment 38

[Fred Wenzel [:wenzel]]

Oh yeah, it reappeared when I just woke my laptop up from sleep (it was gone earlier today). Reopening.

Comment 39

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

(In reply to comment #38)
> Oh yeah, it reappeared when I just woke my laptop up from sleep (it was gone
> earlier today). Reopening.

That sounds like bug 403967.

Comment 40

[Tony Chung [:tchung]]

adding relnote keyword for b1

Comment 43

[Thomas Cloer]

I'm still getting this one with 3.0b1 German on 10.5.1

Comment 44

[Chris Blore]

(In reply to comment #43)
> I'm still getting this one with 3.0b1 German on 10.5.1
> 

As far as I'm aware, this wasn't fixed until after the 3.0b1 cycle and therefore it is to be expected that you would see the bug.

Comment 45

[Marcia Knous [:marcia]]

Yes, you will still see the bug in the beta (en-US and locales), but it is not present in the current trunk nightlies.

Comment 50

[Larry Levine]

For clarification - this bug has been fixed but is not part of the EN beta release?

Comment 51

[Marcia Knous [:marcia]]

Larry: Yes, that is correct.

(In reply to comment #50)
> For clarification - this bug has been fixed but is not part of the EN beta
> release?
> 

Comment 53

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2007123104 Minefield/3.0b3pre
-> Verified fixed