makinacorpus / django-leaflet

Use Leaflet in your Django projects
GNU Lesser General Public License v3.0
716 stars 282 forks source link

fix: change OSM tile link to unified format #375

Closed LaoshuBaby closed 5 months ago

LaoshuBaby commented 6 months ago

Hello, I noticed django-leaflet still using a/b/c subdomain when requesting OSM tiles, according to https://github.com/openstreetmap/operations/issues/737, old format will stop working in the future, so I search all "{s}.tile.openstreetmap" in this repository and change them.

This was inspired by talk in https://github.com/thomersch/openstreetmap-calendar/issues/146

If there are anything improper, please comment to me, thank you!

claudep commented 6 months ago

Could you tell what you changed in leaflet/static/leaflet/leaflet-src.map, as the format is diff-unfriendly and the file comes from upstream?

LaoshuBaby commented 6 months ago

Could you tell what you changed in leaflet/static/leaflet/leaflet-src.map

replace("{s}.tile.openstreetmap","tile.openstreetmap")

As the title of this PR

the file comes from upstream?

This may be the key problem. I found it a bit like the build artifact of javascript, and thought that it might indeed be difficult to diff, so I am not very sure whether to replace the string in this file (but I did not see how it is built in this project, so I guess it is relatively stable because I replaced all such strings in text files in django-leaflet).

However, I really didn't expect it to come from an upstream project. Can you tell me which project it is? If so, I think this PR should be hold temporarily until the format replacement is done upstream.

claudep commented 6 months ago

Upstream is https://github.com/Leaflet/Leaflet. Looks like they fixed it in https://github.com/Leaflet/Leaflet/commit/c2324e52e273

I just figured out that leaflet-src.map is an old file that was forgotten in the tree but is no longer used. I filed a PR to remove that file: https://github.com/makinacorpus/django-leaflet/pull/376

Gagaro commented 5 months ago

Thanks for the PR, I rebased it and merged it in https://github.com/makinacorpus/django-leaflet/commit/6d6eed7a9a77c521f673037c10cf7be21a311771

danieldegroot2 commented 4 months ago

@Gagaro Could you use HTTPS instead of HTTP, please? (use https:// for the new url) (unless some of your clients do not support this, but they should be able to overwrite it already.)

Leaflet/leaflet already do so.