Opened 7 years ago

Closed 7 years ago

#15815 closed defect (fixed)

Stackcontroller: destroyRecursive() throws exception

Reported by: doron Owned by: bill
Priority: undecided Milestone: 1.8
Component: Dijit Version: 1.8.0rc1
Keywords: Cc:
Blocked By: Blocking:

Description

Go to http://download.dojotoolkit.org/release-1.8.0rc1/dojo-release-1.8.0rc1/dijit/tests/layout/StackContainer.html

Run this js:

dijit.byId("holder").destroyRecursive()

Get this js error:

TypeError: root is null
http://download.dojotoolkit.org/release-1.8.0rc1/dojo-release-1.8.0rc1/dojo/dojo.js
Line 29

Change History (3)

comment:1 Changed 7 years ago by doron

The exception occurs in registry.js::findWidgets - the passed in root is null.

comment:2 Changed 7 years ago by bill

Milestone: tbd1.8
Summary: Stackcontroller can't be destroyed, throws exceptionStackcontroller: destroyRecursive() throws exception

OK thanks. I investigated, and there are two issues here:

StackController button widgets destroyed twice

This is complicated: even though the StackController buttons are internal to StackController (ie, not created directly by the app), they are inside of StackController.containerNode, which means that _WidgetBase.destroyRecursive() will destroy them. Then after that, this code in StackController.js runs and tries to destroy them again:

destroy: function(){
        for(var pane in this.pane2button){
                this.onRemoveChild(registry.byId(pane));
        }
...

Probably, the container of the buttons should not be labeled containerNode, since containerNode implies that the buttons are user defined widgets rather than internal widgets. I'll do that for 2.0

destroy() no longer idempotent

As discussed in http://thread.gmane.org/gmane.comp.web.dojo.user/68585, destroy() is no longer idempotent.

This is easy and safe to fix with some guard code, and given the number of people that have had issues with it I think it's worth adding into 1.8 final.

So I'll do that.

comment:3 Changed 7 years ago by bill

Resolution: fixed
Status: newclosed

In [29442]:

Make destroy() idempotent like it was in 1.7, to avoid various issues, fixes #15815 !strict

Note: See TracTickets for help on using tickets.