Last Comment Bug 1102364 - Add microdata to HTML bugmail so GMail can display a "View bug" button
: Add microdata to HTML bugmail so GMail can display a "View bug" button
Status: RESOLVED FIXED
: relnote
Product: Bugzilla
Classification: Server Software
Component: Email Notifications (show other bugs)
: unspecified
: All All
-- enhancement (vote)
: Bugzilla 6.0
Assigned To: Ed Morley [:emorley]
: default-qa
:
Mentors:
: 1114796 (view as bug list)
Depends on:
Blocks: 1139840 1139872
  Show dependency treegraph
 
Reported: 2014-11-20 10:01 PST by YF (Yang)
Modified: 2016-01-08 05:27 PST (History)
8 users (show)
mail: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Add Microdata for a "View bug" action to HTML bugmail (2.30 KB, patch)
2014-11-25 14:57 PST, Ed Morley [:emorley]
glob: review-
Details | Diff | Splinter Review
Example HTML bugmail (1.91 KB, text/plain)
2014-11-25 14:59 PST, Ed Morley [:emorley]
no flags Details
Add Microdata for a "View bug" action to HTML bugmail (2.33 KB, patch)
2014-11-26 15:53 PST, Ed Morley [:emorley]
dkl: review-
Details | Diff | Splinter Review
Add Microdata for a "View bug" action to HTML bugmail (2.33 KB, patch)
2015-02-20 11:20 PST, Ed Morley [:emorley]
dkl: review+
Details | Diff | Splinter Review

Comment 1 User image Frédéric Buclin 2014-11-20 10:42:07 PST
Could you be more specific about what you would like to see implemented instead of simply pasting a URL? The link to the bug report is already in the bugmail, so I don't see which problem you try to solve.
Comment 2 User image YF (Yang) 2014-11-20 10:47:48 PST
I want to be able to directly open the bug page, via a one-touch button, rather than expand the message content to find the bug page link.
Comment 3 User image David Lawrence [:dkl] 2014-11-20 10:59:24 PST
This is similar to a request by a Mozilla developer for BMO in bug 1100476. There is a screenshot also that would show how the buttons look for other similar actions with other sites.

We should have a button for going to the bug for normal bugmail, and a different button for attachment flag email, such as 'review?', to go to the attachment details directly.

dkl
Comment 4 User image Ed Morley [:emorley] 2014-11-21 08:59:40 PST
I'd like both a "view bug" button for normal bugmail, and then for review mails a "View Review".

