Opened 8 years ago

Closed 8 years ago

#12878 closed defect (fixed)

ContentPaneResizeMixin: _startChildren does not check _started flag first

Reported by: Daniel Trang Owned by: bill
Priority: high Milestone: 1.7
Component: Dijit Version: 1.6.1
Keywords: Cc:
Blocked By: Blocking:

Description

the "_startChildren" method in class "dijit.layout._ContentPaneResizeMixin" does not check to make sure the child widget has not been started before calling startup() on the widget.

should look like this: dojo.forEach(this.getChildren(), function(child){

if(!child._started) {

child.startup(); child._started = true;

}

});

thanks

Change History (4)

comment:1 Changed 8 years ago by bill

Component: GeneralDijit

What makes you think that it should check that flag?

comment:2 in reply to:  1 Changed 8 years ago by Daniel Trang

If the flag is not checked startup() may be called more than once during a widgets lifecycle resulting in potentially multiple calls to initiation code.

Ex: I create a widget that I intend to reuse. I run startup after instantiating it. I later add the widget to a dijit Dialog, startup the Dialog, and run show(). Because Dialog extends dijit.layout.ContentPane? it will run the inherited _startChildren method resulting in my reuse widget's startup method being called again.

Replying to bill:

What makes you think that it should check that flag?

comment:3 Changed 8 years ago by bill

Milestone: tbd1.7
Owner: set to bill
Status: newassigned
Summary: _startChildren does not check _started flag firstContentPaneResizeMixin: _startChildren does not check _started flag first

What I meant was that a widget's startup() method is idempotent because widgets check this._started inside of startup():

startup: function(){
	if(this._started){ return; }
	...

Admittedly though there are some places in dijit that also check the _started flag outside of the startup() call, like in parser.js, _WidgetInTemplate.js, _Container.js, AccordionContainer.js ex:

if(this._started && !widget._started){
	widget.startup();
}

Anyway, I'll update that code just to be safe.

comment:4 Changed 8 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [24762]) Check if child._started is set before calling startup(). Not sure if this check is necessary but other code does it. Fixes #12878 !strict, thanks mrnoouts.

Note: See TracTickets for help on using tickets.