Bug 374540: Print out the requirements in an HTML table instead of preformatted text

Patch By Max Kanat-Alexander <mkanat@bugzilla.org> (module owner) a=mkanat

6 files changed, 121 insertions, 21 deletions

diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm
index c090fe1f4..4c62564d7 100644
--- a/Bugzilla/Install/Requirements.pm
+++ b/Bugzilla/Install/Requirements.pm
@@ -25,7 +25,7 @@ package Bugzilla::Install::Requirements;
 
 use strict;
 
-use Bugzilla::Install::Util qw(vers_cmp);
+use Bugzilla::Install::Util qw(vers_cmp install_string is_web);
 use List::Util qw(max);
 use Safe;
 
@@ -262,11 +262,11 @@ sub _get_extension_requirements {
 sub check_requirements {
     my ($output) = @_;
 
-    print "\nChecking perl modules...\n" if $output;
+    print "\n", install_string('checking_modules'), "\n" if $output;
     my $root = ROOT_USER;
     my %missing = _check_missing(REQUIRED_MODULES, $output);
 
-    print "\nChecking available perl DBD modules...\n" if $output;
+    print "\n", install_string('checking_dbd'), "\n" if $output;
     my $have_one_dbd = 0;
     my $db_modules = DB_MODULE;
     foreach my $db (keys %$db_modules) {
@@ -274,7 +274,7 @@ sub check_requirements {
         $have_one_dbd = 1 if have_vers($dbd, $output);
     }
 
-    print "\nThe following Perl modules are optional:\n" if $output;
+    print "\n", install_string('checking_optional'), "\n" if $output;
     my %missing_optional = _check_missing(OPTIONAL_MODULES, $output);
 
     # If we're running on Windows, reset the input line terminator so that
@@ -476,15 +476,10 @@ sub have_vers {
     }
     my $wanted  = $params->{version};
 
-    my ($msg, $vnum, $vstr);
-    no strict 'refs';
-    printf("Checking for %15s %-9s ", $package, !$wanted?'(any)':"(v$wanted)") 
-        if $output;
-
     eval "require $module;";
 
     # VERSION is provided by UNIVERSAL::
-    $vnum = eval { $module->VERSION } || -1;
+    my $vnum = eval { $module->VERSION } || -1;
 
     # CGI's versioning scheme went 2.75, 2.751, 2.752, 2.753, 2.76
     # That breaks the standard version tests, so we need to manually correct
@@ -493,14 +488,15 @@ sub have_vers {
         $vnum = $1 . "." . $2;
     }
 
+    my $vstr;
     if ($vnum eq "-1") { # string compare just in case it's non-numeric
-        $vstr = "not found";
+        $vstr = install_string('module_not_found');
     }
     elsif (vers_cmp($vnum,"0") > -1) {
-        $vstr = "found v$vnum";
+        $vstr = install_string('module_found', { ver => $vnum });
     }
     else {
-        $vstr = "found unknown version";
+        $vstr = install_string('module_unknown_version');
     }
 
     my $vok = (vers_cmp($vnum,$wanted) > -1);
@@ -510,9 +506,29 @@ sub have_vers {
         $vok = 0 if $blacklisted;
     }
 
-    my $ok = $vok ? "ok:" : "";
-    my $black_string = $blacklisted ? "(blacklisted)" : "";
-    print "$ok $vstr $black_string\n" if $output;
+    if ($output) {
+        my $ok           = $vok ? install_string('module_ok') : '';
+        my $black_string = $blacklisted ? install_string('blacklisted') : '';
+        my $want_string  = $wanted ? "v$wanted" : install_string('any');
+
+        # It's impossible to do the printf formatting in the install_string
+        # system, so we do it manually below.
+        if (is_web()) {
+            print install_string('module_details',
+                { package => $package,
+                  wanted  => $want_string,
+                  found   => $vstr,
+                  ok      => $ok,
+                  blacklisted => $black_string,
+                  row_class => $vok ? 'mod_ok' : 'mod_not_ok' });
+        }
+        else {
+            $ok = "$ok:" if $ok;
+            printf "%s %19s %-9s $ok $vstr $black_string\n",
+                install_string('checking_for'), $package, "($want_string)";
+        }
+    }
+    
     return $vok ? 1 : 0;
 }
 
diff --git a/Bugzilla/Install/Util.pm b/Bugzilla/Install/Util.pm
index bb5a84dfd..82edefb3a 100644
--- a/Bugzilla/Install/Util.pm
+++ b/Bugzilla/Install/Util.pm
@@ -37,6 +37,7 @@ our @EXPORT_OK = qw(
     get_version_and_os
     indicate_progress
     install_string
+    is_web
     vers_cmp
 );
 
@@ -80,7 +81,7 @@ sub install_string {
         $string_template = _get_string_from_file($string_id, "$base.html.pl")
             if is_web();
         $string_template = _get_string_from_file($string_id, "$base.txt.pl")
-            if !$string_template;
+            if !defined $string_template;
         last if defined $string_template;
     }
     
diff --git a/setup.cgi b/setup.cgi
index 7b4aa4554..5b3dffd96 100644
--- a/setup.cgi
+++ b/setup.cgi
@@ -53,8 +53,10 @@ print install_string('header', get_version_and_os());
 # Check Requirements #
 ######################
 
-print '<pre>';
 my $module_results = check_requirements(1);
+print '</table>';
+
+print '<pre>';
 Bugzilla::Install::Requirements::print_module_instructions($module_results, 1);
 print '</pre>';
 
diff --git a/skins/standard/setup.css b/skins/standard/setup.css
new file mode 100644
index 000000000..067b34071
--- /dev/null
+++ b/skins/standard/setup.css
@@ -0,0 +1,39 @@
+/* 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 Everything Solved.
+  * Portions created by Everything Solved are Copyright (C) 2007 
+  * Everything Solved. All Rights Reserved.
+  *
+  * Contributor(s): Max Kanat-Alexander <mkanat@bugzilla.org>
+  */
+
+.mod_requirements {
+    border-collapse: collapse;
+    border: none;
+}
+.mod_requirements td, .mod_requirements th {
+    border: 1px solid black;
+    padding: .2em;
+}
+.mod_requirements th.mod_header { border: none; }
+.mod_name {
+    text-align: right;
+}
+
+/* If CSS is working, we don't display the "OK" column. Instead we have
+   colors and we explain them. */
+td.mod_ok, th.mod_ok { display: none; }
+.color_explanation { color: black; background-color: white }
+
+.mod_ok { color: green; }
+.mod_not_ok { color: red; }
\ No newline at end of file
diff --git a/template/en/default/setup/strings.html.pl b/template/en/default/setup/strings.html.pl
index 583a37abd..8e687e508 100644
--- a/template/en/default/setup/strings.html.pl
+++ b/template/en/default/setup/strings.html.pl
@@ -20,6 +20,28 @@
 # setup.cgi).
 
 %strings = (
+    checking_dbd => '<tr><th colspan="4" class="mod_header">Database Modules</th></tr>',
+    checking_optional => '<tr><th colspan="4" class="mod_header">Optional Modules</th></tr>',
+    checking_modules  => <<END_HTML
+<h2>Perl Modules</h2>
+
+<div style="color: white; background-color: white">
+    <p class="color_explanation">Rows that look <span class="mod_ok">like
+    this</span> mean that you have a good version of that module installed.
+    Rows that look <span class="mod_not_ok">like this</span> mean that you
+    either don't have that module installed, or the version you have
+    installed is too old.</p>
+</div>
+    
+<table class="mod_requirements" border="1">
+<tr>
+    <th class="mod_name">Package</th>
+    <th>Version Required</th> <th>Version Found</th>
+    <th class="mod_ok">OK?</th>
+</tr>
+END_HTML
+,
+
     footer => "</div></body></html>",
 
     # This is very simple. It doesn't support the skinning system.
@@ -30,15 +52,25 @@
 <head>
     <title>Installation and Setup for Bugzilla ##bz_ver##</title>
     <link href="skins/standard/global.css" rel="stylesheet" type="text/css" />
+    <link href="skins/standard/setup.css"  rel="stylesheet" type="text/css" />
 </head>
 <body id="bugzilla-installation">
     <h1>Installation and Setup for Bugzilla ##bz_ver##</h1>
     <div id="bugzilla-body">
-    
-        <p><strong>Perl Version</strong>: ##perl_ver##</p>
-        <p><strong>OS</strong>: ##os_name## ##os_ver##</p>
+        <p><strong>Perl Version</strong>: ##perl_ver##
+          <strong>OS</strong>: ##os_name## ##os_ver##</p>
+END_HTML
+,
+    module_details => <<END_HTML
+<tr class="##row_class##">
+    <td class="mod_name">##package##</td>
+    <td class="mod_wanted">##wanted##</td>
+    <td class="mod_found">##found##</td>
+    <td class="mod_ok">##ok##</td>
+</tr>
 END_HTML
 ,
+    module_found => '##ver##',
 );
 
 1;
diff --git a/template/en/default/setup/strings.txt.pl b/template/en/default/setup/strings.txt.pl
index b4768771c..34e447857 100644
--- a/template/en/default/setup/strings.txt.pl
+++ b/template/en/default/setup/strings.txt.pl
@@ -27,8 +27,18 @@
 # Please keep the strings in alphabetical order by their name.
 
 %strings = (
+    any  => 'any',
+    blacklisted => '(blacklisted)',
+    checking_for => 'Checking for',
+    checking_dbd      => 'Checking available perl DBD modules...',
+    checking_optional => 'The following Perl modules are optional:',
+    checking_modules  => 'Checking perl modules...',
     header => "* This is Bugzilla ##bz_ver## on perl ##perl_ver##\n"
             . "* Running on ##os_name## ##os_ver##",
+    module_found => "found v##ver##",
+    module_not_found => "not found",
+    module_ok => 'ok',
+    module_unknown_version => "found unknown version",
 );
 
 1;