Opened 12 years ago

Closed 12 years ago

#6948 closed defect (fixed)

Menu: destoryRecursive() doesn't destroy submenus

Reported by: anthony.fryer@… Owned by: bill
Priority: high Milestone: 1.2
Component: Dijit Version: 1.1.1
Keywords: dojo Widget destroy recursive menu Cc:
Blocked By: Blocking:

Description

If you run dijit/tests/test_menu.html and create/destroy/create the programmatic context menu, when you do the 2nd create you get an error saying "widget id progSubMenu is already registered". This is because pSubMenu is not being destroyed when pMenu.destroyRecursive is called. The reason it isn't being destroyed is because _Widget.destroyDescendants is only calling destroy() on the child widgets, not destroyRecursive(). Either destroyDescendants should always call destroyRecursive() or have a flag that is used to determine wether destroy or destroyRecursive is used.

Attachments (1)

_widget_patch.txt (541 bytes) - added by guest 12 years ago.

Download all attachments as: .zip

Change History (6)

Changed 12 years ago by guest

Attachment: _widget_patch.txt added

comment:1 Changed 12 years ago by guest

related to #4641

comment:2 Changed 12 years ago by bill

Cc: anthony.fryer@… removed
Component: CoreDijit
Milestone: 1.2
Owner: anonymous deleted
Reporter: changed from guest to anthony.fryer@…

No, destroyDescendants() gets a list of every descendant and calls destroy() on each one.

Menu is a complicated case because a child menu actually becomes a sibling of the parent menu (both dom nodes are direct children of <body>), so getDescendants() won't find the "child" of the PopupMenuItem?.... however that's why there is special code in Menu to delete the popup:

	destroyDescendants: function(){
		if(this.popup){
			this.popup.destroyRecursive();
			delete this.popup;   <------------ HERE
		}
		this.inherited(arguments);
	}

Not sure why you are seeing that error but I'll take a look.

comment:3 Changed 12 years ago by bill

Owner: set to bill
Status: newassigned

comment:4 Changed 12 years ago by bill

Summary: _Widget.destroyDescendants should call destroyRecursive() instead of destroy()Menu: destoryRecursive() doesn't destroy submenus

Oh I see... Menu.getDescendants() doesn't catch the submenu hanging off of the PopupMenuItem? (since it's a child of <body>) and thus it's never destroyed. This is a mess, but perhaps I can make things work better by reducing (or ideally eliminating) dependence on getDescendants()... _Container.destroyRecursive() could simply call destroyRecursive() on each child.

comment:5 Changed 12 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [13988]) For _Container based widgets, make destroyRecursive() work hierarchically, thus avoiding use of getDescendants() which has various issues (see tickets for details). Fixes #6948, refs #6954. !strict

Note: See TracTickets for help on using tickets.