-
-
Notifications
You must be signed in to change notification settings - Fork 669
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
Conversation
Ok - thanks, this looks good. I'll try and test it later, for now though I've marked it as work in progress. |
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? |
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? |
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). |
That's a good point. I'll think about it and look at how the image is presented in the HTML.
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. |
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. |
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. |
Will give it a test later today. But should be ok as it is probably already tested by google :-) |
Screenshot example: https://ibb.co/dfFZgv |
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. |
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. |
Well that's sad. I have dim recollections of this from years ago and expected things to be a little better now. |
Yeah we have to use `juice` on our HTML emails at work to inline all the
styles.
https://www.npmjs.com/package/juice
…On Sun, Sep 10, 2017 at 3:34 PM, Matt Simerson ***@***.***> wrote:
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.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2091 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAobY_-84MZP-zHsKl1XLJ1aoXvAsslGks5shDnGgaJpZM4PSUJs>
.
|
What's remaining to do on this pr? |
Testing - I haven't had a chance to do so yet. |
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. |
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:
Checklist: