Patch Viewer, a pretty way of viewing and manipulating patches (bug 174942). Requires PatchIterator to be installed, classes uploaded to that bug and will be soon in CPAN.

12 files changed, 1066 insertions, 109 deletions

diff --git a/attachment.cgi b/attachment.cgi
index e70fb88f4..149ddfd21 100755
--- a/attachment.cgi
+++ b/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
 {
diff --git a/checksetup.pl b/checksetup.pl
index 27542d8e4..b7c1fdd0f 100755
--- a/checksetup.pl
+++ b/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
diff --git a/defparams.pl b/defparams.pl
index e2dcf7533..20700d02d 100644
--- a/defparams.pl
+++ b/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;
diff --git a/globals.pl b/globals.pl
index 134bddb28..67fed5306 100644
--- a/globals.pl
+++ b/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",
diff --git a/t/008filter.t b/t/008filter.t
index fc8f77e69..0d6ec4b49 100644
--- a/t/008filter.t
+++ b/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;
diff --git a/template/en/default/attachment/diff-file.html.tmpl b/template/en/default/attachment/diff-file.html.tmpl
new file mode 100644
index 000000000..51072269d
--- /dev/null
+++ b/template/en/default/attachment/diff-file.html.tmpl
@@ -0,0 +1,129 @@
+<!-- 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>
diff --git a/template/en/default/attachment/diff-footer.html.tmpl b/template/en/default/attachment/diff-footer.html.tmpl
new file mode 100644
index 000000000..4eb94aca2
--- /dev/null
+++ b/template/en/default/attachment/diff-footer.html.tmpl
@@ -0,0 +1,33 @@
+<!-- 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 %]
diff --git a/template/en/default/attachment/diff-header.html.tmpl b/template/en/default/attachment/diff-header.html.tmpl
new file mode 100644
index 000000000..c1b70173e
--- /dev/null
+++ b/template/en/default/attachment/diff-header.html.tmpl
@@ -0,0 +1,307 @@
+<!-- 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">
+
+
diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl
index 14c2dc1fe..2cfc0e088 100644
--- a/template/en/default/attachment/edit.html.tmpl
+++ b/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>
diff --git a/template/en/default/attachment/list.html.tmpl b/template/en/default/attachment/list.html.tmpl
index fc5852923..598f8172b 100644
--- a/template/en/default/attachment/list.html.tmpl
+++ b/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>
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index ba626a21b..60590d4a4 100644
--- a/template/en/default/filterexceptions.pl
+++ b/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',
 ],
 
 );
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 8aa3842c8..de5d60c6c 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/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.