peterbe / premailer

Turns CSS blocks into style attributes
https://premailer.io
BSD 3-Clause "New" or "Revised" License
1.07k stars 188 forks source link

Phone number URLs are broken #140

Closed viernullvier closed 7 years ago

viernullvier commented 9 years ago

Valid tel: urls are prepended with base_url when they shouldn't be. This is somehow related to #8.

May I suggest adding an attribute that disables rewriting for a specific <a> tag? Maybe something like this:

<a href="tel:12345" data-premailer="ignore">Link</a>
peterbe commented 9 years ago

That data-premailer="ignore" feels a little bit to heavy-weight and hard to remember. How about we just notice that the url is special and conclude there and then to avoid it?

viernullvier commented 9 years ago

I just found out that everything works fine if the number includes a proper international prefix (ie. starts with a + sign). It seems to be that urljoin adheres to RFC 3966 a bit too strictly. Anyway, I'd slightly prefer my solution because it's actually not the first time I've run into issues with base_url.

Let's say we're using premailer to render HTML emails before sending them through an email service like Mailgun. If we want to add an unsubscribe link to our email, we have to insert a magic string like %unsubscribe_url% or something, the same goes for inserting URLs stored in template variables for bulk sending. Have a look at this:

0. <a href="foo/bar">I'm a regular link and work as intended.</a>
1. <a href="#foobar">I'm a relative link and work as intended.</a>
2. <a href="mailto:foo@bar.com">I'm an email link and work as intended.</a>
3. <a href="tel:+12345">I'm an international phone number link and work as intended.</a>
4. <a href="tel:12345">I'm a local phone number link and don't work with base_url.</a>
5. <a href="%unsubscribe_url%">I'm an unsubscribe link and don't work with base_url.</a>
6. <a href="{{template_variable}}">I'm a template variable link and don't work with base_url.</a>

While malformed tel: URLs might be common enough to justify a special treatment, cases 4 and 5 are vendor-specific and shouldn't be handled by premailer itself. However, working around premailer leads to some rather hackish solutions:

html = pm.Premailer(source, base_url=base, preserve_internal_links=True).transform()
html = html.replace("#%unsubscribe_url%", "%unsubscribe_url%")
peterbe commented 9 years ago

I think you have me convinced. There might not be an easy solution which we can hack around. Is this unique to when you set the base_url? I wonder why it's able to copy with mailto:foo@bar.com but not other non-regular-URL-looking things.