Merge pull request openlayers#486 from elemoine/486

OpenLayers.Control.SelectFeature - unselectAll is unsafe
kochizufan · Jun 1, 2012 · 066859b · 066859b
2 parents bd27f6a + e3d1d3e
commit 066859b
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 5 deletions.
diff --git a/lib/OpenLayers/Control/SelectFeature.js b/lib/OpenLayers/Control/SelectFeature.js
@@ -300,14 +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<layers.length; ++l) {
+        var layers = this.layers || [this.layer],
+            layer, feature, l, numExcept;
+        for(l=0; l<layers.length; ++l) {
             layer = layers[l];
-            for(var i=layer.selectedFeatures.length-1; i>=0; --i) {
-                feature = layer.selectedFeatures[i];
+            numExcept = 0;
+            while(layer.selectedFeatures.length > numExcept) {
+                feature = layer.selectedFeatures[numExcept];
                 if(!options || options.except != feature) {
                     this.unselect(feature);
+                } else {
+                    ++numExcept;
                 }
             }
         }

diff --git a/tests/Control/SelectFeature.html b/tests/Control/SelectFeature.html
@@ -558,6 +558,46 @@
         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(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, feature4]);
+
+        control.select(feature1);
+        control.select(feature2);
+        control.select(feature3);
+        control.select(feature4);
+
+        layer.events.on({
+            featureunselected: function(e) {
+                // we change the selectedFeatures array while
+                // unselectAll is iterating over that array.
+                if(feature2.layer) {
+                    layer.removeFeatures([feature2]);
+                }
+            }
+        });
+
+        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');
+    }
 
     </script>
 </head>