From e603b067151ca8088793a8451038716f7c14b08d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Lemoine?= Date: Tue, 29 May 2012 23:01:36 +0200 Subject: [PATCH 1/3] make SelectControl.unselectAll safer --- lib/OpenLayers/Control/SelectFeature.js | 8 +++++-- tests/Control/SelectFeature.html | 28 +++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/lib/OpenLayers/Control/SelectFeature.js b/lib/OpenLayers/Control/SelectFeature.js index e5129ce9fc..9d2064bfd7 100644 --- a/lib/OpenLayers/Control/SelectFeature.js +++ b/lib/OpenLayers/Control/SelectFeature.js @@ -306,8 +306,12 @@ OpenLayers.Control.SelectFeature = OpenLayers.Class(OpenLayers.Control, { layer = layers[l]; for(var i=layer.selectedFeatures.length-1; i>=0; --i) { feature = layer.selectedFeatures[i]; - if(!options || options.except != feature) { - this.unselect(feature); + // feature can be undefined here if an unselectfeature + // listeners has unselected or removed other features + if(feature) { + if(!options || options.except != feature) { + this.unselect(feature); + } } } } diff --git a/tests/Control/SelectFeature.html b/tests/Control/SelectFeature.html index 68e3031165..fbf47989f4 100644 --- a/tests/Control/SelectFeature.html +++ b/tests/Control/SelectFeature.html @@ -558,6 +558,34 @@ t.eq(layer1.renderer.getRenderLayerId(), layer1.id, "Root container moved correctly when control is destroyed and layers was an array parameter"); } + + function test_unselectAll(t) { + t.plan(1); + + var layer = new OpenLayers.Layer.Vector(); + + var control = new OpenLayers.Control.SelectFeature(layer); + + var feature1 = new OpenLayers.Feature.Vector(); + var feature2 = new OpenLayers.Feature.Vector(); + var feature3 = new OpenLayers.Feature.Vector(); + + layer.addFeatures([feature1, feature2, feature3]); + + control.select(feature1); + control.select(feature2); + control.select(feature3); + + layer.events.on({ + featureunselected: function(e) { + layer.removeFeatures([feature2]); + } + }); + + control.unselectAll(); + t.eq(layer.selectedFeatures.length, 0, + 'unselectAll unselects all features'); + } From 976554fc825b3b56f2d0d79b05b29a9794cb4a11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Lemoine?= Date: Wed, 30 May 2012 09:00:42 +0200 Subject: [PATCH 2/3] make SelectControl.unselectAll safer, use @ahocevar's implementation --- lib/OpenLayers/Control/SelectFeature.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/OpenLayers/Control/SelectFeature.js b/lib/OpenLayers/Control/SelectFeature.js index 9d2064bfd7..198cef9b1b 100644 --- a/lib/OpenLayers/Control/SelectFeature.js +++ b/lib/OpenLayers/Control/SelectFeature.js @@ -300,18 +300,17 @@ OpenLayers.Control.SelectFeature = OpenLayers.Class(OpenLayers.Control, { */ unselectAll: function(options) { // we'll want an option to supress notification here - var layers = this.layers || [this.layer]; - var layer, feature; - for(var l=0; l=0; --i) { - feature = layer.selectedFeatures[i]; - // feature can be undefined here if an unselectfeature - // listeners has unselected or removed other features - if(feature) { - if(!options || options.except != feature) { - this.unselect(feature); - } + numExcept = 0; + while(layer.selectedFeatures.length > numExcept) { + feature = layer.selectedFeatures[numExcept]; + if(!options || options.except != feature) { + this.unselect(feature); + } else { + ++numExcept; } } } From e3d1d3ea625e380c895090766c6745610b94e4df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89ric=20Lemoine?= Date: Wed, 30 May 2012 09:01:15 +0200 Subject: [PATCH 3/3] more tests for SelectControl.unselectAll --- tests/Control/SelectFeature.html | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/Control/SelectFeature.html b/tests/Control/SelectFeature.html index fbf47989f4..150ab07c5c 100644 --- a/tests/Control/SelectFeature.html +++ b/tests/Control/SelectFeature.html @@ -560,31 +560,43 @@ } function test_unselectAll(t) { - t.plan(1); + t.plan(2); var layer = new OpenLayers.Layer.Vector(); var control = new OpenLayers.Control.SelectFeature(layer); var feature1 = new OpenLayers.Feature.Vector(); + feature1.id = 1; var feature2 = new OpenLayers.Feature.Vector(); + feature2.id = 2; var feature3 = new OpenLayers.Feature.Vector(); + feature3.id = 3; + var feature4 = new OpenLayers.Feature.Vector(); + feature4.id = 4; - layer.addFeatures([feature1, feature2, feature3]); + layer.addFeatures([feature1, feature2, feature3, feature4]); control.select(feature1); control.select(feature2); control.select(feature3); + control.select(feature4); layer.events.on({ featureunselected: function(e) { - layer.removeFeatures([feature2]); + // we change the selectedFeatures array while + // unselectAll is iterating over that array. + if(feature2.layer) { + layer.removeFeatures([feature2]); + } } }); - control.unselectAll(); - t.eq(layer.selectedFeatures.length, 0, - 'unselectAll unselects all features'); + control.unselectAll({except: feature3}); + t.eq(layer.selectedFeatures.length, 1, + 'unselectAll unselected all but one'); + t.eq(layer.selectedFeatures[0].id, 3, + 'the remaining selected features is the one expected'); }