Opened 12 years ago

Closed 12 years ago

#6122 closed defect (fixed)

TabContainer: failure on unload on IE7

Reported by: bill Owned by: bill
Priority: high Milestone: 1.1
Component: Dijit Version: 1.0
Keywords: Cc:
Blocked By: Blocking:

Description

After attaching MSE7 to IE7 running themeTester, and then refreshing the page, get exception while unloading the (initial) page, in Menu::unBindDomNode because it's trying to unbind from dijit_layoutTabButton_5 but dojo.byId("dijit_layoutTabButton_5") is null.

Looks like StackController? isn't properly cleaning up the Menus it created when it's destroyed. StackController? needs to clean up on destroy(), and also on onRemoveChild().

Proably also Menu should be more forgiving if the DOM node to which it was attached is no longer there, but the root problem is StackController? cleaning up Menus.

Attachments (2)

6122.patch (1.7 KB) - added by Becky Gibson 12 years ago.
remove menu in onRemoveChild and call onRemoveChild from stackcontainer.destroy()
StackContainer.patch (556 bytes) - added by guest 12 years ago.
Add a guard to ensure container exists

Download all attachments as: .zip

Change History (9)

comment:1 Changed 12 years ago by Adam Peller

Priority: normalhigh

Changed 12 years ago by Becky Gibson

Attachment: 6122.patch added

remove menu in onRemoveChild and call onRemoveChild from stackcontainer.destroy()

comment:2 Changed 12 years ago by Becky Gibson

Status: newassigned

I stepped this through the debugger and menus are now destroyed in onRemoveChild and onRemoveChild now gets called in stackController.destroy. However, since I could not reproduce the exact conditions I would appreciate it if someone could test before I commit.

comment:3 Changed 12 years ago by bill

Yup, I just tested it; looks good :-). No more errors.

comment:4 Changed 12 years ago by Becky Gibson

Resolution: fixed
Status: assignedclosed

(In [13052]) fixes #6122 Updated stackController.onRemoveChild to remove any menus. Updated stackController.destroy to call onRemoveChild for each page." /Users/bgibson/Documents/workspace/trunk/dijit/layout/StackContainer.js !strict

Changed 12 years ago by guest

Attachment: StackContainer.patch added

Add a guard to ensure container exists

comment:5 Changed 12 years ago by guest

Resolution: fixed
Status: closedreopened

I don't know why, but StackController?.destroy() is being invoked twice on a Rails+Dojo application I'm working on. I submitted a patch to guard against an undefined variable, but I'm not sure this fixes all cases, and it certainly doesn't fix the root cause behind destroy() being invoked twice.

-lalee

comment:6 Changed 12 years ago by bill

Owner: changed from Becky Gibson to bill
Status: reopenednew

Let me work on this some. I'm not sure what's happening with lalee's comment above, but page unload causes a call to

WidgetSet.forEach(function(widget){ widget.destroy(); })

... which unfortunately apparently deletes the widgets in a random order. In particular the TabContainer's children are getting deleted before the TabContainer or the TabController, which means that this code in TabController is doing nothing:

var container = dijit.byId(this.containerId);
dojo.forEach(container.getChildren(), this.onRemoveChild, this);

comment:7 Changed 12 years ago by bill

Resolution: fixed
Status: newclosed

(In [13089]) StackController?.destroy() shouldn't access StackContainer?'s children, because they may already have been deleted. (Case in point: on page unload.). Also, Menu.destroy() needs to be resilient if the target node has already been destroyed (which also happens on page unload).

Fixes #6122 !strict

Note: See TracTickets for help on using tickets.