civicrm / org.civicrm.flexmailer

FlexMailer is now part of civicrm-core. Please see the link below for how to upgrade.
https://civicrm.org/extensions/flexmailer
Other
5 stars 22 forks source link

Flexmailer tries to apply tracking to anchor links, breaking them #43

Open larssandergreen opened 5 years ago

larssandergreen commented 5 years ago

Since flexmailer applies tracking to anything like href='foo' or href="bar", it converts anchor links (of the form href="#foo") to tracked links, which of course breaks those links.

See here: https://github.com/civicrm/org.civicrm.flexmailer/blob/master/src/ClickTracker/HtmlClickTracker.php#L71 and here for text: https://github.com/civicrm/org.civicrm.flexmailer/blob/master/src/ClickTracker/TextClickTracker.php#L59

A simple solution would be simply to ignore any link of the form href="#foo" or href='#bar'. Not much of a change, but would be helpful (this is an issue that a few people have noticed with Mosaico). Does this approach make sense?

sluc23 commented 4 years ago

facing same issue.. @totten any thought on how to fix this? I can work on a PR if you think that excluding non-absolute links from tracking makes sense (in case of #anchors, I think it does)

sluc23 commented 4 years ago

@totten , I think I've found a worse bug related to this.. because the anchor (#myAnchor) link is tracked by Civi, the url is replaced in the email body like: <civi_url>/civicrm/extern/url.php?u=2&qid=1, when this is clicked in email client, Civi tracks the click and then replaces the url for <civi_url>/civicrm/extern/url.php?u=2&qid=1#myAnchor, which tracks again the click, and replaces it again.. getting into a redirection loop, stopped by the browser (after 20 times in my case with FF).

So the result of clicking a tracked "#" link is that the user cannot move into anchors inside same email body page, is redirected to Civi, followed by a browser error page because of the redirection loop, and Civi tracks this click 20 times (in my tests with Firefox)

I'll work on a PR to exclude anchor links from tracking.

sluc23 commented 4 years ago

PR #51 submitted!

totten commented 4 years ago

51 was closed because it treated both internal and external anchors the same. Internal anchors should be untracked, but external anchors should still be tracked.

In the discussion of #51, I added some suggestions for an updated patch.