Review: Needs Fixing
Thanks for the patch.
These were previously plain-text emails sent to non-Gmail users as well.
Dumping HTML-like markup into the start of them without even a Content-Type
change is not going to look good at all to anyone who doesn't use Gmail; and I
would be wary of making them anything other than text/plain anyway.
Can you point to the specification for this actions markup? We need to find
some approach that's a bit less horribly intrusive, if indeed it's worth doing.
As a last resort it may require a new PersonSettings column, but it will
depend on what options the spec affords us. For example, if it were possible
to do this by way of additional message headers, that would be *much* better
than dumping JSON into the message body.
Whatever approach ends up being workable, this needs to include tests.
Diff comments:
> === modified file 'lib/lp/bugs/emailtemplates/bug-notification-verbose.txt'
> --- lib/lp/bugs/emailtemplates/bug-notification-verbose.txt 2011-06-15
> 11:19:21 +0000
> +++ lib/lp/bugs/emailtemplates/bug-notification-verbose.txt 2016-07-10
> 01:02:16 +0000
> @@ -1,6 +1,18 @@
> +<script type="application/ld+json">
> +{
> + "@context": "http://schema.org";,
> + "@type": "EmailMessage",
> + "potentialAction": {
> + "@type": "ViewAction",
> + "name": "View Bug",
> + "url": "%(bug_url)s"
> + },
> + "description": ""
> +}
> +</script>
> %(content)s
>
> ---
> +--
Please don't do this. The trailing space wasn't an error; "\n-- \n" is a
pattern recognised by many mail user agents as separating message body from
signature.
> %(notification_rationale)s%(subscription_filters)s
> %(bug_url)s
>
--
https://code.launchpad.net/~daker/launchpad/fix.1600499/+merge/299616
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
