OpenLayers OpenLayers

Ticket #1697 (closed bug: fixed)

Opened 3 months ago

Last modified 3 months ago

calling layer.destroy twice fails on vector layer

Reported by: tschaub Assigned to: tschaub
Priority: minor Milestone: 2.7 Release
Component: Layer.Vector Version: 2.6
Keywords: Cc:
State: Complete

Description

This is a symptom of a couple other issues in the vector layer, but it is worth fixing in this way. Specifically, if you call layer.removeFeatures with features undefined (and layer.features == null) or if you call layer.destroyFeatures in the same way, things fail. Both of these occur if you destroy a layer that is already destroyed.

Attachments

destroy2x.patch (2.1 kB) - added by tschaub on 08/22/08 19:58:44.
allow layer.destroy to be called twice
destroy2x.2.patch (2.1 kB) - added by tschaub on 09/02/08 17:03:07.
allow layer.destroy to be called twice

Change History

08/22/08 19:58:44 changed by tschaub

  • attachment destroy2x.patch added.

allow layer.destroy to be called twice

08/22/08 20:02:24 changed by tschaub

  • state set to Review.

The first change to the vector layer actually checks if features is undefined (or evaluates to false). Previously, if the features arg was undefined, we created an array of length 1 [undefined]. This never met the next condition (features.length <= 0) and so never returned.

The second change only touches feature.length and only calls removeFeatures if features is defined.

Thanks for the review.

(follow-up: ↓ 3 ) 08/26/08 03:01:17 changed by elemoine

Tim, thanks for the patch. One comment: with your patch if one does: 

layer.removeFeatures([]);

a featuresremoved event will be triggered, do we really want that?

09/02/08 17:03:07 changed by tschaub

  • attachment destroy2x.2.patch added.

allow layer.destroy to be called twice

(in reply to: ↑ 2 ) 09/02/08 17:04:04 changed by tschaub

  • owner changed from crschmidt to tschaub.

Replying to elemoine:

Tim, thanks for the patch. One comment: with your patch if one does: {{{ layer.removeFeatures([]); }}} a featuresremoved event will be triggered, do we really want that?

Good point, Eric. The updated patch corrects this.

Thanks for any review.

09/03/08 03:45:56 changed by elemoine

  • state changed from Review to Commit.

Looks good. Please commit (before committing you could actually change to loop and apply the trick to avoid the calculation of the array length on each iteration).

09/03/08 15:16:29 changed by tschaub

  • status changed from new to closed.
  • state changed from Commit to Complete.
  • resolution set to fixed.

(In [7941]) Allow layer.destroy to be called twice without failing for the vector layer. r=elemoine (closes #1697)