Don't use <pre> for markdown comments in email

RESOLVED FIXED in Bugzilla 6.0

Status

()

Bugzilla
Email Notifications
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Albert Ting, Assigned: Albert Ting)

Tracking

Bugzilla 6.0
Bug Flags:
approval +

Details

Attachments

(1 attachment, 4 obsolete attachments)

(Assignee)

Description

3 years ago
bugmail.html.tmpl wraps markdown comments inside a <pre>.  This doesn't work because it then prints out all blank lines.  Instead, it should <div>.
(Assignee)

Comment 1

3 years ago
Created attachment 8576372 [details] [diff] [review]
v1

Suggested patch
Attachment #8576372 - Flags: review?(dkl)

Comment 2

3 years ago
Comment on attachment 8576372 [details] [diff] [review]
v1

>+          <[% comment_div %] class="bz_comment_text">

comment_div is unfiltered, so your patch won't pass tests.
(Assignee)

Comment 3

3 years ago
Created attachment 8576743 [details] [diff] [review]
v2

Updated version to including filtering
Attachment #8576372 - Attachment is obsolete: true
Attachment #8576372 - Flags: review?(dkl)
Attachment #8576743 - Flags: review?(dkl)
bug 1105568 adds another email template that will need to be updated here; sorry albert.
Assignee: email-notifications → altlist
Comment on attachment 8576743 [details] [diff] [review]
v2

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

As glob said, we need to also patch template/en/default/email/flagmail.html.tmpl as well. Will look at this on the next revision.
Attachment #8576743 - Flags: review?(dkl) → review-
(Assignee)

Comment 6

3 years ago
Created attachment 8580286 [details] [diff] [review]
v3

Updated version to support flagmail (and set the class)
Attachment #8576743 - Attachment is obsolete: true
Attachment #8580286 - Flags: review?(glob)
Comment on attachment 8580286 [details] [diff] [review]
v3

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

this patch doesn't apply due to your source having a bz_comment_text class on the <pre> tag.
this class isn't defined in our bugmail, so it isn't required.

::: template/en/default/email/bugmail.html.tmpl~
@@ +27,5 @@
>                from [% INCLUDE global/user.html.tmpl user = to_user, who = comment.author %]
>              </div>
>            [% END %]
> +          [% comment_div = comment.is_markdown ? "div" : "pre" %]
> +          <[% comment_div FILTER html %] class="bz_comment_text">[% comment.body_full({ wrap => 1 }) FILTER markdown(bug, comment, to_user) %]</[% comment_div FILTER html %]>

because comment_div is a constant, it's safe (and quicker) to use the "none" filter.
Attachment #8580286 - Flags: review?(glob) → review-
(Assignee)

Comment 8

3 years ago
Created attachment 8580807 [details] [diff] [review]
v4

Updated version.

I removed the bz_comment_text class.  This comes from bug 1119336, which I'll update after this bug gets approved.
Attachment #8580286 - Attachment is obsolete: true
Attachment #8580807 - Flags: review?(glob)
(Assignee)

Comment 9

3 years ago
Created attachment 8580833 [details] [diff] [review]
v5

Correction, had to remove other modifications from bug 1119336
Attachment #8580807 - Attachment is obsolete: true
Attachment #8580807 - Flags: review?(glob)
Attachment #8580833 - Flags: review?(glob)
Comment on attachment 8580833 [details] [diff] [review]
v5

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

r=glob
Attachment #8580833 - Flags: review?(glob) → review+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   7559020..2c82105  master -> master
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Flags: approval+
Resolution: --- → FIXED
Target Milestone: --- → Bugzilla 6.0
You need to log in before you can comment on or make changes to this bug.