Patch Viewer, a pretty way of viewing and manipulating patches (bug 174942). …

attachment.cgi

@@ -80,6 +80,21 @@ if ($action eq "view")
   validateID();
   view(); 
 }
+elsif ($action eq "interdiff")
+{
+  validateID('oldid');
+  validateID('newid');
+  validateFormat("html", "raw");
+  validateContext();
+  interdiff();
+}
+elsif ($action eq "diff")
+{
+  validateID();
+  validateFormat("html", "raw");
+  validateContext();
+  diff();
+}
 elsif ($action eq "viewall") 
 { 
   ValidateBugID($::FORM{'bugid'});
@@ -149,16 +164,18 @@ exit;
 
 sub validateID
 {
+    my $param = @_ ? $_[0] : 'id';
+
     # Validate the value of the "id" form field, which must contain an
     # integer that is the ID of an existing attachment.
 
-    $vars->{'attach_id'} = $::FORM{'id'};
+    $vars->{'attach_id'} = $::FORM{$param};
     
-    detaint_natural($::FORM{'id'}) 
+    detaint_natural($::FORM{$param}) 
      || ThrowUserError("invalid_attach_id");
   
     # Make sure the attachment exists in the database.
-    SendSQL("SELECT bug_id, isprivate FROM attachments WHERE attach_id = $::FORM{'id'}");
+    SendSQL("SELECT bug_id, isprivate FROM attachments WHERE attach_id = $::FORM{$param}");
     MoreSQLData()
       || ThrowUserError("invalid_attach_id");
 
@@ -170,6 +187,28 @@ sub validateID
     }
 }
 
+sub validateFormat
+{
+  $::FORM{'format'} ||= $_[0];
+  if (! grep { $_ eq $::FORM{'format'} } @_)
+  {
+     $vars->{'format'} = $::FORM{'format'};
+     $vars->{'formats'} = \@_;
+     ThrowUserError("invalid_format");
+  }
+}
+
+sub validateContext
+{
+  $::FORM{'context'} ||= "patch";
+  if ($::FORM{'context'} ne "file" && $::FORM{'context'} ne "patch") {
+    $vars->{'context'} = $::FORM{'context'};
+    detaint_natural($::FORM{'context'})
+      || ThrowUserError("invalid_context");
+    delete $vars->{'context'};
+  }
+}
+
 sub validateCanEdit
 {
     my ($attach_id) = (@_);
@@ -408,6 +447,238 @@ sub view
     print $thedata;
 }
 
