Ticket #2289 (new bug)

Opened 4 years ago

Last modified 4 years ago

Function ''layer.destroyFeatures()'' doesn't call ''feature.destroy()'', although it is supposed to do so.

Reported by: pigletto Owned by: crschmidt
Priority: minor Milestone: 2.13 Release
Component: Layer.Vector Version: 2.8
Keywords: destroyFeatures Cc: pigletto@…
State:

Description

Function layer.destroyFeatures() doesn't call feature.destroy(), although it is supposed to do so. This is because features array is empty after call to this.removeFeatures(features, options) so subsequent calls to features[i].destroy() make no sense. This causes that popups associated with features may become disconnected zombies...

Current implementation of destroyFeatures:

destroyFeatures: function(features, options) {
        var all = (features == undefined); // evaluates to true if
                                           // features is null
        if(all) {
            features = this.features;
        }
        if(features) {
            this.removeFeatures(features, options);
            for(var i=features.length-1; i>=0; i--) {
                features[i].destroy();
            }
        }
    },

Attachments

patch-2289-r9721-A0.diff Download (0.5 KB) - added by elemoine 4 years ago.
popup_destroy_test.html Download (2.1 KB) - added by pigletto 4 years ago.
Test page showing zombie popups after destroyFeatures

Change History

Changed 4 years ago by elemoine

Changed 4 years ago by elemoine

I can confirm that there's a problem. Can you please try patch-2289-r9721-A0.diff Download?

Changed 4 years ago by pigletto

Test page showing zombie popups after destroyFeatures

Changed 4 years ago by pigletto

Patch works partially. I mean that after adding slice() the for loop with destroy() is executed properly, but the problem with popups that are left on the map still exists.

If you have a feature with popup opened and then call destroyFeatures (this may happen when using Cluster strategy or BBOX strategy) the function removeFeatures which is called inside destroyFeatures executes line:

  feature.layer = null;

This causes that further call to this.layer.map.removePopup(this.popup) in Feature.destroy doesn't work. I think that removeFeatures should check if there are any opened popups and remove them first.

I've attached example page that illustrates the problem.

Changed 4 years ago by elemoine

This is a different problem, which should be addressed in a separate ticket in my opinion.

The problem is that the OpenLayers.Feature.Vector:destroyPopup() method doesn't call its parent method. But I think this is intentional - vector features do not support having popups attached to them, see the comments in the OpenLayers.Feature.Vector class.

What you can do is create your own Feature class:

var Feature = OpenLayers.Class(OpenLayers.Feature.Vector, {
    destroyPopup: function() {
        OpenLayers.Feature.prototype.destroyPopup.apply(this, arguments);
     }
});

Changed 4 years ago by pigletto

Proposed solution with own Feature class works (after using your patch) :)

As suggested, I've submitted another ticket:  http://trac.openlayers.org/ticket/2296. If it is behaviour is intentional then I think it should be at last documented or maybe new Example page can be added.

Note: See TracTickets for help on using tickets.