#10813 closed defect (fixed)
crash when deleting SplitContainer's sizer - same id as it's thumb
Reported by: | Owned by: | bill | |
---|---|---|---|
Priority: | high | Milestone: | 1.5 |
Component: | Dijit | Version: | 1.3.0 |
Keywords: | Cc: | [email protected]… | |
Blocked By: | Blocking: |
Description
Problem
My application wants to remove a whole container hierarchy. It does this recursively by looping over all child elements. This fails: IE6 crashes and FF3.6 runs into an endless loop.
The 'cause' is that my recursion tries to continuously remove a dijit_layout_SplitterContainer_Splitter_<num> node.
A quick look into the DOM unveiled that the sizer has the same id attribute as it's child - the thumb.
The place in the code was found very quickly:
_addSizer: function(index){ index = index===undefined?this.sizers.length:index; // TODO: use a template for this!!! var sizer = dojo.doc.createElement('div'); sizer.id=dijit.getUniqueId('dijit_layout_SplitterContainer_Splitter'); this.sizers.splice(index,0,sizer); this.domNode.appendChild(sizer); sizer.className = this.isHorizontal ? 'dijitSplitContainerSizerH' : 'dijitSplitContainerSizerV'; // add the thumb div var thumb = dojo.doc.createElement('div'); thumb.className = 'thumb'; thumb.id = sizer.id; sizer.appendChild(thumb); // FIXME: are you serious? why aren't we using mover start/stop combo? this.connect(sizer, "onmousedown", '_onSizerMouseDown'); dojo.setSelectable(sizer, false); },
The offending line obviously is:
thumb.id = sizer.id;
This is still the case in trunk, revision 20689: http://trac.dojotoolkit.org/browser/dijit/trunk/layout/SplitContainer.js#L145
Fix
Appending "_thumb" to the thumbs id works:
thumb.id = sizer.id + '_thumb';
Change History (6)
comment:1 Changed 12 years ago by
Component: | General → Dijit |
---|---|
Owner: | anonymous deleted |
comment:4 follow-up: 6 Changed 12 years ago by
Milestone: | tbd → 1.5 |
---|---|
Owner: | set to bill |
Status: | new → assigned |
Yah, looks like the id is unneeded too (the tests work w/out it), so I'll take it out.
(BTW the thumb isn't visible at all in tundra but it is in soria.)
I'm not sure why you have to do manual recursion to destroy the SplitContainer though, destroyRecursive() works for most widgets. Maybe it doesn't work for SplitContainer since that code is deprecated.
comment:5 Changed 12 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:6 Changed 12 years ago by
Replying to bill:
I'm not sure why you have to do manual recursion to destroy the SplitContainer though, destroyRecursive() works for most widgets.
"works for most widgets" sounds nice but is not sufficient. The problems are the nodes which are not supported. We suffered from major memory leaks when we tried that (it was back in 1.1, now we're using 1.3.1), so we had to write our own recursive deletion code.
Thanks for the commit.
the FIXME comment a few lines below is curious also. there are many unresolved issue with this deprecated widget, but this one appears simple enough. thanks for this information