+sub interdiff
+{
+  # Get old patch data
+  my ($old_bugid, $old_description, $old_filename, $old_file_list) =
+      get_unified_diff($::FORM{'oldid'});
+
+  # Get new patch data
+  my ($new_bugid, $new_description, $new_filename, $new_file_list) =
+      get_unified_diff($::FORM{'newid'});
+
+  my $warning = warn_if_interdiff_might_fail($old_file_list, $new_file_list);
+
+  #
+  # send through interdiff, send output directly to template
+  #
+  # Must hack path so that interdiff will work.
+  #
+  $ENV{'PATH'} = $::diffpath;
+  open my $interdiff_fh, "$::interdiffbin $old_filename $new_filename|";
+  binmode $interdiff_fh;
+  my ($iter, $last_iter) = setup_iterators("");
+  if ($::FORM{'format'} eq "raw")
+  {
+    require PatchIterator::DiffPrinter::raw;
+    $last_iter->sends_data_to(new PatchIterator::DiffPrinter::raw());
+    # Actually print out the patch
+    print $cgi->header(-type => 'text/plain',
+                       -expires => '+3M');
+  }
+  else
+  {
+    $vars->{warning} = $warning if $warning;
+    $vars->{bugid} = $new_bugid;
+    $vars->{oldid} = $::FORM{'oldid'};
+    $vars->{old_desc} = $old_description;
+    $vars->{newid} = $::FORM{'newid'};
+    $vars->{new_desc} = $new_description;
+    delete $vars->{attachid};
+    delete $vars->{do_context};
+    delete $vars->{context};
+    setup_template_iterator($iter, $last_iter);
+  }
+  $iter->iterate_fh($interdiff_fh, "interdiff #$::FORM{'oldid'} #$::FORM{'newid'}");
+  close $interdiff_fh;
+  $ENV{'PATH'} = '';
+
+  #
+  # Delete temporary files
+  #
+  unlink($old_filename) or warn "Could not unlink $old_filename: $!";
+  unlink($new_filename) or warn "Could not unlink $new_filename: $!";
+}
+
+sub get_unified_diff
+{
+  my ($id) = @_;
+
+  # Bring in the modules we need
+  require PatchIterator::Raw;
+  require PatchIterator::FixPatchRoot;
+  require PatchIterator::DiffPrinter::raw;
+  require PatchIterator::PatchInfoGrabber;
+  require File::Temp;
+
+  # Get the patch
+  SendSQL("SELECT bug_id, description, ispatch, thedata FROM attachments WHERE attach_id = $id");
+  my ($bugid, $description, $ispatch, $thedata) = FetchSQLData();
+  if (!$ispatch) {
+    $vars->{'attach_id'} = $id;
+    ThrowCodeError("must_be_patch");
+  }
+
+  # Reads in the patch, converting to unified diff in a temp file
+  my $iter = new PatchIterator::Raw;
+  # fixes patch root (makes canonical if possible)
+  my $fix_patch_root = new PatchIterator::FixPatchRoot(Param('cvsroot'));
+  $iter->sends_data_to($fix_patch_root);
+  # Grabs the patch file info
+  my $patch_info_grabber = new PatchIterator::PatchInfoGrabber();
+  $fix_patch_root->sends_data_to($patch_info_grabber);
+  # Prints out to temporary file
+  my ($fh, $filename) = File::Temp::tempfile();
+  $patch_info_grabber->sends_data_to(new PatchIterator::DiffPrinter::raw($fh));
+  # Iterate!
+  $iter->iterate_string($id, $thedata);
+
+  return ($bugid, $description, $filename, $patch_info_grabber->patch_info()->{files});
+}
+
+sub warn_if_interdiff_might_fail {
+  my ($old_file_list, $new_file_list) = @_;
+  # Verify that the list of files diffed is the same
+  my @old_files = sort keys %{$old_file_list};
+  my @new_files = sort keys %{$new_file_list};
+  if (@old_files != @new_files ||
+      join(' ', @old_files) ne join(' ', @new_files)) {
+    return "interdiff1";
+  }
+
+  # Verify that the revisions in the files are the same
+  foreach my $file (keys %{$old_file_list}) {
+    if ($old_file_list->{$file}{old_revision} ne
+        $new_file_list->{$file}{old_revision}) {
+      return "interdiff2";
+    }
+  }
+
+  return undef;
+}
+
+sub setup_iterators {
+  my ($diff_root) = @_;
+
+  #
+  # Parameters:
+  # format=raw|html
+  # context=patch|file|0-n
+  # collapsed=0|1
+  # headers=0|1
+  #
+
+  # Define the iterators
+  # The iterator that reads the patch in (whatever its format)
+  require PatchIterator::Raw;
+  my $iter = new PatchIterator::Raw;
+  my $last_iter = $iter;
+  # Fix the patch root if we have a cvs root
+  if (Param('cvsroot'))
+  {
+    require PatchIterator::FixPatchRoot;
+    $last_iter->sends_data_to(new PatchIterator::FixPatchRoot(Param('cvsroot')));
+    $last_iter->sends_data_to->diff_root($diff_root) if defined($diff_root);
+    $last_iter = $last_iter->sends_data_to;
+  }
+  # Add in cvs context if we have the necessary info to do it
+  if ($::FORM{'context'} ne "patch" && $::cvsbin && Param('cvsroot_get'))
+  {
+    require PatchIterator::AddCVSContext;
+    $last_iter->sends_data_to(
+        new PatchIterator::AddCVSContext($::FORM{'context'},
+                                         Param('cvsroot_get')));
+    $last_iter = $last_iter->sends_data_to;
+  }
+  return ($iter, $last_iter);
+}
+
+sub setup_template_iterator
+{
+  my ($iter, $last_iter) = @_;
+
+  require PatchIterator::DiffPrinter::template;
+
+  my $format = $::FORM{'format'};
+
+  # Define the vars for templates
+  if (defined($::FORM{'headers'})) {
+    $vars->{headers} = $::FORM{'headers'};
+  } else {
+    $vars->{headers} = 1 if !defined($::FORM{'headers'});
+  }
+  $vars->{collapsed} = $::FORM{'collapsed'};
+  $vars->{context} = $::FORM{'context'};
+  $vars->{do_context} = $::cvsbin && Param('cvsroot_get') && !$vars->{'newid'};
+
+  # Print everything out
+  print $cgi->header(-type => 'text/html',
+                     -expires => '+3M');
+  $last_iter->sends_data_to(new PatchIterator::DiffPrinter::template($template,
+                             "attachment/diff-header.$format.tmpl",
+                             "attachment/diff-file.$format.tmpl",
+                             "attachment/diff-footer.$format.tmpl",
+                             { %{$vars},
+                               bonsai_url => Param('bonsai_url'),
+                               lxr_url => Param('lxr_url'),
+                               lxr_root => Param('lxr_root'),
+                             }));
+}
+
+sub diff
+{
+  # Get patch data
+  SendSQL("SELECT bug_id, description, ispatch, thedata FROM attachments WHERE attach_id = $::FORM{'id'}");
+  my ($bugid, $description, $ispatch, $thedata) = FetchSQLData();
+
+  # If it is not a patch, view normally
+  if (!$ispatch)
+  {
+    view();
+    return;
+  }
+
+  my ($iter, $last_iter) = setup_iterators();
+
+  if ($::FORM{'format'} eq "raw")
+  {
+    require PatchIterator::DiffPrinter::raw;
+    $last_iter->sends_data_to(new PatchIterator::DiffPrinter::raw());
+    # Actually print out the patch
+    use vars qw($cgi);
+    print $cgi->header(-type => 'text/plain',
+                       -expires => '+3M');
+    $iter->iterate_string("Attachment " . $::FORM{'id'}, $thedata);
+  }
+  else
+  {
+    $vars->{other_patches} = [];
+    if ($::interdiffbin && $::diffpath) {
+      # Get list of attachments on this bug.
+      # Ignore the current patch, but select the one right before it
+      # chronologically.
+      SendSQL("SELECT attach_id, description FROM attachments WHERE bug_id = $bugid AND ispatch = 1 ORDER BY creation_ts DESC");
+      my $select_next_patch = 0;
+      while (my ($other_id, $other_desc) = FetchSQLData()) {
+        if ($other_id eq $::FORM{'id'}) {
+          $select_next_patch = 1;
+        } else {
+          push @{$vars->{other_patches}}, { id => $other_id, desc => $other_desc, selected => $select_next_patch };
+          if ($select_next_patch) {
+            $select_next_patch = 0;
+          }
+        }
+      }
+    }
+
+    $vars->{bugid} = $bugid;
+    $vars->{attachid} = $::FORM{'id'};
+    $vars->{description} = $description;
+    setup_template_iterator($iter, $last_iter);
+    # Actually print out the patch
+    $iter->iterate_string("Attachment " . $::FORM{'id'}, $thedata);
+  }
+}
 
 sub viewall
 {

checksetup.pl

@@ -430,6 +430,60 @@ LocalVar('mysqlpath', <<"END");
 END
 
 
+my $cvs_executable = `which cvs`;
+if ($cvs_executable =~ /no cvs/) {
+    # If which didn't find it, just set to blank
+    $cvs_executable = "";
+} else {
+    chomp $cvs_executable;
+}
+
+LocalVar('cvsbin', <<"END");
+#
+# For some optional functions of Bugzilla (such as the pretty-print patch
+# viewer), we need the cvs binary to access files and revisions.
+# Because it's possible that this program is not in your path, you can specify
+# its location here.  Please specify the full path to the executable.
+\$cvsbin = "$cvs_executable";
+END
+
+
+my $interdiff_executable = `which interdiff`;
+if ($interdiff_executable =~ /no interdiff/) {
+    # If which didn't find it, set to blank
+    $interdiff_executable = "";
+} else {
+    chomp $interdiff_executable;
+}
+
+LocalVar('interdiffbin', <<"END");
+
+#
+# For some optional functions of Bugzilla (such as the pretty-print patch
+# viewer), we need the interdiff binary to make diffs between two patches.
+# Because it's possible that this program is not in your path, you can specify
+# its location here.  Please specify the full path to the executable.
+\$interdiffbin = "$interdiff_executable";
+END
+
+
+my $diff_binaries = `which diff`;
+if ($diff_binaries =~ /no diff/) {
+    # If which didn't find it, set to blank
+    $diff_binaries = "";
+} else {
+    $diff_binaries =~ s:/diff\n$::;
+}
+
+LocalVar('diffpath', <<"END");
+
+#
+# The interdiff feature needs diff, so we have to have that path.
+# Please specify only the directory name, with no trailing slash.
+\$diffpath = "$diff_binaries";
+END
+
+
 LocalVar('create_htaccess', <<'END');
 #
 # If you are using Apache for your web server, Bugzilla can create .htaccess

defparams.pl

@@ -1057,6 +1057,73 @@ Reason: %reason%
    default => 1,
   },
 
