Bug 514970: Clean up duplicates.cgi and make it use Bug objects

r=LpSolit, a=LpSolit

11 files changed, 300 insertions, 282 deletions

diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 92353b6a0..4ee362945 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -134,7 +134,8 @@ sub check {
         # We don't want to override the normal template "user" object if
         # "user" is one of the params.
         delete $param->{user};
-        ThrowUserError('object_does_not_exist', { %$param, class => $class });
+        my $error = delete $param->{_error} || 'object_does_not_exist';
+        ThrowUserError($error, { %$param, class => $class });
     }
     return $obj;
 }
diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm
index 0228aca02..c993905db 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -913,6 +913,17 @@ sub check_product {
     return $product;
 }
 
+sub check {
+    my ($class, $params) = @_;
+    $params = { name => $params } if !ref $params;
+    $params->{_error} = 'product_access_denied';
+    my $product = $class->SUPER::check($params);
+    if (!Bugzilla->user->can_access_product($product)) {
+        ThrowUserError('product_access_denied', $params);
+    }
+    return $product;
+}
+
 1;
 
 __END__
diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm
index 59324383f..06c6be5cb 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -810,8 +810,8 @@ sub get_enterable_products {
 }
 
 sub can_access_product {
-    my ($self, $product_name) = @_;
-
+    my ($self, $product) = @_;
+    my $product_name = blessed($product) ? $product->name : $product;
     return scalar(grep {$_->name eq $product_name} @{$self->get_accessible_products});
 }
 
@@ -2055,10 +2055,11 @@ the database again. Used mostly by L<Bugzilla::Product>.
 
  Returns:     an array of product objects.
 
-=item C<can_access_product(product_name)>
+=item C<can_access_product($product)>
 
-Returns 1 if the user can search or enter bugs into the specified product,
-and 0 if the user should not be aware of the existence of the product.
+Returns 1 if the user can search or enter bugs into the specified product
+(either a L<Bugzilla::Product> or a product name), and 0 if the user should
+not be aware of the existence of the product.
 
 =item C<get_accessible_products>
 
