[2.1][FIXED] EditorGridPanel.destroy() does not destroy associated GridEditors

[mystix]

Destroying an EditorGridPanel's ColumnModel does not destroy any GridEditors defined in its ColumnModel, resulting in a memory leak.

to reproduce this problem, type the following into the Firebug console in the Editor Grid Example:

var grid = Ext.getCmp(Ext.get('editor-grid').first().id);
var id = grid.getColumnModel().getCellEditor(3, 0).field.id; // get the "Available" column's DateField id

console.log(Ext.getCmp(id)); // verify that we've got the correct component

grid.destroy(); // destroy the editorgrid
console.log(Ext.getCmp(id)); // DateField is still registered with the ComponentMgr -- i.e. it hasn't been destroyed

the following override resolves this (without destroying the ColumnModel, but rendering the same ColumnModel unusable for other EditorGridPanels):

Ext.override(Ext.grid.EditorGridPanel, {
  onDestroy: function() {
    var cols = this.getColumnModel().config;
    for(var i = 0, len = cols.length; i < len; i++){
      var c = cols[i];
      Ext.destroy(c.editor);
    }
    Ext.grid.EditorGridPanel.superclass.onDestroy.call(this);
  }
});

1 question remains though -- should the ColumnModel for the EditorGridPanel also be destroyed when it is destroyed?
For a GridPanel, it makes sense not to destroy the ColumnModel since it can in theory be re-used (so the GridPanel.onDestroy() method is correct in simply purging the ColumnModel's listeners).

For an EditorGridPanel though, though it is perfectly ok to reuse its ColumnModel in a different EditorGridPanel, there's no ColumnModel.destroy() method to mop up any lingering GridEditors should the ColumnModel be destroyed (i.e. simply nullfied).

[brian.moeskau]

Added in SVN, thanks. I think if someone is reusing a column model, they will just need to know to destroy their final grid before nulling the column model. I doubt that's a common issue.

[keztrel]

I had some problems with this addition, since it was added in Ext 2.2

I didn't had time to go into but with

Ext.override(Ext.Editor, {
    beforeDestroy : function(){
        if (this.field) {
            this.field.destroy();
            this.field = null;
        }
    }
}); 

it worked.

[mystix]

I had some problems with this addition, since it was added in Ext 2.2

I didn't had time to go into but with

Ext.override(Ext.Editor, {
    beforeDestroy : function(){
        if (this.field) {
            this.field.destroy();
            this.field = null;
        }
    }
}); 

it worked.

what "problems" exactly?

[Condor]

what "problems" exactly?

Problem described here.

[mystix]

Problem described here.

current code in SVN is this actually:

Ext.override(Ext.Editor, {
    beforeDestroy : function(){
        Ext.destroy(this.field);
        this.field = null;
    }
});

so that issue should be taken care of nicely.