Skip to content

Fix issue with trailing slash in Matomo analytics templates #7737

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

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

fnagel
Copy link

@fnagel fnagel commented Jan 17, 2025

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets fixes #7732
Related issues/PRs #7692 #7693
License MIT

What's in this PR?

Fixed for missing trailing slashes.

Why?

In order to fix Matomo URLs.

Context

Follow-up to 84a6d7b

@@ -1 +1 @@
<noscript><p><img referrerpolicy="no-referrer-when-downgrade" src="{{ analytics.content.url }}/matomo.php?idsite={{ analytics.content.siteId }}&amp;rec=1" style="border:0;" alt="" /></p></noscript>
<noscript><p><img referrerpolicy="no-referrer-when-downgrade" src="{{ analytics.content.url|trim('/') }}/matomo.php?idsite={{ analytics.content.siteId }}&amp;rec=1" style="border:0;" alt="" /></p></noscript>
Copy link
Member

@alexander-schranz alexander-schranz Jan 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why its more technical we should only rtrim url here only, technical URLs are allowed to start with // which are "Protocol-relative URLs":

Suggested change
<noscript><p><img referrerpolicy="no-referrer-when-downgrade" src="{{ analytics.content.url|trim('/') }}/matomo.php?idsite={{ analytics.content.siteId }}&amp;rec=1" style="border:0;" alt="" /></p></noscript>
<noscript><p><img referrerpolicy="no-referrer-when-downgrade" src="{{ analytics.content.url|trim('/', 'right') }}/matomo.php?idsite={{ analytics.content.siteId }}&amp;rec=1" style="border:0;" alt="" /></p></noscript>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. Changed the commit and updated the PR.

@fnagel fnagel force-pushed the fix-matomo-analytic-templates-2 branch from 0c118c4 to 3a6d0e4 Compare January 17, 2025 12:49
@alexander-schranz alexander-schranz enabled auto-merge (squash) January 17, 2025 12:52
@alexander-schranz
Copy link
Member

@fnagel thank you!

@alexander-schranz alexander-schranz merged commit 4ace6eb into sulu:3.0 Jan 17, 2025
8 of 9 checks passed
@benr77
Copy link

benr77 commented Jan 17, 2025

I've updated my 3.0 branch and this is now working perfectly. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants