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

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, defect)

Description

[Albert Ting]

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>.

Comment 1

[Albert Ting]

Attachment: v1

Suggested patch

Comment 2

[Frédéric Buclin]

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.

Comment 3

[Albert Ting]

Attachment: v2

Updated version to including filtering

Comment 4

[:glob ✱]

bug 1105568 adds another email template that will need to be updated here; sorry albert.

Comment 5

[David Lawrence [:dkl]]

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.

Comment 6

[Albert Ting]

Attachment: v3

Updated version to support flagmail (and set the class)

Comment 7

[: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.

Comment 8

[Albert Ting]

Attachment: v4

Updated version.

I removed the bz_comment_text class.  This comes from bug 1119336, which I'll update after this bug gets approved.

Comment 9

[Albert Ting]

Attachment: v5

Correction, had to remove other modifications from bug 1119336

Comment 10

[:glob ✱]

Comment on attachment 8580833 [details] [diff] [review]
v5

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

r=glob

Comment 11

[:glob ✱]

To ssh://[e-mail]/bugzilla/bugzilla.git
   7559020..2c82105  master -> master