Skip to content

Conversation

@hostep
Copy link
Contributor

@hostep hostep commented Oct 9, 2025

The last 2 versions released introduced improved escaping, but it looks like some wrong methods got used.

Especially in the javascript area, you should use escapeJs instead of escapeHtml (see below for example).
Also, to output an url in javascript, the documentation recommends to still use escapeJs instead of escapeUrl:

//Do not use HTMl context methods like escapeUrl

Also the url for the <noscript> tag was generated in a strange way by escaping parts of the url in 2 different ways, this has been simplified which makes the code more readable and more correct.

Watch out, I've only tested the changes in iframe.phtml and script.phtml myself, maybe double check the other changes as well yourself.

//cc @jissereitsma


Locally, I created a small proof of this in the script.phtml by changing the php variable $gtmId to "test'ing" as a way to demonstrate the problem.

With the old escapeHtml method, this would output: 'test&#039;ing', which Javascript interprets as: test&#039;ing
With the new escapeJs method, this will output: 'test\u0027ing', which Javascript interprets as test'ing which is what we want.

@jissereitsma
Copy link
Collaborator

Thanks for diving into this. I have been adding escaping to all of my extensions (200+) and mostly have been checking whether the output was ok, instead of whether the right method was used in all places. Apparently this failed in some places.

I'll merge and test things properly.

@jissereitsma jissereitsma merged commit a68f154 into yireo:master Oct 20, 2025
2 checks passed
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.

2 participants