Since the "View Review" part is specific to the Review extension (I'm presuming native Bugzilla doesn't have patch review support?), let's keep bug 1100476 to be about that, and this bug to be about "View bug", which we can actually implement downstream :-)
Comment 5 User image Ed Morley [:emorley] 2014-11-25 14:16:17 PST
(In reply to Ed Morley (moved to Treeherder team) [:edmorley] from comment #4)
> I'd like both a "view bug" button for normal bugmail, and then for review
> mails a "View Review".
> 
> Since the "View Review" part is specific to the Review extension (I'm
> presuming native Bugzilla doesn't have patch review support?), let's keep
> bug 1100476 to be about that, and this bug to be about "View bug", which we
> can actually implement downstream :-)

Ok I misunderstood what the Review extension does. However, whilst upstream Bugzilla does support request flags, the emails are plaintext only, so I'll land the "View bug" button part separately, since it will be simpler.
Comment 6 User image Ed Morley [:emorley] 2014-11-25 14:57:07 PST
Created attachment 8528625 [details] [diff] [review]
Add Microdata for a "View bug" action to HTML bugmail

This allows GMail to display a "View bug" action button, once the email sender has registered using these steps:
https://developers.google.com/gmail/markup/registering-with-google

For bug changes where a comment was added, the link used for "View bug" includes the "#cN" comment anchor. For cases where there are multiple comments in one Bugmail, the patch intentionally clobbers the value of "comment_anchor" on each iteration, since new_comments is reversed, and so we want the last value (ie the oldest comment number) for the anchor.

Whilst Github's markup (see bug 1100476 comment 2) uses a description that's specific to the action (eg "View this Foo on GitHub", the spec [1] actually states that it should be a description of the email, so I instead went with "Bugzilla bug update notification". Happy to change that to whatever - it isn't surfaced anywhere I can see in the GMail UI.

Similarly, Github uses title case for the action name (eg "View Issue"), but the GMail page examples are all in sentence case, so I went with "View bug".

I tested this in my local Docker instance, and the resultant HTML markup part of the Bugmail in mailer.testfile (will attach shortly) passed validation at https://www.google.com/webmasters/markup-tester/

There's the obvious "why should we do this, since Gmail....", however given:
- the Microdata spec can (and hopefully will) be used by other providers/clients
- the change is fairly small
- GMail is fairly widely used compared to other providers
- Mozilla is switching to GMail for its own email
...I hope this should be fine.

[1] https://developers.google.com/gmail/markup/reference/go-to-action#specification and http://schema.org/EmailMessage
Comment 7 User image Ed Morley [:emorley] 2014-11-25 14:59:01 PST
Created attachment 8528631 [details]
Example HTML bugmail
Comment 8 User image Ed Morley [:emorley] 2014-11-25 15:09:57 PST
Note this will require bug 240169 before we're able to register to use this on BMO, however I imagine this feature may still be useful for other deployments, even if that bug proves to be challenging.
Comment 9 User image Byron Jones ‹:glob› 2014-11-25 21:31:16 PST
Comment on attachment 8528625 [details] [diff] [review]
Add Microdata for a "View bug" action to HTML bugmail

Review of attachment 8528625 [details] [diff] [review]:
-----------------------------------------------------------------

thanks for the patch ed.
overall this looks good.  just a few minor tweaks required.

::: template/en/default/email/bugmail.html.tmpl
@@ +64,5 @@
>        [% END %]
>        </ul>
> +      <div itemscope itemtype="http://schema.org/EmailMessage">
> +        <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
> +          <link itemprop="url" href="show_bug.cgi?id=[% bug.id %][% comment_anchor %]"/>

please make that a full url:
href="[% urlbase FILTER none %]show_bug.cgi?..

you need to filter bug.id and comment_anchor.
executing "runtests.pl" will catch this issue for you:

t/008filter.t ........ 55/286
#   Failed test '(en/default) template/en/default/email/bugmail.html.tmpl has unfiltered directives:
#   68: bug.id
#   68: comment_anchor
# --ERROR'
#   at t/008filter.t line 118.
# Looks like you failed 1 test of 286.
Comment 10 User image Ed Morley [:emorley] 2014-11-26 15:42:23 PST
(In reply to Byron Jones ‹:glob› from comment #9)
> please make that a full url:
> href="[% urlbase FILTER none %]show_bug.cgi?..

Further up the template there is:
<base href="[% urlbase FILTER html %]">

It would presumably make sense for these to be consistent. Would you like me to use 'html' or 'none' for both?

> you need to filter bug.id and comment_anchor.
> executing "runtests.pl" will catch this issue for you:

bug.id is presumably always an int.
comment_anchor is a fixed string that we specify, concatenated with an array size, so also fairly safe.

Should I use none or html for them? Guess safer against future accidental changes if I use html?

> t/008filter.t ........ 55/286
> #   Failed test '(en/default) template/en/default/email/bugmail.html.tmpl
> has unfiltered directives:
> #   68: bug.id
> #   68: comment_anchor
> # --ERROR'
> #   at t/008filter.t line 118.
> # Looks like you failed 1 test of 286.

Sorry for not running the tests, I'd glanced at them and didn't see anything with bugmail in the name, so didn't think to run them, my bad (and checksetup.pl was failing after I'd switched to master, so thought it was going to be fairly involved). I've got them running now.
Comment 11 User image Ed Morley [:emorley] 2014-11-26 15:45:46 PST
(In reply to Ed Morley (moved to Treeherder team) [:edmorley] from comment #10)
> Further up the template there is:
> <base href="[% urlbase FILTER html %]">
> 
> It would presumably make sense for these to be consistent. Would you like me
> to use 'html' or 'none' for both?

There seems to be a precedent of using html elsewhere:
https://github.com/bugzilla/bugzilla/search?l=perl&q=urlbase
Comment 12 User image Ed Morley [:emorley] 2014-11-26 15:53:17 PST
Created attachment 8529346 [details] [diff] [review]
Add Microdata for a "View bug" action to HTML bugmail

Picked one way, happy to change :-)

Also I'm not sure what the convention is re:
  [% "$bug.id$comment_anchor" FILTER html %]
vs
  [% bug.id FILTER html %][%comment_anchor FILTER html %]
Comment 13 User image Frédéric Buclin 2014-12-22 15:04:45 PST
*** Bug 1114796 has been marked as a duplicate of this bug. ***
Comment 14 User image Ed Morley [:emorley] 2015-02-19 02:02:59 PST
Byron, is there someone else who would be better to ask for review? I know you've got a lot on your plate at the moment, but I'm keen to land this 9 line patch (+on bmo) so I can start hassling IT about DKIM and then getting Google to enable on their side - and the review has been pending for almost 3 months. Thanks :-)
Comment 15 User image David Lawrence [:dkl] 2015-02-19 08:45:56 PST
Comment on attachment 8529346 [details] [diff] [review]
Add Microdata for a "View bug" action to HTML bugmail

Review of attachment 8529346 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Fixes on commit. r=dkl

::: template/en/default/email/bugmail.html.tmpl
@@ +64,5 @@
>        [% END %]
>        </ul>
> +      <div itemscope itemtype="http://schema.org/EmailMessage">
> +        <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
> +          <link itemprop="url" href="[% urlbase FILTER html %]show_bug.cgi?id=[% "$bug.id$comment_anchor" FILTER html %]"/>

[% urlbase FILTER none %] and [% "$bug.id$comment_anchor" FILTER uri %] instead.

@@ +65,5 @@
>        </ul>
> +      <div itemscope itemtype="http://schema.org/EmailMessage">
> +        <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
> +          <link itemprop="url" href="[% urlbase FILTER html %]show_bug.cgi?id=[% "$bug.id$comment_anchor" FILTER html %]"/>
> +          <meta itemprop="name" content="View [% terms.bug %]"/>

Should this be "View Bug" instead? ([% terms.Bug %])
Comment 16 User image Ed Morley [:emorley] 2015-02-19 08:50:04 PST
Thank you for the review :-)

