Closed Bug 1139872 Opened 5 years ago Closed 5 years ago

The URL in bugmail microdata has the '#' URI encoded, causing the links to break

Categories

(Bugzilla :: Email Notifications, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Bugzilla 6.0

People

(Reporter: emorley, Assigned: emorley)

References

Details

Attachments

(1 file)

Broken out from bug 1102364:

(In reply to Ed Morley [:edmorley] from comment #28)
> (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?
(In reply to Ed Morley [:edmorley] from comment #0)
> > 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?

it's safe to use |FILTER none| here.

adding a template comment indicating that filtering isn't required and is in fact detrimental would help future developers touching this file.
Target Milestone: --- → Bugzilla 6.0
Version: unspecified → 5.1
Assignee: email-notifications → emorley
Status: NEW → ASSIGNED
Attachment #8574388 - Flags: review?(glob)
Comment on attachment 8574388 [details] [diff] [review]
Don't filter the bugmail microdata URL params

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

r=glob
Attachment #8574388 - Flags: review?(glob) → review+
Flags: approval?
Comment on attachment 8574388 [details] [diff] [review]
Don't filter the bugmail microdata URL params

>+          [%# Filtering of the URL param is not required & would break the URL when the comment anchor is set %]
>+          <link itemprop="url" href="[% urlbase FILTER html %]show_bug.cgi?id=[% "$bug.id$comment_anchor" FILTER none %]"/>
>           <meta itemprop="name" content="View [% terms.bug %]"/>
>         </div>
>         <meta itemprop="description" content="[% terms.Bugzilla %] [%+ terms.bug %] update notification"/>

Could you please remove all these useless /> on checkin? HTML5 is not XHTML. This is the single HTML template in the whole codebase where this is in use.
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   e3fc6a0..592e6fd  master -> master
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: approval? → approval+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.