Revert "Revert "use / for font url to fix production-bundle issues""

[thet]

This reverts commit 9bc2a08.

In production mode, icon fonts are not found due to urls like:
http://localhost:8080/Plone/++plone++production/++unique++2021-10-22T12:54:21.278234/++plone++static/++plone++static/fonts/plone-fontello.ttf?89786008

[mister-roboto]

@thet thanks for creating this Pull Request and help improve Plone!

To ensure that these changes do not break other parts of Plone, the Plone test suite matrix needs to pass.

Whenever you feel that the pull request is ready to be tested, either start all jenkins jobs pull requests by yourself, or simply add a comment in this pull request stating:

@jenkins-plone-org please run jobs

With this simple comment all the jobs will be started automatically.

Happy hacking!

[thet]

@jenkins-plone-org please run jobs

[sneridagh]

@thet could you please elaborate why did you revert this? Also, why is it needed?

In a site deployed in a different directory it will fail... because there's no Plone in the root...

Also, I realised of this is present also in 5.2.7 after upgrading a site to that.

/cc @erral @tisto

[erral]

There was an error for Plone sites not deployed in the root of the domain, reported by @cekk here:

#162

I thought I found the issue and added a PR to fix that:

#168

But it looks like it did not fixed the issue, and I think it was merged without properly testing it. I think @agitator pinged me about it, so I reverted my PR and reopened the original issue:

#171

[erral]

So the current status should be the original: fontello icons are no properly rendered it Plone is not deployed in the root of the domain.

[tisto]

@thet we really need feedback from you here. This is a serious issue for us and IMHO for the Plone community as well. We shipped the latest Plone version to lots of clients that are important to us, and all icons of the site are broken.

I understand if you don't have time right now to look into this. Though, then the right cause of action here would be to revert whatever commit/PR caused this and ship a new plone.staticresources package right away. IMHO this issue is serious enough to ask the @plone/release-team to ship Plone 5.2.8 asap after creating a fix.

@erral can you pinpoint the PR/commit that initially caused this?

[erral]

I think that the issue came after this commit: 0228cd7

[thet]

@tisto I agree this needs to be fixed.
If I remember correctly making the font paths relative works in development mode but not in production mode, where the ++unique++ and ++production++ paths are inserted.
@agitator should know about some other implications on this, he was providing an initial fix on that.

In current not-yet-released Plone 6 with the new resource registry this shouldn't happen anymore AFAIK /cc @jensens

I'm open for any good solution on this and do not have strong opinions other than a fix should not break another valid and common use case.
so go ahead with this!

[jensens]

++unique++ is replaced by ++webresource++. ++production++ is no longer needed since merging does not make sense anymore.

Overall all three traversers are based on the same plone.resource.traversal.UniqueResourceTraverser, so they work exactly the same. The only difference is the way the unique part is created. So overall, without looking further, the behavior should be the same and the failure as well, I guess, just with a different name between the 2 ++.

The idea to base the traversers on the root/navigation root is to make all cachable at its best.
This happens here:
https://github.com/plone/Products.CMFPlone/blob/1f09a8585c152ed559fdc24056bd8b77941b248a/Products/CMFPlone/resources/browser/resource.py#L250-L256

[sneridagh]

But this is all about Plone 6. I assume Plone 6 will be fine.

How we address the problem that in production mode a relative link to that is broken? Also, I remember that something like that happen in Diazo, but it manages to inject the right path always.

[jensens]

Right, in 6 we also do not have fontello anymore, we use the new icon sub-system.