100122 - Dynamic Menus not showing on OSX

Status: RESOLVED FIXED

(Core :: XUL, defect, P1)

Description

[Joseph Elwell]

We dynamically hide and show a whole menu.

We have our Applications xul window contain various menu items, such as File,
Edit, etc. And one which starts hidden.

To do this we grab the menu id and setAttribute("hidden", "false")
and the menupopup we setAttribute("collapsed", "false").

When we try to show the menu item on OS X, nothing happens. The same code works
on Classic builds as well as Linux and Windows.

Comment 1

[Joseph Elwell]

pinkerton: can we target this for 0.9.5?

Comment 2

[Mike Pinkerton (not reading bugmail)]

ok, found why this doesn't work...

  // show this menu
  NS_ASSERTION(PR_FALSE, "nsMenuX::AttributeChanged: WRITE SHOW CODE.");

thanks beard ;) ;)

Comment 3

[saari (gone)]

hahahah :-)

Comment 4

[Mike Pinkerton (not reading bugmail)]

Attachment: patch to implement showing/hiding top-level menus

Comment 5

[Mike Pinkerton (not reading bugmail)]

need some r/sr luvin' on the patch. thanks!

Comment 6

[saari (gone)]

r=saari

Comment 7

[Simon Fraser [no longer active]]

+  if ( hiddenValue == NS_LITERAL_STRING("true") || collapsedValue == 
NS_LITERAL_STRING("true") )

Is that the optimal way to compare strings these days? How about 
hiddenValue.Equals()? I'm not sure.

+    if ( currMenu == this ) {

slicing may break this compare in future if the menu classes start implementing 
other APIs. Probably best to compare interface pointers.

+              ::InsertMenuItem(menubar, ::GetMenuTitle(mMacMenuHandle, title), 
indexToInsert + 1);

Does InsertMenuItem do meta-character interpretation? Could this cause problems?

Comment 8

[Mike Pinkerton (not reading bugmail)]

Attachment: take 2, addressing smfr's concerns

Comment 9

[Mike Pinkerton (not reading bugmail)]

new patch addresses the latter 2 of smfr's concerns. According to jag, .Equals() 
and == are the same and should be inlined to the same code.

Comment 10

[Simon Fraser [no longer active]]

Comment on attachment 50150 [details] [diff] [review]
take 2, addressing smfr's concerns

sr=sfraser

Comment 11

[chris hofmann]

a=chofmann for the 0.9.4 branch

Comment 12

[Mike Pinkerton (not reading bugmail)]

landed on branch. waiting for trunk to open.

Comment 13

[Mike Pinkerton (not reading bugmail)]

landed on trunk