+# Added for Patch Viewer stuff (attachment.cgi?action=diff)
+  {
+   name    => 'cvsroot',
+   desc    => 'The <a href="http://www.cvshome.org">CVS</a> root that most ' .
+              'users of your system will be using for "cvs diff".  Used in ' .
+              'Patch Viewer ("Diff" option on patches) to figure out where ' .
+              'patches are rooted even if users did the "cvs diff" from ' .
+              'different places in the directory structure.  (NOTE: if your ' .
+              'CVS repository is remote and requires a password, you must ' .
+              'either ensure the Bugzilla user has done a "cvs login" or ' .
+              'specify the password ' .
+              '<a href="http://www.cvshome.org/docs/manual/cvs_2.html#SEC26">as ' .
+              'part of the CVS root.</a>)  Leave this blank if you have no ' .
+              'CVS repository.',
+   type    => 't',
+   default => '',
+  },
+
+  {
+   name    => 'cvsroot_get',
+   desc    => 'The CVS root Bugzilla will be using to get patches from.  ' .
+              'Some installations may want to mirror their CVS repository on ' .
+              'the Bugzilla server or even have it on that same server, and ' .
+              'thus the repository can be the local file system (and much ' .
+              'faster).  Make this the same as cvsroot if you don\'t ' .
+              'understand what this is (if cvsroot is blank, make this blank ' .
+              'too).',
+   type    => 't',
+   default => '',
+  },
+
+  {
+   name    => 'bonsai_url',
+   desc    => 'The URL to a ' .
+              '<a href="http://www.mozilla.org/bonsai.html">Bonsai</a> ' .
+              'server containing information about your CVS repository.  ' .
+              'Patch Viewer will use this information to create links to ' .
+              'bonsai\'s blame for each section of a patch (it will append ' .
+              '"/cvsblame.cgi?..." to this url).  Leave this blank if you ' .
+              'don\'t understand what this is.',
+   type    => 't',
+   default => ''
+  },
+
+  {
+   name    => 'lxr_url',
+   desc    => 'The URL to an ' .
+              '<a href="http://sourceforge.net/projects/lxr">LXR</a> server ' .
+              'that indexes your CVS repository.  Patch Viewer will use this ' .
+              'information to create links to LXR for each file in a patch.  ' .
+              'Leave this blank if you don\'t understand what this is.',
+   type    => 't',
+   default => ''
+  },
+
+  {
+   name    => 'lxr_root',
+   desc    => 'Some LXR installations do not index the CVS repository from ' .
+              'the root--' .
+              '<a href="http://lxr.mozilla.org/mozilla">Mozilla\'s</a>, for ' .
+              'example, starts indexing under <code>mozilla/</code>.  This ' .
+              'means URLs are relative to that extra path under the root.  ' .
+              'Enter this if you have a similar situation.  Leave it blank ' .
+              'if you don\'t know what this is.',
+   type    => 't',
+   default => '',
+  },
 );
 
 1;

globals.pl

@@ -75,7 +75,7 @@ use DBI;
 
 use Date::Format;               # For time2str().
 use Date::Parse;               # For str2time().
-#use Carp;                       # for confess
+use Carp;                       # for confess
 use RelationSet;
 
 # Use standard Perl libraries for cross-platform file/directory manipulation.
@@ -98,12 +98,12 @@ $::SIG{PIPE} = 'IGNORE';
 $::defaultqueryname = "(Default query)"; # This string not exposed in UI
 $::unconfirmedstate = "UNCONFIRMED";
 
