12247 - [BLOCKER]Can not add dynamic menuitems to toolbar menus

Status: VERIFIED FIXED

(Core :: XUL, defect, P1)

Description

[Radha on family leave (not reading bugmail)]

I want to add history menu items to the 'Go' menu dynamically. Though the
creation and attaching code etc... works, the menu actually doesn't show up. As
per last thursday's meeting, marking this as a blocker bug.

The code is already checked in, If you uncomment the UpdateGoMenu() call at the
bottom of the nsBrowserAppCore::OnEndDocumentLoad() you can see what is being
done.

Comment 1

[saari (gone)]

This worked last week... and it doesn't work on any platform?

Comment 2

[saari (gone)]

Well, this works for bookmarks, and there is no reason it shouldn't work here.

Comment 3

[saari (gone)]

I just walked through UpdateGoMenu and it doesn't seem to ever find the go menu.
We never make it to line 707 that breaks the for loop.

Comment 4

[saari (gone)]

radha do you want to double check this based on what I wrote above?

Comment 5

[Radha on family leave (not reading bugmail)]

Chris,  Sorry about that. Since I didn't want to turn on this feature with some
of my other work for back & forward menu, I didn't checkin navigator.xul where I
had added an id for 'Go' menu. I'll check it in as soon as the tree opens.Once
that is done, it s'd break out of the for loop and continue with creation of
menu. You can also set APP_DEBUG to 1, at the top of the file and get more debug
information.


I'll let you know as soon as I check in the xul file.

Comment 6

[Radha on family leave (not reading bugmail)]

Chris,  I have checked in navigator.xul which adds an id to go menu.

Comment 7

[saari (gone)]

Ok, I walked through UpdateGoMenu, and CreateMenuItem, and things look OK at
first glance, but the dynamic menu construction code doesn't see the new menuitem
at all...

I'm going to need some help with this one from hyatt or waterson at the content
model level.

Comment 8

[David Hyatt]

You can't set the id attribute dynamically right now.  You should try to work
around this bug if possible, since it may take a while to fix (e.g., just give
the go menu an id in the XUL).

Comment 9

[Chris Waterson]

Why isn't UpdateGoMenu written in JavaScript?

Comment 10

[Radha on family leave (not reading bugmail)]

The 'Go'  menu already has an id in XUL. The problem is, dynamically adding
children to the 'Go' menu seems to  succeed in all stages, but it doesn't
actually show up on the screen.

Comment 11

[Radha on family leave (not reading bugmail)]

I think I found what the problem is, Calling AppendChild after I have set the
menu items label, and other attributes shows the item on the screen. Originally,
I appended the child and then set the label.

Is onaction obsolete at this time?

Comment 12

[Radha on family leave (not reading bugmail)]

I got this working. Marking it fixed.

Comment 13

[phillip]

code level fix, marking verified. reopen if i'm wrong here...