Opened 10 years ago

Closed 6 years ago

#10296 closed defect (invalid)

Menu: Destroying menu items with focus leaves widget in a bad state

Reported by: Adam Peller Owned by: Adam Peller
Priority: low Milestone: future
Component: Dijit Version: 1.4.0b
Keywords: Cc:
Blocked By: Blocking:

Description

If a child of a dijit.Menu is removed or otherwise destroyed, such as with destroyDescendants(), and that item had focus, this.focusedChild will point at a stale widget reference. A subsequent call to focus will fail because the domNode of that widget is null if it was destroyed, otherwise it may just point at a widget no longer in the hierarchy.

To fix this, we might override removeChild() to clean out focusedChild as necessary and have the focus method check this.focusedChild.domNode != null. Alternatively, we could track mutation events

Change History (8)

comment:1 Changed 10 years ago by Adam Peller

the workaround is to set menu.focusedChild=null when removing or destroying children with focus.

comment:2 Changed 10 years ago by bill

Actually dijit.form.Select has that workaround code:

// this.inherited destroys this.dropDown's child widgets (MenuItems).
// Avoid this.dropDown (Menu widget) having a pointer to a destroyed widget (which will cause
// issues later in _setSelected).
if(this.dropDown){
	delete this.dropDown.focusedChild;
}

It can (and should) be removed when this bug is fixed.

comment:3 Changed 10 years ago by bill

Milestone: tbd1.5
Owner: set to bill
Status: newassigned

comment:4 Changed 9 years ago by Adam Peller

Milestone: 1.51.6

comment:5 Changed 9 years ago by bill

Milestone: 1.6future
Summary: Destroying menu items with focus leaves widget in a bad stateMenu: Destroying menu items with focus leaves widget in a bad state

comment:6 Changed 7 years ago by bill

Priority: highlow

comment:7 Changed 6 years ago by bill

Owner: changed from bill to Adam Peller
Status: assignedpending

Is this still an issue? I agree that this.focusedChild will point to an abandoned node, but I don't see any focus() method that tries to focus this.focusedChild.

Note that deleting or hiding a node with focus is bad news in general, at least on IE, but probably it should be up to the app code to put focus somewhere else before deleting the focused node.

comment:8 Changed 6 years ago by trac-o-bot

Resolution: invalid
Status: pendingclosed

Because we get so many tickets, we often need to return them to the initial reporter for more information. If that person does not reply within 14 days, the ticket will automatically be closed, and that has happened in this case. If you still are interested in pursuing this issue, feel free to add a comment with the requested information and we will be happy to reopen the ticket if it is still valid. Thanks!

Note: See TracTickets for help on using tickets.