-#sub die_with_dignity {
-#    my ($err_msg) = @_;
-#    print $err_msg;
-#    confess($err_msg);
-#}
-#$::SIG{__DIE__} = \&die_with_dignity;
+sub die_with_dignity {
+    my ($err_msg) = @_;
+    print $err_msg;
+    confess($err_msg);
+}
+$::SIG{__DIE__} = \&die_with_dignity;
 
 @::default_column_list = ("bug_severity", "priority", "rep_platform", 
                           "assigned_to", "bug_status", "resolution",

t/008filter.t

@@ -101,60 +101,13 @@ foreach my $path (@Support::Templates::include_paths) {
             my @lineno = ($` =~ m/\n/gs);
             my $lineno = scalar(@lineno) + 1;
 
-            # Comments
-            next if $directive =~ /^[+-]?#/;        
+            if (!directive_ok($file, $directive)) {
 
-            # Remove any leading/trailing + or - and whitespace.
-            $directive =~ s/^[+-]?\s*//;
-            $directive =~ s/\s*[+-]?$//;
-
-            # Directives
-            next if $directive =~ /^(IF|END|UNLESS|FOREACH|PROCESS|INCLUDE|
-                                     BLOCK|USE|ELSE|NEXT|LAST|DEFAULT|FLUSH|
-                                     ELSIF|SET|SWITCH|CASE)/x;
-
-            # Simple assignments
-            next if $directive =~ /^[\w\.\$]+\s+=\s+/;
-
-            # Conditional literals with either sort of quotes 
-            # There must be no $ in the string for it to be a literal
-            next if $directive =~ /^(["'])[^\$]*[^\\]\1/;
-
-            # Special values always used for numbers
-            next if $directive =~ /^[ijkn]$/;
-            next if $directive =~ /^count$/;
-            
-            # Params
-            next if $directive =~ /^Param\(/;
-
-            # Other functions guaranteed to return OK output
-            next if $directive =~ /^(time2str|GetBugLink)\(/;
-
-            # Safe Template Toolkit virtual methods
-            next if $directive =~ /\.(size)$/;
-
-            # Special Template Toolkit loop variable
-            next if $directive =~ /^loop\.(index|count)$/;
-            
-            # Branding terms
-            next if $directive =~ /^terms\./;
-            
-            # Things which are already filtered
-            # Note: If a single directive prints two things, and only one is 
-            # filtered, we may not catch that case.
-            next if $directive =~ /FILTER\ (html|csv|js|url_quote|quoteUrls|
-                                            time|uri|xml)/x;
-
-            # Exclude those on the nofilter list
-            if (defined($safe{$file}{$directive})) {
-                $safe{$file}{$directive}++;
-                next;
-            };
-
-            # This intentionally makes no effort to eliminate duplicates; to do
-            # so would merely make it more likely that the user would not 
-            # escape all instances when attempting to correct an error.
-            push(@unfiltered, "$lineno:$directive");
+              # This intentionally makes no effort to eliminate duplicates; to do
+              # so would merely make it more likely that the user would not 
+              # escape all instances when attempting to correct an error.
+              push(@unfiltered, "$lineno:$directive");
+            }
         }  
 
         my $fullpath = File::Spec->catfile($path, $file);
@@ -183,6 +136,74 @@ foreach my $path (@Support::Templates::include_paths) {
     }
 }
 
+sub directive_ok {
+    my ($file, $directive) = @_;
+
+    # Comments
+    return 1 if $directive =~ /^[+-]?#/;        
+
+    # Remove any leading/trailing + or - and whitespace.
+    $directive =~ s/^[+-]?\s*//;
+    $directive =~ s/\s*[+-]?$//;
+
+    # Exclude those on the nofilter list
+    if (defined($safe{$file}{$directive})) {
+        $safe{$file}{$directive}++;
+        return 1;
+    };
+
+    # Directives
+    return 1 if $directive =~ /^(IF|END|UNLESS|FOREACH|PROCESS|INCLUDE|
+                                 BLOCK|USE|ELSE|NEXT|LAST|DEFAULT|FLUSH|
+                                 ELSIF|SET|SWITCH|CASE|WHILE)/x;
+
+    # ? :
+    if ($directive =~ /.+\?(.+):(.+)/) {
+        return 1 if directive_ok($file, $1) && directive_ok($file, $2);
+    }
+
+    # + - * /
+    return 1 if $directive =~ /[+\-*\/]/;
+
+    # Numbers
+    return 1 if $directive =~ /^[0-9]+$/;
+
+    # Simple assignments
+    return 1 if $directive =~ /^[\w\.\$]+\s+=\s+/;
+
+    # Conditional literals with either sort of quotes 
+    # There must be no $ in the string for it to be a literal
+    return 1 if $directive =~ /^(["'])[^\$]*[^\\]\1/;
+    return 1 if $directive =~ /^(["'])\1/;
+
+    # Special values always used for numbers
+    return 1 if $directive =~ /^[ijkn]$/;
+    return 1 if $directive =~ /^count$/;
+    
+    # Params
+    return 1 if $directive =~ /^Param\(/;
+
+    # Other functions guaranteed to return OK output
+    return 1 if $directive =~ /^(time2str|GetBugLink|url)\(/;
+
+    # Safe Template Toolkit virtual methods
+    return 1 if $directive =~ /\.(size)$/;
+
+    # Special Template Toolkit loop variable
+    return 1 if $directive =~ /^loop\.(index|count)$/;
+    
+    # Branding terms
+    return 1 if $directive =~ /^terms\./;
+            
+    # Things which are already filtered
+    # Note: If a single directive prints two things, and only one is 
+    # filtered, we may not catch that case.
+    return 1 if $directive =~ /FILTER\ (html|csv|js|url_quote|quoteUrls|
+                                        time|uri|xml|lower)/x;
+
+    return 0;
+}
+
 $/ = $oldrecsep;
 
 exit 0;

template/en/default/attachment/diff-file.html.tmpl

+<!-- 1.0@bugzilla.org -->
+[%# 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 Netscape Communications
+  # Corporation. Portions created by Netscape are
+  # Copyright (C) 1998 Netscape Communications Corporation. All
+  # Rights Reserved.
+  #
+  # Contributor(s): John Keiser <jkeiser@netscape.com>
+  #%]
+
+[%# This line is really long for a reason: to get rid of any possible textnodes
+  # between the elements.  This is necessary because DOM parent-child-sibling
+  # relations can change and screw up the javascript for restoring, collapsing
+  # and expanding.  Do not change without testing all three of those.
+  #%]
+<table class="file_table"><thead><tr><td class="file_head" colspan="2"><a href="#" onclick="return twisty_click(this)">[% collapsed ? '(+)' : '(-)' %]</a><input type="checkbox" name="[% file.filename FILTER html %]"[% collapsed ? '' : ' checked' %] style="display: none"> 
+  [% IF lxr_prefix && !file.is_add %]
+    <a href="[% lxr_prefix %]">[% file.filename FILTER html %]</a>
+  [% ELSE %]
+    [% file.filename FILTER html %]
+  [% END %]
+  [% IF file.plus_lines %]
+    [% IF file.minus_lines %]
+      (-[% file.minus_lines %]&nbsp;/&nbsp;+[% file.plus_lines %]&nbsp;lines)
+    [% ELSE %]
+      (+[% file.plus_lines %]&nbsp;lines)
+    [% END %]
+  [% ELSE %]
+    [% IF file.minus_lines %]
+      (-[% file.minus_lines %]&nbsp;lines)
+    [% END %]
+  [% END %]
+</td></tr></thead><tbody class="[% collapsed ? 'file_collapse' : 'file' %]">
+<script type="application/x-javascript" language="JavaScript">
+incremental_restore()
+</script>
+
+[% section_num = 0 %]
+[% FOREACH section = sections %]
+  [% section_num = section_num + 1 %]
+  <tr><th class="section_head" colspan="2">
+  [% IF file.is_add %]
+    Added
+  [% ELSIF file.is_remove %]
+    [% IF bonsai_prefix %]
+      <a href="[% bonsai_prefix %]">Removed</a>
+    [% ELSE %]
+      Removed
+    [% END %]
+  [% ELSE %]
+    [% IF bonsai_prefix %]
+      <a href="[% bonsai_prefix %]#[% section.old_start %]">
+    [% END %]
+    [% IF section.old_lines > 1 %]
+      Lines [% section.old_start %]-[% section.old_start + section.old_lines - 1 %]
+    [% ELSE %]
+      Line [% section.old_start %]
+    [% END %]
+    [% IF bonsai_prefix %]
+      </a>
+    [% END %]
+  [% END %] 
+  (<a name="[% file.filename FILTER html %]_sec[% section_num %]"><a href="#[% file.filename FILTER html %]_sec[% section_num %]">Link Here</a></a>)
+  </th></tr>
+  [% FOREACH group = section.groups %]
+    [% IF group.context %]
+      [% FOREACH line = group.context %]
+        <tr><td><pre>[% line FILTER html %]</pre></td><td><pre>[% line FILTER html %]</pre></td></tr>
+      [% END %]
+    [% END %]
+    [% IF group.plus.size %]
+      [% IF group.minus.size %]
+        [% i = 0 %]
+        [% WHILE (i < group.plus.size || i < group.minus.size) %]
+          [% currentloop = 0 %]
+          [% WHILE currentloop < 500 && (i < group.plus.size || i < group.minus.size) %]
+            <tr class="changed">
+              <td><pre>[% group.minus.$i FILTER html %]</pre></td>
+              <td><pre>[% group.plus.$i FILTER html %]</pre></td>
+            </tr>
+            [% currentloop = currentloop + 1 %]
+            [% i = i + 1 %]
+          [% END %]
+        [% END %]
+      [% ELSE %]
+        [% FOREACH line = group.plus %]
+          [% IF file.is_add %]
+            <tr>
+              <td class="added" colspan="2"><pre>[% line FILTER html %]</pre></td>
+            </tr>
+          [% ELSE %]
+            <tr>
+              <td></td>
+              <td class="added"><pre>[% line FILTER html %]</pre></td>
+            </tr>
+          [% END %]
+        [% END %]
+      [% END %]
+    [% ELSE %]
+      [% IF group.minus.size %]
+        [% FOREACH line = group.minus %]
+          [% IF file.is_remove %]
+            <tr>
+              <td class="removed" colspan="2"><pre>[% line FILTER html %]</pre></td>
+            </tr>
+          [% ELSE %]
+            <tr>
+              <td class="removed"><pre>[% line FILTER html %]</pre></td>
+              <td></td>
+            </tr>
+          [% END %]
+        [% END %]
+      [% END %]
+    [% END %]
+  [% END %]
+[% END %]
+
+</table>

template/en/default/attachment/diff-footer.html.tmpl

+<!-- 1.0@bugzilla.org -->
+[%# 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 Netscape Communications
+  # Corporation. Portions created by Netscape are
+  # Copyright (C) 1998 Netscape Communications Corporation. All
+  # Rights Reserved.
+  #
+  # Contributor(s): John Keiser <jkeiser@netscape.com>
+  #%]
+
+</form>
+
+[% IF headers %]
+
+  <br>
+
+  [% PROCESS global/footer.html.tmpl %]
+ 
+[% ELSE %]
+</body>
+</html>
+[% END %]

template/en/default/attachment/diff-header.html.tmpl

+<!-- 1.0@bugzilla.org -->
+[%# 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 Netscape Communications
+  # Corporation. Portions created by Netscape are
+  # Copyright (C) 1998 Netscape Communications Corporation. All
+  # Rights Reserved.
+  #
+  # Contributor(s): John Keiser <jkeiser@netscape.com>
+  #%]
+
+[%# Define strings that will serve as the title and header of this page %]
+
+[% title = BLOCK %]Attachment #[% attachid %] for Bug #[% bugid %][% END %]
+
+[% style = BLOCK %]
+.file_head {
+  font-size: x-large;
+  font-weight: bold;
+  background-color: #d3d3d3;
+  border: 1px solid black;
+  width: 100%;
+}
+.file_collapse {
+  display: none;
+}
+.section_head {
+  width: 100%;
+  font-weight: bold;
+  background-color: #d3d3d3;
+  border: 1px solid black;
+  text-align: left;
+}
+table.file_table {
+  table-layout: fixed;
+  width: 100%;
+  empty-cells: show;
+  border-spacing: 0px;
+  border-collapse: collapse;
+}
+tbody.file td {
+  border-left: 1px dashed black;
+  border-right: 1px dashed black;
+  width: 50%;
+}
+tbody.file pre {
+  display: inline;
+  white-space: -moz-pre-wrap;
+  font-size: 0.9em;
+}
+tbody.file pre:empty {
+  display: block;
+  height: 1em;
+}
+.changed {
+  background-color: lightblue;
+}
+.added {
+  background-color: lightgreen;
+}
+.removed {
+  background-color: #FFCC99;
+}
+.warning {
+  color: red
+}
+[% END %]
+
+[%# SCRIPT FUNCTIONS %]
+[% javascript = BLOCK %]
+  function collapse_all() {
+    var elem = document.checkboxform.firstChild;
+    while (elem != null) {
+      if (elem.firstChild != null) {
+        var tbody = elem.firstChild.nextSibling;
+        if (tbody.className == 'file') {
+          tbody.className = 'file_collapse';
+          twisty = get_twisty_from_tbody(tbody);
+          twisty.firstChild.nodeValue = '(+)';
+          twisty.nextSibling.checked = false;
+        }
+      }
+      elem = elem.nextSibling;
+    }
+    return false;
+  }
+
+  function expand_all() {
+    var elem = document.checkboxform.firstChild;
+    while (elem != null) {
+      if (elem.firstChild != null) {
+        var tbody = elem.firstChild.nextSibling;
+        if (tbody.className == 'file_collapse') {
+          tbody.className = 'file';
+          twisty = get_twisty_from_tbody(tbody);
+          twisty.firstChild.nodeValue = '(-)';
+          twisty.nextSibling.checked = true;
+        }
+      }
+      elem = elem.nextSibling;
+    }
+    return false;
+  }
+
+  var current_restore_elem;
+
+  function restore_all() {
+    current_restore_elem = null;
+    incremental_restore();
+  }
+
+  function incremental_restore() {
+    if (!document.checkboxform.restore_indicator.checked) {
+      return;
+    }
+    var next_restore_elem;
+    if (current_restore_elem) {
+      next_restore_elem = current_restore_elem.nextSibling;
+    } else {
+      next_restore_elem = document.checkboxform.firstChild;
+    }
+    while (next_restore_elem != null) {
+      current_restore_elem = next_restore_elem;
+      if (current_restore_elem.firstChild != null) {
+        restore_elem(current_restore_elem.firstChild.nextSibling);
+      }
+      next_restore_elem = current_restore_elem.nextSibling;
+    }
+  }
+
+  function restore_elem(elem, alertme) {
+    if (elem.className == 'file_collapse') {
+      twisty = get_twisty_from_tbody(elem);
+      if (twisty.nextSibling.checked) {
+        elem.className = 'file';
+        twisty.firstChild.nodeValue = '(-)';
+      }
+    } else if (elem.className == 'file') {
+      twisty = get_twisty_from_tbody(elem);
+      if (!twisty.nextSibling.checked) {
+        elem.className = 'file_collapse';
+        twisty.firstChild.nodeValue = '(+)';
+      }
+    }
+  }
+
+  function twisty_click(twisty) {
+    tbody = get_tbody_from_twisty(twisty);
+    if (tbody.className == 'file') {
+      tbody.className = 'file_collapse';
+      twisty.firstChild.nodeValue = '(+)';
+      twisty.nextSibling.checked = false;
+    } else {
+      tbody.className = 'file';
+      twisty.firstChild.nodeValue = '(-)';
+      twisty.nextSibling.checked = true;
+    }
+    return false;
+  }
+
+  function get_tbody_from_twisty(twisty) {
+    return twisty.parentNode.parentNode.parentNode.nextSibling;
+  }
+  function get_twisty_from_tbody(tbody) {
+    return tbody.previousSibling.firstChild.firstChild.firstChild;
+  }
+[% END %]
+
+[% onload = 'restore_all(); document.checkboxform.restore_indicator.checked = true' %]
+
+[% IF headers %]
+  [% h1 = BLOCK %]
+    [% IF attachid %]
+      [% description FILTER html %] (#[% attachid %])
+    [% ELSE %]
+      [% old_url = url('attachment.cgi', action = 'diff', id = oldid) %]
+      [% new_url = url('attachment.cgi', action = 'diff', id = newid) %]
+      Diff Between 
+      <a href="[% old_url %]">[% old_desc FILTER html %]</a>
+      (<a href="[% old_url %]">#[% oldid %]</a>)
+      and 
+      <a href="[% new_url %]">[% new_desc FILTER html %]</a>
+      (<a href="[% new_url %]">#[% newid %]</a>)
+    [% END %]
+    for <a href="show_bug.cgi?id=[% bugid %]">Bug #[% bugid %]</a>
+  [% END %]
+  [% h2 = BLOCK %]
+    [% bugsummary FILTER html %]
+  [% END %]
+  [% PROCESS global/header.html.tmpl %]
+[% ELSE %]
+  <html>
+  <head>
+  <style type="text/css">
+  [% style %]
+  </style>
+  <script type="text/javascript" language="JavaScript">
+  <!--
+  [% javascript %]
+  -->
+  </script>
+  </head>
+  <body onload="[% onload FILTER html %]">
+[% END %]
+  
+[%# If we have attachid, we are in diff, otherwise we're in interdiff %]
+[% IF attachid %]
+  [%# HEADER %]
+  [% IF headers %]
+    [% USE url('attachment.cgi', id = attachid) %]
+    <a href="[% url() %]">View</a>
+    | <a href="[% url(action = 'edit') %]">Edit</a>
+    [% USE url('attachment.cgi', id = attachid, context = context,
+                                 collapsed = collapsed, headers = headers,
+                                 action = 'diff') %]
+    | <a href="[% url(format = 'raw') %]">Raw Unified</a>
+  [% END %]
+  [% IF other_patches %]
+    [% IF headers %] |[%END%]
+    Differences between
+    <form style="display: inline">
+      <select name="oldid">
+      [% FOREACH patch = other_patches %]
+        <option value="[% patch.id %]"
+        [% IF patch.selected %] selected[% END %]
+        >[% patch.desc FILTER html %]</option>
+      [% END %]
+      </select>
+      and this patch
+      <input type="submit" value="Diff">
+      <input type="hidden" name="action" value="interdiff">
+      <input type="hidden" name="newid" value="[% attachid %]">
+      <input type="hidden" name="headers" value="[% headers FILTER html %]">
+    </form>
+  [% END %]
+  <br>
+[% ELSE %]
+  [% IF headers %]
+    [% USE url('attachment.cgi', newid = newid, oldid = oldid, action = 'interdiff') %]
+    <a href="[% url(format = 'raw') %]">Raw Unified</a>
+    [% IF attachid %]
+    <br>
+    [% ELSE %]
+    |
+    [% END %]
+  [% END %]
+[% END %]
+  
+[%# Collapse / Expand %]
+<a href="#"
+   onmouseover="lastStatus = window.status; window.status='Collapse All'; return true"
+   onmouseout="window.status = lastStatus; return true"
+   onclick="return collapse_all()">Collapse All</a> | 
+<a href="#"
+   onmouseover="lastStatus = window.status; window.status='Expand All'; return true"
+   onmouseout="window.status = lastStatus; return true"
+   onclick="return expand_all()">Expand All</a>
+
+[% IF do_context %]
+  | <span style='font-weight: bold'>Context:</span>
+  [% IF context == "patch" %]
+    (<strong>Patch</strong> / 
+  [% ELSE %]
+    (<a href="[% url(context = '') %]">Patch</a> / 
+  [% END %]
+  [% IF context == "file" %]
+    <strong>File</strong> /
+  [% ELSE %]
+    <a href="[% url(context = 'file') %]">File</a> / 
+  [% END %]
+
+  [% IF context == "patch" || context == "file" %]
+    [% context = 3 %]
+  [% END %]
+  [%# textbox for context %]
+  <form style="display: inline"><input type="hidden" name="action" value="diff"><input type="hidden" name="id" value="[% attachid %]"><input type="hidden" name="collapsed" value="[% collapsed FILTER html %]"><input type="hidden" name="headers" value="[% headers FILTER html %]"><input type="text" name="context" value="[% context FILTER html %]" size="3"></form>)
+[% END %]
+
+[% IF warning %]
+<h2 class="warning">Warning: 
+  [% IF warning == "interdiff1" %]
+  this difference between two patches may show things in the wrong places due
+  to a limitation in Bugzilla when comparing patches with different sets of
+  files.
+  [% END %]
+  [% IF warning == "interdiff2" %]
+  this difference between two patches may be inaccurate due to a limitation in
+  Bugzilla when comparing patches made against different revisions.
+  [% END %]
+</h2>
+[% END %]
+ 
+[%# Restore Stuff %]
+<form name="checkboxform">
+<input type="checkbox" name="restore_indicator" style="display: none">
+
+

template/en/default/attachment/edit.html.tmpl

@@ -42,6 +42,10 @@
 
 <script type="application/x-javascript" language="JavaScript">
   <!--
+  var prev_mode = 'raw';
+  var current_mode = 'raw';
+  var has_edited = 0;
+  var has_viewed_as_diff = 0;
   function editAsComment()
     {
       // Get the content of the document as a string.
@@ -69,44 +73,81 @@
       // with a newline.
       theContent = theContent.replace( /(.*\n|.+)/g , ">$1" );
 
-      hideElementById('viewFrame');
-      hideElementById('editButton');
-      hideElementById('smallCommentFrame');
-
-      showElementById('undoEditButton');
-
-      // Show the TEXTAREA that will contain the editable attachment
-      // and copy the content of the attachment into it.
-      showElementById('editFrame');
+      switchToMode('edit');
 
+      // Copy the contents of the diff into the textarea
       var editFrame = document.getElementById('editFrame');
       editFrame.value = theContent;
       editFrame.value += "\n\n";
+
+      has_edited = 1;
     }
   function undoEditAsComment()
     {
-      // Hide the "edit attachment as comment" TEXTAREA and the "undo" button.
-      hideElementById('undoEditButton');
-      hideElementById('editFrame');
-
-      // Show the "view attachment" IFRAME, the "redo" button that allows the user
-      // to go back to editing the attachment as a comment, and the small comment field.
-      showElementById('viewFrame');
-      showElementById('redoEditButton');
-      showElementById('smallCommentFrame');
-
+      switchToMode(prev_mode);
     }
   function redoEditAsComment()
     {
-      // Hide the "view attachment" IFRAME, the "redo" button that allows the user
-      // to go back to editing the attachment as a comment, and the small comment field.
-      hideElementById('viewFrame');
-      hideElementById('redoEditButton');
-      hideElementById('smallCommentFrame');
-
-      // Show the "edit attachment as comment" TEXTAREA and the "undo" button.
-      showElementById('undoEditButton');
-      showElementById('editFrame');
+      switchToMode('edit');
+    }
+  function viewDiff()
+    {
+      switchToMode('diff');
+
+      // If we have not viewed as diff before, set the view diff frame URL
+      if (!has_viewed_as_diff) {
+        var viewDiffFrame = document.getElementById('viewDiffFrame');
+        viewDiffFrame.src =
+            'attachment.cgi?id=[% attachid %]&action=diff&headers=0';
+        has_viewed_as_diff = 1;
+      }
+    }
+  function viewRaw()
+    {
+      switchToMode('raw');
+    }
+
+  function switchToMode(mode)
+    {
+      if (mode == current_mode) {
+        alert('switched to same mode!  This should not happen.');
+        return;
+      }
+
+      // Switch out of current mode
+      if (current_mode == 'edit') {
+        hideElementById('editFrame');
+        hideElementById('undoEditButton');
+      } else if (current_mode == 'raw') {
+        hideElementById('viewFrame');
+        hideElementById('viewDiffButton');
+        hideElementById(has_edited ? 'redoEditButton' : 'editButton');
+        hideElementById('smallCommentFrame');
+      } else if (current_mode == 'diff') {
+        hideElementById('viewDiffFrame');
+        hideElementById('viewRawButton');
+        hideElementById(has_edited ? 'redoEditButton' : 'editButton');
+        hideElementById('smallCommentFrame');
+      }
+
+      // Switch into new mode
+      if (mode == 'edit') {
+        showElementById('editFrame');
+        showElementById('undoEditButton');
+      } else if (mode == 'raw') {
+        showElementById('viewFrame');
+        showElementById('viewDiffButton');
+        showElementById(has_edited ? 'redoEditButton' : 'editButton');
+        showElementById('smallCommentFrame');
+      } else if (mode == 'diff') {
+        showElementById('viewDiffFrame');
+        showElementById('viewRawButton');
+        showElementById(has_edited ? 'redoEditButton' : 'editButton');
+        showElementById('smallCommentFrame');
+      }
+
+      prev_mode = current_mode;
+      current_mode = mode;
     }
 
   function hideElementById(id)
@@ -184,8 +225,11 @@
             <textarea name="comment" rows="5" cols="25" wrap="soft"></textarea><br>
         </div>
 
-        <input type="submit" value="Submit">
-
+        <input type="submit" value="Submit"><br><br>
+        <strong>Actions:</strong> <a href="attachment.cgi?id=[% attachid %]">View</a>
+        [% IF ispatch %]
+         | <a href="attachment.cgi?id=[% attachid %]&action=diff">Diff</a>
+        [% END %]
         </small>
       </td>
 
@@ -199,9 +243,12 @@
           <script type="application/x-javascript" language="JavaScript">
             <!--
             if (typeof document.getElementById == "function") {
+              document.write('<iframe id="viewDiffFrame" style="height: 400px; width: 100%; display: none;"></iframe>');
               document.write('<button type="button" id="editButton" onclick="editAsComment();">Edit Attachment As Comment</button>');
               document.write('<button type="button" id="undoEditButton" onclick="undoEditAsComment();" style="display: none;">Undo Edit As Comment</button>');
               document.write('<button type="button" id="redoEditButton" onclick="redoEditAsComment();" style="display: none;">Redo Edit As Comment</button>');
+              document.write('<button type="button" id="viewDiffButton" onclick="viewDiff();">View Attachment As Diff</button>');
+              document.write('<button type="button" id="viewRawButton" onclick="viewRaw();" style="display: none;">View Attachment As Raw</button>');
             }
             //-->
           </script>

template/en/default/attachment/list.html.tmpl

@@ -69,8 +69,12 @@
       <td valign="top">
         [% IF attachment.canedit %]
           <a href="attachment.cgi?id=[% attachment.attachid %]&amp;action=edit">Edit</a>
-        [% ELSE %]
-          None
+        [% END %]
+        [% IF attachment.ispatch %]
+          [% IF attachment.canedit %]
+            |
+          [% END %]
+          <a href="attachment.cgi?id=[% attachment.attachid %]&amp;action=diff">Diff</a>
         [% END %]
       </td>
     </tr>

template/en/default/filterexceptions.pl

@@ -105,7 +105,6 @@
 
 'reports/components.html.tmpl' => [
   'numcols',
-  'numcols - 1',
   'comp.description', 
   'comp.initialowner', # email address
   'comp.initialqacontact', # email address
@@ -181,10 +180,6 @@
   'other_format.name', 
   'other_format.description', #
   'sizeurl', 
-  'height + 100', 
-  'height - 100', 
-  'width + 100', 
-  'width - 100', 
   'switchbase',
   'format',
   'cumulate',
@@ -257,7 +252,6 @@
 
 'list/table.html.tmpl' => [
   'id', 
-  'splitheader ? 2 : 1', 
   'abbrev.$id.title || field_descs.$id || column.title', #
   'tableheader',
   'bug.bug_severity', #
@@ -387,9 +381,6 @@
   'dependson_ids.join(",")', 
   'blocked_ids.join(",")', 
   'dep_id', 
-  'hide_resolved ? 0 : 1', 
-  'hide_resolved ? "Show" : "Hide"', 
-  'realdepth < 2 || maxdepth == 1 ? "disabled" : ""', 
   'hide_resolved', 
   'realdepth < 2 ? "disabled" : ""', 
   'maxdepth + 1', 
@@ -420,7 +411,6 @@
 ],
 
 'bug/navigate.html.tmpl' => [
-  'this_bug_idx + 1', 
   'bug_list.first', 
   'bug_list.last', 
   'bug_list.$prev_bug', 
@@ -540,7 +530,6 @@
   'flag.type.name', 
   'flag.status',
   'flag.requestee.nick', # Email
-  'show_attachment_flags ? 4 : 3',
   'bugid',
 ],
 
@@ -553,6 +542,27 @@
   'bugid', 
 ],
 
+'attachment/diff-header.html.tmpl' => [
+  'attachid',
+  'bugid',
+  'old_url',
+  'new_url',
+  'oldid',
+  'newid',
+  'style',
+  'javascript',
+  'patch.id',
+],
+
+'attachment/diff-file.html.tmpl' => [
+  'lxr_prefix',
+  'file.minus_lines',
+  'file.plus_lines',
+  'bonsai_prefix',
+  'section.old_start',
+  'section_num'
+],
+
 'admin/products/groupcontrol/confirm-edit.html.tmpl' => [
   'group.count', 
 ],
@@ -586,7 +596,6 @@
 ],
 
 'admin/flag-type/list.html.tmpl' => [
-  'type.is_active ? "active" : "inactive"', 
   'type.id', 
   'type.flag_count', 
 ],
@@ -601,7 +610,6 @@
 
 'account/prefs/email.html.tmpl' => [
   'watchedusers', # Email
-  'useqacontact ? \'5\' : \'4\'', 
   'role', 
   'reason.name', 
   'reason.description',
@@ -617,7 +625,6 @@
   'tab.description', 
   'current_tab.name', 
   'current_tab.description', 
-  'current_tab.description FILTER lower',
 ],
 
 );

template/en/default/global/user-error.html.tmpl

@@ -344,6 +344,19 @@
     Valid types must be of the form <em>foo/bar</em> where <em>foo</em>
     is either <em>application, audio, image, message, model, multipart,
     text,</em> or <em>video</em>.
+    
+  [% ELSIF error == "invalid_context" %]
+    [% title = "Invalid Context" %]
+    The context [% context FILTER html %] is invalid (must be a number,
+    "file" or "patch").
+
+  [% ELSIF error == "invalid_format" %]
+    [% title = "Invalid Format" %]
+    The format "[% format FILTER html %]" is invalid (must be one of
+    [% FOREACH my_format = formats %]
+      "[% my_format FILTER html %]"
+    [% END %]
+    ).
 
   [% ELSIF error == "invalid_maxrow" %]
     [% title = "Invalid Max Rows" %]
@@ -427,6 +440,10 @@
     The query named <em>[% queryname FILTER html %]</em> does not
     exist.
         
+  [% ELSIF error == "must_be_patch" %]
+    [% title = "Attachment Must Be Patch" %]
+    Attachment #[% attach_id FILTER html %] must be a patch.
+
   [% ELSIF error == "missing_subcategory" %]
     [% title = "Missing Subcategory" %]
     You did not specify a subcategory for this series.