* [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