PDA

View Full Version : [FIXED-274][3.??] Calling close() on a hidden window skips destruction?



arandal
28 Sep 2009, 8:37 AM
I just ran into a possible problem the other night while checking my app for leaks. I have a TabPanel in a ViewPort where the customer can open and close many Panels (interfaces) at will. Some of these interfaces manage dialog windows. The first time the customer requests the dialog, I create it. When the customer dismisses the dialog I hide it. On subsequent requests I re-show it.

The problem occurs when the customer closes the tab page and I call close() on a hidden window. When calling close() on a hidden window, the close event does not fire and the window is not destroyed.

Here's the problem:

Looking at the close() method we see that it unconditionally calls hide() and then relies upon the callback from hide() to fire the close event and call destroy().


close : function(){
if(this.fireEvent("beforeclose", this) !== false){
this.hide(null, function(){
this.fireEvent('close', this);
this.destroy();
}, this);
}
}
If we look at the hide() method, we see that it returns prior to using the callback if the window is hidden causing the cleanup to be skipped.


hide : function(animateTarget, cb, scope){
if(this.hidden || this.fireEvent("beforehide", this) === false){
return;
}
if(cb){
this.on('hide', cb, scope, {single:true});
}
...
...
...
}
I realize of course that I could simply call the destroy() method, however, just about any other dev tool I have used that provides dialog window functionality normally gives the option to close or hide the dialog and if you choose to hide it, you have to remember to close it to clean it up.

Now for my question: is this by design or is it a bug? If it is by design, then I guess I have to remember to call destroy() when my window is hidden. If it is a bug then I propose the following as a fix.



close : function(){
if(this.fireEvent("beforeclose", this) !== false){
if( this.hidden === false ){
this.hide(null, this.doClose, this);
}else{
this.doClose();
}
}
},
doClose: function(){
this.fireEvent('close', this);
this.destroy();
}
Thank you for your attention to this.

evant
28 Sep 2009, 8:50 AM
Agreed, this is a bug.

Ultra simple test case:



Ext.onReady(function(){
var w = new Ext.Window({
width: 400,
height: 400,
title: 'foo',
listeners: {
destroy: function(){
console.log('hai');
}
}
});
w.show();
w.hide.defer(1000, w);
w.close.defer(2000, w);
});

evant
28 Sep 2009, 8:54 AM
Fix applied to svn in rev #5430 for patch release 3.0.3.

arandal
28 Sep 2009, 9:00 AM
Thanks for the quick response Evan.