Send mails as HTML as default. Setting for send as plain text. #1648
Conversation
|
I actually think mails should be sent as text by default. The fact that the text looks crappy is the bug to fix. I think it depends on mail templates being HTML. They should be turned to text. |
tboerger
added
the
lgtm/need 2
|
I think many users like nicer e-mails. Even if HTML is default, it's sent as multipart and if you don't like then disable it. |
|
I vote for multipart as default as that is the same github/gitlab is doing |
| @@ -1250,9 +1250,9 @@ func newMailService() { | |||
| } | |||
|
|
|||
| MailService = &Mailer{ | |||
| QueueLength: sec.Key("SEND_BUFFER_LEN").MustInt(100), | |||
| Name: sec.Key("NAME").MustString(AppName), | |||
| EnableHTMLAlternative: sec.Key("ENABLE_HTML_ALTERNATIVE").MustBool(), | |||
There was a problem hiding this comment.
Don't remove old configs, deprecate them...
modules/mailer/mailer.go
Outdated
| body, err := html2text.FromString(htmlBody) | ||
| var plainBody string | ||
| var err error | ||
| if strings.Contains(body[:100], "<html>") { |
There was a problem hiding this comment.
Why are we checking this? What if the body starts with <body> (as is customary for templates) or <div>?
There was a problem hiding this comment.
At this stage it should be a complete HTML file. Anyway I removed it but still check for HTML when sending as plain to generate warning should there be a in the body.
| @@ -1269,6 +1269,10 @@ func newMailService() { | |||
| } | |||
| MailService.From = sec.Key("FROM").MustString(MailService.User) | |||
|
|
|||
| if sec.HasKey("ENABLE_HTML_ALTERNATIVE") { | |||
| log.Warn("ENABLE_HTML_ALTERNATIVE is deprecated, use SEND_AS_PLAIN_TEXT") | |||
There was a problem hiding this comment.
Still need to use if SEND_AS_PLAIN_TEXT isn't being set :/
|
LGTM |
tboerger
added
lgtm/need 1
modules/mailer/mailer.go
Outdated
| msg.SetBody("text/plain", body) | ||
| if setting.MailService.EnableHTMLAlternative { | ||
| msg.AddAlternative("text/html", htmlBody) | ||
| plainBody, _ := html2text.FromString(body) |
There was a problem hiding this comment.
Why do we ignore the error?
There was a problem hiding this comment.
good catch :)
if err != nil || setting.MailService.SendAsPlainText {should do the trick :)
There was a problem hiding this comment.
Not sure why I did that, but I have changed it now :)
* Add new option SendAsPlainText. remove EnableHTMLAlternative * Send HTML mails as default * Add html check if html2text should be performed
|
LGTM |
tboerger
added
lgtm/done
As the default mail templates are all HTML and look rather crappy if sent as plain text I think the default should be to send as HTML. HTML is added as mime/alternative so it's still possible to view as plain text.
I have added a new setting, SEND_AS_PLAIN_TEXT, to send as plain text as default instead. Old setting, ENABLE_HTML_ALTERNATIVE, is removed.
Fixes #1342