-
-
Notifications
You must be signed in to change notification settings - Fork 26
Fix #462: Normalize resolveuid links to avoid portal-prefixed URLs #655
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix #462: Normalize resolveuid links to avoid portal-prefixed URLs #655
Conversation
When editing tiles with TinyMCE, internal links can sometimes be saved with portal-prefixed absolute URLs (e.g., '/Plone/resolveuid/...') instead of relative paths (e.g., '../resolveuid/...'). This causes the resolveuid transform to fail when the viewing site URL differs from the editing URL, resulting in broken links. This change normalizes resolveuid links before saving tile content by: - Converting portal-context prefixed URLs to relative '../resolveuid/...' - Converting full absolute URLs containing resolveuid to relative paths - Generic fallback for any '/sitename/resolveuid/...' patterns The normalization is applied in the tile save method for app/textapp tiles to ensure consistent relative URLs are stored, allowing the resolveuid transform to work correctly regardless of where the site is accessed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the fix! This is a very welcome improvement - I've seen incorrect resolveuid URLs also. Also very clever cases, which are covered here.
But I think the we always want "../resolveuid" (actually not "resolveuid/", as this would not work if the URL contains a view name). Therefore we can simplify the 3 regexes into one, as suggested in the comment.
I see a problem with my suggestion, which is also in the orignial PR: on portal root and not virtual hosted, so e.g. for https://localhost:8080/Plone, a ../resolveuid in the portal root would break.
Note: I haven't tested this!
While we're are at this: Tests would be fine and best practice too. But I don't require them here, as this could delay the PR finalization. Tests can be added later.
Oh, we don't have any mosaic JavaScript tests, so tests are obsolete anyways.
|
@petschki please check this PR also, I'm curious of your opinion. |
Co-authored-by: Johannes Raggam <[email protected]>
Co-authored-by: Johannes Raggam <[email protected]>
|
I've just tried mosaic with master branch and with this PR on a vanilla classic-ui Plone Site. I've tested several scenarios without and with VHM and unfortunately could not reproduce the initial issue where the I think the metioned issue is outdated for Mosaic 3+ as there was a complete ES6 rewrite in the meantime. |
|
@petschki Thank you for your kind words! Will be eagerly waiting for your comment. Looking forward to keep contributing to Plone 😊 |
|
Also, I had raised 2 PRs in ploneorg.theme, a long time ago. All the checks had passed, but no maintainer merged the code. Would be grateful if any of you could kindly check it out. |
|
@thet if OK for you, I will close this without merging since it targets an outdated issue. |
|
@petschki Yes, that's ok. I thought I saw these cases regularly, but it's perfectly possible that I've seen this in old tinymce resp. Plone instances. The work isn't lost anyways as GitHub keeps the branch internally alive. |
Problem
When editing tiles with TinyMCE, internal links can sometimes be saved with portal-prefixed absolute URLs (e.g.,
/Plone/resolveuid/...) instead of relative paths (e.g.,../resolveuid/...). This causes the resolveuid transform to fail when the viewing site URL differs from the editing URL, resulting in broken links.As described in #462, this happens when:
/Plone/resolveuid/3c99581261e6479bb0500daea17f57fe../resolveuid/3c99581261e6479bb0500daea17f57feThe relative URL is correct, but the initial absolute URL breaks when the Plone site is viewed on a different URL from the editing site.
Solution
This PR adds normalization of resolveuid links before saving tile content. The normalization:
../resolveuid/.../sitename/resolveuid/...patternsThe normalization is applied in the
Tile.save()method for app/textapp tiles, ensuring consistent relative URLs are stored. This allows the resolveuid transform to work correctly regardless of where the site is accessed.Changes
resources/js/mosaic.tile.js:save()method before POSTing tile content../resolveuid/formatlastSavedDatato prevent unnecessary savesTesting
Manual testing steps:
../resolveuid/<uid>instead of/Plone/resolveuid/<uid>Fixes #462