375011 - menu separators cannot be unhidden if they are created hidden

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Josh Aas]

Right now you can't unhide menu separator DOM nodes after creating them hidden. That is because we don't create an actual nsIMenuItem object for separators, which would listen for DOM node attribute changes. We should fix this someday though usage patterns make this bug not such a big deal. If anyone messes with the hidden attribute on a separator they are probably going to touch one of the separator's sibling menu items at the same time and cause the whole menu to get rebuilt.

Comment 1

[Josh Aas]

Attachment: fix v1.0

This makes our code much more clean and capable re: separators, and it would be even better if we could modify the nsIMenu* apis but I believe we're not allowed to do that now.

Comment 2

[Colin Barrett [:cbarrett]]

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

We don't create a lot of these types of menu items, so I don't think this will impact memory usage or performance too much.

Now that we have a bit better handle on lifetime issues for menu items and can insert and add things more flexibly, should we look at reducing the number of rebuilds we do?

Comment 3

[Josh Aas]

Perhaps for Mozilla 2 but that is too risky for questionable perf wins at this point and it wouldn't make our behavior any better.

Comment 4

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

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

You're allowed to modify nsIMenu* APIs anytime, they're not real APIs that other people use.

Comment 5

[Josh Aas]

Attachment: fix v1.0.1

This is the patch I'm going to check in, it is the same as v1.0 but I removed unused stuff from the interfaces to reduce the number of arguments passed around.

Comment 6

[Josh Aas]

fixed on trunk, also bumped UUIDs