Fix for HTML entity decoding of single quotes in campaigns and csv/xml exports

[bx80]

Description:

Fixes #14695

Tweaked the campaign referrer name decoding so that HTML entity single quotes are properly rendered when displayed on the visitor log.

Also changed the XML and CSV datatable render to use HTML entity decode with the ENT_QUOTES option instead of the ENT_COMPAT option so that exported data with entity encoded quotes is correctly converted.

Review

• Functional review done
• Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
• Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
• Security review done
• Wording review done
• Code review done
• Tests were added if useful/possible
• Reviewed for breaking changes
• Developer changelog updated if needed
• Documentation added if needed
• Existing documentation updated if needed

[sgiehl]

@bx80 Are you still working on that one? Had a quick look, but it didn't seem to work as expected. Feel free to readd the Needs Review label, once you are done.

[bx80]

@sgiehl I've added an additional commit to fix issue #18190 where outlinks with single quotes are not decoded on the visits log. I've checked both the campaign referrer name and outlinks and they both correctly show the single quotes for me. If it's still not working for you, could you provide a screenshot or the test string used? Then I can try to recreate. Maybe character set is a factor?

My test steps for the campaign referrer:

1. Create a new campaign with a name containing a single quote
2. Visit a tracked page using the campaign URL.
3. Checked the visit log campaign name. (contained ' before the fix, single quote after)
   

For the outlinks:

1. On my test tracked website, added a new link to the wikipedia article reported in the issue.
2. Followed the link to record a tracked visit.
3. Checked the visit log actions outlink. (contained ' before the fix, single quote after)
   

Thanks!

[sgiehl]

@bx80 How did you track the campaign? I have a local site that simply tracks a page view. Adding ?pk_campaign=te%27%22st to the url, triggers tracking of a campaign with the name te'"st.

Visitor log shows this for the visit:

The outlink seems to work correct now.

[bx80]

@sgiehl That's pretty much how I did my tests, a local test site tracking page views with a campaign url. I've just copied and pasted your example campaign url from the comment above and repeated the test - it still works fine for me. It seems like there is some other factor here. I wonder if it could be database collation / character set dependent? SELECT @@character_set_database, @@collation_database; shows that I'm using utf8mb4 / utf8mb4_general_ci, how about you?

[sgiehl]

My database collation ist actually still latin, but all tables are utf8mb4. But I don't think that should be the issue.
Maybe you could extend/change the UI tests of the Live plugin by some data containing such special chars, so we can see in the tests if it works or not.

[justinvelluppillai]

@sgiehl it'd be good to know if you see any issues left on this needing fixed or what needs to be done to get it merged as well as the useful smaller tips.

[sgiehl]

Seems that actually works as expected. It took me a while to figure out why it doesn't work for campaigns locally. 🤯 It's actually the MarketingCampaignsReporting plugin that I have active, as it overwrites the campaign in that case. Would you mind checking how to fix that for that plugin as well, and maybe create a pull request there as well? Guess it might need to be added here: https://github.com/matomo-org/plugin-MarketingCampaignsReporting/blob/c7d437e9bf4232b29ec5e1f16dcd21dd457bec7e/VisitorDetails.php#L52

[sgiehl]

@bx80 be good not to update a submodule to a specific branch without mentioning it. After merging the plugin PR, the submodule would now be broken as the reference doesn't exist anymore. Will update the submodule again and merge the PR if tests are passing afterwards

[sgiehl]

failing tests seem to be random failures. Will merge this now 👍