Skip to content
This repository has been archived by the owner. It is now read-only.

Pass {{link}} url into outlook-specific markup in email templates. #1028

Merged
merged 1 commit into from Apr 29, 2014

Conversation

@rfk
Copy link
Member

@rfk rfk commented Apr 29, 2014

Fixes #1027 in my testing with Outlook2013

@rfk
Copy link
Member Author

@rfk rfk commented Apr 29, 2014

Are there email-rending tests that we could usefully add to, to prevent this re-appearing in future?

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Apr 29, 2014

@rfk - @johngruen used Email on Acid, if you are inclined, there are discussions in #706 and #761.

@rfk
Copy link
Member Author

@rfk rfk commented Apr 29, 2014

...yeah, I'm not sure what to make of this travis failure

@johngruen
Copy link
Contributor

@johngruen johngruen commented Apr 29, 2014

@rfk @shane-tomlinson I am an idiot. FWIW Email on Acid didn't catch the broken links under the Outlook conditionals.
screen shot 2014-04-29 at 9 38 04 am
(first link 408s b/c localhost)

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Apr 29, 2014

Here is what the initial confirmation email looks like in Outlook 2011 on OSX (in Spanish, of course):

firefox accounts in outlook

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Apr 29, 2014

Ahha, from https://litmus.com/blog/a-guide-to-rendering-differences-in-microsoft-outlook-clients, it looks like Outlook 2011 uses Webkit, my test was invalid.

EMAIL RENDERING IN OUTLOOK 2011

Outlook 2011, also known as “Outlook for Mac,” runs on OS X, Apple’s proprietary operating system for Macintosh (Mac) computers, and uses WebKit to render emails. This is great news for email designers, since Outlook for Mac has excellent support for HTML and CSS. There is no PC/Windows equivalent, although Outlook for Mac renders quite similarly to Apple Mail, which also uses WebKit to render.

@johngruen
Copy link
Contributor

@johngruen johngruen commented Apr 29, 2014

@shane-tomlinson: the offending clients are 2007,2011,and 2013. I got someone in QA to test on 2013 and confirm that this patch works. I believe all there have the same rendering engine.

@nchapman
Copy link
Contributor

@nchapman nchapman commented Apr 29, 2014

Nick CI (run against Sauce) gives this a 👍

TOTAL: tested 1 platforms, 0/105 tests failed
@pdehaan pdehaan added the email label Apr 29, 2014
@pdehaan
Copy link
Contributor

@pdehaan pdehaan commented Apr 29, 2014

I'm fine w/ somebody merging this.
Travis is still, well, being Travis and 1 test is failing.

I verified the raw email from ux test server is sending the correct URL (instead of "http://").
Attempting to test w/ Outlook in a native Win 8 environment proved challenging as after speaking w/ a MS rep for about 15 minutes in webchat, my only options are to buy Outlook 2013 for $109 or else download a "free" trial to Office 365, but still have to give them my credit card or paypal information "just in case I want to keep using it after my free trial". So it looks like I have to hand over my most sensitive banking information in order to test their software and confirm this issue.

I vote we merge now this and see if its worth getting into Prod today/tomorrow and in the meantime I'll dig around internally and try and find if I can get the MSDN information to download copies of Outlook to test on without having to give anybody my banking details.

/cc @ckarlof @edmoz @zaach @nchapman @shane-tomlinson @rfk

@nchapman
Copy link
Contributor

@nchapman nchapman commented Apr 29, 2014

I vote we merge now this

All the tests pass locally and against Sauce (from my local machine) so I shall merge this with caution.

nchapman added a commit that referenced this pull request Apr 29, 2014
fix(email): Pass {{link}} url into outlook-specific markup in email templates.
@nchapman nchapman merged commit c391a5e into master Apr 29, 2014
1 check failed
1 check failed
@graydon
continuous-integration/travis-ci The Travis CI build failed
Details
@nchapman nchapman deleted the rfk/fix-link-rendering-in-outlook branch Apr 29, 2014
@pdehaan
Copy link
Contributor

@pdehaan pdehaan commented Apr 29, 2014

i shall merge this with caution

? Like, hit the green button, but softly?

@pdehaan
Copy link
Contributor

@pdehaan pdehaan commented Apr 29, 2014

Looks good in prod:

<!--[if mso]>
<v:roundrect =
xmlns:v=3D=22urn:schemas-microsoft-com:vml=22 xmlns:w=3D=22urn:schemas-micr=
osoft-com:office:word=22 href=3D=22https://api.accounts.firefox.=
com/v1/verify=5Femail=3Fuid=3D73008a8414614b6bb2f0bee0fee0ca0e&code=3D64540=
9bec56a481b9a1ff32b4126cfa9=22 style=3D=22height:40px;v-text-anchor:middle;=
width:300px;=22 arcsize=3D=2210%=22 stroke=3D=22f=22 =
fillcolor=3D=22#0095DD=22>
<w:anchorlock/>
<center>
<![endif]-->
<a href=3D=22https://api.=
accounts.firefox.com/v1/verify=5Femail=3Fuid=3D73008a8414614b6bb2f0bee0fee0=
ca0e&code=3D645409bec56a481b9a1ff32b4126cfa9=22 style=3D=22text-decoration:=
none;font-size:20px;font-family:Helvetica;display:block;color:#FFFFFF;paddi=
ng:15px !important;-ms-text-size-adjust:100%;-webkit-text-size-adjust:100%;=
=22>Verify</a>
<!--[if mso]>
</center>
</v:roundrect>
<![endif]-->
@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented May 1, 2014

Thanks @rfk for coming up with this fix so quickly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants