calling map.destroy causes problem with legend and tree components

demonstration of the problem

Hey Tim, you mean so we can stop sending remove events from the LayerStore when the OL map is destroying? I also think that's the solution to these problems.

For my problem I was able to "fix" things by overriding the Ext.Store remove function like this:

    remove : function(record){
        var index = this.data.indexOf(record);
        this.data.removeAt(index);
        if(this.pruneModifiedRecords){
            this.modified.remove(record);
        }
        if(this.snapshot){
            this.snapshot.remove(record);
        }
        if (this.map.paddingForPopups !== null) {
            this.fireEvent("remove", this, record, index);
        }
    },

Ofcourse the paddingForPopups part is just a hack right now to check if the OL map is destroying, ideally we would have events like you correctly state above.

Added an OpenLayers enhancement ticket for the destroy events (I think the most important one right now is the beforedestroy event on the Map object):

http://trac.openlayers.org/ticket/2136

patch fixing the issue, depends on OL ticket 2136

(In [1067]) Avoid endlessly calling onImageLoadError by registering it as a single listener each time the image source is set. r=ahocever (closes #90)

Apologies for the typo in the commit. r1067 was meant to close #91.

fix the problem at its root

The above patch fixes a couple of things that were wrong in LayerNode and LayerContainer: onDestroy is not available in all Ext classes! We have to override destroy here. Also, the LayerNode has to take care to remove events when being destroyed.

Anyway, no need for beforedestroy events from OpenLayers here. Reason: the destroy process of the OL map removes layers, triggering removelayer events, and these get handled by our GeoExt components properly. The problem is that we destroy the tree, but all the destroyed nodes still have all event handlers.

The attached patch fixes the issue. Bart, can you please report which of your issues reported in #88 still remain?

Please review.

Nice Andreas!

I'll try to sneak in a review sometime soon (may be Monday). Thanks for tackling this.

Hi Andreas, I tested your patch but I am still getting errors in IE7 on reload. I've removed the legend from my app to be sure it's coming from the layer tree. It's still complaining about El._flyweights not being there.

Bart: I would say we continue tracking the IE unload issues in #88 then. I am sure there are more places where we do not take care to clean up after ourselves properly, and we have to fix these. To me, it feels like if OpenLayers destroys everything properly, there should be no need to know when it starts destroying. I'll see if I can come up with a test case showing the unload issues in IE.

proper destroy and listener handling

This looks good. I made some changes to the OL event on/un calls (they only take a single arg, not three).

Please commit.

Thanks Tim for fixing the OL on/un calls -- I was confused with the similar-but-not-the-same on/un methods in OL and Ext.

(In [1086]) clean up after ourselves properly, which we did not do enough and not right before. onDestroy() is not available everywhere in Ext, we have to override destroy(). r=tschaub (closes #90)