Closed Bug 1102364 Opened 10 years ago Closed 9 years ago

Add microdata to HTML bugmail so GMail can display a "View bug" button

Categories

(Bugzilla :: Email Notifications, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Bugzilla 6.0

People

(Reporter: yfdyh000, Assigned: emorley)

References

Details

(Keywords: relnote)

Attachments

(2 files, 2 obsolete files)

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.
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.
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
See Also: → 1100476
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 :-)
(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.
Assignee: email-notifications → emorley
Status: NEW → ASSIGNED
Summary: View bug buttons for Gmail → Add microdata to Bugmail so GMail can display "View bug" buttons
Summary: Add microdata to Bugmail so GMail can display "View bug" buttons → Add microdata to bug change emails so GMail can display a "View bug" button
Summary: Add microdata to bug change emails so GMail can display a "View bug" button → Add microdata to HTML bugmail so GMail can display a "View bug" button
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
Attachment #8528625 - Flags: review?(glob)
Attached file Example HTML bugmail
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 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.
Attachment #8528625 - Flags: review?(glob) → review-
(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.
Flags: needinfo?(glob)
(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
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 %]
Flags: needinfo?(glob)
Attachment #8529346 - Flags: review?(glob)
Attachment #8528625 - Attachment is obsolete: true
Depends on: 807013
No longer depends on: 807013
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 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 %])
Attachment #8529346 - Flags: review?(glob) → review+
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
Flags: approval?
(In reply to David Lawrence [:dkl] from comment #15)
> [% urlbase FILTER none %]

No, it must be HTML-escaped. FILTER html is right.
Target Milestone: --- → Bugzilla 6.0
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
Attachment #8529346 - Flags: review+ → review-
(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. ;)
(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
Flags: approval?
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 :-)).
Attachment #8529346 - Attachment is obsolete: true
Attachment #8567227 - Flags: review?(dkl)
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
Attachment #8567227 - Flags: review?(dkl) → review+
Flags: approval?
Flags: approval? → approval+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   a500926..240dec4  master -> master
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Many thanks :-)
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
Keywords: relnote
Is this actually working? I don't see any "View Bug" buttons on bugmail in GMail.
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.
Depends on: 240169
(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?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 1139840
No longer depends on: 240169
(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).
(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.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Blocks: 1139872
You need to log in before you can comment on or make changes to this bug.