@bx80 opened this Pull Request on October 18th 2021 Contributor

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

@sgiehl commented on October 20th 2021 Member

@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 commented on October 21st 2021 Contributor

@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)
visit log campaigns single quote fix

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)
visit log outlink single quote fix

Thanks!

@sgiehl commented on October 25th 2021 Member

@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:
image

The outlink seems to work correct now.

@bx80 commented on October 25th 2021 Contributor

@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 @<a class='mention' href='https://github.com/character_set_database'>@character_set_database</a>, @<a class='mention' href='https://github.com/collation_database'>@collation_database</a>; shows that I'm using utf8mb4 / utf8mb4_general_ci, how about you?

@sgiehl commented on October 28th 2021 Member

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 commented on November 4th 2021 Contributor

@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 commented on November 5th 2021 Member

@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 commented on November 5th 2021 Member

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

This Pull Request was closed on November 5th 2021
Powered by GitHub Issue Mirror