Bug 208761: Move GetFormat() from globals.pl into Bugzilla::Template - Patch by Frédéric Buclin <LpSolit@gmail.com> r=wicked a=justdave

14 files changed, 108 insertions, 74 deletions

diff --git a/Bugzilla/Template.pm b/Bugzilla/Template.pm
index 711144a6f..52a1bf150 100644
--- a/Bugzilla/Template.pm
+++ b/Bugzilla/Template.pm
@@ -25,12 +25,14 @@
 #                 Tobias Burnus <burnus@net-b.de>
 #                 Myk Melez <myk@mozilla.org>
 #                 Max Kanat-Alexander <mkanat@bugzilla.org>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 
 package Bugzilla::Template;
 
 use strict;
 
+use Bugzilla::Constants;
 use Bugzilla::Config qw(:DEFAULT $templatedir $datadir);
 use Bugzilla::Util;
 use Bugzilla::User;
@@ -132,7 +134,6 @@ sub getTemplateIncludePath {
              @usedlanguages)];
 }
 
-# Write the header for non yet templatized .cgi files.
 sub put_header {
     my $self = shift;
     ($vars->{'title'}, $vars->{'h1'}, $vars->{'h2'}) = (@_);
@@ -142,13 +143,51 @@ sub put_header {
     $vars->{'header_done'} = 1;
 }
 
-# Write the footer for non yet templatized .cgi files.
 sub put_footer {
     my $self = shift;
     $self->process("global/footer.html.tmpl", $vars)
       || ThrowTemplateError($self->error());
 }
 
+sub get_format {
+    my $self = shift;
+    my ($template, $format, $ctype) = @_;
+
+    $ctype ||= 'html';
+    $format ||= '';
+
+    # Security - allow letters and a hyphen only
+    $ctype =~ s/[^a-zA-Z\-]//g;
+    $format =~ s/[^a-zA-Z\-]//g;
+    trick_taint($ctype);
+    trick_taint($format);
+
+    $template .= ($format ? "-$format" : "");
+    $template .= ".$ctype.tmpl";
+
+    # Now check that the template actually exists. We only want to check
+    # if the template exists; any other errors (eg parse errors) will
+    # end up being detected later.
+    eval {
+        $self->context->template($template);
+    };
+    # This parsing may seem fragile, but its OK:
+    # http://lists.template-toolkit.org/pipermail/templates/2003-March/004370.html
+    # Even if it is wrong, any sort of error is going to cause a failure
+    # eventually, so the only issue would be an incorrect error message
+    if ($@ && $@->info =~ /: not found$/) {
+        ThrowUserError('format_not_found', {'format' => $format,
+                                            'ctype'  => $ctype});
+    }
+
+    # Else, just return the info
+    return
+    {
+        'template'    => $template,
+        'extension'   => $ctype,
+        'ctype'       => Bugzilla::Constants::contenttypes->{$ctype}
+    };
+}
 
 ###############################################################################
 # Templatization Code
@@ -449,12 +488,19 @@ __END__
 
 =head1 NAME
 
-Bugzilla::Template - Wrapper arround the Template Toolkit C<Template> object
+Bugzilla::Template - Wrapper around the Template Toolkit C<Template> object
 
 =head1 SYNOPSYS
 
   my $template = Bugzilla::Template->create;
 
+  $template->put_header($title, $h1, $h2);
+  $template->put_footer();
+
+  my $format = $template->get_format("foo/bar",
+                                     scalar($cgi->param('format')),
+                                     scalar($cgi->param('ctype')));
+
 =head1 DESCRIPTION
 
 This is basically a wrapper so that the correct arguments get passed into
@@ -463,6 +509,41 @@ the C<Template> constructor.
 It should not be used directly by scripts or modules - instead, use
 C<Bugzilla-E<gt>instance-E<gt>template> to get an already created module.
 
+=head1 METHODS
+
+=over
+
+=item C<put_header($title, $h1, $h2)>
+
+ Description: Display the header of the page.
+
+ Params:      $title - Page title.
+              $h1    - Main page header.
+              $h2    - Page subheader.
+
+ Returns:     nothing
+
+=item C<put_footer()>
+
+ Description: Display the footer of the page.
+
+ Params:      none
+
+ Returns:     nothing
+
+=item C<get_format($file, $format, $ctype)>
+
+ Description: Construct a format object from URL parameters.
+
+ Params:      $file   - Name of the template to display.
+              $format - When the template exists under several formats
+                        (e.g. table or graph), specify the one to choose.
+              $ctype  - Content type, see Bugzilla::Constants::contenttypes.
+
+ Returns:     A format object.
+
+=back
+
 =head1 SEE ALSO
 
 L<Bugzilla>, L<Template>
diff --git a/buglist.cgi b/buglist.cgi
index 70cc46bae..44854b237 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -115,8 +115,8 @@ if ((defined $cgi->param('ctype')) && ($cgi->param('ctype') eq "js")) {
 # Determine the format in which the user would like to receive the output.
 # Uses the default format if the user did not specify an output format;
 # otherwise validates the user's choice against the list of available formats.
-my $format = GetFormat("list/list", scalar $cgi->param('format'),
-                       scalar $cgi->param('ctype'));
+my $format = $template->get_format("list/list", scalar $cgi->param('format'),
+                                   scalar $cgi->param('ctype'));
 
 # Use server push to display a "Please wait..." message for the user while
 # executing their query if their browser supports it and they are viewing
diff --git a/chart.cgi b/chart.cgi
index 812803199..2b8d39b56 100755
--- a/chart.cgi
+++ b/chart.cgi
@@ -265,7 +265,7 @@ sub plot {
     validateWidthAndHeight();
     $vars->{'chart'} = new Bugzilla::Chart($cgi);
 
-    my $format = &::GetFormat("reports/chart", "", scalar($cgi->param('ctype')));
+    my $format = $template->get_format("reports/chart", "", scalar($cgi->param('ctype')));
 
     # Debugging PNGs is a pain; we need to be able to see the error messages
     if ($cgi->param('debug')) {
diff --git a/config.cgi b/config.cgi
index bbffe20d8..e3ecef3ff 100755
--- a/config.cgi
+++ b/config.cgi
@@ -87,8 +87,8 @@ $vars->{'field'} = [Bugzilla->dbh->bz_get_field_defs()];
 # Determine how the user would like to receive the output; 
 # default is JavaScript.
 my $cgi = Bugzilla->cgi;
-my $format = GetFormat("config", scalar($cgi->param('format')),
-                       scalar($cgi->param('ctype')) || "js");
+my $format = $template->get_format("config", scalar($cgi->param('format')),
+                                   scalar($cgi->param('ctype')) || "js");
 
 # Return HTTP headers.
 print "Content-Type: $format->{'ctype'}\n\n";
diff --git a/docs/xml/customization.xml b/docs/xml/customization.xml
index 49b73319e..37c64cca0 100644
--- a/docs/xml/customization.xml
+++ b/docs/xml/customization.xml
@@ -202,7 +202,7 @@
       
       <para>
         To see if a CGI supports multiple output formats and types, grep the
-        CGI for <quote>GetFormat</quote>. If it's not present, adding
+        CGI for <quote>get_format</quote>. If it's not present, adding
         multiple format/type support isn't too hard - see how it's done in
         other CGIs, e.g. config.cgi.
       </para>
diff --git a/duplicates.cgi b/duplicates.cgi
index 92c697f53..2aa0df263 100755
--- a/duplicates.cgi
+++ b/duplicates.cgi
@@ -269,8 +269,9 @@ my @selectable_products = GetSelectableProducts();
 $vars->{'products'} = \@selectable_products;
 
 
-my $format = GetFormat("reports/duplicates", scalar($cgi->param('format')),
-                       scalar($cgi->param('ctype')));
+my $format = $template->get_format("reports/duplicates",
+                                   scalar($cgi->param('format')),
+                                   scalar($cgi->param('ctype')));
 
 print $cgi->header($format->{'ctype'});
 
diff --git a/enter_bug.cgi b/enter_bug.cgi
index d60ac7bc7..fd009e518 100755
--- a/enter_bug.cgi
+++ b/enter_bug.cgi
@@ -581,9 +581,9 @@ $vars->{'group'} = \@groups;
 
 $vars->{'default'} = \%default;
 
-my $format = 
-  GetFormat("bug/create/create", scalar $cgi->param('format'), 
-            scalar $cgi->param('ctype'));
+my $format = $template->get_format("bug/create/create",
+                                   scalar $cgi->param('format'), 
+                                   scalar $cgi->param('ctype'));
 
 print $cgi->header($format->{'ctype'});
 $template->process($format->{'template'}, $vars)
diff --git a/globals.pl b/globals.pl
index 17174540a..6e9dcbeba 100644
--- a/globals.pl
+++ b/globals.pl
@@ -1082,54 +1082,6 @@ sub OpenStates {
     return ('NEW', 'REOPENED', 'ASSIGNED', 'UNCONFIRMED');
 }
 
-
-###############################################################################
-
-# Constructs a format object from URL parameters. You most commonly call it 
-# like this:
-# my $format = GetFormat("foo/bar", scalar($cgi->param('format')),
-#                        scalar($cgi->param('ctype')));
-
-sub GetFormat {
-    my ($template, $format, $ctype) = @_;
-
-    $ctype ||= "html";
-    $format ||= "";
-
-    # Security - allow letters and a hyphen only
-    $ctype =~ s/[^a-zA-Z\-]//g;
-    $format =~ s/[^a-zA-Z\-]//g;
-    trick_taint($ctype);
-    trick_taint($format);
-
-    $template .= ($format ? "-$format" : "");
-    $template .= ".$ctype.tmpl";
-
-    # Now check that the template actually exists. We only want to check
-    # if the template exists; any other errors (eg parse errors) will
-    # end up being detected later.
-    eval {
-        Bugzilla->template->context->template($template);
-    };
-    # This parsing may seem fragile, but its OK:
-    # http://lists.template-toolkit.org/pipermail/templates/2003-March/004370.html
-    # Even if it is wrong, any sort of error is going to cause a failure
-    # eventually, so the only issue would be an incorrect error message
-    if ($@ && $@->info =~ /: not found$/) {
-        ThrowUserError("format_not_found", { 'format' => $format,
-                                             'ctype' => $ctype,
-                                           });
-    }
-
-    # Else, just return the info
-    return
-    {
-        'template'    => $template ,
-        'extension'   => $ctype ,
-        'ctype'       => Bugzilla::Constants::contenttypes->{$ctype} ,
-    };
-}
-
 ############# Live code below here (that is, not subroutine defs) #############
 
 use Bugzilla;
diff --git a/page.cgi b/page.cgi
index 9954c64a8..97f425ba2 100755
--- a/page.cgi
+++ b/page.cgi
@@ -52,7 +52,7 @@ if ($id) {
         ThrowCodeError("bad_page_cgi_id", { "page_id" => $id });
     }
 
-    my $format = GetFormat("pages/$1", undef, $2);
+    my $format = $template->get_format("pages/$1", undef, $2);
     
     $cgi->param('id', $id);
 
diff --git a/post_bug.cgi b/post_bug.cgi
index f0c2de65a..77f076139 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -67,8 +67,8 @@ my $dbh = Bugzilla->dbh;
 # enter_bug template and then referencing them in the comment template.
 my $comment;
 
-my $format = GetFormat("bug/create/comment",
-                       scalar($cgi->param('format')), "txt");
+my $format = $template->get_format("bug/create/comment",
+                                   scalar($cgi->param('format')), "txt");
 
 $template->process($format->{'template'}, $vars, \$comment)
   || ThrowTemplateError($template->error());
diff --git a/query.cgi b/query.cgi
index 51f9832f3..2f73f0602 100755
--- a/query.cgi
+++ b/query.cgi
@@ -454,9 +454,9 @@ if (defined($vars->{'format'}) && IsValidQueryType($vars->{'format'})) {
 # If we submit back to ourselves (for e.g. boolean charts), we need to
 # preserve format information; hence query_format taking priority over
 # format.
-my $format = GetFormat("search/search", 
-                       $vars->{'query_format'} || $vars->{'format'}, 
-                       scalar $cgi->param('ctype'));
+my $format = $template->get_format("search/search", 
+                                   $vars->{'query_format'} || $vars->{'format'}, 
+                                   scalar $cgi->param('ctype'));
 
 print $cgi->header($format->{'ctype'});
 
diff --git a/report.cgi b/report.cgi
index 6effd485e..f9a56588f 100755
--- a/report.cgi
+++ b/report.cgi
@@ -293,7 +293,8 @@ else {
     ThrowUserError("unknown_action", {action => $cgi->param('action')});
 }
 
-my $format = GetFormat("reports/report", $formatparam, scalar($cgi->param('ctype')));
+my $format = $template->get_format("reports/report", $formatparam,
+                                   scalar($cgi->param('ctype')));
 
 # If we get a template or CGI error, it comes out as HTML, which isn't valid
 # PNG data, and the browser just displays a "corrupt PNG" message. So, you can
diff --git a/show_bug.cgi b/show_bug.cgi
index 76bf353d9..ddb6b2313 100755
--- a/show_bug.cgi
+++ b/show_bug.cgi
@@ -54,8 +54,8 @@ if (!$cgi->param('id') && $single) {
     exit;
 }
 
-my $format = GetFormat("bug/show", scalar $cgi->param('format'), 
-                       scalar $cgi->param('ctype'));
+my $format = $template->get_format("bug/show", scalar $cgi->param('format'), 
+                                   scalar $cgi->param('ctype'));
 
 GetVersionTable();
 
diff --git a/summarize_time.cgi b/summarize_time.cgi
index 31aedd9e1..c04a956d7 100755
--- a/summarize_time.cgi
+++ b/summarize_time.cgi
@@ -485,10 +485,9 @@ $vars->{'check_time'} = \&check_time;
 $vars->{'sort_bug_keys'} = \&sort_bug_keys;
 $vars->{'GetBugLink'} = \&GetBugLink;
 
-$ctype = "html" if !$ctype;
-my $format = GetFormat("bug/summarize-time", undef, $ctype);
+my $format = $template->get_format("bug/summarize-time", undef, $ctype);
 
 # Get the proper content-type
-print $cgi->header(-type=> Bugzilla::Constants::contenttypes->{$ctype});
+print $cgi->header(-type=> $format->{'ctype'});
 $template->process("$format->{'template'}", $vars)
   || ThrowTemplateError($template->error());