[DUPE-452][3.x] Ext.menu.Menu hide event fires after the destroy event

[draduc]

The code i used for the test:



Ext.onReady(function(){
Ext.getDoc().on({
'contextmenu':{
scope:this,
fn:function(e){
e.preventDefault();
if(!this.testMenu){
this.testMenu = new Ext.menu.Menu({
items:[
{
text:'destroy menu',
scope:this,
handler:function(){
this.testMenu.destroy();
}
}
]
});
Ext.util.Observable.capture(this.testMenu,function(eventName){
console.log('menu fire: ' + eventName);
});
this.testMenu.showAt(e.getXY());
}
}
}
});
});



The problem: Throws the following JS error :



me.dom is undefined
.../script/lib/ext/ext-all-debug.js
Line 3844



The cause: The menu beforehide / hide set of events fires after the beforedestroy/destroy set of events as shown by the console output posted below


...
menu fire: mouseout
menu fire: mouseover
menu fire: beforedestroy
menu fire: destroy
menu fire: itemclick
menu fire: click
menu fire: beforehide


Suggested resolution: hide the menu before the handler is executed. As i'm not using any special menu items, this workaround seems to mask the problem.

The above case is the most obvious one, however, the same error appears when clicking a menu item triggers the destruction of a component that it is bound to.

Thank you!

[Condor]

Destroying components from within handlers is usually a bad idea, because the code that follows the handler call could still rely on the component being present.

I would recommend using:

this.testMenu.destroy.defer(1, this.testMenu);

This way the click can be fully handled before the destroy is executed.

[draduc]

Shouldn't this be fixed? Or at least made 'dumb' proof so that it checks the objects before calling methods on them?

This is the most basic isolation i found for this problem, initially it popped out in situations quite different from this one.

In my case the menu is bound to a grid as a context menu. The grid itself is part of a tabpanel with closable tabs. The same error appears if i:
- right click on a grid row (row context menu pops up)
- close the tab that contains the grid while the context menu is still displayed

Is this a bug or just expected behaviour? I'm asking because that event chain just doesn't look right....i mean i'm ok with the itemClick and handler priority but the component tries to hide itself after it has been destroyed...

[Jamie Avins]

We will look at it for 3.2.

[Pyja]

Hi,

1. Draduc suggested hotfix : Suggested resolution: hide the menu before the handler is executed
First i found this solution, but when your menu has to stay open, u don't have to decide to hide it. Simple to write for a simple example, but difficult for complex usages.

2. Condor suggested hotfix :
this.testMenu.destroy.defer(1, this.testMenu)) can be applied in this (simple) testcase, but when you have params that can add menus, and some superclasses, and more, u don't know if you have a menu, floating menus,... And put a defer EVERYWHERE is not the solution for me

I don't agree for these 2 previous solutions, because it's the framework role to manage this case for u !

some pretty solutions for me :

1. @ end of Component.destroy, put
this.rendered = false; So, when some event tells to hide, it first test rendered, and so do not hide (it's okay, because it's destroyed)


Ext.Component.override({
destroy : function(){
if(!this.isDestroyed){
if(this.fireEvent('beforedestroy', this) !== false){
this.destroying = true;
this.beforeDestroy();
if(this.ownerCt && this.ownerCt.remove){
this.ownerCt.remove(this, false);
}
if(this.rendered){
this.el.remove();
if(this.actionMode == 'container' || this.removeMode == 'container'){
this.container.remove();
}
}
this.onDestroy();
Ext.ComponentMgr.unregister(this);
this.fireEvent('destroy', this);
this.purgeListeners();
this.destroying = false;
this.isDestroyed = true;
this.rendered = false; // ADDED LINE
}
}
}
});


2. in some events, check that element is not destroyed :
if (!this.isDestroyed)

I prefer the Solution 2, pretty for me, but in my project i set the Solution 1, more easy to write (Solution 2 requires to know all events/methods that want to do some work on el


Bye

Pyja

[Jamie Avins]

The speciic issue is a dupe of Bug #438. Solution 2 is actually what was taken there. There's a race condition for the onWindowResize event we cannot stop (without a buffer). An additional isDestroyed check has been added and resolves the issue.