diff --git a/Bugzilla/WebService/Bug.pm b/Bugzilla/WebService/Bug.pm
index 6051a1d1c..53f3255d1 100644
--- a/Bugzilla/WebService/Bug.pm
+++ b/Bugzilla/WebService/Bug.pm
@@ -434,7 +434,7 @@ sub legal_values {
         defined $id || ThrowCodeError('param_required',
             { function => 'Bug.legal_values', param => 'product_id' });
         grep($_->id eq $id, @{Bugzilla->user->get_accessible_products})
-            || ThrowUserError('product_access_denied', { product => $id });
+            || ThrowUserError('product_access_denied', { id => $id });
 
         my $product = new Bugzilla::Product($id);
         my @objects;
diff --git a/duplicates.cgi b/duplicates.cgi
index 4c0509864..cbc991b7d 100755
--- a/duplicates.cgi
+++ b/duplicates.cgi
@@ -33,6 +33,24 @@ use Bugzilla::Search;
 use Bugzilla::Field;
 use Bugzilla::Product;
 
+use constant DEFAULTS => {
+    # We want to show bugs which:
+    # a) Aren't CLOSED; and
+    # b)  i) Aren't VERIFIED; OR
+    #    ii) Were resolved INVALID/WONTFIX
+    #
+    # The rationale behind this is that people will eventually stop
+    # reporting fixed bugs when they get newer versions of the software,
+    # but if the bug is determined to be erroneous, people will still
+    # keep reporting it, so we do need to show it here.
+    fully_exclude_status  => ['CLOSED'],
+    partly_exclude_status => ['VERIFIED'],
+    except_resolution => ['INVALID', 'WONTFIX'],
+    changedsince => 7,
+    maxrows      => 20,
+    sortby       => 'count',
+};
+
 ###############
 # Subroutines #
 ###############
@@ -55,8 +73,13 @@ sub add_indirect_dups {
 sub walk_dup_chain {
     my ($dups, $from_id) = @_;
     my $to_id = $dups->{$from_id};
+    my %seen;
     while (my $bug_id = $dups->{$to_id}) {
-        last if $bug_id == $from_id; # avoid duplicate loops
+        if ($seen{$bug_id}) {
+            warn "Duplicate loop: $to_id -> $bug_id\n";
+            last;
+        }
+        $seen{$bug_id} = 1;
         $to_id = $bug_id;
     }
     # Optimize for future calls to add_indirect_dups.
@@ -64,41 +87,70 @@ sub walk_dup_chain {
     return $to_id;
 }
 
+# Get params from URL
+sub formvalue {
+    my ($name) = (@_);
+    my $cgi = Bugzilla->cgi;
+    if (defined $cgi->param($name)) {
+        return $cgi->param($name);
+    }
+    elsif (exists DEFAULTS->{$name}) {
+        return ref DEFAULTS->{$name} ? @{ DEFAULTS->{$name} } 
+                                     : DEFAULTS->{$name};
+    }
+    return undef;
+}
+
+sub sort_duplicates {
+    my ($a, $b, $sort_by) = @_;
+    if ($sort_by eq 'count' or $sort_by eq 'delta') {
+        return $a->{$sort_by} <=> $b->{$sort_by};
+    }
+    if ($sort_by =~ /^(bug_)?id$/) {
+        return $a->{'bug'}->$sort_by <=> $b->{'bug'}->$sort_by;
+    }
+    return $a->{'bug'}->$sort_by cmp $b->{'bug'}->$sort_by;
+    
+}
+
 ###############
 # Main Script #
 ###############
 
 my $cgi = Bugzilla->cgi;
 my $template = Bugzilla->template;
-my $vars = {};
-
-Bugzilla->login();
+my $user = Bugzilla->login();
 
 my $dbh = Bugzilla->switch_to_shadow_db();
 
-# Get params from URL
-sub formvalue {
-    my ($name, $default) = (@_);
-    return Bugzilla->cgi->param($name) || $default || "";
-}
-
-my $sortby = formvalue("sortby");
-my $changedsince = formvalue("changedsince", 7);
-my $maxrows = formvalue("maxrows", 100);
+my $changedsince = formvalue("changedsince");
+my $maxrows = formvalue("maxrows");
 my $openonly = formvalue("openonly");
-my $reverse = formvalue("reverse") ? 1 : 0;
+my $sortby = formvalue("sortby");
+if (!grep(lc($_) eq lc($sortby), qw(count delta id))) {
+    Bugzilla::Field->check($sortby);
+}
+my $reverse = formvalue("reverse");
+# Reverse count and delta by default.
+if (!defined $reverse) {
+    if ($sortby eq 'count' or $sortby eq 'delta') {
+        $reverse = 1;
+    }
+    else {
+        $reverse = 0;
+    }
+}
 my @query_products = $cgi->param('product');
 my $sortvisible = formvalue("sortvisible");
-my @buglist = (split(/[:,]/, formvalue("bug_id")));
-detaint_natural($_) foreach @buglist;
-# If we got any non-numeric items, they will now be undef. Remove them from
-# the list.
-@buglist = grep($_, @buglist);
+my @bugs;
+if ($sortvisible) {
+    my @limit_to_ids = (split(/[:,]/, formvalue("bug_id") || ''));
+    @bugs = @{ Bugzilla::Bug->new_from_list(\@limit_to_ids) };
+    @bugs = @{ $user->visible_bugs(\@bugs) };
+}
 
 # Make sure all products are valid.
-foreach my $p (@query_products) {
-    Bugzilla::Product::check_product($p);
-}
+@query_products = map { Bugzilla::Product->check($_) } @query_products;
 
 # Small backwards-compatibility hack, dated 2002-04-10.
 $sortby = "count" if $sortby eq "dup_count";
@@ -112,7 +164,6 @@ detaint_natural($changedsince)
   || ThrowUserError("invalid_changedsince", 
                     { changedsince => $origchangedsince });
 
-
 my %total_dups = @{$dbh->selectcol_arrayref(
     "SELECT dupe_of, COUNT(dupe)
        FROM duplicates
@@ -136,117 +187,76 @@ my %since_dups = @{$dbh->selectcol_arrayref(
     $reso_field_id, $changedsince)};
 add_indirect_dups(\%since_dups, \%dupe_relation);
 
-my (@bugs, @bug_ids);
-
+# Enforce the mostfreqthreshold parameter and the "bug_id" cgi param.
 foreach my $id (keys %total_dups) {
     if ($total_dups{$id} < Bugzilla->params->{'mostfreqthreshold'}) {
         delete $total_dups{$id};
         next;
     }
-    if ($sortvisible and @buglist and !grep($_ == $id, @buglist)) {
+    if ($sortvisible and !grep($_->id == $id, @bugs)) {
         delete $total_dups{$id};
     }
 }
 
-if (scalar %total_dups) {
-    # use Bugzilla::Search so that we get the security checking
-    my $params = new Bugzilla::CGI({ 'bug_id' => [keys %total_dups] });
-
-    if ($openonly) {
-        $params->param('resolution', '---');
-    } else {
-        # We want to show bugs which:
-        # a) Aren't CLOSED; and
-        # b)  i) Aren't VERIFIED; OR
-        #    ii) Were resolved INVALID/WONTFIX
-
-        # The rationale behind this is that people will eventually stop
-        # reporting fixed bugs when they get newer versions of the software,
-        # but if the bug is determined to be erroneous, people will still
-        # keep reporting it, so we do need to show it here.
-
-        # a)
-        $params->param('field0-0-0', 'bug_status');
-        $params->param('type0-0-0', 'notequals');
-        $params->param('value0-0-0', 'CLOSED');
-
-        # b) i)
-        $params->param('field0-1-0', 'bug_status');
-        $params->param('type0-1-0', 'notequals');
-        $params->param('value0-1-0', 'VERIFIED');
-
-        # b) ii)
-        $params->param('field0-1-1', 'resolution');
-        $params->param('type0-1-1', 'anyexact');
-        $params->param('value0-1-1', 'INVALID,WONTFIX');
-    }
+if (!@bugs) {
+    @bugs = @{ Bugzilla::Bug->new_from_list([keys %total_dups]) };
+    @bugs = @{ $user->visible_bugs(\@bugs) };
+}
 
-    # Restrict to product if requested
-    if ($cgi->param('product')) {
-        $params->param('product', join(',', @query_products));
+my @fully_exclude_status = formvalue('fully_exclude_status');
+my @partly_exclude_status = formvalue('partly_exclude_status');
+my @except_resolution = formvalue('except_resolution');
+
+# Filter bugs by criteria
+my @result_bugs;
+foreach my $bug (@bugs) {
+    # It's possible, if somebody specified a bug ID that wasn't a dup
+    # in the "buglist" parameter and specified $sortvisible that there
+    # would be bugs in the list with 0 dups, so we want to avoid that.
+    next if !$total_dups{$bug->id};
+
+    next if ($openonly and !$bug->isopened);
+    # If the bug has a status in @fully_exclude_status, we skip it,
+    # no question.
+    next if grep($_ eq $bug->bug_status, @fully_exclude_status);
+    # If the bug has a status in @partly_exclude_status, we skip it...
+    if (grep($_ eq $bug->bug_status, @partly_exclude_status)) {
+        # ...unless it has a resolution in @except_resolution.
+        next if !grep($_ eq $bug->resolution, @except_resolution);
     }
 
-    my $query = new Bugzilla::Search('fields' => [qw(bug_id
-                                                     component
-                                                     bug_severity
-                                                     op_sys
-                                                     target_milestone
-                                                     short_desc
-                                                     bug_status
-                                                     resolution
-                                                    )
-                                                 ],
-                                     'params' => $params,
-                                    );
-
-    my $results = $dbh->selectall_arrayref($query->getSQL());
-
-    foreach my $result (@$results) {
-        # Note: maximum row count is dealt with in the template.
-
-        my ($id, $component, $bug_severity, $op_sys, $target_milestone, 
-            $short_desc, $bug_status, $resolution) = @$result;
-
-        push (@bugs, { id => $id,
-                       count => $total_dups{$id},
-                       delta => $since_dups{$id} || 0, 
-                       component => $component,
-                       bug_severity => $bug_severity,
-                       op_sys => $op_sys,
-                       target_milestone => $target_milestone,
-                       short_desc => $short_desc,
-                       bug_status => $bug_status, 
-                       resolution => $resolution });
-        push (@bug_ids, $id); 
+    if (scalar @query_products) {
+        next if !grep($_->id == $bug->product_id, @query_products);
     }
-}
 
-$vars->{'bugs'} = \@bugs;
-$vars->{'bug_ids'} = \@bug_ids;
-
-$vars->{'sortby'} = $sortby;
-$vars->{'sortvisible'} = $sortvisible;
-$vars->{'changedsince'} = $changedsince;
-$vars->{'maxrows'} = $maxrows;
-$vars->{'openonly'} = $openonly;
-$vars->{'reverse'} = $reverse;
-$vars->{'format'} = $cgi->param('format');
-$vars->{'query_products'} = \@query_products;
-$vars->{'products'} = Bugzilla->user->get_selectable_products;
-
-
-my $format = $template->get_format("reports/duplicates",
-                                   scalar($cgi->param('format')),
-                                   scalar($cgi->param('ctype')));
-
-# We set the charset in Bugzilla::CGI, but CGI.pm ignores it unless the
-# Content-Type is a text type. In some cases, such as when we are
-# generating RDF, it isn't, so we specify the charset again here.
-print $cgi->header(
-    -type => $format->{'ctype'},
-    (Bugzilla->params->{'utf8'} ? ('charset', 'utf8') : () )
+    # Note: maximum row count is dealt with later.
+    push (@result_bugs, { bug => $bug,
+                          count => $total_dups{$bug->id},
+                          delta => $since_dups{$bug->id} || 0 });
+}
+@bugs = @result_bugs;
+@bugs = sort { sort_duplicates($a, $b, $sortby) } @bugs;
+if ($reverse) {
+    @bugs = reverse @bugs;
+}
+@bugs = @bugs[0..$maxrows-1] if scalar(@bugs) > $maxrows;
+
+my %vars = (
+    bugs     => \@bugs,
+    bug_ids  => [map { $_->{'bug'}->id } @bugs],
+    sortby   => $sortby,
+    openonly => $openonly,
+    maxrows  => $maxrows,
+    reverse  => $reverse,
+    format   => scalar $cgi->param('format'),
+    product  => [map { $_->name } @query_products],
+    sortvisible  => $sortvisible,
+    changedsince => $changedsince,
 );
 
+my $format = $template->get_format("reports/duplicates", $vars{'format'});
+print $cgi->header;
+
 # Generate and return the UI (HTML page) from the appropriate template.
-$template->process($format->{'template'}, $vars)
+$template->process($format->{'template'}, \%vars)
   || ThrowTemplateError($template->error());
diff --git a/skins/standard/duplicates.css b/skins/standard/duplicates.css
index 9948b789e..e89b72c59 100644
--- a/skins/standard/duplicates.css
+++ b/skins/standard/duplicates.css
@@ -10,25 +10,40 @@
  * 
  * 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.
+ * The Initial Developer of the Original Code is Everything Solved, Inc.
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
  * 
- * Contributor(s): Myk Melez <myk@mozilla.org>
+ * Contributor(s):
+ *   Max Kanat-Alexander <mkanat@bugzilla.org>
  */
 
-tree#results-tree {
-  margin-right: 0px;
-  border-right-width: 0px;
-  margin-left: 0px;
-  border-left-width: 0px;
+#duplicates_table {
+  border-collapse: collapse;
 }
 
-treechildren:-moz-tree-cell-text(resolution-FIXED) { 
-  text-decoration: line-through;
+#duplicates_table .resolved {
+  background-color: #d9d9d9; 
+  color: black;
 }
 
-treecol#id_column { width: 6em; }
-treecol#duplicate_count_column { width: 5em; }
-treecol#duplicate_delta_column { width: 5em; }
+#duplicates_table thead tr {
+  background-color: #ccc;
+  color: black;
+}
+
+#duplicates_table thead tr th {
+  vertical-align: middle;
+}
+
+#duplicates_table td, #duplicates_table th {
+  border: 1px solid black;
+  padding: .1em .25em;
+}
+
+#duplicates_table tbody td {
+  text-align: center;
+}
+#duplicates_table tbody td.short_desc {
+  text-align: left;
+}
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index c4d0c064f..2c096df73 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -92,20 +92,6 @@
   'request.attach_id', 
 ],
 
-'reports/duplicates-table.html.tmpl' => [
-  'column.name', 
-  'column.description',
-  'bug.count', 
-  'bug.delta', 
-],
-
-'reports/duplicates.html.tmpl' => [
-  'bug_ids_string', 
-  'maxrows',
-  'changedsince',
-  'reverse',
-],
-
 'reports/keywords.html.tmpl' => [
   'keyword.bug_count', 
 ],
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 467d4a174..79faabbfd 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -1301,8 +1301,13 @@
     Try splitting your patch into several pieces.
 
   [% ELSIF error == "product_access_denied" %]
-    Either the product '[% product FILTER html %]' does not exist or
-    you don't have access to it.
+    Either the product 
+    [%+ IF id.defined %]
+      with the id [% id FILTER html %]
+    [% ELSE %]
+      '[% name FILTER html %]'
+    [% END %]
+    does not exist or you don't have access to it.
 
   [% ELSIF error == "product_doesnt_exist" %]
     [% title = "Specified Product Does Not Exist" %]
diff --git a/template/en/default/reports/duplicates-simple.html.tmpl b/template/en/default/reports/duplicates-simple.html.tmpl
index 61d0c6fd2..bef002193 100644
--- a/template/en/default/reports/duplicates-simple.html.tmpl
+++ b/template/en/default/reports/duplicates-simple.html.tmpl
@@ -15,7 +15,9 @@
   # Copyright (C) 1998 Netscape Communications Corporation. All
   # Rights Reserved.
   #
-  # Contributor(s): Gervase Markham <gerv@gerv.net>
+  # Contributor(s): 
+  #  Gervase Markham <gerv@gerv.net>
+  #  Max Kanat-Alexander <mkanat@bugzilla.org>
   #%]
 
 [%# INTERFACE:
@@ -24,8 +26,9 @@
 
 [% PROCESS global/variables.none.tmpl %]
 
+<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"
+                      "http://www.w3.org/TR/html4/loose.dtd">
 <html>
-
   [% IF product %]
     [% title = "Most Frequently Reported $terms.Bugs for $product" %]
   [% ELSE %]
@@ -39,5 +42,4 @@
   <body>
     [% PROCESS "reports/duplicates-table.html.tmpl" %]
   </body>
-
 </html>
diff --git a/template/en/default/reports/duplicates-table.html.tmpl b/template/en/default/reports/duplicates-table.html.tmpl
index 8950f340a..38ab2d56b 100644
--- a/template/en/default/reports/duplicates-table.html.tmpl
+++ b/template/en/default/reports/duplicates-table.html.tmpl
@@ -15,21 +15,16 @@
   # Copyright (C) 1998 Netscape Communications Corporation. All
   # Rights Reserved.
   #
-  # Contributor(s): Gervase Markham <gerv@gerv.net>
+  # Contributor(s): 
+  #   Gervase Markham <gerv@gerv.net>
+  #   Max Kanat-Alexander <mkanat@bugzilla.org>
   #%]
 
 [%# INTERFACE:
-  # bugs: list of hashes. May be empty. Each hash has nine members:
-  #   id: integer. The bug number
+  # bugs: list of hashes. May be empty. Each hash has three members:
+  #   bug: A Bugzilla::Bug object
   #   count: integer. The number of dupes
   #   delta: integer. The change in count in the last $changedsince days
-  #   component: string. The bug's component
-  #   bug_severity: string. The bug's severity.
-  #   op_sys: string. The bug's reported OS.
-  #   target_milestone: string. The bug's TM.
-  #   short_desc: string. The bug's summary.
-  #   bug_status: string. The bug's status.
-  #   resolution: string. The bug's resolution, if any.
   #
   # bug_ids: list of integers. May be empty. The IDs of the bugs in $bugs.
   #
@@ -38,99 +33,89 @@
   # maxrows: integer. Max number of rows to display.
   # changedsince: integer. The number of days ago for the changedsince column.
   # openonly: boolean. True if we are only showing open bugs.
-  # query_products: list of strings. Restrict to these products only.
+  # product: array of strings. Restrict to these products only.
   #%]
 
-[% PROCESS global/variables.none.tmpl %]
+[% PROCESS "global/field-descs.none.tmpl" %]
 
 [%# *** Column Headers *** %]
 
-[% IF bug_ids.size > 0 %]
-  <table border>
-    <thead>
-      <tr bgcolor="#CCCCCC">
-        [% FOREACH column = [ { name => "id", description => "$terms.Bug #" },
-                              { name => "count", description => "Dupe<br>Count" },
-                              { name => "delta",
-                                description => "Change in last<br>$changedsince day(s)" },
-                              { name => "component", description => "Component" },
-                              { name => "bug_severity", description => "Severity" },
-                              { name => "op_sys", description => "Op Sys" },
-                              { name => "target_milestone",
-                                description => "Target<br>Milestone" },
-                              { name => "short_desc", description => "Summary" } ]
-         %]
+[% SET columns = [
+    { name => "id", description => "$terms.Bug #" },
+    { name => "count", description => "Dupe<br>Count" },
+    { name => "delta",
+      description => "Change in last<br>$changedsince day(s)" },
+    { name => "component", description => field_descs.component },
+    { name => "bug_severity", description => field_descs.bug_severity },
+    { name => "op_sys", description => field_descs.op_sys },
+    { name => "target_milestone", description => field_descs.target_milestone },
+    { name => "short_desc", description => field_descs.short_desc },
+] %]
 
-          <th>
-            [% bug_ids_string = bug_ids.join(',') %]
-            <a href="duplicates.cgi?sortby=[% column.name %]
-              [% IF sortby == column.name %]
-                [% "&amp;reverse=1" IF NOT reverse %]
-              [% ELSE %]
-                [%-# Some columns start off reversed %]
-                [% "&amp;reverse=1" IF column.name.match('delta|count') %]
-              [% END %]
-              [% IF maxrows %]&amp;maxrows=[% maxrows FILTER html %][% END %]
-              [% IF changedsince %]&amp;changedsince=[% changedsince FILTER html %][% END %]
-              [% "&amp;openonly=1" IF openonly %]
-              [% FOREACH p = query_products %]&amp;product=[% p FILTER html %][% END %]
-              [% IF format %]&amp;format=[% format FILTER html %][% END %]
-              [% IF sortvisible %]&amp;bug_id=[% bug_ids_string FILTER html %]&amp;sortvisible=1[% END %]">
-              [% column.description %]</a>
+[% SET base_args = [] %]
+[% FOREACH param = ['maxrows', 'openonly', 'format', 'sortvisible',
+                    'changedsince', 'product'] 
+%]
+  [% NEXT IF NOT ${param}.defined %]
+  [% FOREACH value = ${param} %]
+    [% filtered_value = value FILTER url_quote %]
+    [% base_args.push("$param=$filtered_value") %]
+  [% END %]
+[% END %]
+[% IF sortvisible %]
+  [% bug_ids_string = bug_ids.nsort.join(',') FILTER url_quote %]
+  [% base_args.push("bug_id=$bug_ids_string") %]
+[% END %]
+[% base_args_string = base_args.join('&amp;') %]
+
+[% IF bugs.size %]
+  <table id="duplicates_table" cellpadding="0" cellspacing="0">
+    <thead>
+      <tr>
+        [% FOREACH column = columns %]
+          [% IF column.name == sortby %]
+            [%# We add this to the column object so it doesn't affect future 
+              # iterations of the loop.
+              #%]
+            [% column.reverse_sort = reverse ? 0 : 1 %]
+          [% END %]
+          <th class="[% column.name FILTER html %]">
+            <a href="duplicates.cgi?sortby=[% column.name FILTER url_quote %]
+                     [% IF column.reverse_sort.defined %]
+                      [%- %]&amp;reverse=[% column.reverse_sort FILTER url_quote %]
+                     [% END %]
+                     [% IF base_args_string %]
+                       [% "&amp;$base_args_string" FILTER none %]
+                     [% END %]"
+            >[% column.description FILTER none %]</a>
           </th>
         [% END %]
       </tr>
     </thead>
 
-    [% IF NOT sortby %]
-      [% sortby = "count"; reverse = "1" %]
-    [% END %]
-
-    [% IF sortby == "id" OR sortby == "count" OR sortby == "delta" %]
-      [%# Numeric sort %]
-      [% sortedbugs = bugs.nsort(sortby) %]
-    [% ELSE %]
-      [% sortedbugs = bugs.sort(sortby) %]
-    [% END %]
-
-    [% IF reverse %]
-      [% bugs = sortedbugs.reverse %]
-    [% ELSE %]
-      [% bugs = sortedbugs %]
-    [% END %]
-
     [%# *** Buglist *** %]
-    <tbody>
 
-      [%# We need to keep track of the bug IDs we are actually displaying, because
-        # if the user decides to sort the visible list, we need to know what that
-        # list actually is. %]
-      [% vis_bug_ids = [] %]
-
-      [% FOREACH bug = bugs %]
-        [% LAST IF loop.index() >= maxrows %]
-        [% vis_bug_ids.push(bug.id) %]
-
-        <tr [% "class='resolved'" IF bug.resolution != "" %]>
-          <td>
-            <center>
-              [% bug.id FILTER bug_link(bug.id) FILTER none %]
-            </center>
+    <tbody>
+      [% FOREACH item = bugs %]
+        [% SET bug = item.bug %]
+        <tr [% " class='resolved'" IF NOT bug.isopened %]>
+          <td class="id">
+            [% bug.id FILTER bug_link(bug) FILTER none %]
           </td>
-
-          <td>
-            <center>
-              [% bug.count %]
-            </center>
+          <td class="count">[% item.count FILTER html %]</td>
+          <td class="delta">[% item.delta FILTER html %]</td>
+          <td class="component">[% bug.component FILTER html %]</td>
+          <td class="bug_severity">
+            [%- display_value('bug_severity', bug.bug_severity) FILTER html %]
           </td>
-
-          <td><center>[% bug.delta %]</center></td>
-
-          <td>[% bug.component FILTER html %]</td>
-          <td><center>[% display_value("bug_severity",     bug.bug_severity    ) FILTER html %]</center></td>
-          <td><center>[% display_value("op_sys",           bug.op_sys          ) FILTER html %]</center></td>
-          <td><center>[% display_value("target_milestone", bug.target_milestone) FILTER html %]</center></td>
-          <td>[% bug.short_desc FILTER html %]</td>
+          <td class="op_sys">
+            [%- display_value('op_sys', bug.op_sys) FILTER html %]
+          </td>
+          <td class="target_milestone">
+            [% display_value('target_milestone', 
+                             bug.target_milestone) FILTER html %]
+          </td>
+          <td class="short_desc">[% bug.short_desc FILTER html %]</td>
         </tr>
       [% END %]
     </tbody>
diff --git a/template/en/default/reports/duplicates.html.tmpl b/template/en/default/reports/duplicates.html.tmpl
index e4ea73882..6b49a23c6 100644
--- a/template/en/default/reports/duplicates.html.tmpl
+++ b/template/en/default/reports/duplicates.html.tmpl
@@ -19,14 +19,12 @@
   #%]
 
 [%# INTERFACE:
-  # products: an array of product objects this user can see.
-  #
   # sortby: string. the column on which we are sorting the buglist.
   # reverse: boolean. True if we are reversing the current sort.
   # maxrows: integer. Max number of rows to display.
   # changedsince: integer. The number of days ago for the changedsince column.
   # openonly: boolean. True if we are only showing open bugs.
-  # query_products: list of strings. The set of products we check for dups.
+  # product: array of strings. The set of products we check for dups.
   #
   # Additionally, you need to fulfill the interface to
   # duplicates-table.html.tmpl.
@@ -34,9 +32,10 @@
 
 [% PROCESS global/variables.none.tmpl %]
 
-[% IF query_products.size %]
+[% IF product.size %]
   [% title = BLOCK %]
-    Most Frequently Reported [% terms.Bugs %] for [% query_products.join(', ') FILTER html %]
+    Most Frequently Reported [% terms.Bugs %] for 
+    [%+ product.join(', ') FILTER html %]
   [% END %]
 [% ELSE %]
   [% title = "Most Frequently Reported $terms.Bugs" %]
@@ -44,7 +43,7 @@
 
 [% PROCESS global/header.html.tmpl
    title = title
-   style = ".resolved { background-color: #d9d9d9; color: #000000; }"
+   style_urls = ['skins/standard/duplicates.css']
 %]
 
 <p>
@@ -57,27 +56,26 @@
 
 [%# *** Parameters *** %]
 
-[% bug_ids_string = vis_bug_ids.join(',') %]
+[% bug_ids_string = bug_ids.join(',') %]
 
 <h3><a name="params">Change Parameters</a></h3>
 
 <form method="get" action="duplicates.cgi">
   <input type="hidden" name="sortby" value="[% sortby FILTER html %]">
-  <input type="hidden" name="reverse" value="[% reverse %]">
-  <input type="hidden" name="bug_id" value="[% bug_ids_string %]">
+  <input type="hidden" name="reverse" value="[% reverse FILTER html %]">
+  <input type="hidden" name="bug_id" value="[% bug_ids_string FILTER html %]">
   <table>
     <tr>
-      <td>When sorting or restricting,
-          work with:</td>
+      <td>When sorting or restricting, work with:</td>
       <td>
         <input type="radio" name="sortvisible" id="entirelist" value="0"
-          [%+ "checked" IF NOT sortvisible %]>
+          [% ' checked="checked"' IF NOT sortvisible %]>
         <label for="entirelist">
           entire list
         </label>
         <br>
         <input type="radio" name="sortvisible" id="visiblelist" value="1"
-          [%+ "checked" IF sortvisible %]>
+          [% ' checked="checked"' IF sortvisible %]>
         <label for="visiblelist">
           currently visible list
         </label>
@@ -85,9 +83,9 @@
       <td rowspan="4" valign="top">Restrict to products:</td>
       <td rowspan="4" valign="top">
         <select name="product" size="5" multiple="multiple">
-          [% FOREACH p = products %]
+          [% FOREACH p = user.get_selectable_products %]
             <option name="[% p.name FILTER html %]"
-            [% " selected" IF lsearch(query_products, p.name) != -1 %]
+            [% ' selected="selected"' IF product.contains(p.name) %]
             >[% p.name FILTER html %]</option>
           [% END %]
         </select>
@@ -95,16 +93,20 @@
      </tr>
 
     <tr>
-      <td>Max rows:</td>
+      <td><label for="maxrows">Max rows:</label></td>
       <td>
-        <input size="4" name="maxrows" value="[% maxrows %]">
+        <input size="4" name="maxrows" id="maxrows" 
+               value="[% maxrows FILTER html %]">
       </td>
     </tr>
 
     <tr>
-      <td>Change column is change in the last:</td>
       <td>
-        <input size="4" name="changedsince" value="[% changedsince %]"> days
+        <label for="changedsince">Change column is change in the last:</label>
+      </td>
+      <td>
+        <input size="4" name="changedsince" id="changedsince"
+               value="[% changedsince FILTER html %]"> days
       </td>
     </tr>
 
@@ -116,7 +118,7 @@
       </td>
       <td>
         <input type="checkbox" name="openonly" id="openonly" value="1"
-          [%+ "checked" IF openonly %]>
+          [% ' checked="checked"' IF openonly %]>
       </td>
     </tr>
 
@@ -126,10 +128,9 @@
 </form>
 
 <form method="post" action="buglist.cgi">
-  <input type="hidden" name="bug_id" value="[% bug_ids_string %]">
-  <input type="hidden" name="order" value="Reuse same sort as last time">
+  <input type="hidden" name="bug_id" value="[% bug_ids_string FILTER html %]">
   Or just give this to me as a <input type="submit" id="list" 
-                                value="[% terms.bug %] list">.
+                                      value="[% terms.bug %] list">.
   (Note: the order may not be the same.)
 </form>
 
@@ -139,15 +140,15 @@
   <a name="explanation">What are "Most Frequently Reported [% terms.Bugs %]"?</a>
 </b>
 
-<blockquote>
-  The Most Frequent [% terms.Bugs %] page lists the known open [% terms.bugs %] which
-  are reported most frequently. It is
-  automatically generated from the [% terms.Bugzilla %] database every 24 hours, by
+<p>
+  The Most Frequent [% terms.Bugs %] page lists the known open 
+  [%+ terms.bugs %] which are reported most frequently,
   counting the number of direct and indirect duplicates of [% terms.bugs %].
   This information is provided in order to assist in minimizing
-  the amount of duplicate [% terms.bugs %] entered into [% terms.Bugzilla %], which
-  saves time for Quality Assurance engineers who have to triage the [% terms.bugs %].
-</blockquote>
+  the amount of duplicate [% terms.bugs %] entered into [% terms.Bugzilla %],
+  which saves time for Quality Assurance engineers who have to triage
+  the [% terms.bugs %].
+</p>
 
 <b>How do I use this list?</b>
 
@@ -166,11 +167,12 @@
 
   <ul>
     <li><a href="query.cgi">Try and locate a similar [% terms.bug %]</a>
-        that has already been filed.</li>
+      that has already been filed.</li>
     <li>If you find your [% terms.bug %] in [% terms.Bugzilla %],
-        feel free to comment with any new or additional data you may have.</li>
-    <li>If you cannot find your problem already documented in [% terms.Bugzilla %],
-        <a href="enter_bug.cgi">file a new [% terms.bug %]</a>.</li>
+      feel free to comment with any new or additional data you may have.</li>
+    <li>If you cannot find your problem already documented in 
+      [%+ terms.Bugzilla %],
+      <a href="enter_bug.cgi">file a new [% terms.bug %]</a>.</li>
   </ul>
 </ul>