[PATCH] Allow ZIP files whose paths are relative to the ZIP root in JOSM's map styling

[skorasaurus]

HOT OSM is making a map styling for josm and would like to distribute it through a zip file since many of our users will be in areas with limited and sporadic internet connections.

We're also currently using github to manage it and Github automatically generates zip files based on the tag, (i.e. : ​https://github.com/hotosm/HDM-JOSM-style/archive/0.0.1.zip

Unfortunately, these zip files aren't usable in josm for the map coloring because the icons path is relative to the zip root, not to the file itself. Using the default zip export from github could make the workflow much easier

If you could, we'd greatly appreciate it.

[stoecker]

Why don't you simply change your mapcss file, so path is correct?

[yohanboniface]

@stoecker: Github create a zip of the *folder* of the project, so this means that the root of the git project is *in* a folder in the zip.
For example:

MyZip /
 |____MyProjName /
         |_________my-mapcss-file-usually-at-git-repo-root.txt
         |_________icons/
                     |_________my-icon1.png
                     |_________my-icon2.png

This is why resolving the icons path relative to the mapcss file instead of to the ZIP root can solve the problem.

[stoecker]

Yes, I understand this. Though the issue remains. Why don't you simply fix the mapcss, so that the path is correct?

[yohanboniface]

You mean for example using MyProjName/icons/my-icon1.png instead of icons/my-icon1.png as icon-image value ?

If this is what you mean, the answer is: doing that way, the path isn't correct any more when pointing from JOSM directly to the mapcss file. Which is for example the case when you work on the style itself.

And by the way, I think we can say that having the file paths relative to a CSS like stylesheet is the standard way to handle this.
I mean, in pure CSS, if you have the path icons/my-icon.png, this means relatively to the CSS stylesheet path. Otherwise, the path must be absolute (/icons/my-icon.png or /mypath/icons/my-icon.png). So I think following this in JOSM makes sense, no?

[bastiK]

I agree that it makes more sense to consider the path relative to the mapcss file. As the style is usually in the root of the zip file, we shouldn't break to much with this change.

[skyper]

@yohanboniface:

Please, either add an external link to Styles or add the style to the wiki but not both.

Thanks

[yohanboniface]

Well, I've asked don-vip on IRC to delete the wiki page, but he seems idle.
Can you do it for us? Thanks.
BTW, are you on #josm?

[skyper]

Replying to yohanboniface:

Well, I've asked don-vip on IRC to delete the wiki page, but he seems idle.
Can you do it for us? Thanks.

Yes, I can do it.

BTW, are you on #josm?

sometimes.

EDT: Why did you use ubuntu one and not github as link.

[yohanboniface]

EDT: Why did you use ubuntu one and not github as link.

Because for now the Github link can't work (as explained in the current ticket ;) )

[skorasaurus]

What information do you still need that I could provide ?

[dommage]

Patch to allow more lenient searching of style zip files for icons.

[dommage]

Patched ImageProvider.getIfAvailableZip with code from MirroredInputStream.getZipEntry to allow a more lenient search for resources in style zipfiles. This is consistent with the way the the .mapcss file is found within the zipfile, but that leniency was not being extended to the icon images.

While this does increase the cost of looking up image files, it is only for zipped style resources and they are subsequently cached.

NOTE: I considered capturing the zipfile path prefix of the .mapcss file and propagating it to the ImageProvider, but it would have entailed much greater changes for what should be a simple request.

[stoecker]

With your patch an "hello.png" would also find "Myhello.png"? Don't sounds right to me.

[dommage]

Updated patch that tries exact path match first.

[dommage]

True enough. I have added the exact match check back in (the old behaviour). That should (overall) improve performance in the general case and still help when the images are not hanging off the root.
In the case where the same file name or file ending exists in two different sub-folders there is no issue, the key for each will include the folder name.

1) hello.png
2) my_hello.png
3) foo/hello.png
4) foo/my_hello.png
5) bar/hello.png

With this change number 4 could still cause problems, but only in zipfiles that suffer from the original 'nested-root' issue, not in existing zipfiles.

[AlfonZ]

I've tried

test/test.mapcss

node[name=foo] { icon-image:"foo.png"; text:auto; }
node[name=barfoo] { icon-image:"barfoo.png"; text:auto; }

test/foo.png
test/barfoo.png

and it used barfoo.png for both nodes

[bastiK]

Replying to dommage:

Patched ImageProvider.getIfAvailableZip with code from MirroredInputStream.getZipEntry to allow a more lenient search for resources in style zipfiles. This is consistent with the way the the .mapcss file is found within the zipfile, but that leniency was not being extended to the icon images.

While this does increase the cost of looking up image files, it is only for zipped style resources and they are subsequently cached.

NOTE: I considered capturing the zipfile path prefix of the .mapcss file and propagating it to the ImageProvider, but it would have entailed much greater changes for what should be a simple request.

I think the endsWith approach is a bit too pragmatic. What greater changes are we talking about?

[dommage]

Prevent matching files with the same postfix.

[dommage]

The only reason the style is processed at all is that the MirroredInputStream.getZipEntry goes hunting for the .mapcss in much the same way. That is the only place that JOSM is currently aware that the style is not located in the root of the .zip file.

I ran through a few possibilities before settling on the endsWith (which I agree is a bit simplistic).

• 'correcting' the paths in the mapcss post-parsing
• 'correcting' the zip structure on the fly (by stripping any directories above the .mapcss file)
• making the zip lookup (ImageProvider.getIfAvailableZip) 'smarter'

Unfortunately, all of these require the app to be aware that there is a .zip structure/.mapcss path mismatch; Information that currently only exists in MirroredInputStream.
There is an added problem in that this particular style file also includes references to builtin images which would have to be left unchanged. (The v2 diff manages that by putting back the exact match check.)

I've added one last attempt using endsWith as an attachment (patch_8686_3.diff). It relies on the initial getEntry to keep everything that already works working. It then makes the assumption that it is looking for an entry in a sub-directory and uses a '/' to make the name match exact.

[AlfonZ]

Another edge case:

test1/foo.png
test2/test.mapcss
test2/foo.png

uses test1/foo.png instead of intended test2/foo.png.
Although I admit it's unlikely to happen.

[dommage]

If entry not found, search relative to the mapcss file location.

[dommage]

If the entry is not found with the given full_name, find the mapcss file and use it's location as the root of the requested images.

[stoecker]

Hmm, these are all workarounds which I don't like. Fixing the system by properly passing the path seems much better to me.

[bastiK]

Replying to dommage:

If the entry is not found with the given full_name, find the mapcss file and use it's location as the root of the requested images.

Patch 4 goes in the right direction, but MapCSSStyleSource and MapImage should be aware of the path of the corresponding .mapcss file inside the zip, so ImageProvider can be called with the right parameters directly.

[dommage]

Replying to bastiK:

Patch 4 goes in the right direction, but MapCSSStyleSource and MapImage should be aware of the path of the corresponding .mapcss file inside the zip, so ImageProvider can be called with the right parameters directly.

I don't disagree, but I have reached the limits of my comfort with the codebase. The patch as given solves the users issue without (so far as I can tell) breaking any existing use cases. They've waited three months, perhaps we could release this and open a new ticket to investigate enhanced mapcss handling?

[bastiK]

In 6148/josm:

fixed #8686 - Allow ZIP files whose paths are relative to the ZIP root in JOSM's map styling