(In reply to David Lawrence [:dkl] from comment #15)
> @@ +65,5 @@
> >        </ul>
> > +      <div itemscope itemtype="http://schema.org/EmailMessage">
> > +        <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
> > +          <link itemprop="url" href="[% urlbase FILTER html %]show_bug.cgi?id=[% "$bug.id$comment_anchor" FILTER html %]"/>
> > +          <meta itemprop="name" content="View [% terms.bug %]"/>
> 
> Should this be "View Bug" instead? ([% terms.Bug %])


(In reply to Ed Morley [:edmorley] from comment #6)
> Similarly, Github uses title case for the action name (eg "View Issue"), but
> the GMail page examples are all in sentence case, so I went with "View bug".

The example page is at: https://developers.google.com/gmail/markup/reference/go-to-action
Comment 17 User image Frédéric Buclin 2015-02-19 10:12:12 PST
(In reply to David Lawrence [:dkl] from comment #15)
> [% urlbase FILTER none %]

No, it must be HTML-escaped. FILTER html is right.
Comment 18 User image David Lawrence [:dkl] 2015-02-19 11:27:13 PST
Comment on attachment 8529346 [details] [diff] [review]
Add Microdata for a "View bug" action to HTML bugmail

Review of attachment 8529346 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Frédéric Buclin from comment #17)
> (In reply to David Lawrence [:dkl] from comment #15)
> > [% urlbase FILTER none %]
> 
> No, it must be HTML-escaped. FILTER html is right.

Sorry bout that. Ed, please use FILTER html in the next revision.

dkl
Comment 19 User image Frédéric Buclin 2015-02-19 11:35:45 PST
(In reply to David Lawrence [:dkl] from comment #18)
> Sorry bout that. Ed, please use FILTER html in the next revision.

That's what he did. You said to use FILTER none instead. ;)
Comment 20 User image David Lawrence [:dkl] 2015-02-19 12:29:57 PST
(In reply to Frédéric Buclin from comment #19)
> (In reply to David Lawrence [:dkl] from comment #18)
> > Sorry bout that. Ed, please use FILTER html in the next revision.
> 
> That's what he did. You said to use FILTER none instead. ;)

True. Reason I made this comment is he said he would make a new revision to be committed with the suggested changes even tho I r+'ed. We still need to the [% "$bug.id$comment_anchor" FILTER uri %] fix as well.

dkl
Comment 21 User image Ed Morley [:emorley] 2015-02-20 11:20:52 PST
Created attachment 8567227 [details] [diff] [review]
Add Microdata for a "View bug" action to HTML bugmail

s/html/uri/ for the URL param.

Commit message is in the header of the patch, please can you copy it in when you land (unless whatever tool you use does that with Git already; seem to remember it doesn't from before :-)).
Comment 22 User image David Lawrence [:dkl] 2015-02-20 14:26:58 PST
Comment on attachment 8567227 [details] [diff] [review]
Add Microdata for a "View bug" action to HTML bugmail

Review of attachment 8567227 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Comment 23 User image David Lawrence [:dkl] 2015-02-22 14:00:43 PST
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   a500926..240dec4  master -> master
Comment 24 User image Ed Morley [:emorley] 2015-02-23 03:47:28 PST
Many thanks :-)
Comment 25 User image Ed Morley [:emorley] 2015-02-23 03:55:03 PST
It would be good to mention this is the relnote, using something like:

* HTML bugmail now contains Microdata for a "View bug" action, that allows providers such as GMail to display a quick access button next to each Bugzilla email in users' inboxes. To register for GMail, follow the steps at: https://developers.google.com/gmail/markup/registering-with-google
Comment 26 User image Justin Dolske [:Dolske] 2015-02-27 19:11:18 PST
Is this actually working? I don't see any "View Bug" buttons on bugmail in GMail.
Comment 27 User image Mark Côté [:mcote] 2015-02-27 19:49:57 PST
Dolkse: I actually asked about this the other day and was referred to comment #8. :\ We'll try to get some movement on bug 240169.
Comment 28 User image Ed Morley [:emorley] 2015-03-05 03:35:43 PST
(In reply to David Lawrence [:dkl] from comment #15)
> ::: template/en/default/email/bugmail.html.tmpl
> @@ +64,5 @@
> >        [% END %]
> >        </ul>
> > +      <div itemscope itemtype="http://schema.org/EmailMessage">
> > +        <div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
> > +          <link itemprop="url" href="[% urlbase FILTER html %]show_bug.cgi?id=[% "$bug.id$comment_anchor" FILTER html %]"/>
> 
> [% urlbase FILTER none %] and [% "$bug.id$comment_anchor" FILTER uri %]
> instead.

Switching the latter to FILTER uri seems to have broken the URL, since the output (at least from what I receive; I can't test locally to see what is sent), is:
https://bugzilla.mozilla.org/show_bug.cgi?id=1139664%23c2

Which results in "'1139664#c2' is not a valid bug number nor an alias to a bug."

Is there another filter type I can use instead that's more appropriate? Or can I just use |filter none| given the anchor is only generated from a .count? Or should I split the params up more and encode the bug number and comment number separately, and leave the hash untouched?
Comment 29 User image Ed Morley [:emorley] 2015-03-05 03:40:52 PST
(In reply to Justin Dolske [:Dolske] from comment #26)
> Is this actually working? I don't see any "View Bug" buttons on bugmail in
> GMail.

I've broken the registration part out to bug 1139840 to avoid confusion (and since this bug is Bugzilla upstream, whereas we're talking about BMO).
Comment 30 User image Byron Jones ‹:glob› 2015-03-05 04:49:30 PST
(In reply to Ed Morley [:edmorley] from comment #28)
> Switching the latter to FILTER uri seems to have broken the URL, since the
> output (at least from what I receive; I can't test locally to see what is
> sent), is:
> https://bugzilla.mozilla.org/show_bug.cgi?id=1139664%23c2

please don't re-open fixed and committed bugs; file a new bug.

Note You need to log in before you can comment on or make changes to this bug.