Bug 472217: Create a Bugzilla::Comment object and eliminate GetComments

Patch by Max Kanat-Alexander <mkanat@bugzilla.org> r=LpSolit, a=LpSolit

17 files changed, 326 insertions, 225 deletions

diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 14fe20c02..2a248905d 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -46,6 +46,7 @@ use Bugzilla::Product;
 use Bugzilla::Component;
 use Bugzilla::Group;
 use Bugzilla::Status;
+use Bugzilla::Comment;
 
 use List::Util qw(min);
 use Storable qw(dclone);
@@ -1835,12 +1836,12 @@ sub set_cclist_accessible { $_[0]->set('cclist_accessible', $_[1]); }
 sub set_comment_is_private {
     my ($self, $comment_id, $isprivate) = @_;
     return unless Bugzilla->user->is_insider;
-    my ($comment) = grep($comment_id eq $_->{id}, @{$self->longdescs});
+    my ($comment) = grep($comment_id == $_->id, @{ $self->comments });
     ThrowUserError('comment_invalid_isprivate', { id => $comment_id }) 
         if !$comment;
 
     $isprivate = $isprivate ? 1 : 0;
-    if ($isprivate != $comment->{isprivate}) {
+    if ($isprivate != $comment->is_private) {
         $self->{comment_isprivate} ||= {};
         $self->{comment_isprivate}->{$comment_id} = $isprivate;
     }
@@ -2665,12 +2666,38 @@ sub keyword_objects {
     return $self->{'keyword_objects'};
 }
 
-sub longdescs {
-    my ($self) = @_;
-    return $self->{'longdescs'} if exists $self->{'longdescs'};
+sub comments {
+    my ($self, $params) = @_;
     return [] if $self->{'error'};
-    $self->{'longdescs'} = GetComments($self->{bug_id});
-    return $self->{'longdescs'};
+    $params ||= {};
+
+    if (!defined $self->{'comments'}) {
+        $self->{'comments'} = Bugzilla::Comment->match({ bug_id => $self->id });
+        my $count = 0;
+        foreach my $comment (@{ $self->{'comments'} }) {
+            $comment->{count} = $count++;
+        }
+    }
+    my @comments = @{ $self->{'comments'} };
+
+    my $order = $params->{order} 
+        || Bugzilla->user->settings->{'comment_sort_order'}->{'value'};
+    if ($order ne 'oldest_to_newest') {
+        @comments = reverse @comments;
+        if ($order eq 'newest_to_oldest_desc_first') {
+            unshift(@comments, pop @comments);
+        }
+    }
+
+    if ($params->{after}) {
+        my $from = datetime_from($params->{after});
+        @comments = grep { datetime_from($_->creation_ts) > $from } @comments;
+    }
+    if ($params->{to}) {
+        my $to = datetime_from($params->{to});
+        @comments = grep { datetime_from($_->creation_ts) <= $to } @comments;
+    }
+    return \@comments;
 }
 
 sub milestoneurl {
@@ -2955,7 +2982,7 @@ sub update_comment {
       || ThrowCodeError('bad_arg', {argument => 'comment_id', function => 'update_comment'});
 
     # The comment ID must belong to this bug.
-    my @current_comment_obj = grep {$_->{'id'} == $comment_id} @{$self->longdescs};
+    my @current_comment_obj = grep {$_->id == $comment_id} @{$self->comments};
     scalar(@current_comment_obj)
       || ThrowCodeError('bad_arg', {argument => 'comment_id', function => 'update_comment'});
 
@@ -2972,7 +2999,7 @@ sub update_comment {
     $self->_sync_fulltext();
 
     # Update the comment object with this new text.
-    $current_comment_obj[0]->{'body'} = $new_comment;
+    $current_comment_obj[0]->{'thetext'} = $new_comment;
 }
 
 # Represents which fields from the bugs table are handled by process_bug.cgi.
@@ -3032,74 +3059,6 @@ sub ValidateTime {
     }
 }
 
-sub GetComments {
-    my ($id, $comment_sort_order, $start, $end, $raw) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    $comment_sort_order = $comment_sort_order ||
-        Bugzilla->user->settings->{'comment_sort_order'}->{'value'};
-
-    my $sort_order = ($comment_sort_order eq "oldest_to_newest") ? 'asc' : 'desc';
-
-    my @comments;
-    my @args = ($id);
-
-    my $query = 'SELECT longdescs.comment_id AS id, profiles.userid, ' .
-                        $dbh->sql_date_format('longdescs.bug_when', '%Y.%m.%d %H:%i:%s') .
-                      ' AS time, longdescs.thetext AS body, longdescs.work_time,
-                        isprivate, already_wrapped, type, extra_data
-                   FROM longdescs
-             INNER JOIN profiles
-                     ON profiles.userid = longdescs.who
-                  WHERE longdescs.bug_id = ?';
-
-    if ($start) {
-        $query .= ' AND longdescs.bug_when > ?';
-        push(@args, $start);
-    }
-    if ($end) {
-        $query .= ' AND longdescs.bug_when <= ?';
-        push(@args, $end);
-    }
-
-    $query .= " ORDER BY longdescs.bug_when $sort_order";
-    my $sth = $dbh->prepare($query);
-    $sth->execute(@args);
-
-    # Cache the users we look up
-    my %users;
-
-    while (my $comment_ref = $sth->fetchrow_hashref()) {
-        my %comment = %$comment_ref;
-        $users{$comment{'userid'}} ||= new Bugzilla::User($comment{'userid'});
-        $comment{'author'} = $users{$comment{'userid'}};
-
-        # If raw data is requested, do not format 'special' comments.
-        $comment{'body'} = format_comment(\%comment) unless $raw;
-
-        push (@comments, \%comment);
-    }
-   
-    if ($comment_sort_order eq "newest_to_oldest_desc_first") {
-        unshift(@comments, pop @comments);
-    }
-
-    return \@comments;
-}
-
-# Format language specific comments.
-sub format_comment {
-    my $comment = shift;
-    my $template = Bugzilla->template_inner;
-    my $vars = {comment => $comment};
-    my $body;
-
-    $template->process("bug/format_comment.txt.tmpl", $vars, \$body)
-        || ThrowTemplateError($template->error());
-    $body =~ s/^X//;
-    return $body;
-}
-
 # Get the activity of a bug, starting from $starttime (if given).
 # This routine assumes Bugzilla::Bug->check has been previously called.
 sub GetBugActivity {
@@ -3644,7 +3603,7 @@ sub _validate_attribute {
     my @valid_attributes = (
         # Miscellaneous properties and methods.
         qw(error groups product_id component_id
-           longdescs milestoneurl attachments
+           comments milestoneurl attachments
            isopened isunconfirmed
            flag_types num_attachment_flag_types
            show_attachment_flags any_flags_requesteeble),
diff --git a/Bugzilla/BugMail.pm b/Bugzilla/BugMail.pm
index d654a139b..2aff7daf4 100644
--- a/Bugzilla/BugMail.pm
+++ b/Bugzilla/BugMail.pm
@@ -119,6 +119,7 @@ sub Send {
     my $msg = "";
 
     my $dbh = Bugzilla->dbh;
+    my $bug = new Bugzilla::Bug($id);
 
     # XXX - These variables below are useless. We could use field object
     # methods directly. But we first have to implement a cache in
@@ -356,7 +357,7 @@ sub Send {
         }
     }
 
-    my $comments = get_comments_by_bug($id, $start, $end);
+    my $comments = $bug->comments({ after => $start, to => $end });
 
     ###########################################################################
     # Start of email filtering code
@@ -569,7 +570,7 @@ sub sendMail {
     }
 
     if (!$user->is_insider) {
-        @send_comments = grep { !$_->{isprivate} } @send_comments;
+        @send_comments = grep { !$_->is_private } @send_comments;
     }
 
     if ($difftext eq "" && !scalar(@send_comments) && !$isnew) {
@@ -650,38 +651,4 @@ sub sendMail {
     return 1;
 }
 
-# Get bug comments for the given period.
-sub get_comments_by_bug {
-    my ($id, $start, $end) = @_;
-    my $dbh = Bugzilla->dbh;
-
-    my $result = "";
-    my $count = 0;
-
-    # $start will be undef for new bugs, and defined for pre-existing bugs.
-    if ($start) {
-        # If $start is not NULL, obtain the count-index
-        # of this comment for the leading "Comment #xxx" line.
-        $count = $dbh->selectrow_array('SELECT COUNT(*) FROM longdescs
-                                        WHERE bug_id = ? AND bug_when <= ?',
-                                        undef, ($id, $start));
-    }
-
-    my $raw = 1; # Do not format comments which are not of type CMT_NORMAL.
-    my $comments = Bugzilla::Bug::GetComments($id, "oldest_to_newest", $start, $end, $raw);
-    my $attach_base = correct_urlbase() . 'attachment.cgi?id=';
-
-    foreach my $comment (@$comments) {
-        $comment->{count} = $count++;
-        # If an attachment was created, then add an URL. (Note: the 'g'lobal
-        # replace should work with comments with multiple attachments.)
-        if ($comment->{body} =~ /Created an attachment \(/) {
-            $comment->{body} =~ s/(Created an attachment \(id=([0-9]+)\))/$1\n --> \($attach_base$2\)/g;
-        }
-        $comment->{body} = $comment->{'already_wrapped'} ? $comment->{body} : wrap_comment($comment->{body});
-    }
-
-    return $comments;
-}
-
 1;
diff --git a/Bugzilla/Comment.pm b/Bugzilla/Comment.pm
new file mode 100644
index 000000000..7072dbbee
--- /dev/null
+++ b/Bugzilla/Comment.pm
@@ -0,0 +1,173 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# The contents of this file are subject to the Mozilla Public
+# License Version 1.1 (the "License"); you may not use this file
+# except in compliance with the License. You may obtain a copy of
+# the License at http://www.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS
+# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
+# implied. See the License for the specific language governing
+# rights and limitations under the License.
+#
+# The Original Code is the Bugzilla Bug Tracking System.
+#
+# The Initial Developer of the Original Code is James Robson.
+# Portions created by James Robson are Copyright (c) 2009 James Robson.
+# All rights reserved.
+#
+# Contributor(s): James Robson <arbingersys@gmail.com> 
+
+use strict;
+
+package Bugzilla::Comment;
+
+use base qw(Bugzilla::Object);
+
+use Bugzilla::Constants;
+use Bugzilla::Error;
+use Bugzilla::Util;
+
+###############################
+####    Initialization     ####
+###############################
+
+use constant DB_COLUMNS => qw(
+    comment_id
+    bug_id
+    who
+    bug_when
+    work_time
+    thetext
+    isprivate
+    already_wrapped
+    type
+    extra_data
+);
+
+use constant DB_TABLE => 'longdescs';
+use constant ID_FIELD => 'comment_id';
+use constant LIST_ORDER => 'bug_when';
+
+###############################
+####      Accessors      ######
+###############################
+
+sub already_wrapped { return $_[0]->{'already_wrapped'}; }
+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 work_time   { return $_[0]->{'work_time'}; }
+
+sub author { 
+    my $self = shift;
+    $self->{'author'} ||= new Bugzilla::User($self->{'who'});
+    return $self->{'author'};
+}
+
+sub body_full {
+    my ($self, $params) = @_;
+    $params ||= {};
+    my $template = Bugzilla->template_inner;
+    my $body;
+    $template->process("bug/format_comment.txt.tmpl", 
+                       { comment => $self, %$params }, \$body)
+        || ThrowTemplateError($template->error());
+    $body =~ s/^X//;
+    if ($params->{wrap} and !$self->already_wrapped) {
+        $body = wrap_comment($body);
+    }
+    return $body;
+}
+
+1;
+
+__END__
+
+=head1 NAME
+
+Bugzilla::Comment - A Comment for a given bug 
+
+=head1 SYNOPSIS
+
+ use Bugzilla::Comment;
+
+ my $comment = Bugzilla::Comment->new($comment_id);
+ my $comments = Bugzilla::Comment->new_from_list($comment_ids);
+
+=head1 DESCRIPTION
+
+Bugzilla::Comment represents a comment attached to a bug.
+
+This implements all standard C<Bugzilla::Object> methods. See 
+L<Bugzilla::Object> for more details.
+
+=head2 Accessors
+
+=over
+
+=item C<bug_id>
+
+C<int> The ID of the bug to which the comment belongs.
+
+=item C<creation_ts>
+
+C<string> The comment creation timestamp.
+
+=item C<body>
+
+C<string> The body without any special additional text.
+
+=item C<work_time>
+
+C<string> Time spent as related to this comment.
+
+=item C<is_private>
+
+C<boolean> Comment is marked as private
+
+=item C<already_wrapped>
+
+If this comment is stored in the database word-wrapped, this will be C<1>.
+C<0> otherwise.
+
+=item C<author>
+
+L<Bugzilla::User> who created the comment.
+
+=item C<body_full>
+
+=over
+
+=item B<Description>
+
+C<string> Body of the comment, including any special text (such as
+"this bug was marked as a duplicate of...").
+
+=item B<Params>
+
+=over
+
+=item C<is_bugmail>
+
+C<boolean>. C<1> if this comment should be formatted specifically for
+bugmail.
+
+=item C<wrap>
+
+C<boolean>. C<1> if the comment should be returned word-wrapped.
+
+=back
+
+=item B<Returns>
+
+A string, the full text of the comment as it would be displayed to an end-user.
+
+=back
+
+
+
+=back
+
+=cut
diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index d5e371f64..fa7247243 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -170,7 +170,7 @@ sub quoteUrls {
 
     # If the comment is already wrapped, we should ignore newlines when
     # looking for matching regexps. Else we should take them into account.
-    my $s = ($comment && $comment->{already_wrapped}) 
+    my $s = ($comment && $comment->already_wrapped) 
             ? qr/\s/ : qr/[[:blank:]]/;
 
     # However, note that adding the title (for buglinks) can affect things
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 3843062fb..0fc2db336 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -1390,7 +1390,7 @@ sub wants_bug_mail {
     my $self = shift;
     my ($bug_id, $relationship, $fieldDiffs, $comments, $dependencyText,
         $changer, $bug_is_new) = @_;
-    my $comments_concatenated = join("\n", map { $_->{body} } (@$comments));
+    my $comments_concatenated = join("\n", map { $_->body } @$comments);
 
     # Make a list of the events which have happened during this bug change,
     # from the point of view of this user.    
diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm
index 21588417c..d1a52a5fc 100644
--- a/Bugzilla/Util.pm
+++ b/Bugzilla/Util.pm
@@ -40,7 +40,7 @@ use base qw(Exporter);
                              diff_arrays
                              trim wrap_hard wrap_comment find_wrap_point
                              format_time format_time_decimal validate_date
-                             validate_time
+                             validate_time datetime_from
                              file_mod_time is_7bit_clean
                              bz_crypt generate_random_password
                              validate_email_syntax clean_text
@@ -396,7 +396,9 @@ sub format_time {
 
     # If $format is undefined, try to guess the correct date format.
     if (!defined($format)) {
-        if ($date =~ m/^(\d{4})[-\.](\d{2})[-\.](\d{2}) (\d{2}):(\d{2})(:(\d{2}))?$/) {
+        if (!ref $date
+            && $date =~ /^(\d{4})[-\.](\d{2})[-\.](\d{2}) (\d{2}):(\d{2})(:(\d{2}))?$/) 
+        {
             my $sec = $7;
             if (defined $sec) {
                 $format = "%Y-%m-%d %T %Z";
@@ -409,44 +411,49 @@ sub format_time {
         }
     }
 
-    # strptime($date) returns an empty array if $date has an invalid date format.
+    my $dt = ref $date ? $date : datetime_from($date, $timezone);
+    $date = defined $dt ? $dt->strftime($format) : '';
+    return trim($date);
+}
+
+sub datetime_from {
+    my ($date, $timezone) = @_;
+
+    # strptime($date) returns an empty array if $date has an invalid
+    # date format.
     my @time = strptime($date);
 
     unless (scalar @time) {
-        # If an unknown timezone is passed (such as MSK, for Moskow), strptime() is
-        # unable to parse the date. We try again, but we first remove the timezone.
+        # If an unknown timezone is passed (such as MSK, for Moskow),
+        # strptime() is unable to parse the date. We try again, but we first
+        # remove the timezone.
         $date =~ s/\s+\S+$//;
         @time = strptime($date);
     }
 
-    if (scalar @time) {
-        # Fix a bug in strptime() where seconds can be undefined in some cases.
-        $time[0] ||= 0;
-
-        # strptime() counts years from 1900, and months from 0 (January).
-        # We have to fix both values.
-        my $dt = DateTime->new({year   => 1900 + $time[5],
-                                month  => ++$time[4],
-                                day    => $time[3],
-                                hour   => $time[2],
-                                minute => $time[1],
-                                # DateTime doesn't like fractional seconds.
-                                second => int($time[0]),
-                                # If importing, use the specified timezone, otherwise 
-                                # use the timezone specified by the server.
-                                time_zone => Bugzilla->local_timezone->offset_as_string($time[6]) 
-                                          || Bugzilla->local_timezone});
-
-        # Now display the date using the given timezone,
-        # or the user's timezone if none is given.
-        $dt->set_time_zone($timezone || Bugzilla->user->timezone);
-        $date = $dt->strftime($format);
-    }
-    else {
-        # Don't let invalid (time) strings to be passed to templates!
-        $date = '';
-    }
-    return trim($date);
+    return undef if !@time;
+
+    # strptime() counts years from 1900, and months from 0 (January).
+    # We have to fix both values.
+    my $dt = DateTime->new({
+        year   => $time[5] + 1900,
+        month  => $time[4] + 1,
+        day    => $time[3],
+        hour   => $time[2],
+        minute => $time[1],
+        # DateTime doesn't like fractional seconds.
+        # Also, sometimes seconds are undef.
+        second => int($time[0] || 0),
+        # If a timezone was specified, use it. Otherwise, use the
+        # local timezone.
+        time_zone => Bugzilla->local_timezone->offset_as_string($time[6]) 
+                     || Bugzilla->local_timezone,
+    });
+
+    # Now display the date using the given timezone,
+    # or the user's timezone if none is given.
+    $dt->set_time_zone($timezone || Bugzilla->user->timezone);
+    return $dt;
 }
 
 sub format_time_decimal {
@@ -641,6 +648,7 @@ Bugzilla::Util - Generic utility functions for bugzilla
 
   # Functions for formatting time
   format_time($time);
+  datetime_from($time, $timezone);
 
   # Functions for dealing with files
   $time = file_mod_time($filename);
@@ -894,6 +902,15 @@ This routine is mainly called from templates to filter dates, see
 Returns a number with 2 digit precision, unless the last digit is a 0. Then it 
 returns only 1 digit precision.
 
+=item C<datetime_from($time, $timezone)>
+
+Returns a DateTime object given a date string. If the string is not in some
+valid date format that C<strptime> understands, we return C<undef>.
+
+You can optionally specify a timezone for the returned date. If not
+specified, defaults to the currently-logged-in user's timezone, or
+the Bugzilla server's local timezone if there isn't a logged-in user.
+
 =back
 
 
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 6a3e93519..aea30cedb 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -23,6 +23,7 @@ package Bugzilla::WebService::Bug;
 use strict;
 use base qw(Bugzilla::WebService);
 
+use Bugzilla::Comment;
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Field;
@@ -95,12 +96,12 @@ sub comments {
     foreach my $bug_id (@$bug_ids) {
         my $bug = Bugzilla::Bug->check($bug_id);
         # We want the API to always return comments in the same order.
-        my $comments = Bugzilla::Bug::GetComments(
-            $bug->id, 'oldest_to_newest', $params->{new_since});
+   
+        my $comments = $bug->comments({ order => 'oldest_to_newest',
+                                        after => $params->{new_since} });
         my @result;
         foreach my $comment (@$comments) {
-            next if $comment->{isprivate} && !$user->is_insider;
-            $comment->{bug_id} = $bug->id;
+            next if $comment->is_private && !$user->is_insider;
             push(@result, $self->_translate_comment($comment, $params));
         }
         $bugs{$bug->id}{'comments'} = \@result;
@@ -109,15 +110,10 @@ sub comments {
     my %comments;
     if (scalar @$comment_ids) {
         my @ids = map { trim($_) } @$comment_ids;
-        my @sql_ids = map { $dbh->quote($_) } @ids;
-        my $comment_data = $dbh->selectall_arrayref(
-            'SELECT comment_id AS id, bug_id, who, bug_when AS time,
-                    isprivate, thetext AS body, type, extra_data
-               FROM longdescs WHERE ' . $dbh->sql_in('comment_id', \@sql_ids),
-            {Slice=>{}});
+        my $comment_data = Bugzilla::Comment->new_from_list(\@ids);
 
         # See if we were passed any invalid comment ids.
-        my %got_ids = map { $_->{id} => 1 } @$comment_data;
+        my %got_ids = map { $_->id => 1 } @$comment_data;
         foreach my $comment_id (@ids) {
             if (!$got_ids{$comment_id}) {
                 ThrowUserError('comment_id_invalid', { id => $comment_id });
@@ -125,16 +121,14 @@ sub comments {
         }
  
         # Now make sure that we can see all the associated bugs.
-        my %got_bug_ids = map { $_->{bug_id} => 1 } @$comment_data;
+        my %got_bug_ids = map { $_->bug_id => 1 } @$comment_data;
         Bugzilla::Bug->check($_) foreach (keys %got_bug_ids);
 
         foreach my $comment (@$comment_data) {
-            if ($comment->{isprivate} && !$user->is_insider) {
-                ThrowUserError('comment_is_private', { id => $comment->{id} });
+            if ($comment->is_private && !$user->is_insider) {
+                ThrowUserError('comment_is_private', { id => $comment->id });
             }
-            $comment->{author} = new Bugzilla::User($comment->{who});
-            $comment->{body} = Bugzilla::Bug::format_comment($comment);
-            $comments{$comment->{id}} =
+            $comments{$comment->id} =
                 $self->_translate_comment($comment, $params);
         }
     }
@@ -146,12 +140,12 @@ sub comments {
 sub _translate_comment {
     my ($self, $comment, $filters) = @_;
     return filter $filters, {
-        id         => $self->type('int', $comment->{id}),
-        bug_id     => $self->type('int', $comment->{bug_id}),
-        author     => $self->type('string', $comment->{author}->login),
-        time       => $self->type('dateTime', $comment->{'time'}),
-        is_private => $self->type('boolean', $comment->{isprivate}),
-        text       => $self->type('string', $comment->{body}),
+        id         => $self->type('int', $comment->id),
+        bug_id     => $self->type('int', $comment->bug_id),
+        author     => $self->type('string', $comment->author->login),
+        time       => $self->type('dateTime', $comment->creation_ts),
+        is_private => $self->type('boolean', $comment->is_private),
+        text       => $self->type('string', $comment->body_full),
     };
 }
 
diff --git a/email_in.pl b/email_in.pl
index 1ec2a19df..2d0ebe571 100755
--- a/email_in.pl
+++ b/email_in.pl
@@ -204,7 +204,7 @@ sub process_bug {
     foreach my $field (keys %fields) {
         $cgi->param(-name => $field, -value => $fields{$field});
     }
-    $cgi->param('longdesclength', scalar $bug->longdescs);
+    $cgi->param('longdesclength', scalar @{ $bug->comments });
     $cgi->param('token', issue_hash_token([$bug->id, $bug->delta_ts]));
 
     require 'process_bug.cgi';
diff --git a/enter_bug.cgi b/enter_bug.cgi
index 071276f1d..2ae05b4d6 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -434,17 +434,16 @@ if ($cloned_bug_id) {
     # We need to ensure that we respect the 'insider' status of
     # the first comment, if it has one. Either way, make a note
     # that this bug was cloned from another bug.
-    # We cannot use $cloned_bug->longdescs because this method
-    # depends on the "comment_sort_order" user pref, and we
-    # really want the first comment of the bug.
-    my $bug_desc = Bugzilla::Bug::GetComments($cloned_bug_id, 'oldest_to_newest');
-    my $isprivate = $bug_desc->[0]->{'isprivate'};
+    my $bug_desc = $cloned_bug->comments({ order => 'oldest_to_newest' })->[0];
+    my $isprivate = $bug_desc->is_private;
 
     $vars->{'comment'}        = "";
     $vars->{'commentprivacy'} = 0;
 
     if (!$isprivate || Bugzilla->user->is_insider) {
-        $vars->{'comment'}        = $bug_desc->[0]->{'body'};
+        # We use "body" to avoid any format_comment text, which would be
+        # pointless to clone.
+        $vars->{'comment'}        = $bug_desc->body;
         $vars->{'commentprivacy'} = $isprivate;
     }
 
diff --git a/post_bug.cgi b/post_bug.cgi
index 323e005f4..a482913cd 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -229,13 +229,13 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
         # expects to find this exact string.
         my $new_comment = "Created an attachment (id=" . $attachment->id . ")\n" .
                           $attachment->description . "\n";
-        # We can use $bug->longdescs here because we are sure that the bug
+        # We can use $bug->comments here because we are sure that the bug
         # description is of type CMT_NORMAL. No need to include it if it's
         # empty, though.
-        if ($bug->longdescs->[0]->{'body'} !~ /^\s+$/) {
-            $new_comment .= "\n" . $bug->longdescs->[0]->{'body'};
+        if ($bug->comments->[0]->body !~ /^\s+$/) {
+            $new_comment .= "\n" . $bug->comments->[0]->body;
         }
-        $bug->update_comment($bug->longdescs->[0]->{'id'}, $new_comment);
+        $bug->update_comment($bug->comments->[0]->id, $new_comment);
     }
     else {
         $vars->{'message'} = 'attachment_creation_failed';
diff --git a/process_bug.cgi b/process_bug.cgi
index 36091b892..504fcacc3 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -168,8 +168,7 @@ if (defined $cgi->param('delta_ts')
     $vars->{'start_at'} = $cgi->param('longdesclength');
     # Always sort midair collision comments oldest to newest,
     # regardless of the user's personal preference.
-    $vars->{'comments'} = Bugzilla::Bug::GetComments($first_bug->id,
-                                                     "oldest_to_newest");
+    $vars->{'comments'} = $first_bug->comments({ order => "oldest_to_newest" });
     $vars->{'bug'} = $first_bug;
     # The token contains the old delta_ts. We need a new one.
     $cgi->param('token', issue_hash_token([$first_bug->id, $first_bug->delta_ts]));
diff --git a/template/en/default/bug/comments.html.tmpl b/template/en/default/bug/comments.html.tmpl
index 2f9eeebf6..41436f9b2 100644
--- a/template/en/default/bug/comments.html.tmpl
+++ b/template/en/default/bug/comments.html.tmpl
@@ -94,7 +94,6 @@
 
 
 [% DEFAULT start_at = 0 mode = "show" %]
-[% isinsider = Param("insidergroup") && user.in_group(Param("insidergroup")) %]
 [% sort_order = user.settings.comment_sort_order.value %]
 
 [%# NOTE: (start_at > 0) means we came here from a midair collision,
@@ -145,8 +144,9 @@
 [%############################################################################%]
 
 [% BLOCK a_comment %]
-  [% IF NOT comment.isprivate || isinsider %]
-    <div class="bz_comment[% " bz_private" IF comment.isprivate %]
+  [% RETURN IF comment.is_private AND ! user.is_insider %]
+
+    <div class="bz_comment[% " bz_private" IF comment.is_private %]
                 [% " bz_comment_hilite" IF marks.$count %]
                 [% " bz_first_comment" IF count == description %]">
       [% IF count == description %]
@@ -168,7 +168,7 @@
           </span>
         [% END %]
 
-        [% IF mode == "edit" && isinsider %]
+        [% IF mode == "edit" && user.is_insider %]
           <div class="bz_private_checkbox">
             <input type="hidden" value="1"
                    name="defined_isprivate_[% comment.id %]">
@@ -176,7 +176,7 @@
                    name="isprivate_[% comment.id %]" value="1"
                    id="isprivate_[% comment.id %]"
                    onClick="updateCommentPrivacy(this, [% count %])"
-                   [% " checked=\"checked\"" IF comment.isprivate %]>
+                   [% " checked=\"checked\"" IF comment.is_private %]>
             <label for="isprivate_[% comment.id %]">Private</label>
           </div>
         [% END %]
@@ -201,7 +201,7 @@
         </span>
 
         <span class="bz_comment_time">
-          [%+ comment.time FILTER time %]
+          [%+ comment.creation_ts FILTER time %]
         </span>
       </div>
 
@@ -215,15 +215,9 @@
 [%# Don't indent the <pre> block, since then the spaces are displayed in the
   # generated HTML
   #%]
-[% IF comment.already_wrapped %]
-    [% wrapped_comment = comment.body %]
-[% ELSE %]
-    [% wrapped_comment = comment.body FILTER wrap_comment %]
-[% END %]
 <pre class="bz_comment_text" 
      [% ' id="comment_text_' _ count _ '"' IF mode == "edit" %]>
-  [%- wrapped_comment FILTER quoteUrls(bug, comment) -%]
+  [%- comment.body_full({ wrap => 1 }) FILTER quoteUrls(bug, comment) -%]
 </pre>
     </div>
-  [% END %]
 [% END %]
diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl
index d5a345182..006603f59 100644
--- a/template/en/default/bug/edit.html.tmpl
+++ b/template/en/default/bug/edit.html.tmpl
@@ -142,7 +142,7 @@
 <form name="changeform" method="post" action="process_bug.cgi">
 
   <input type="hidden" name="delta_ts" value="[% bug.delta_ts %]">
-  <input type="hidden" name="longdesclength" value="[% bug.longdescs.size %]">
+  <input type="hidden" name="longdesclength" value="[% bug.comments.size %]">
   <input type="hidden" name="id" value="[% bug.bug_id %]">
   <input type="hidden" name="token" value="[% issue_hash_token([bug.id, bug.delta_ts]) FILTER html %]">
 
@@ -287,7 +287,7 @@
         <hr>
         <div id="comments">
         [% PROCESS bug/comments.html.tmpl
-           comments = bug.longdescs
+           comments = bug.comments
            mode = user.id ? "edit" : "show"
          %]
         </div>
diff --git a/template/en/default/bug/format_comment.txt.tmpl b/template/en/default/bug/format_comment.txt.tmpl
index e0881e4e7..8e97d4d08 100644
--- a/template/en/default/bug/format_comment.txt.tmpl
+++ b/template/en/default/bug/format_comment.txt.tmpl
@@ -23,23 +23,23 @@
   #%]
 
 [%# INTERFACE:
-  #   comment: A hash containing comment information.
-  #              count:           The comment number (on the bug it belongs to)
-  #              author:          The Bugzilla::User object of the comment's
-  #                               author
-  #              time:            The time at which the comment has been
-  #                               committed
-  #              body:            The comment text
-  #              type:            One of the CMT_* constants (not given if none
-  #                               applies)
-  #              extra_data:      Extra data (type specific)
-  #              already_wrapped: Determines whether the comment is pre-wrapped
+  #   comment: A Bugzilla::Comment object.
+  #   is_bugmail: boolean; True if this comment is going into a plain-text
+  #               bugmail.
   #%]
 
-[% PROCESS 'global/field-descs.none.tmpl' %]
+[%# Please don't use field-descs here. It can slow down Bugzilla. %]
+[% PROCESS 'global/variables.none.tmpl' %]
+
+[% SET comment_body = comment.body %]
+[% IF is_bugmail %]
+  [% comment_body = comment_body.replace( '(Created an attachment \(id=([0-9]+)\))',
+                                          '$1' _ "\n" _ ' --> (' _ urlbase 
+                                          _ 'attachment.cgi?id=$2)' ) %]
+[% END %]
 
 [% IF comment.type == constants.CMT_DUPE_OF %]
-X[% comment.body %]
+X[% comment_body %]
 
 *** This [% terms.bug %] has been marked as a duplicate of [% terms.bug %] [%+ comment.extra_data %] ***
 [% ELSIF comment.type == constants.CMT_HAS_DUPE %]
@@ -47,14 +47,13 @@ X[% comment.body %]
 [% ELSIF comment.type == constants.CMT_POPULAR_VOTES %]
 *** This [% terms.bug %] has been confirmed by popular vote. ***
 [% ELSIF comment.type == constants.CMT_MOVED_TO %]
-X[% comment.body %]
+X[% comment_body %]
 
 [%+ terms.Bug %] moved to [% Param("move-to-url") %].
 If the move succeeded, [% comment.extra_data %] will receive a mail containing
 the number of the new [% terms.bug %] in the other database.
-If all went well, please mark this [% terms.bug %]
-[%+ display_value("bug_status", 'VERIFIED') %], and paste in a link to the new [% terms.bug %].
+If all went well, please paste in a link to the new [% terms.bug %].
 Otherwise, reopen this [% terms.bug %].
 [% ELSE %]
-X[% comment.body %]
+X[% comment_body %]
 [% END %]
diff --git a/template/en/default/bug/show-multiple.html.tmpl b/template/en/default/bug/show-multiple.html.tmpl
index 473453eb5..903d9d622 100644
--- a/template/en/default/bug/show-multiple.html.tmpl
+++ b/template/en/default/bug/show-multiple.html.tmpl
@@ -289,7 +289,7 @@
   <br>
 
   [% PROCESS bug/comments.html.tmpl
-     comments = bug.longdescs %]
+     comments = bug.comments %]
 
 [% END %]
 
diff --git a/template/en/default/bug/show.xml.tmpl b/template/en/default/bug/show.xml.tmpl
index 1db320c4f..42464aa14 100644
--- a/template/en/default/bug/show.xml.tmpl
+++ b/template/en/default/bug/show.xml.tmpl
@@ -65,16 +65,16 @@
       [% PROCESS section_flags obj => bug %]
 
       [% IF displayfields.long_desc %]
-        [% FOREACH c = bug.longdescs %]
-          [% NEXT IF c.isprivate && !user.in_group(Param("insidergroup")) %]
-          <long_desc isprivate="[% c.isprivate FILTER xml %]">
+        [% FOREACH c = bug.comments %]
+          [% NEXT IF c.is_private && !user.in_group(Param("insidergroup")) %]
+          <long_desc isprivate="[% c.is_private FILTER xml %]">
             <commentid>[% c.id FILTER xml %]</commentid>
             <who name="[% c.author.name FILTER xml %]">[% c.author.email FILTER email FILTER xml %]</who>
-            <bug_when>[% c.time FILTER time("%Y-%m-%d %T %z") FILTER xml %]</bug_when>
+            <bug_when>[% c.creation_ts FILTER time("%Y-%m-%d %T %z") FILTER xml %]</bug_when>
             [% IF user.in_group(Param('timetrackinggroup')) && (c.work_time - 0 != 0) %]
               <work_time>[% PROCESS formattimeunit time_unit = c.work_time FILTER xml %]</work_time>
             [% END %]
-            <thetext>[% c.body FILTER xml %]</thetext>
+            <thetext>[% c.body_full FILTER xml %]</thetext>
           </long_desc>
         [% END %]
       [% END %]
@@ -151,4 +151,4 @@
       [% END %]
     />
   [% END %]
-[% END %]
\ No newline at end of file
+[% END %]
diff --git a/template/en/default/email/newchangedmail.txt.tmpl b/template/en/default/email/newchangedmail.txt.tmpl
index 9b1443bc2..368c23713 100644
--- a/template/en/default/email/newchangedmail.txt.tmpl
+++ b/template/en/default/email/newchangedmail.txt.tmpl
@@ -50,7 +50,7 @@ X-Bugzilla-Changed-Fields: [% changedfields %]
 [%- IF comment.count %]
 --- Comment #[% comment.count %] from [% comment.author.identity %] [%+ comment.time FILTER time %] ---
 [% END %]
-[%+ FILTER remove('^X') %][% PROCESS bug/format_comment.txt.tmpl %][% END %]
+[%+ comment.body_full({ is_bugmail => 1, wrap => 1 }) %]
 [% END %]
 
 -- [%# Protect the trailing space of the signature marker %]