* [pbs-devel] [PATCH proxmox-backup] email_notifications: make html template more valid @ 2022-11-29 16:40 Stefan Sterz 2022-11-30 14:26 ` Dominik Csapak 0 siblings, 1 reply; 6+ messages in thread From: Stefan Sterz @ 2022-11-29 16:40 UTC (permalink / raw) To: pbs-devel some webmail interfaces may behave unexpectedly if the html is invalid. thus, close tags and declare a language. Signed-off-by: Stefan Sterz <s.sterz@proxmox.com> --- src/server/email_notifications.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs index b3298cf9..378e805e 100644 --- a/src/server/email_notifications.rs +++ b/src/server/email_notifications.rs @@ -294,7 +294,7 @@ fn send_job_status_mail(email: &str, subject: &str, text: &str) -> Result<(), Er // Note: OX has serious problems displaying text mails, // so we include html as well let html = format!( - "<html><body><pre>\n{}\n<pre>", + "<html lang=\"en\"><head></head><body><pre>\n{}\n</pre></body></html>", handlebars::html_escape(text) ); -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] email_notifications: make html template more valid 2022-11-29 16:40 [pbs-devel] [PATCH proxmox-backup] email_notifications: make html template more valid Stefan Sterz @ 2022-11-30 14:26 ` Dominik Csapak 2022-11-30 15:02 ` Stefan Sterz 0 siblings, 1 reply; 6+ messages in thread From: Dominik Csapak @ 2022-11-30 14:26 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Stefan Sterz some thoughts: i don't think 'more valid' is possible, either the markup is valid or not On 11/29/22 17:40, Stefan Sterz wrote: > some webmail interfaces may behave unexpectedly if the html is > invalid. thus, close tags and declare a language. which webmail interfaces? and according to whom is the old invalid and the new valid? i checked the html spec of the whatwg[0] and it seems to me that empty tags ('head' too) can be omitted also the 'lang' attribute is only recommended, not required the correct closing of the tags good though 0: https://html.spec.whatwg.org/multipage/semantics.html > > Signed-off-by: Stefan Sterz <s.sterz@proxmox.com> > --- > src/server/email_notifications.rs | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs > index b3298cf9..378e805e 100644 > --- a/src/server/email_notifications.rs > +++ b/src/server/email_notifications.rs > @@ -294,7 +294,7 @@ fn send_job_status_mail(email: &str, subject: &str, text: &str) -> Result<(), Er > // Note: OX has serious problems displaying text mails, > // so we include html as well > let html = format!( > - "<html><body><pre>\n{}\n<pre>", > + "<html lang=\"en\"><head></head><body><pre>\n{}\n</pre></body></html>", > handlebars::html_escape(text) > ); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] email_notifications: make html template more valid 2022-11-30 14:26 ` Dominik Csapak @ 2022-11-30 15:02 ` Stefan Sterz 2022-11-30 15:14 ` Dominik Csapak 0 siblings, 1 reply; 6+ messages in thread From: Stefan Sterz @ 2022-11-30 15:02 UTC (permalink / raw) To: Dominik Csapak, Proxmox Backup Server development discussion On 11/30/22 15:26, Dominik Csapak wrote: > some thoughts: > > i don't think 'more valid' is possible, either the markup is valid or not > fair, but entirely valid html in emails is .. interesting. for example, it would be nice to add a doctype, but from what i found i am not 100% certain that all email clients can handle it well (e.g. outlook 2019 can't handle the html 5 doctype apparently and ignores it [1]). it seems that a common practice is to use an older html 4 doctype, though [2]. [1]: https://www.caniemail.com/search/?s=doctype [2]: https://stackoverflow.com/questions/34319889/doctype-for-html-email > On 11/29/22 17:40, Stefan Sterz wrote: >> some webmail interfaces may behave unexpectedly if the html is >> invalid. thus, close tags and declare a language. > > which webmail interfaces? and according to whom is the old invalid and > the new valid? > well i checked it against w3's nu validator [3]. i started out with something the validator was happy with and then slowly removed things i wasn't certain would render correctly due to html mail weirdness. lost the doctype, because see above. lost the title, because apparently android 4.4 used it in notifications. so if you used it for e.g. the subject, in notifications the subject would display twice? couldn't confirm that, but i thought it would be inconvenient. the last part that valid html requires is a character encoding, but that should be set in emails via the `Content-Type` header. so i wasn't sure if it makes sense to add it to the html too. [3]: https://validator.w3.org/nu/#file > i checked the html spec of the whatwg[0] and it seems to me that > empty tags ('head' too) can be omitted > true, sorry, should have removed it when removing the title. > also the 'lang' attribute is only recommended, not required > yes, however, why not follow the recommendation? sorry maybe i am missing something super obvious but is there are good reason? the reasoning behind the recommendation is accessibility, making it easier for speech synthesis to pick up the right intonation. > the correct closing of the tags good though > > 0: https://html.spec.whatwg.org/multipage/semantics.html > >> >> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com> >> --- >> src/server/email_notifications.rs | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/server/email_notifications.rs >> b/src/server/email_notifications.rs >> index b3298cf9..378e805e 100644 >> --- a/src/server/email_notifications.rs >> +++ b/src/server/email_notifications.rs >> @@ -294,7 +294,7 @@ fn send_job_status_mail(email: &str, subject: >> &str, text: &str) -> Result<(), Er >> // Note: OX has serious problems displaying text mails, >> // so we include html as well >> let html = format!( >> - "<html><body><pre>\n{}\n<pre>", >> + "<html >> lang=\"en\"><head></head><body><pre>\n{}\n</pre></body></html>", >> handlebars::html_escape(text) >> ); >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] email_notifications: make html template more valid 2022-11-30 15:02 ` Stefan Sterz @ 2022-11-30 15:14 ` Dominik Csapak 2022-11-30 15:19 ` Stefan Sterz 0 siblings, 1 reply; 6+ messages in thread From: Dominik Csapak @ 2022-11-30 15:14 UTC (permalink / raw) To: Stefan Sterz, Proxmox Backup Server development discussion On 11/30/22 16:02, Stefan Sterz wrote: > On 11/30/22 15:26, Dominik Csapak wrote: >> some thoughts: >> >> i don't think 'more valid' is possible, either the markup is valid or not >> > > fair, but entirely valid html in emails is .. interesting. for example, > it would be nice to add a doctype, but from what i found i am not 100% > certain that all email clients can handle it well (e.g. outlook 2019 > can't handle the html 5 doctype apparently and ignores it [1]). > > it seems that a common practice is to use an older html 4 doctype, > though [2]. i checked my personal inbox for html mails, and i found everything from html4/5 doctype, to no doctype, and even many mails that do not even declare <html> or <body> but start direct with e.g. <p> so maybe just dropping that and directly using the <pre> here would also be ok? > > [1]: https://www.caniemail.com/search/?s=doctype > [2]: https://stackoverflow.com/questions/34319889/doctype-for-html-email > >> On 11/29/22 17:40, Stefan Sterz wrote: >>> some webmail interfaces may behave unexpectedly if the html is >>> invalid. thus, close tags and declare a language. >> >> which webmail interfaces? and according to whom is the old invalid and >> the new valid? >> > > well i checked it against w3's nu validator [3]. i started out with > something the validator was happy with and then slowly removed things i > wasn't certain would render correctly due to html mail weirdness. lost > the doctype, because see above. lost the title, because apparently > android 4.4 used it in notifications. so if you used it for e.g. the > subject, in notifications the subject would display twice? couldn't > confirm that, but i thought it would be inconvenient. yeah in the html spec it says that when there is a seperate 'title' in the document (e.g. in mails) one can omit the title as for the doctype, i also could not really find any resource that would confirm the need for it in mails. when it renders correctly on (most) clients without though, it's fine to omit imo > > the last part that valid html requires is a character encoding, but that > should be set in emails via the `Content-Type` header. so i wasn't sure > if it makes sense to add it to the html too. > > [3]: https://validator.w3.org/nu/#file > >> i checked the html spec of the whatwg[0] and it seems to me that >> empty tags ('head' too) can be omitted >> > > true, sorry, should have removed it when removing the title. > >> also the 'lang' attribute is only recommended, not required >> > yes, however, why not follow the recommendation? sorry maybe i am > missing something super obvious but is there are good reason? the > reasoning behind the recommendation is accessibility, making it easier > for speech synthesis to pick up the right intonation. true, but it has nothing to do with validity, so it would at least deserve a sentences in the commit message ;) > >> the correct closing of the tags good though >> >> 0: https://html.spec.whatwg.org/multipage/semantics.html >> >>> >>> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com> >>> --- >>> src/server/email_notifications.rs | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/server/email_notifications.rs >>> b/src/server/email_notifications.rs >>> index b3298cf9..378e805e 100644 >>> --- a/src/server/email_notifications.rs >>> +++ b/src/server/email_notifications.rs >>> @@ -294,7 +294,7 @@ fn send_job_status_mail(email: &str, subject: >>> &str, text: &str) -> Result<(), Er >>> // Note: OX has serious problems displaying text mails, >>> // so we include html as well >>> let html = format!( >>> - "<html><body><pre>\n{}\n<pre>", >>> + "<html >>> lang=\"en\"><head></head><body><pre>\n{}\n</pre></body></html>", >>> handlebars::html_escape(text) >>> ); >>> >> >> > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] email_notifications: make html template more valid 2022-11-30 15:14 ` Dominik Csapak @ 2022-11-30 15:19 ` Stefan Sterz 2022-11-30 16:09 ` Thomas Lamprecht 0 siblings, 1 reply; 6+ messages in thread From: Stefan Sterz @ 2022-11-30 15:19 UTC (permalink / raw) To: Dominik Csapak, Proxmox Backup Server development discussion On 11/30/22 16:14, Dominik Csapak wrote: > On 11/30/22 16:02, Stefan Sterz wrote: >> On 11/30/22 15:26, Dominik Csapak wrote: >>> some thoughts: >>> >>> i don't think 'more valid' is possible, either the markup is valid or >>> not >>> >> >> fair, but entirely valid html in emails is .. interesting. for example, >> it would be nice to add a doctype, but from what i found i am not 100% >> certain that all email clients can handle it well (e.g. outlook 2019 >> can't handle the html 5 doctype apparently and ignores it [1]). >> >> it seems that a common practice is to use an older html 4 doctype, >> though [2]. > > i checked my personal inbox for html mails, and i found everything from > html4/5 doctype, to no doctype, and even many mails that do not even > declare <html> or <body> but start direct with e.g. <p> > > so maybe just dropping that and directly using the <pre> here > would also be ok? > maybe, i can't tell you what will happen though, html emails are weird, at least to me ^^' i'll go with what you prefer :) >> >> [1]: https://www.caniemail.com/search/?s=doctype >> [2]: https://stackoverflow.com/questions/34319889/doctype-for-html-email >> >>> On 11/29/22 17:40, Stefan Sterz wrote: >>>> some webmail interfaces may behave unexpectedly if the html is >>>> invalid. thus, close tags and declare a language. >>> >>> which webmail interfaces? and according to whom is the old invalid and >>> the new valid? >>> >> >> well i checked it against w3's nu validator [3]. i started out with >> something the validator was happy with and then slowly removed things i >> wasn't certain would render correctly due to html mail weirdness. lost >> the doctype, because see above. lost the title, because apparently >> android 4.4 used it in notifications. so if you used it for e.g. the >> subject, in notifications the subject would display twice? couldn't >> confirm that, but i thought it would be inconvenient. > > yeah in the html spec it says that when there is a seperate > 'title' in the document (e.g. in mails) one can omit the title > > as for the doctype, i also could not really find any resource > that would confirm the need for it in mails. when it renders > correctly on (most) clients without though, it's fine to omit imo > >> >> the last part that valid html requires is a character encoding, but that >> should be set in emails via the `Content-Type` header. so i wasn't sure >> if it makes sense to add it to the html too. >> >> [3]: https://validator.w3.org/nu/#file >> >>> i checked the html spec of the whatwg[0] and it seems to me that >>> empty tags ('head' too) can be omitted >>> >> >> true, sorry, should have removed it when removing the title. >> >>> also the 'lang' attribute is only recommended, not required >>> >> yes, however, why not follow the recommendation? sorry maybe i am >> missing something super obvious but is there are good reason? the >> reasoning behind the recommendation is accessibility, making it easier >> for speech synthesis to pick up the right intonation. > > true, but it has nothing to do with validity, so it would > at least deserve a sentences in the commit message ;) ill add it if we go with the "keep the full html structure" option instead of the "<pre>-only" one :) > >> >>> the correct closing of the tags good though >>> >>> 0: https://html.spec.whatwg.org/multipage/semantics.html >>> >>>> >>>> Signed-off-by: Stefan Sterz <s.sterz@proxmox.com> >>>> --- >>>> src/server/email_notifications.rs | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/src/server/email_notifications.rs >>>> b/src/server/email_notifications.rs >>>> index b3298cf9..378e805e 100644 >>>> --- a/src/server/email_notifications.rs >>>> +++ b/src/server/email_notifications.rs >>>> @@ -294,7 +294,7 @@ fn send_job_status_mail(email: &str, subject: >>>> &str, text: &str) -> Result<(), Er >>>> // Note: OX has serious problems displaying text mails, >>>> // so we include html as well >>>> let html = format!( >>>> - "<html><body><pre>\n{}\n<pre>", >>>> + "<html >>>> lang=\"en\"><head></head><body><pre>\n{}\n</pre></body></html>", >>>> handlebars::html_escape(text) >>>> ); >>>> >>> >>> >> > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] email_notifications: make html template more valid 2022-11-30 15:19 ` Stefan Sterz @ 2022-11-30 16:09 ` Thomas Lamprecht 0 siblings, 0 replies; 6+ messages in thread From: Thomas Lamprecht @ 2022-11-30 16:09 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Stefan Sterz, Dominik Csapak Am 30/11/2022 um 16:19 schrieb Stefan Sterz: > ill add it if we go with the "keep the full html structure" option > instead of the "<pre>-only" one 😄 If we change this I'd prefer a "full" HTML structure. Not much of a point in over-optimization here, and _if_ some client renders this badly, and the user checks the source of the mail and sees missing tags or the like, the technicality of it being valid w3c; we highly probably still will be the one blamed first. Anyhow, if there's no known (and not totally obscure) MUA that has issues with the current form I'd avoid touching it. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-30 16:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-29 16:40 [pbs-devel] [PATCH proxmox-backup] email_notifications: make html template more valid Stefan Sterz 2022-11-30 14:26 ` Dominik Csapak 2022-11-30 15:02 ` Stefan Sterz 2022-11-30 15:14 ` Dominik Csapak 2022-11-30 15:19 ` Stefan Sterz 2022-11-30 16:09 ` Thomas Lamprecht
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox