fix(#299): stop TinyMCE double-prefixing URLs in email templates (and every editor) - #300
Conversation
… every editor)
The email-template WYSIWYG editor saved links double-prefixed with the admin
base, e.g. an href of {{login_url}} came back as
https://host/admin/{{login_url}}, so the sent email link resolved to
https://host/admin/https://host/accedi.
Root cause: TinyMCE's convert_urls (on by default) resolves every href against
the editing page's base URL — the admin page — prefixing links and even
placeholder tokens with /admin. relative_urls:false alone does NOT prevent this;
it only picks absolute vs relative for the (still-performed) conversion. The fix
is convert_urls:false, which tells TinyMCE never to rewrite URLs.
- Set convert_urls:false on ALL six TinyMCE inits: settings/index (email
templates — the reported case), book_form (#descrizione), events/form
(#event_content), cms/edit-home (#text_content_body), admin/cms-edit
(#tinymce-editor), and the legacy admin/settings view (dead code — not routed
anywhere — fixed for consistency).
- EmailService::replaceVariables now heals templates ALREADY saved corrupted on
existing installs: it strips an absolute "…/admin/" prefix sitting directly in
front of a {{placeholder}} before substitution, so a user doesn't have to
re-save every template after updating. A legitimate template never puts a
host+/admin/ in front of a token, so this is safe.
Tests:
- tests/email-template-url-prefix-299.unit.php (9) — the healing: corrupted
href → correct single URL, http/https, subdir base, multiple links, and
no-ops for real /admin/ URLs and non-URL placeholders.
- tests/tinymce-url-prefix-299.spec.js (4) — real browser feeds an href with a
placeholder through each editor and asserts convert_urls is false AND the
placeholder round-trips verbatim with no /admin/ prefix and no %7B%7B encoding.
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughLa modifica impedisce a TinyMCE di riscrivere gli URL nei contenuti amministrativi e aggiunge una riparazione in ChangesCorrezione dei prefissi URL
Estimated code review effort: 3 (Moderate) | ~20 minuti Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/tinymce-url-prefix-299.spec.js`:
- Line 47: Rimuovi l’attesa page.waitForLoadState('networkidle') nel test e
affidati al successivo waitForFunction per attendere la condizione applicativa
necessaria. Mantieni invariato il resto del flusso del test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eac3d7a7-a1df-4def-9bdd-b664e11904c0
📒 Files selected for processing (9)
app/Support/EmailService.phpapp/Views/admin/cms-edit.phpapp/Views/admin/settings.phpapp/Views/cms/edit-home.phpapp/Views/events/form.phpapp/Views/libri/partials/book_form.phpapp/Views/settings/index.phptests/email-template-url-prefix-299.unit.phptests/tinymce-url-prefix-299.spec.js
… at render
Follow-up so existing installs get their STORED template values fixed, not only
the rendered email. TinyMCE no longer rewrites URLs (convert_urls:false), and
EmailService heals at render time, but a template saved before the fix keeps the
double-prefixed link in the DB — visible in the editor and wrong if read directly.
- Extract the render-time repair into EmailService::healPlaceholderUrls() (static,
reused by the DB heal — one place for the pattern).
- SettingsRepository::healCorruptedTemplateUrls() repairs stored rows: a cheap
LIKE '%/admin/{{%' narrows to affected templates, the fix is done in PHP
(portable across MySQL 5.7+/MariaDB — no REGEXP_REPLACE, and the host varies so
a literal REPLACE won't do). Idempotent, all locales.
- SettingsController runs it right after ensureEmailTemplates, so opening the
settings page repairs the stored values.
Also address CodeRabbit: drop waitForLoadState('networkidle') (flaky) from the
E2E — the following waitForFunction is the real readiness signal.
Tests: tinymce-url-prefix-299.spec.js adds a DB-repair case — a template stored
with an /admin/ prefix is healed to the plain placeholder after the settings page
is opened. unit suite unchanged (9) and still green after the extraction.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Problem
#299: email templates come out with links double-prefixed by the admin base after the template is edited once in the WYSIWYG editor. An href of
{{login_url}}is saved ashttps://host/admin/{{login_url}}, so the sent email link becomeshttps://host/admin/https://host/accedi.Root cause
TinyMCE's
convert_urls(on by default) resolves every href against the editing page's base URL — which is the admin page — so it prefixes links, and even placeholder tokens like{{login_url}}, with/admin. On re-edit it compounds.relative_urls: falsealone does not prevent this: it only chooses absolute vs relative output for a conversion that still happens. The correct switch isconvert_urls: false, which tells TinyMCE to leave URLs exactly as written.Fix
convert_urls: falseon all six TinyMCE inits:settings/index(email templates — the reported case),book_form(#descrizione),events/form(#event_content),cms/edit-home(#text_content_body),admin/cms-edit(#tinymce-editor), and the legacyadmin/settingsview (dead code — not routed — fixed for consistency).EmailService::replaceVariablesnow strips an absolute…/admin/prefix sitting directly in front of a{{placeholder}}before substitution, so existing installs (like the reporter's) don't need every template re-saved after updating. A legitimate template never puts a host+/admin/in front of a token, so this is safe.Tests
tests/email-template-url-prefix-299.unit.php(9)/admin/URLs and non-URL placeholders.tests/tinymce-url-prefix-299.spec.js(4)convert_urlsis false AND the placeholder round-trips verbatim — no/admin/prefix, no%7B%7Bencoding.All green. Full-tree PHPStan level 5 clean.
Closes #299.
Summary by CodeRabbit
Correzioni
/admin/nei link con segnaposto dei template email.Test