Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

links_in_email includes doctype uri and namespace #72

Open
nicholaides opened this Issue · 5 comments

5 participants

@nicholaides

In HTML emails links_in_email will include the URIs specified in the Doctype URI, any HTML namespace tags.

e.g.

<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html xmlns='http://www.w3.org/1999/xhtml' xml:lang='en' lang='en'>

This breaks click_first_link_in_email for HTML emails because the first link is http://www.w3.org/TR/html4/loose.dtd in this example.

I patched my version to this hacky solulution:

def links_in_email(email)
  URI.extract(email.default_part_body.to_s, ['http', 'https']).reject do |link|
    ['www.w3.org', 'www.openmobilealliance.org'].include? URI.parse(link).host
  end
end

An ideal solution would probably use Nokogiri to get the links from all the <a href=...> elements

@nicholaides

Here's a better solution:

def links_in_email(email)
  part = email.default_part
  if part.content_type =~ /text\/html/
    Nokogiri::HTML::Document.parse(part.body.to_s).search('a').map{|a| a[:href] }
  else
    URI.extract(part.body.to_s, ['http', 'https'])
  end
end
@iblue

URI.extract also does not work for URIs in single quotes (which are generated when using HAML).

URI.extract("<a href=3D'http://example.org/'>Click me.</a>") # => ["http://example.org/'"]

Using @nicholaides solution would require Nokogiri, so I would suggest using some simple regular expressions instead.

@TylerRick

Thanks for the workaround, @nicholaides!

It would be nice if this could be fixed in this gem. Nokogiri seems like a much more reliable solution than regular expressions.

If the dependency on Nokogiri is a problem (it's certainly not for me), what about making it optional?

One could either require 'nokogiri' and rescue LoadError, or check for defined?(Nokogiri)... and only if it's loaded, use Nokogiri in links_in_email....

@bmabey
Owner

Yeah, that sounds reasonable. I'll merge in a PR with optional Nokogiri support.

@coorasse

this issue is pretty old, but still valid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.