Opened 11 years ago

Closed 11 years ago

#7727 closed defect (fixed)

ContentPane: destroy() is destroying children, should only destroy ContentPane itself

Reported by: hotheartboy Owned by:
Priority: high Milestone: 1.2
Component: Dijit Version: 1.2beta
Keywords: Cc: bill
Blocked By: Blocking:

Description

Sorry!I had maybe reported the same question.But I explained vaguely at that time.

Now I try!

The code in dijit.layout.ContentPane?:

	destroy: function(){
		if(this._beingDestroyed){
			return;
		}
		this._onUnloadHandler();
		this._beingDestroyed = true;
		this.inherited(arguments);
	}

the "this._onUnloadHandler()" will call the _contentSetter.empty();

The code is:

	_onUnloadHandler: function(){
		this.isLoaded = false;
		this.cancel();
		
		if(this._contentSetter) {
			this._contentSetter.empty(); 
		}
		try{
			this.onUnload.call(this);
		}catch(e){
                         ...
		}
	}

The empty() will destroy child widgets in html content.

I'm confused! Is the destroy() the same with the destroyDescendants()?

If it should be true,it cause that the dijit.layout.StackContainer? happens wrongs when the closeChild() exeutes.

	closeChild: function(/*Widget*/ page){
		var remove = page.onClose(this, page);
		if(remove){
			this.removeChild(page);
			page.destroyRecursive();
		}
	}

The code in the dijit._Widget:

	destroyRecursive: function(/*Boolean?*/ preserveDom){
		this.destroyDescendants(preserveDom);
		this.destroy(preserveDom);
	}

Since the destroy() is the same with the destroyDescendants(),wrongs will happen when the destroyRecursive() executes.

I'm from china.I'm poor in eglish.please not joke to me.

Thank you!

Change History (4)

comment:1 Changed 11 years ago by hotheartboy

If destroyDescendants() is the destroy(),it will cause child widgets to destory twice.

comment:2 Changed 11 years ago by bill

Summary: Is the destroy() the same with the destroyDescendants() in dijit.layout.ContentPane?ContentPane: destroy() is destroying children, should only destroy ContentPane itself

Looks like a bug. Thanks for pointing it out. As you suspected, destroy() should only destroy the ContentPane itself and should *not* destroy the children/descendants.

After [15332] I don't think the child widgets will be destroyed twice (although this._contentSetter.empty() will be called twice). Can you try with the latest code?

There's still a bug in that ContentPane.destroy() isn't operating according to spec, but at least (I think) the double destroy problem is fixed. I think the call to

		if(this._contentSetter) {
			this._contentSetter.empty(); 
		}

in _onUnloadHandler() should be removed?

comment:3 Changed 11 years ago by hotheartboy

I agree with you!

comment:4 Changed 11 years ago by bill

Resolution: fixed
Status: newclosed

(In [15339]) More ContentPane? destruction related fixes:

  1. Emptying of contents now handled exclusively in destroyDescendants(); removed other calls to setter.empty().
  2. destroyDescendants() calls _onUnloadHandler() instead of destroy() (TODO: loading an href will call _onUnloadHandler() twice, once when the "Loading..." message is displayed and once when the actual new content is displayed)
  3. make ContentPane? override destroyRecursive() instead of overriding destroy(), since the standard way to destroy widgets is destroyRecursive() (see #5796)
  4. comments, plus mising semicolon

Fixes #7727 !strict

Note: See TracTickets for help on using tickets.