thoughtbot / griddler

Simplify receiving email in Rails (deprecated)
http://griddler.io/
MIT License
1.38k stars 199 forks source link

Griddler inexorably prefers text over html for raw_body #35

Closed svanderbleek closed 11 years ago

svanderbleek commented 11 years ago

I would like to have a configuration option to tell Griddler to prefer the HTML if available. If that sounds ok I'll implement it.

calebhearth commented 11 years ago

This would require some changes to how Griddler splits the body (reply_delimiter) as well, I think. You'd need to make sure that you removed the entire delimiter element, then also make sure that the result continues to be valid html.

For example, splitting on <p>delimiter</p> here would result in invalid HTML if you were to just use String#split.

<div class='email-body'>
  <p>Hey, here's my response to this email that you sent.</p>
  <p><a ...>and here's a link</a></p>
  <p>delimiter</p>
  <!-- some more stuff -->
</div>

We could rely on something like sanitize for ensuring that we maintain valid html after truncation, but that adds a lot of complexity and dependencies.

You'd also probably be expecting your users to be sending html emails, which isn't always going to be the case, whereas you'll (almost?) always be able to get a text version even of html.

@jayroh, @gabebw do you have any thoughts on this?

gabebw commented 11 years ago

It sounds hard and error-prone, as Caleb points out.

If it can be done cleanly, without adding significant technical debt - why not?

svanderbleek commented 11 years ago

I'm working on an implementation for my job, but might axe it because it is hard and error prone. I'll close the issue if nothing comes from it. :email:

calebhearth commented 11 years ago

I was hesitant to suggest trying it out, but if you're already working on this for work we'd be happy to take a look at the result.

calebhearth commented 11 years ago

I'm going to close this for now. Feel free to reopen/push code to this issue if you end up taking a whack at this and want a review, @svanderbleek.