8000 Use the available multipart content-type to add HTML code to bounces by Bramzor · Pull Request #2091 · haraka/Haraka · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Use the available multipart content-type to add HTML code to bounces #2091

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

Merged
merged 6 commits into from
Sep 30, 2017
Merged

Use the available multipart content-type to add HTML code to bounces #2091

merged 6 commits into from
Sep 30, 2017

Conversation

Bramzor
Copy link
Contributor
@Bramzor Bramzor commented Sep 10, 2017

Fixes #2090

My first pull request so sorry if it isn't perfect.
(Needs to be tested. Was unable to test it myself in my current setup. Code itself is copied over but I was behind of the master.)
Todo: Fix Address objects so they print out an email address without the < >.

Changes proposed in this pull request:

  • Added HTML Bounces to Haraka

Checklist:

  • docs updated
  • tests updated
  • Changes updated

@smfreegard
Copy link
Collaborator

Ok - thanks, this looks good. I'll try and test it later, for now though I've marked it as work in progress.

@msimerson
Copy link
Member

Are we intending to change the default bounce to include the HTML and image? If not, shouldn't the outbound/bounce_message_* files live in test/config?

@smfreegard
Copy link
Collaborator

Are we intending to change the default bounce to include the HTML and image?

I think we should make this the default. It should give us some free advertising and it's easier to read for most people.

What do you think?

@msimerson
Copy link
Member

I was more curious than caring. My only caution would be getting "free advertising" we don't want. "Why are you Haraka people spamming me with these messages?," type of stuff, b/c recipients don't understand the message (IME, very few people grok DSN/bounces).

@smfreegard
Copy link
Collaborator

"Why are you Haraka people spamming me with these messages?

That's a good point. I'll think about it and look at how the image is presented in the HTML.

b/c recipients don't understand the message (IME, very few people grok DSN/bounces).

Indeed - although that is one of the reasons for including an HTML version of the bounce as we can address this shortcoming and make it far more clearer than it otherwise would.

Once I get around to testing this - I'll include a screenshot here so you can see the proposed changes and wording.

@Bramzor
Copy link
Contributor Author
Bramzor commented Sep 10, 2017

We could start making it optional now as intended, so good point, we can move it to test/config in that case. But we can make it the default afterwards as it is a real improvement to users. Bounces are indeed not user friendly but with the HTML, it is at least an improvement on that.
The image that is included by default is the image from the google bounce. Just an error image. It's not the purpose to add additional images for marketing or anything like that. Just to make it clear the message is not delivered.

@msimerson
Copy link
Member

Something else useful to test is how the bounce HTML is rendered in gmail/outlook/yahoo webmail. MUA's are generally pretty good but the webmail providers sometimes cruelly butcher the appearance of otherwise well-formed HTML.

@Bramzor
Copy link
Contributor Author
Bramzor commented Sep 10, 2017

Will give it a test later today. But should be ok as it is probably already tested by google :-)

@Bramzor
Copy link
Contributor Author
Bramzor commented Sep 10, 2017

Screenshot example: https://ibb.co/dfFZgv
Remembers me that I had to do an additional fix to show the "to" field as the current includes the < and >.

@msimerson
Copy link
Member

Looks great! Although I think I want to clean up the HTML and replace most of the inline styling and table with a tiny bit of CSS and a couple divs.

@Bramzor
Copy link
Contributor Author
Bramzor commented Sep 10, 2017

Than you might introduce issues with webmail, that is why they don't use that. The HTML looks ugly and old, but it does work in webmail clients as it cannot be impacted by external css and you don't see the HTML code anyway unless in the source.

@msimerson
Copy link
Member

Than you might introduce issues with webmail, that is why they don't use that.

Well that's sad. I have dim recollections of this from years ago and expected things to be a little better now.

@baudehlo
Copy link
Collaborator
baudehlo commented Sep 11, 2017 via email

@KingNoosh
Copy link
Member

What's remaining to do on this pr?

@smfreegard
Copy link
Collaborator

Testing - I haven't had a chance to do so yet.

@msimerson
Copy link
Member

Same with this one, I'd like to merge this tomorrow, after cutting a release. It's got some manual testing and it can get more widely tested in HEAD.

@msimerson msimerson merged commit adadf83 into 7885 haraka:master Sep 30, 2017
@msimerson msimerson mentioned this pull request Feb 16, 2018
@haraka haraka deleted a comment from codecov bot Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0