Fix TinyMCE external url handling #1320
Conversation
…al links/images urls
thet
dismissed
their stale review
The scenario outlined in #1116 (comment) might not be relevant, see the discussion there.
| tinyOptions["urlconverter_callback"] = (url, node, on_save, name) => { | ||
| if (url.indexOf("http") === 0) { | ||
| // if url starts with "http" return it as is | ||
| return url; | ||
| } | ||
| // otherwise default tiny behavior | ||
| if (self.tiny.settings.relative_urls) { | ||
| return self.tiny.documentBaseURI.toRelative(url); | ||
| } | ||
| url = self.tiny.documentBaseURI.toAbsolute(url, self.tiny.settings.remove_script_host); |
There was a problem hiding this comment.
According to the discussion on #1116 (comment) ff, this can be simplified to:
| tinyOptions["urlconverter_callback"] = (url, node, on_save, name) => { | |
| if (url.indexOf("http") === 0) { | |
| // if url starts with "http" return it as is | |
| return url; | |
| } | |
| // otherwise default tiny behavior | |
| if (self.tiny.settings.relative_urls) { | |
| return self.tiny.documentBaseURI.toRelative(url); | |
| } | |
| url = self.tiny.documentBaseURI.toAbsolute(url, self.tiny.settings.remove_script_host); | |
| tinyOptions["urlconverter_callback"] = (url) => { |
There was a problem hiding this comment.
I've just tried that locally and this breaks plone.outputfilters regex matching resolveuid in an url. So no transformation is done for /Plone/resolveuid/<uid> which is returned by the plonelink/ploneimage plugin for internal links/images.
There was a problem hiding this comment.
Ah, that's interesting. Then just ignore my suggestion.
There was a problem hiding this comment.
yap ... unfortunately this cannot work without further changes to plone.outputfilters though it's a simple non-breaking change to the resolveuid_re from
resolveuid_re = re.compile("^[./]*resolve[Uu]id/([^/]*)/?(.*)$")
to
resolveuid_re = re.compile("^.*/resolve[Uu]id/([^/]*)/?(.*)$")
otherwise url patterns like /<plonesiteid>/resolveuid/<uid> will not get resolved.
Note that this is what you get if you edit your Plone site without virtual hosting. My local virtualhosting test resolves to /resolveuid/<uid> which the outputfilter would match though ...
There was a problem hiding this comment.
Let's be not too intrusive for now and keep this as a fix for the external urls. We can fix/generalize internal url transformation later...
There was a problem hiding this comment.
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.
<thinking/>
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: plone/Products.CMFPlone@243773d - 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. 🤯
Since that's getting absurdly complex, let's stick with your PR as-is!
There was a problem hiding this comment.
Created an issue out of this possible enhancement: #1321
There was a problem hiding this comment.
yap ... unfortunately this cannot work without further changes to
plone.outputfiltersthough it's a simple non-breaking change to theresolveuid_refromresolveuid_re = re.compile("^[./]*resolve[Uu]id/([^/]*)/?(.*)$")to
resolveuid_re = re.compile("^.*/resolve[Uu]id/([^/]*)/?(.*)$")otherwise url patterns like
/<plonesiteid>/resolveuid/<uid>will not get resolved.Note that this is what you get if you edit your Plone site without virtual hosting. My local virtualhosting test resolves to
/resolveuid/<uid>which the outputfilter would match though ...
As in plone/plone.app.contenttypes#675 the problem is that plone.outputfilters and the WYSIWYG editor (TinyMCE) does not share the same code for UID resolving. The plone.outputfilters uid resolver should be an util imported and exposed by a plone.api call which than TinyMCE can call via a browser view. The same code should be imported in plone.restapi. So one place, one policy, one service and everybody is happy.
Fixes #1116
Add
urlconverter_callbackand skip converting external urls.IMPLEMENTATION NOTES:
This is a quick and rough solution which checks if inserted url starts with "http" ... it would be more solid if we query the inserted element and check for the
dataset.linktypevalue, but right now I have no clue, how to get the element in the callback.