Merge pull request #1972 from elemoine/modifyinteraction

Fix bug at modify interaction construction time

src/ol/interaction/modifyinteraction.js

@@ -112,18 +112,6 @@ ol.interaction.Modify = function(options) {
         ol.interaction.Modify.getDefaultStyleFunction()
   });
 
-  /**
-   * @type {ol.Collection}
-   * @private
-   */
-  this.features_ = options.features;
-
-  this.features_.forEach(this.addFeature_, this);
-  goog.events.listen(this.features_, ol.CollectionEventType.ADD,
-      this.addFeature_, false, this);
-  goog.events.listen(this.features_, ol.CollectionEventType.REMOVE,
-      this.removeFeature_, false, this);
-
   /**
   * @const
   * @private
@@ -140,10 +128,38 @@ ol.interaction.Modify = function(options) {
     'GeometryCollection': this.writeGeometryCollectionGeometry_
   };
 
+  /**
+   * @type {ol.Collection}
+   * @private
+   */
+  this.features_ = options.features;
+
+  this.features_.forEach(this.addFeature_, this);
+  goog.events.listen(this.features_, ol.CollectionEventType.ADD,
+      this.handleFeatureAdd_, false, this);
+  goog.events.listen(this.features_, ol.CollectionEventType.REMOVE,
+      this.handleFeatureRemove_, false, this);
+
 };
 goog.inherits(ol.interaction.Modify, ol.interaction.Pointer);
 
 
+/**
+ * @param {ol.Feature} feature Feature.
+ * @private
+ */
+ol.interaction.Modify.prototype.addFeature_ = function(feature) {
+  var geometry = feature.getGeometry();
+  if (goog.isDef(this.SEGMENT_WRITERS_[geometry.getType()])) {
+    this.SEGMENT_WRITERS_[geometry.getType()].call(this, feature, geometry);
+  }
+  var map = this.getMap();
+  if (!goog.isNull(map)) {
+    this.handlePointerAtPixel_(this.lastPixel_, map);
+  }
+};
+
+
 /**
  * @inheritDoc
  */
@@ -157,16 +173,34 @@ ol.interaction.Modify.prototype.setMap = function(map) {
  * @param {ol.CollectionEvent} evt Event.
  * @private
  */
-ol.interaction.Modify.prototype.addFeature_ = function(evt) {
+ol.interaction.Modify.prototype.handleFeatureAdd_ = function(evt) {
   var feature = evt.element;
   goog.asserts.assertInstanceof(feature, ol.Feature);
-  var geometry = feature.getGeometry();
+  this.addFeature_(feature);
-  if (goog.isDef(this.SEGMENT_WRITERS_[geometry.getType()])) {
+};
-    this.SEGMENT_WRITERS_[geometry.getType()].call(this, feature, geometry);
+
+
+/**
+ * @param {ol.CollectionEvent} evt Event.
+ * @private
+ */
+ol.interaction.Modify.prototype.handleFeatureRemove_ = function(evt) {
+  var feature = evt.element;
+  var rBush = this.rBush_;
+  var i, nodesToRemove = [];
+  rBush.forEachInExtent(feature.getGeometry().getExtent(), function(node) {
+    if (feature === node.feature) {
+      nodesToRemove.push(node);
+    }
+  });
+  for (i = nodesToRemove.length - 1; i >= 0; --i) {
+    rBush.remove(nodesToRemove[i]);
   }
-  var map = this.getMap();
+  // There remains only vertexFeature…
-  if (!goog.isNull(map)) {
+  if (!goog.isNull(this.vertexFeature_) &&
-    this.handlePointerAtPixel_(this.lastPixel_, map);
+      this.features_.getLength() === 0) {
+    this.overlay_.removeFeature(this.vertexFeature_);
+    this.vertexFeature_ = null;
   }
 };
 
@@ -329,31 +363,6 @@ ol.interaction.Modify.prototype.writeGeometryCollectionGeometry_ =
 };
 
 
-/**
- * @param {ol.CollectionEvent} evt Event.
- * @private
- */
-ol.interaction.Modify.prototype.removeFeature_ = function(evt) {
-  var feature = evt.element;
-  var rBush = this.rBush_;
-  var i, nodesToRemove = [];
-  rBush.forEachInExtent(feature.getGeometry().getExtent(), function(node) {
-    if (feature === node.feature) {
-      nodesToRemove.push(node);
-    }
-  });
-  for (i = nodesToRemove.length - 1; i >= 0; --i) {
-    rBush.remove(nodesToRemove[i]);
-  }
-  // There remains only vertexFeature…
-  if (!goog.isNull(this.vertexFeature_) &&
-      this.features_.getLength() === 0) {
-    this.overlay_.removeFeature(this.vertexFeature_);
-    this.vertexFeature_ = null;
-  }
-};
-
-
 /**
  * @param {ol.Coordinate} coordinates Coordinates.
  * @return {ol.Feature} Vertex feature.

test/spec/ol/interaction/modifyinteraction.test.js

@@ -0,0 +1,24 @@
+goog.provide('ol.test.interaction.Modify');
+
+describe('ol.interaction.Modify', function() {
+
+  describe('constructor', function() {
+    it('adds features to the RTree', function() {
+      var feature = new ol.Feature(
+          new ol.geom.Point([0, 0]));
+      var features = new ol.Collection([feature]);
+      var modify = new ol.interaction.Modify({
+        features: features
+      });
+      var rbushEntries = modify.rBush_.getAll();
+      expect(rbushEntries.length).to.be(1);
+      expect(rbushEntries[0].feature === feature).to.be.ok();
+    });
+  });
+
+});
+
+goog.require('ol.Feature');
+goog.require('ol.Collection');
+goog.require('ol.geom.Point');
+goog.require('ol.interaction.Modify');