Opening another issue as the (super helpful!) patch in #928244: Allow zoom-to-layer to apply to multiple layers was already committed. I think there is a missing line in this patch that causes a bug.

I had two maps

  • one with multiple layers and multiple points - which worked brilliantly with the patch;
  • the second with a single layer and single point - which worked before the patch, and failed directly after the patch was committed (reverted back and forth a few times to confirm)

Disclaimer is the OpenLayers code itself - especially on the JS side - is a black box for me as this is the first I've dove into it, so not sure exactly what map.zoomToExtent(), map.fullExtent.extend(), and map.zoomTo() are really doing, so I could be way off base.

However, when I compared the old code and the new kml and geojson code against the new else for non-kml/geojson layers, there was a line missing. I added that in and voila, now my single-layer, single-point map works (along with my multi-layer, multi-point still working).

Maybe this is a larger issue, so just posting the teensy patch here so others more familiar can decide.

For quick review:

Current dev branch code with new line this patch adds highlighted with comment at the bottom:

<?php
     
if (layers[i].layer_handler == 'kml' || layers[i].layer_handler == 'geojson') {
       
layers[i].events.register('loadend', layers[i], function() {
         
layerextent = layers[i].getDataExtent().scale(zoomtolayer_scale);
         
map.fullExtent.extend(layerextent);
         
map.zoomToExtent(map.fullExtent);
        });
      }
      else {
       
layerextent = layers[i].getDataExtent();
       
// Check for valid layer extent
       
if (layerextent != null) {
         
map.fullExtent.extend(layerextent);
         
map.zoomToExtent(map.fullExtent); /****** NEW LINE ******/
       
}
      }
?>

I know there is the new call in the final else, but with a single point that is never hit.

Files: 
CommentFileSizeAuthor
#2 bug-fix-for--openlayers-zoom_multiple_layers-928244-22.patch660 bytescovenantd
PASSED: [[SimpleTest]]: [MySQL] 238 pass(es).
[ View ]

Comments

Status:Needs review» Needs work

Hi !

Thanks for the finding ;-)
Can you provide a patch ?

Thanks !

Status:Needs work» Needs review
StatusFileSize
new660 bytes
PASSED: [[SimpleTest]]: [MySQL] 238 pass(es).
[ View ]

Oops, patch was missing extension so d.o. rejected the upload - here we go!

Status:Needs review» Fixed

Wow, you're fast - thanks!

Oops, I was commenting as you committed :)

The calls to map.fullExtent.extend() expand the OpenLayers.Bounds() area to include another set of bounds (ie, the bounding box around everything in a layer, obtained with layer.getExtent()). (OpenLayers docs for OpenLayers.Bounds.extend()).

Basically, this code is stepping through all of the layers and building a "Bounds" object that contains all of the things. To avoid zooming the map multiple times, map.zoomToExtent(map.fullExtent) shouldn't be called before all of the extents are gathered--so instead of putting the missing call inside the for loop, try it at the end. (For json and KML layers, we can't avoid zooming multiple times, because we don't know when all of the layers will be finished loading.)

... and it looks like it's already being called at the end, but maybe the .scale() method isn't functioning as expected here:

<?php
 
...
 
// If unable to find width due to single point,
  // zoom in with point_zoom_level option.
 
if (map.fullExtent.getWidth() == 0.0) {
   
map.zoomTo(options.point_zoom_level);
  }
  else {
   
map.zoomToExtent(map.fullExtent.scale(zoomtolayer_scale));      // line 53
 
}
});
?>

Yeah, I was wondering about that - the odd thing I found was that final else was never being called with a single-point. Instead the map.zoomTo() was hit - otherwise I think that call at the end would have done it (and more efficiently as you mention).

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.