plone / mockup

A collection of client side patterns for faster and easier web development
http://plone.github.io/mockup/
BSD 3-Clause "New" or "Revised" License
47 stars 93 forks source link

Simplify URL handling in pat-tinymce #1321

Open thet opened 1 year ago

thet commented 1 year ago

For internal URLs, don't insert the resolveuid URL‌ with the full path but instead insert it as relative-to-the-context URL. It's converted to a relative-to-the-context URL by TinyMCE urlconverter_callback anyways AFAIK. If we do that, we could also fully bypass the urlconverter_callback as suggested in: https://github.com/plone/mockup/pull/1320#discussion_r1195242599

See the whole discussion in https://github.com/plone/mockup/pull/1320#discussion_r1195242599 and especially:

Actually, that kind of url converting is probably really needed. Otherwise the portal path could end up being added to the text, which is not what we want.

I was wondering why this resolveuid is inserted - with the - full path, where in this case a relative-to-the-context URL could be inserted in the first place and thus not needing to rely on the TinyMCE urlconverter_callback.

I did some software archeology. The appendToUrl setting was initially relative-to-the-context. It become an relative URL with the full path due to the wrong assumptions in this commit: https://github.com/plone/Products.CMFPlone/commit/243773d8d1b0f1804cf6db6a92b961fa85598f84 - also see the comments there.

So, there would be another angle to fix the issue:‌ Reverting the change where the full path was inserted in CMFPlone, adding @@resolveuid instead only resuloveuid as mentioned in the comment of the commit referenced above and allowing an @@ in the resoloveuid_and_caption filter in plone.outputfilters. exploding_head

Since that's getting absurdly complex, let's stick with your PR as-is!

petschki commented 11 months ago

There's maybe a similar problem solved? https://github.com/plone/plone.app.contenttypes/pull/675