Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#7706 closed defect (fixed)

ContentPane: widgets in templates getting destroyed twice

Reported by: hotheartboy Owned by: bill
Priority: low Milestone: 1.3
Component: Dijit Version: 1.2beta
Keywords: widget destroy Cc: bill
Blocked By: Blocking:

Description (last modified by bill)

Destroying a ContentPane that contains widgets with widgetsInTemplate=true, will destroy widgets defined in templates twice:

  1. when the host widget is destroyed (IE, InlineEditBox.destroy() will destroy the buttons used in it's template)
  2. ContentPane.destroyDescendants() calls getDescendants(), which incorrectly returns those buttons in InlineEditBox?'s template in addition to the InlinEditBox itself.

Original summary (dup of #7709)


When a template file includes widgets,the top widget will call the included widgets's destroy() twice.One happened in the destroyDescendants() of the dijit._Widgets. Other happened in the empty() of the dojo.html.

dijit._Widgets.destroyDescendants():

dojo.forEach(this.getDescendants(), function(widget){
                        if(widget.destroy){
                                widget.destroy(preserveDom);
                        }
                });

dojo.html.empty():

dojo.forEach(this.parseResults, function(w) {
                        if(w.destroy){
                                w.destroy();
                        }
                });

Should I fix it? Please help me. Thank you!

Change History (8)

comment:1 Changed 11 years ago by bill

Component: GeneralDijit
Milestone: 1.21.3
Owner: anonymous deleted
Summary: a question about dijit 1.2widgets in templates getting destroyed twice

This should be not be happening. getDescendants() should not return widgets defined in templates (such as the buttons in InlineEditBox). As far as I know, it doesn't, because it searches inside of this.containerNode only.

If you have a test case where this fails then please attach it using the attach file button.

comment:2 Changed 11 years ago by hotheartboy

Sorry.I should tell that it happened in dijit.layout.ContentPan?.

The code in dijit.layout.ContentPan?._setContent():

setter = this._contentSetter = new dojo.html._ContentSetter();
....
setter.set( (dojo.isObject(cont) && cont.domNode) ? cont.domNode : cont );

the setter will parse widgets in the content and fill these widgets instances into the parseResults variable.

The code in dojo.html._ContentSetter._parse():

...
this.parseResults = dojo.parser.parse(rootNode, true);
...

The code in dojo.html._ContentSetter.empty():

...
dojo.forEach(this.parseResults, function(w) {
	if(w.destroy){
		w.destroy();
	}
});
...

It causes that the ContentPane? widget will call the included widgets's destroy() twice.One happened in the destroyDescendants() of the dijit._Widgets. The other one happened in the empty() of the dojo.html.

Thank you.

comment:3 Changed 11 years ago by bill

Resolution: duplicate
Status: newclosed

Closing in favor of #7709.

comment:4 Changed 11 years ago by bill

Description: modified (diff)
Milestone: 1.31.4
Priority: highlow
Resolution: duplicate
Status: closedreopened
Summary: widgets in templates getting destroyed twiceContentPane: widgets in templates getting destroyed twice

Actually, there is a separate issue here (even after fixing #7709) in that if a ContentPane contains a widget with widgetsInTemplate==true, ContentPane's getDescendants() will include those widgets too.

Updating ticket description to reflect this.

comment:5 Changed 11 years ago by bill

See #6954 (umbrella ticket).

comment:6 Changed 11 years ago by bill

Milestone: 1.41.3
Owner: set to bill
Status: reopenednew

comment:7 Changed 11 years ago by bill

Resolution: fixed
Status: newclosed

Fixed in [15610]: Add code to ContentPane so it will destroy all widgets inside it's containerNode, even if they were inserted manually by the user. As before, ContentPane will also track things like Menus, even though the domNode is moved to <body>, provided that the Menu was added to the ContentPane via an href or an attr('content', ...).

Also fixed problem where nested ContentPanes wouldn't destroy properly because the outer ContentPane was calling destroy() on the inner ContentPane, and it's contents weren't being destroyed.

Finally, fixed issue with widgetsInTemplates getting destroyed twice by taking advantage of new getDescendants(true) API.

Fixes #7706, #7784, #7823. !strict

Patch is in conjunction with [15607]

comment:8 Changed 11 years ago by bill

(In [16904]) 1. Refactor [15607] (refs #6954, #7550, #7706, #7784, #7823) so that getDescendants() works as in 1.2 again, returning all descendant widgets, even nested widgets that are defined in templates. Expose the new functionality from [15607], to find direct descendants only, into a new _Widget.getChildren() method, rather than as flag to getDescendants().

This is because:

  • [15607] gave getDescendants() a subtle API change: it no longer found widgets inside of a dijit.Declaration (since no containerNode was defined)
  • because of that, Form was broken in that it didn't find form widgets inside of a custom widgets, declared with dijit.Declaration or dojo.declare, which didn't define this.containerNode
  1. Also, rolling back #7819: ContentPane?: missing an addChild() method (refs #7819), because of the backwards compatibility issue for custom widgets that extend ContentPane? and also extend _Container/_Contained.

Summary:

  • getChildren() is now supported by all widgets, and _Container widgets have (as before) an optimized implementation of getChildren() that overrides the one in _Widget.
  • ContentPane? not supporting any _Container methods except getChildren(). Users can set a ContentPane? child by calling attr('value', ...) etc.

!strict

Note: See TracTickets for help on using tickets.