Bug 1456878 - Support markdown comments

5 files changed, 69 insertions, 39 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index ee48ed7a2..9c820eedc 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -732,7 +732,7 @@ sub _preload_referenced_bugs {
         }
         else {
             # bugs referenced in comments
-            Bugzilla::Template::quoteUrls($comment->body, undef, undef, undef,
+            Bugzilla::Template::renderComment($comment->body, undef, undef, 1,
                 sub {
                     my $bug_id = $_[0];
                     push @referenced_bug_ids, $bug_id
@@ -999,6 +999,7 @@ sub create {
 
     # We now have a bug id so we can fill this out
     $creation_comment->{'bug_id'} = $bug->id;
+    $creation_comment->{'is_markdown'} = 1;
 
     # Insert the comment. We always insert a comment on bug creation,
     # but sometimes it's blank.
@@ -2662,7 +2663,8 @@ sub set_all {
         # there are lots of things that want to check if we added a comment.
         $self->add_comment($params->{'comment'}->{'body'},
             { isprivate => $params->{'comment'}->{'is_private'},
-              work_time => $params->{'work_time'} });
+              work_time => $params->{'work_time'},
+              is_markdown => 1 });
     }
 
     if (defined $params->{comment_tags} && Bugzilla->user->can_tag_comments()) {
@@ -3143,7 +3145,7 @@ sub remove_cc {
     @$cc_users = grep { $_->id != $user->id } @$cc_users;
 }
 
-# $bug->add_comment("comment", {isprivate => 1, work_time => 10.5,
+# $bug->add_comment("comment", {isprivate => 1, work_time => 10.5, is_markdown => 1,
 #                               type => CMT_NORMAL, extra_data => $data});
 sub add_comment {
     my ($self, $comment, $params) = @_;
diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm
index f9a6f7d3a..937cd1203 100644
--- a/Bugzilla/Comment.pm
+++ b/Bugzilla/Comment.pm
@@ -45,6 +45,7 @@ use constant DB_COLUMNS => qw(
     already_wrapped
     type
     extra_data
+    is_markdown
 );
 
 use constant UPDATE_COLUMNS => qw(
@@ -67,6 +68,7 @@ use constant VALIDATORS => {
     work_time   => \&_check_work_time,
     thetext     => \&_check_thetext,
     isprivate   => \&_check_isprivate,
+    is_markdown => \&Bugzilla::Object::check_boolean,
     extra_data  => \&_check_extra_data,
     type        => \&_check_type,
 };
@@ -233,6 +235,7 @@ sub body        { return $_[0]->{'thetext'};   }
 sub bug_id      { return $_[0]->{'bug_id'};    }
 sub creation_ts { return $_[0]->{'bug_when'};  }
 sub is_private  { return $_[0]->{'isprivate'}; }
+sub is_markdown { return $_[0]->{'is_markdown'}; }
 sub work_time   {
     # Work time is returned as a string (see bug 607909)
     return 0 if $_[0]->{'work_time'} + 0 == 0;
@@ -576,6 +579,10 @@ C<string> Time spent as related to this comment.
 
 C<boolean> Comment is marked as private.
 
+=item C<is_markdown>
+
+C<boolean> Whether this comment needs Markdown rendering to be applied.
+
 =item C<already_wrapped>
 
 If this comment is stored in the database word-wrapped, this will be C<1>.
diff --git a/Bugzilla/Hook.pm b/Bugzilla/Hook.pm
index bed6a53b0..d27468f55 100644
--- a/Bugzilla/Hook.pm
+++ b/Bugzilla/Hook.pm
@@ -438,12 +438,6 @@ Sometimes this is C<undef>, meaning that we are parsing text that is
 not a bug comment (but could still be some other part of a bug, like
 the summary line).
 
-=item C<user>
-
-The L<Bugzilla::User> object representing the user who will see the text.
-This is useful to determine how much confidential information can be displayed
-to the user.
-
 =back
 
 =head2 bug_start_of_update
diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index 299734d64..f74565302 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -130,17 +130,20 @@ sub get_format {
     };
 }
 
-# This routine quoteUrls contains inspirations from the HTML::FromText CPAN
+# This routine renderComment contains inspirations from the HTML::FromText CPAN
 # module by Gareth Rees <garethr@cre.canon.co.uk>.  It has been heavily hacked,
 # all that is really recognizable from the original is bits of the regular
 # expressions.
 # This has been rewritten to be faster, mainly by substituting 'as we go'.
 # If you want to modify this routine, read the comments carefully
+# Renamed from 'quoteUrls' to 'renderComment' after markdown support was added.
 
-sub quoteUrls {
-    my ($text, $bug, $comment, $user, $bug_link_func) = @_;
+sub renderComment {
+    my ($text, $bug, $comment, $skip_markdown, $bug_link_func) = @_;
     return $text unless $text;
-    $user ||= Bugzilla->user;
+    my $anon_user = Bugzilla::User->new;
+    # We choose to render markdown by default, unless the comment explicitly isn't.
+    $skip_markdown ||= $comment && !$comment->is_markdown;
     $bug_link_func ||= \&get_bug_link;
 
     # We use /g for speed, but uris can have other things inside them
@@ -173,7 +176,7 @@ sub quoteUrls {
     my @hook_regexes;
     Bugzilla::Hook::process('bug_format_comment',
         { text => \$text, bug => $bug, regexes => \@hook_regexes,
-          comment => $comment, user => $user });
+          comment => $comment, user => undef });
 
     foreach my $re (@hook_regexes) {
         my ($match, $replace) = @$re{qw(match replace)};
@@ -193,37 +196,47 @@ sub quoteUrls {
     # Provide tooltips for full bug links (Bug 74355)
     my $urlbase_re = '(' . quotemeta(Bugzilla->localconfig->{urlbase}) . ')';
     $text =~ s~\b(${urlbase_re}\Qshow_bug.cgi?id=\E([0-9]+)(\#c([0-9]+))?)\b
-              ~($things[$count++] = $bug_link_func->($3, $1, { comment_num => $5, user => $user })) &&
+              ~($things[$count++] = $bug_link_func->($3, $1, { comment_num => $5, user => $anon_user })) &&
                ("\x{FDD2}" . ($count-1) . "\x{FDD3}")
               ~egox;
 
-    # non-mailto protocols
-    my $safe_protocols = SAFE_URL_REGEXP();
-    $text =~ s~\b($safe_protocols)
+
+    if ($skip_markdown) {
+        # non-mailto protocols
+        my $safe_protocols = SAFE_URL_REGEXP();
+        $text =~ s~\b($safe_protocols)
               ~($tmp = html_quote($1)) &&
                ($things[$count++] = "<a rel=\"nofollow\" href=\"$tmp\">$tmp</a>") &&
                ("\x{FDD2}" . ($count-1) . "\x{FDD3}")
               ~egox;
 
-    # We have to quote now, otherwise the html itself is escaped
-    # THIS MEANS THAT A LITERAL ", <, >, ' MUST BE ESCAPED FOR A MATCH
+        # We have to quote now, otherwise the html itself is escaped
+        # THIS MEANS THAT A LITERAL ", <, >, ' MUST BE ESCAPED FOR A MATCH
+        $text = html_quote($text);
 
-    $text = html_quote($text);
+        # Color quoted text
+        $text =~ s~^(&gt;.+)$~<span class="quote">$1</span >~mg;
+        $text =~ s~</span >\n<span class="quote">~\n~g;
 
-    # Color quoted text
-    $text =~ s~^(&gt;.+)$~<span class="quote">$1</span >~mg;
-    $text =~ s~</span >\n<span class="quote">~\n~g;
+        # mailto:
+        # Use |<nothing> so that $1 is defined regardless
+        # &#64; is the encoded '@' character.
+        $text =~ s~\b(mailto:|)?([\w\.\-\+\=]+&\#64;[\w\-]+(?:\.[\w\-]+)+)\b
+                 ~<a href=\"mailto:$2\">$1$2</a>~igx;
+    }
+    else {
+        # We intentionally disable all html tags. Users should use markdown syntax.
+        # This prevents things like inline styles on anchor tags, which otherwise would be valid.
+        $text =~ s/([<])/&lt;/g;
 
-    # mailto:
-    # Use |<nothing> so that $1 is defined regardless
-    # &#64; is the encoded '@' character.
-    $text =~ s~\b(mailto:|)?([\w\.\-\+\=]+&\#64;[\w\-]+(?:\.[\w\-]+)+)\b
-              ~<a href=\"mailto:$2\">$1$2</a>~igx;
+        # As a preference, we opt into all new line breaks being rendered as a new line.
+        $text =~ s/(\r?\n)/  $1/g;
+    }
 
     # attachment links
     # BMO: don't make diff view the default for patches (Bug 652332)
     $text =~ s~\b(attachment$s*\#?$s*(\d+)(?:$s+\[diff\])?(?:\s+\[details\])?)
-              ~($things[$count++] = get_attachment_link($2, $1, $user)) &&
+              ~($things[$count++] = get_attachment_link($2, $1, $anon_user)) &&
                ("\x{FDD2}" . ($count-1) . "\x{FDD3}")
               ~egmxi;
 
@@ -240,7 +253,7 @@ sub quoteUrls {
     $text =~ s~\b($bug_re(?:$s*,?$s*$comment_re)?|$comment_re)
               ~ # We have several choices. $1 here is the link, and $2-4 are set
                 # depending on which part matched
-               (defined($2) ? $bug_link_func->($2, $1, { comment_num => $3, user => $user }) :
+               (defined($2) ? $bug_link_func->($2, $1, { comment_num => $3, user => $anon_user }) :
                               "<a href=\"$current_bugurl#c$4\">$1</a>")
               ~egx;
 
@@ -249,7 +262,7 @@ sub quoteUrls {
     $text =~ s~(?<=^\*\*\*\ This\ bug\ has\ been\ marked\ as\ a\ duplicate\ of\ )
                (\d+)
                (?=\ \*\*\*\Z)
-              ~$bug_link_func->($1, $1, { user => $user })
+              ~$bug_link_func->($1, $1, { user => $anon_user })
               ~egmx;
 
     # Now remove the encoding hacks in reverse order
@@ -257,7 +270,12 @@ sub quoteUrls {
         $text =~ s/\x{FDD2}($i)\x{FDD3}/$things[$i]/eg;
     }
 
-    return $text;
+    if ($skip_markdown) {
+        return $text;
+    }
+    else {
+        return Bugzilla->markdown_parser->render_html($text);
+    }
 }
 
 # Creates a link to an attachment, including its title.
@@ -271,11 +289,17 @@ sub get_attachment_link {
     if ($attachment) {
         my $title = "";
         my $className = "";
+        my $linkClass = "";
+
         if ($user->can_see_bug($attachment->bug_id)
             && (!$attachment->isprivate || $user->is_insider))
         {
             $title = $attachment->description;
         }
+        else{
+            $linkClass = "bz_private_link";
+        }
+
         if ($attachment->isobsolete) {
             $className = "bz_obsolete";
         }
@@ -296,7 +320,7 @@ sub get_attachment_link {
 
         # Whitespace matters here because these links are in <pre> tags.
         return qq|<span class="$className">|
-               . qq|<a href="${linkval}" name="attach_${attachid}" title="$title">$link_text</a>|
+               . qq|<a href="${linkval}" class="$linkClass" name="attach_${attachid}" title="$title">$link_text</a>|
                . qq| <a href="${linkval}&amp;action=edit" title="$title">[details]</a>|
                . qq|${patchlink}|
                . qq|</span>|;
@@ -706,11 +730,11 @@ sub create {
             # Removes control characters and trims extra whitespace.
             clean_text => \&Bugzilla::Util::clean_text ,
 
-            quoteUrls => [ sub {
-                               my ($context, $bug, $comment, $user) = @_;
+            renderComment => [ sub {
+                               my ($context, $bug, $comment, $skip_markdown) = @_;
                                return sub {
                                    my $text = shift;
-                                   return quoteUrls($text, $bug, $comment, $user);
+                                   return renderComment($text, $bug, $comment, $skip_markdown);
                                };
                            },
                            1
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index feb541c2e..d14300f6f 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -362,7 +362,7 @@ sub render_comment {
     Bugzilla->switch_to_shadow_db();
     my $bug = $params->{id} ? Bugzilla::Bug->check($params->{id}) : undef;
 
-    my $html = Bugzilla::Template::quoteUrls($params->{text}, $bug);
+    my $html = Bugzilla::Template::renderComment($params->{text}, $bug);
 
     return { html => $html };
 }
@@ -381,6 +381,7 @@ sub _translate_comment {
         time       => $self->type('dateTime', $comment->creation_ts),
         creation_time => $self->type('dateTime', $comment->creation_ts),
         is_private => $self->type('boolean', $comment->is_private),
+        is_markdown => $self->type('boolean', $comment->is_markdown),
         text       => $self->type('string', $comment->body_full),
         attachment_id => $self->type('int', $attach_id),
         count      => $self->type('int', $comment->count),
@@ -1112,9 +1113,11 @@ sub add_comment {
     if (defined $params->{private}) {
         $params->{is_private} = delete $params->{private};
     }
+
     # Append comment
     $bug->add_comment($comment, { isprivate => $params->{is_private},
-                                  work_time => $params->{work_time} });
+                                  work_time => $params->{work_time},
+                                  is_markdown => 1 });
 
     # Add comment tags
     $bug->set_all({ comment_tags => $params->{comment_tags} })