Make a universal routine to report errors in forms, so that we can

have a universal (and more noticable) way of displaying them.

5 files changed, 60 insertions, 95 deletions

diff --git a/CGI.pl b/CGI.pl
index 373572986..e70fd3f87 100644
--- a/CGI.pl
+++ b/CGI.pl
@@ -799,6 +799,15 @@ sub PutFooter {
 }
 
 
+sub PuntTryAgain ($) {
+    my ($str) = (@_);
+    print PerformSubsts(Param("errorhtml"),
+                        {errormsg => $str});
+    PutFooter();
+    exit;
+}
+
+
 sub CheckIfVotedConfirmed {
     my ($id, $who) = (@_);
     SendSQL("SELECT bugs.votes, bugs.bug_status, products.votestoconfirm, " .
@@ -878,18 +887,6 @@ sub DumpBugActivity {
 }
 
 
-#
-# Prints a warnbanner incl. image with given message
-# 
-sub warnBanner( $ )
-{
-  my ($msg) = (@_);
-  print Param("warnbannerhtml");
-  print $msg;
-  print Param("warnfooterhtml");
-}
-
-
 sub GetCommandMenu {
     my $loggedin = quietly_check_login();
     my $html = qq{<FORM METHOD=GET ACTION="show_bug.cgi">};
diff --git a/buglist.cgi b/buglist.cgi
index df6d136de..cd41ce682 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -82,10 +82,7 @@ sub SqlifyDate {
     }
     my $date = str2time($str);
     if (!defined $date) {
-        print "\n\n<P>The string '<tt>$str</tt>' is not a legal date.\n";
-        print "<P>Please click the <B>Back</B> button and try again.\n";
-        PutFooter();
-        exit;
+        PuntTryAgain("The string '<tt>$str</tt>' is not a legal date.");
     }
     return time2str("%Y/%m/%d %H:%M:%S", $date);
 }
@@ -117,10 +114,7 @@ sub Error {
     if (!$serverpush) {
         print "Content-type: text/html\n\n";
     }
-    print $str;
-    print "\n<P>Please press <B>Back</B> and try again.\n";
-    PutFooter();
-    exit();
+    PuntTryAgain($str);
 }
     
         
diff --git a/defparams.pl b/defparams.pl
index 5f8d1ccc2..14fecf10c 100644
--- a/defparams.pl
+++ b/defparams.pl
@@ -207,6 +207,13 @@ DefParam("footerhtml",
 %commandmenu%
 </TD></TR></TABLE></TD></TR></TABLE>');
 
+DefParam("errorhtml",
+         "This is what is printed out when a form is improperly filled out.  %errormsg% is replaced by the actual error itself; %<i>anythingelse</i>% gets replaced by the definition of that parameter (as defined on this page).",
+         "l",
+         qq{<TABLE CELLPADDING=20><TR><TD BGCOLOR="#ff0000">
+<FONT SIZE="+2">%errormsg%</FONT></TD></TR></TABLE>
+<P>Please press <B>Back</B> and try again.<P>});
+
 
 
 DefParam("bannerhtml",
@@ -243,17 +250,6 @@ DefParam("shutdownhtml",
          "");
 
 
-DefParam("warnbannerhtml",
-         "HTML to prepend to warning messages.",
-         "l",
-         "<font size=+1>");
-
-DefParam("warnfooterhtml",
-         "HTML to append to warning messages.",
-         "l",
-         "</font>");
-
-
 DefParam("passwordmail",
 q{The email that gets sent to people to tell them their password.  Within
 this text, %mailaddress% gets replaced by the person's email address,
diff --git a/post_bug.cgi b/post_bug.cgi
index df5e78e77..233280f2b 100755
--- a/post_bug.cgi
+++ b/post_bug.cgi
@@ -33,7 +33,16 @@ sub sillyness {
     my $zz;
     $zz = $::buffer;
     $zz = %::COOKIE;
+    $zz = %::components;
+    $zz = %::versions;
+    $zz = @::legal_bug_status;
+    $zz = @::legal_opsys;
+    $zz = @::legal_platform;
+    $zz = @::legal_priority;
+    $zz = @::legal_product;
+    $zz = @::legal_severity;
 }
+
 confirm_login();
 
 print "Set-Cookie: PLATFORM=$::FORM{'product'} ; path=/ ; expires=Sun, 30-Jun-2029 00:00:00 GMT\n";
@@ -59,18 +68,12 @@ umask 0;
 ConnectToDatabase();
 
 if (!defined $::FORM{'component'} || $::FORM{'component'} eq "") {
-    print "You must choose a component that corresponds to this bug.  If\n";
-    print "necessary, just guess.  But please hit the <B>Back</B> button\n";
-    print "and choose a component.\n";
-    PutFooter();
-    exit 0
+    PuntTryAgain("You must choose a component that corresponds to this bug. " .
+                 "If necessary, just guess.");
 }
 
 if (!defined $::FORM{'short_desc'} || trim($::FORM{'short_desc'}) eq "") {
-    print "You must enter a summary for this bug.  Please hit the\n";
-    print "<B>Back</B> button and try again.\n";
-    PutFooter();
-    exit;
+    PuntTryAgain("You must enter a summary for this bug.");
 }
 
 my $forceAssignedOK = 0;
diff --git a/process_bug.cgi b/process_bug.cgi
index 047016b04..ae55fd8da 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -179,17 +179,14 @@ sub CheckCanChangeField {
     SendSQL("UNLOCK TABLES");
     $oldvalue = value_quote($oldvalue);
     $newvalue = value_quote($newvalue);
-    print qq{
-<H1>Sorry, not allowed.</H1>
+    print PuntTryAgain(qq{
 Only the owner or submitter of the bug, or a sufficiently
 empowered user, may make that change to the $f field.
 <TABLE>
 <TR><TH ALIGN="right">Old value:</TH><TD>$oldvalue</TD></TR>
 <TR><TH ALIGN="right">New value:</TH><TD>$newvalue</TD></TR>
 </TABLE>
-
-<P>Click <B>Back</B> and try again.
-};
+});
     PutFooter();
     exit();
 }
@@ -310,12 +307,9 @@ sub CheckonComment( $ ) {
     if( $ret ) {
         if (!defined $::FORM{'comment'} || $::FORM{'comment'} =~ /^\s*$/) {
             # No comment - sorry, action not allowed !
-            warnBanner("You have to specify a <b>comment</b> on this change." .
-                       "<p>" .
-                       "Please press <b>Back</b> and give some words " .
-                       "on the reason of the your change.\n" );
-            PutFooter();
-            exit( 0 );
+            PuntTryAgain("You have to specify a <b>comment</b> on this " .
+                         "change.  Please give some words " .
+                         "on the reason for your change.");
         } else {
             $ret = 0;
         }
@@ -420,10 +414,10 @@ SWITCH: for ($::FORM{'knob'}) {
         if ( Param("strictvaluechecks") ) {
           if ( !defined$::FORM{'assigned_to'} ||
                trim($::FORM{'assigned_to'}) eq "") {
-            print "You cannot reassign to a bug to noone.  Unless you intentionally cleared out the \"Reassign bug to\" field, ";
-            print Param("browserbugmessage");
-            PutFooter();
-            exit 0;
+            PuntTryAgain("You cannot reassign to a bug to nobody.  Unless " .
+                         "you intentionally cleared out the " .
+                         "\"Reassign bug to\" field, " .
+                         Param("browserbugmessage"));
           }
         }
         my $newid = DBNameToIdAndCheck($::FORM{'assigned_to'});
@@ -432,16 +426,12 @@ SWITCH: for ($::FORM{'knob'}) {
     };
     /^reassignbycomponent$/  && CheckonComment( "reassignbycomponent" ) && do {
         if ($::FORM{'product'} eq $::dontchange) {
-            print "You must specify a product to help determine the new\n";
-            print "owner of these bugs.\n";
-            PutFooter();
-            exit 0
+            PuntTryAgain("You must specify a product to help determine the " .
+                         "new owner of these bugs.");
         }
         if ($::FORM{'component'} eq $::dontchange) {
-            print "You must specify a component whose owner should get\n";
-            print "assigned these bugs.\n";
-            PutFooter();
-            exit 0
+            PuntTryAgain("You must specify a component whose owner should " .
+                         "get assigned these bugs.");
         }
         ChangeStatus('NEW');
         SendSQL("select initialowner from components where program=" .
@@ -487,16 +477,13 @@ SWITCH: for ($::FORM{'knob'}) {
         SendSQL("SELECT bug_id FROM bugs WHERE bug_id = " . SqlQuote($num));
         $num = FetchOneColumn();
         if (!$num) {
-            print "You must specify a bug number of which this bug is a\n";
-            print "duplicate.  The bug has not been changed.\n";
-            PutFooter();
-            exit;
+            PuntTryAgain("You must specify a bug number of which this bug " .
+                         "is a duplicate.  The bug has not been changed.")
         }
         if (!defined($::FORM{'id'}) || $num == $::FORM{'id'}) {
-            print "Nice try, $::FORM{'who'}.  But it doesn't really make sense to mark a\n";
-            print "bug as a duplicate of itself, does it?\n";
-            PutFooter();
-            exit;
+            PuntTryAgain("Nice try, $::FORM{'who'}.  But it doesn't really ".
+                         "make sense to mark a bug as a duplicate of " .
+                         "itself, does it?");
         }
         AppendComment($num, $::FORM{'who'}, "*** Bug $::FORM{'id'} has been marked as a duplicate of this bug. ***");
         if ( Param('strictvaluechecks') ) {
@@ -518,10 +505,7 @@ SWITCH: for ($::FORM{'knob'}) {
 
 
 if ($#idlist < 0) {
-    print "You apparently didn't choose any bugs to modify.\n";
-    print "<p>Click <b>Back</b> and try again.\n";
-    PutFooter();
-    exit;
+    PuntTryAgain("You apparently didn't choose any bugs to modify.");
 }
 
 
@@ -535,12 +519,10 @@ if ($::FORM{'keywords'}) {
         }
         my $i = $::keywordsbyname{$keyword};
         if (!$i) {
-            print "Unknown keyword named <code>$keyword</code>.\n";
-            print "<P>The legal keyword names are <A HREF=describekeywords.cgi>";
-            print "listed here</A>.\n";
-            print "<P>Please click the <B>Back</B> button and try again.\n";
-            PutFooter();
-            exit;
+            PuntTryAgain("Unknown keyword named <code>$keyword</code>. " .
+                         "<P>The legal keyword names are " .
+                         "<A HREF=describekeywords.cgi>" .
+                         "listed here</A>.");
         }
         if (!$keywordseen{$i}) {
             push(@keywordlist, $i);
@@ -553,10 +535,8 @@ my $keywordaction = $::FORM{'keywordaction'} || "makeexact";
 
 if ($::comma eq "" && 0 == @keywordlist && $keywordaction ne "makeexact") {
     if (!defined $::FORM{'comment'} || $::FORM{'comment'} =~ /^\s*$/) {
-        print "Um, you apparently did not change anything on the selected\n";
-        print "bugs. <p>Click <b>Back</b> and try again.\n";
-        PutFooter();
-        exit;
+        PuntTryAgain("Um, you apparently did not change anything on the " .
+                     "selected bugs.");
     }
 }
 
@@ -679,10 +659,7 @@ The changes made were:
                         SqlQuote($i));
                 my $comp = FetchOneColumn();
                 if ($comp ne $i) {
-                    print "<H1>$i is not a legal bug number</H1>\n";
-                    print "<p>Click <b>Back</b> and try again.\n";
-                    PutFooter();
-                    exit;
+                    PuntTryAgain("$i is not a legal bug number");
                 }
                 if (!exists $seen{$i}) {
                     push(@{$deps{$target}}, $i);
@@ -696,12 +673,10 @@ The changes made were:
                 while (MoreSQLData()) {
                     my $t = FetchOneColumn();
                     if ($t == $id) {
-                        print "<H1>Dependency loop detected!</H1>\n";
-                        print "The change you are making to dependencies\n";
-                        print "has caused a circular dependency chain.\n";
-                        print "<p>Click <b>Back</b> and try again.\n";
-                        PutFooter();
-                        exit;
+                        PuntTryAgain("Dependency loop detected!<P>" .
+                                     "The change you are making to " .
+                                     "dependencies has caused a circular " .
+                                     "dependency chain.");
                     }
                     if (!exists $seen{$t}) {
                         push @stack, $t;