Leaflet icon indicating copy to clipboard operation
Leaflet copied to clipboard

min/maxZoom Limit not working correct with `NaN`?

Open Falke-Design opened this issue 3 years ago β€’ 1 comments

I amended the commit to include tests that check if two layers with different zoom levels will update the zoom limit. For example, if minZoom is 6 for the first layer and 4 for the second layer, then the min zoom limit of the map will updated to 4.

However, in the process I discovered an edge case that I think is a bug but wanted your opinion on. When only minZoom or only maxZoom is explicitly set in at least one layer of one or many layers added to the map, and when the set value is NaN, then the corresponding zoom limit in the map is updated to NaN. This is because:

  • minZoom has a default value of 0 and maxZoom has a default value of 18 if not explicitly set in a layer
  • therefore if either minZoom or maxZoom is set to NaN but the other is not explicitly set, then the other will get its default value which will cause the boolean condition in _addZoomLimit to evaluate to true https://github.com/Leaflet/Leaflet/blob/17dcbe9b087c9fd16899bf8e0b27b72c34f7063e/src/layer/Layer.js#L229-L234
  • then when _updateZoomLevels is called, both Math.min(NaN, <some_number>) and Math.min(NaN, <some_number>) will always return NaN causing the corresponding zoom limit to be updated to NaN https://github.com/Leaflet/Leaflet/blob/17dcbe9b087c9fd16899bf8e0b27b72c34f7063e/src/layer/Layer.js#L250-L258

This can be a hard-to-find edge case if the almost all the layers have only valid numerical values, but one of the layers (in any order of being added into the map) met the condition I described. I have added test cases that check for NaN to show what I mean.

I searched for issues with the NaN keyword but I could not find a issue that referenced this error. I am wondering if:

  1. this is a bug that needs to be addressed?
  2. if yes to question 1, should a separate issue be opened up to describe this bug?

Originally posted by @zishiwu123 in https://github.com/Leaflet/Leaflet/issues/8037#issuecomment-1062014280

Falke-Design avatar Jul 24 '22 12:07 Falke-Design

With NaN for minZoom while TileLayer creation, the maxZoom results to be NaN too..

L.tileLayer("", {minZoom: NaN, maxZoom: 18})

Falke-Design avatar Jul 24 '22 12:07 Falke-Design