Skip to content
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

(dev/mail#41) "Preview as HTML" - Fix error with tracking urls #14081

Merged
merged 1 commit into from
Apr 19, 2019

Conversation

seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 commented Apr 17, 2019

Overview

Fix regression in email preview. Use-case:

  • Compose a "Mailing => New Mailing"
  • Fill in basic in fields.
  • In the body of the mailing, include a hyperlink
  • Press the button "Preview as HTML"

5.12 port of #13956

ping @colemanw @eileenmcnaughton @monishdeb @mattwire

@civibot
Copy link

civibot bot commented Apr 17, 2019

(Standard links)

@civibot civibot bot added the 5.12 label Apr 17, 2019
@totten totten changed the title dev/mail#41 Do not generate tracking urls if no mailing id has been p… (dev/mail#41) "Preview as HTML" - Fix error with tracking urls Apr 19, 2019
@totten
Copy link
Member

totten commented Apr 19, 2019

Updated both PR descriptions with clearer steps to reproduce.

Confirmed that this gets the pop-up to open. Relatedly, if I use the "Send Test" button, the generated test does produce a working/trackable link.

Interestingly, I noticed putting a hyperlink to www.google.com is problematic -- the preview renders well, but clicking the link leads to a white screen. This appears to be something unique to www.google.com, though -- other URLs (like en.wikipedia.org) seem fine. And it only affects the preview -- if "Send Test", all links were properly clickable. (Guess: google.com has some kind of anti-iframe mechanism?)

@totten totten merged commit 0dc68c5 into civicrm:5.12 Apr 19, 2019
@seamuslee001 seamuslee001 deleted the dev_mail_41_5_12 branch April 19, 2019 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants