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:
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.
Comment | File | Size | Author |
---|---|---|---|
#2 | bug-fix-for--openlayers-zoom_multiple_layers-928244-22.patch | 660 bytes | sokrplare |
Comments
Comment #1
PolHi !
Thanks for the finding ;-)
Can you provide a patch ?
Thanks !
Comment #2
sokrplare CreditAttribution: sokrplare commentedOops, patch was missing extension so d.o. rejected the upload - here we go!
Comment #3
PolThanks, committed !
http://drupalcode.org/project/openlayers.git/commit/0613bed
Comment #4
sokrplare CreditAttribution: sokrplare commentedWow, you're fast - thanks!
Comment #5
becw CreditAttribution: becw commentedOops, I was commenting as you committed :)
The calls to
map.fullExtent.extend()
expand theOpenLayers.Bounds()
area to include another set of bounds (ie, the bounding box around everything in a layer, obtained withlayer.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:Comment #6
sokrplare CreditAttribution: sokrplare commentedYeah, I was wondering about that - the odd thing I found was that final
else
was never being called with a single-point. Instead themap.zoomTo()
was hit - otherwise I think that call at the end would have done it (and more efficiently as you mention).