357153 - In eMail composer some menus are "SomeMenuItem"

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Georg Maaß]

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

When composing a new eMail the menus INSERT and FORMAT use "SomeMenuItem" as menu title instead if "Insert" and "Format"

Reproducible: Always

Steps to Reproduce:
It happens at least after click into the email body

Comment 1

[Georg Maaß]

Attachment: Screenshot of the menubar

See screenshot

Comment 2

[Stefan [:stefanh]]

Any extensions installed? Can you track this down to when it first started to appear?

Comment 3

[Georg Maaß]

I did not install any extension to this SeaMonkey. I just loaded down the nighly build from 16.10.2006 and started the application inside the disk image. Then I got that result.

But in earlier versions og SeaMonkey I installed the livehttpheaders-0.12.xpi, which seems to be a corrupted installation, because it never worked, but I did not find any of the files /Mozilla/Component/compreg.dat nor /Mozilla/Component/xpti.dat. They probably have a different name or location on the Mac. So I could not delete them.

In earlier versions I also installed the German spellchecker, which works with Mozilla, but does not work (it is never listed in the menu of dictionaries) in SeaMonkey. On Windows I do not have any such problems with the spell checker.

Comment 4

[Stefan [:stefanh]]

(In reply to comment #3)
> I did not install any extension to this SeaMonkey. I just loaded down the
> nighly build from 16.10.2006 and started the application inside the disk image.
> Then I got that result.
> 
> But in earlier versions og SeaMonkey I installed the livehttpheaders-0.12.xpi,
> which seems to be a corrupted installation, because it never worked, but I did
> not find any of the files /Mozilla/Component/compreg.dat nor
> /Mozilla/Component/xpti.dat. They probably have a different name or location on
> the Mac. So I could not delete them.
> 
> In earlier versions I also installed the German spellchecker, which works with
> Mozilla, but does not work (it is never listed in the menu of dictionaries) in
> SeaMonkey. On Windows I do not have any such problems with the spell checker.
> 

Do you still see the issue if you create a new, fresh profile?

Comment 5

[Georg Maaß]

(In reply to comment #4)

> Do you still see the issue if you create a new, fresh profile?

I testet this now. The answer is: Yes. Same problem for the menu with the new created profile. 

The Headers tab is dammeged in the page info, without installing that extension to this SeaMonkey. This seems to be in the global profile. 

Comment 6

[Hiro]

It reproduces. 

Mac OS X 10.3.9
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.9a1) Gecko/20061020 SeaMonkey/1.5a

Comment 7

[Hiro]

The name of a menu("Insert" and "Format") the correct between a few seconds is displayed. 
And They change into "SomeMenuItem". 
The menu that changes into "SomeMenuItem" is insensitive. 

Comment 8

[Hiro]

Trunk Thunderbird reproduces too...

Comment 9

[Stefan [:stefanh]]

(In reply to comment #8)
> Trunk Thunderbird reproduces too...
> 

Yeah, I see this too now. The menus are there and seem to work (they're only disabled if you don't have focus in the message area). As Hiro says, original menu titles are displaying for a half sec or so - then they switch to "SomeMenuItem". Looks like they come from nsMenuBarX.mm (placeholders). I wonder when this regressed?

Comment 10

[Hiro]

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

Comment 11

[Stefan [:stefanh]]

Regression window:

2006-10-10-06 (Thunderbird & SeaMonkey) --> OK

2006-10-11-04 (Tb) --> Broken (the SeaMonkey build is 2 hours older and also broken, of course)

In a build without the issue, the Insert and Format menus are not disabled when you open the msg compose window. Also, all menuitems are shown as "enabled". That is, they should be disabled - when you open the window, focus is in the address field and none of the menuitems work.

When you open the msg compose window in a build after Oct 10, the Insert and Format menu labels are shown briefly (enabled), then they're substituted with the disabled placeholders.

I guess this is a fall-out from bug 356078.

Comment 12

[Stefan [:stefanh]]

--> cocoa (should probably be handled there)

Comment 13

[Hiro]

In Firefox, if the customizing seat of the toolbar is shut, all menus become "SomeMenuItem". 

Comment 14

[Josh Aas]

Attachment: fix v1.0

This code isn't really necessary and it causes this problem. Whether or not the submenu item is enabled only matters when the menu is showing, and that is take care of when the menu is constructed.

Comment 15

[Georg Maaß]

Is that fix already available inside a nighly build for testing?

Comment 16

[Josh Aas]

No, it won't be in a nightly build until about 24 hours after this bug is marked as "FIXED".

Comment 17

[Håkan Waara]

(In reply to comment #14)
> Created an attachment (id=244300) [edit]
> fix v1.0
> 
> This code isn't really necessary and it causes this problem. Whether or not the
> submenu item is enabled only matters when the menu is showing, and that is take
> care of when the menu is constructed.

Josh, you sure you posted the right patch?

Comment 18

[Josh Aas]

Attachment: fix v1.0 (for real!)

Oops, that was the wrong patch. Here is the right one.

Comment 19

[Håkan Waara]

Comment on attachment 244340 [details] [diff] [review]
fix v1.0 (for real!)

Now that you rely on the behavior, I think you need to ensure the menu is always rebuilt and set mNeedsRebuild to true too (maybe in this case you were lucky, and someone else did for you?), see http://landfill.mozilla.org/mxr-test/mozilla/source/widget/src/cocoa/nsMenuX.mm#411 and other places where this is significant.

Comment 20

[Josh Aas]

I don't think any dependencies are different here - we rebuild any time an attribute (including disabled state) changes, and we also rebuild when we are based on a new DOM node, and there really isn't anywhere else we need to do it that I know of. Did you have something specific in mind? Maybe see nsMenuX::AttributeChanged.

Comment 21

[Håkan Waara]

(In reply to comment #20)
> I don't think any dependencies are different here - we rebuild any time an
> attribute (including disabled state) changes, and we also rebuild when we are
> based on a new DOM node, and there really isn't anywhere else we need to do it
> that I know of. Did you have something specific in mind? Maybe see
> nsMenuX::AttributeChanged.

Sorry, I'll explain myself better.

What I meant is, since SetEnabled() now does nothing on runtime (whereas it used to try and disable existing items), and relies on the rebuild, it would be nice to be explicit about this assumption for safety, and do something like:

SetEnabled(PRBool aEnable) 
{
 if (aEnable != mEnabled) {
   // this call will have no effect without rebuilding, so why not be explicit about it?
   SetRebuild(PR_TRUE); 
   mEnabled = aEnable;
 }
 return NS_OK;
}

As you point out, SetRebuild() is already called shortly before SetEnabled() from AttributeChanged(), but what if someone else uses SetEnabled() later and forgets about it?  I just prefer making assumptions explicit, in general... What do you think?

Comment 22

[Josh Aas]

Attachment: fix v1.1

Comment 23

[Josh Aas]

fixed on trunk

Comment 24

[Stefan [:stefanh]]

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