Bug 163114 - Templatise all calls to DisplayError. First patch - attachment.cgi. Patch by gerv; r=burnus.

3 files changed, 118 insertions, 66 deletions

diff --git a/attachment.cgi b/attachment.cgi
index 45538ae23..fcbe86a78 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -114,7 +114,7 @@ elsif ($action eq "update")
 }
 else 
 { 
-  DisplayError("I could not figure out what you wanted to do.")
+  ThrowCodeError("unknown_action");
 }
 
 exit;
@@ -128,15 +128,15 @@ sub validateID
     # Validate the value of the "id" form field, which must contain an
     # integer that is the ID of an existing attachment.
 
-    detaint_natural($::FORM{'id'})
-      || DisplayError("You did not enter a valid attachment number.") 
-      && exit;
+    $vars->{'attach_id'} = $::FORM{'id'};
+    
+    detaint_natural($::FORM{'id'}) 
+     || ThrowUserError("invalid_attach_id");
   
     # Make sure the attachment exists in the database.
     SendSQL("SELECT bug_id, isprivate FROM attachments WHERE attach_id = $::FORM{'id'}");
     MoreSQLData()
-      || DisplayError("Attachment #$::FORM{'id'} does not exist.") 
-      && exit;
+      || ThrowUserError("invalid_attach_id");
 
     # Make sure the user is authorized to access this attachment's bug.
     my ($bugid, $isprivate) = FetchSQLData();
@@ -164,15 +164,13 @@ sub validateCanEdit
             "attach_id = $attach_id AND submitter_id = $::userid");
 
     FetchSQLData()
-      || DisplayError("You are not authorised to edit attachment #$attach_id")
-      && exit;
+      || ThrowUserError("illegal_attachment_edit");
 }
 
 sub validateDescription
 {
   $::FORM{'description'}
-    || DisplayError("You must enter a description for the attachment.")
-      && exit;
+    || ThrowUserError("missing_attachment_description");
 }
 
 sub validateIsPatch
@@ -190,10 +188,7 @@ sub validateContentType
 {
   if (!$::FORM{'contenttypemethod'})
   {
-    DisplayError("You must choose a method for determining the content type,
-      either <em>auto-detect</em>, <em>select from list</em>, or <em>enter 
-      manually</em>.");
-    exit;
+    ThrowUserError("missing_content_type_method");
   }
   elsif ($::FORM{'contenttypemethod'} eq 'autodetect')
   {
@@ -201,10 +196,7 @@ sub validateContentType
     # specified in the HTTP request headers.
     if ( !$::FILE{'data'}->{'contenttype'} )
     {
-      DisplayError("You asked Bugzilla to auto-detect the content type, but
-        your browser did not specify a content type when uploading the file, 
-        so you must enter a content type manually.");
-      exit;
+      ThrowUserError("missing_content_type");
     }
     $::FORM{'contenttype'} = $::FILE{'data'}->{'contenttype'};
   }
@@ -220,22 +212,14 @@ sub validateContentType
   }
   else
   {
-    my $htmlcontenttypemethod = html_quote($::FORM{'contenttypemethod'});
-    DisplayError("Your form submission got corrupted somehow.  The <em>content
-      method</em> field, which specifies how the content type gets determined,
-      should have been either <em>autodetect</em>, <em>list</em>, 
-      or <em>manual</em>, but was instead <em>$htmlcontenttypemethod</em>.");
-    exit;
+    $vars->{'contenttypemethod'} = $::FORM{'contenttypemethod'};
+    ThrowCodeError("illegal_content_type_method");
   }
 
   if ( $::FORM{'contenttype'} !~ /^(application|audio|image|message|model|multipart|text|video)\/.+$/ )
   {
-    my $htmlcontenttype = html_quote($::FORM{'contenttype'});
-    DisplayError("The content type <em>$htmlcontenttype</em> is invalid.
-      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>.");
-    exit;
+    $vars->{'contenttype'} = $::FORM{'contenttype'};
+    ThrowUserError("invalid_content_type");
   }
 }
 
@@ -271,9 +255,8 @@ sub validateStatuses
   foreach my $status (@{$::MFORM{'status'}})
   {
     grep($_ == $status, @statusdefs)
-      || DisplayError("One of the statuses you entered is not a valid status
-                       for this attachment.")
-        && exit;
+      || ThrowUserError("invalid_attach_status");
+      
     # We have tested that the status is valid, so it can be detainted
     detaint_natural($status);
   }
@@ -282,8 +265,7 @@ sub validateStatuses
 sub validateData
 {
   $::FORM{'data'}
-    || DisplayError("The file you are trying to attach is empty!")
-      && exit;
+    || ThrowUserError("zero_length_file");
 
   my $len = length($::FORM{'data'});
 
@@ -294,27 +276,18 @@ sub validateData
   # the "maxattachmentsize" parameter.
   if ( $::FORM{'ispatch'} && $maxpatchsize && $len > $maxpatchsize*1024 )
   {
-    my $lenkb = sprintf("%.0f", $len/1024);
-    DisplayError("The file you are trying to attach is ${lenkb} kilobytes (KB) in size.  
-                  Patches cannot be more than ${maxpatchsize}KB in size.
-                  Try breaking your patch into several pieces.");
-    exit;
+    $vars->{'filesize'} = sprintf("%.0f", $len/1024);
+    ThrowUserError("patch_too_large");
   } elsif ( !$::FORM{'ispatch'} && $maxattachmentsize && $len > $maxattachmentsize*1024 ) {
-    my $lenkb = sprintf("%.0f", $len/1024);
-    DisplayError("The file you are trying to attach is ${lenkb} kilobytes (KB) in size.  
-                  Non-patch attachments cannot be more than ${maxattachmentsize}KB.
-                  If your attachment is an image, try converting it to a compressable
-                  format like JPG or PNG, or put it elsewhere on the web and
-                  link to it from the bug's URL field or in a comment on the bug.");
-    exit;
+    $vars->{'filesize'} = sprintf("%.0f", $len/1024);
+    ThrowUserError("file_too_large");
   }
 }
 
 sub validateFilename
 {
   defined $::FILE{'data'}
-    || DisplayError("You did not specify a file to attach.")
-      && exit;
+    || ThrowUserError("file_not_specified");
 }
 
 sub validateObsolete
@@ -322,35 +295,32 @@ sub validateObsolete
   # Make sure the attachment id is valid and the user has permissions to view
   # the bug to which it is attached.
   foreach my $attachid (@{$::MFORM{'obsolete'}}) {
+    $vars->{'attach_id'} = $attachid;
+    
     detaint_natural($attachid)
-      || DisplayError("The attachment number of one of the attachments 
-           you wanted to obsolete is invalid.") 
-        && exit;
+      || ThrowCodeError("invalid_attach_id_to_obsolete");
   
     SendSQL("SELECT bug_id, isobsolete, description 
              FROM attachments WHERE attach_id = $attachid");
 
     # Make sure the attachment exists in the database.
     MoreSQLData()
-      || DisplayError("Attachment #$attachid does not exist.") 
-        && exit;
+      || ThrowUserError("invalid_attach_id");
 
     my ($bugid, $isobsolete, $description) = FetchSQLData();
 
+    $vars->{'description'} = $description;
+    
     if ($bugid != $::FORM{'bugid'})
     {
-      $description = html_quote($description);
-      DisplayError("Attachment #$attachid ($description) is attached 
-        to bug #$bugid, but you tried to flag it as obsolete while
-        creating a new attachment to bug #$::FORM{'bugid'}.");
-      exit;
+      $vars->{'my_bug_id'} = $::FORM{'bugid'};
+      $vars->{'attach_bug_id'} = $bugid;
+      ThrowCodeError("mismatched_bug_ids_on_obsolete");
     }
 
     if ( $isobsolete )
     {
-      $description = html_quote($description);
-      DisplayError("Attachment #$attachid ($description) is already obsolete.");
-      exit;
+      ThrowCodeError("attachment_already_obsolete");
     }
 
     # Check that the user can modify this attachment
@@ -632,10 +602,13 @@ sub update
 
   # Get the bug ID for the bug to which this attachment is attached.
   SendSQL("SELECT bug_id FROM attachments WHERE attach_id = $::FORM{'id'}");
-  my $bugid = FetchSQLData() 
-    || DisplayError("Cannot figure out bug number.")
-    && exit;
-
+  my $bugid = FetchSQLData();
+  unless ($bugid) 
+  {
+    $vars->{'bug_id'} = $bugid;
+    ThrowUserError("invalid_bug_id");
+  }
+  
   # Lock database tables in preparation for updating the attachment.
   SendSQL("LOCK TABLES attachments WRITE , attachstatuses WRITE , 
            attachstatusdefs READ , fielddefs READ , bugs_activity WRITE");
diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl
index a74d014cd..55fac2aca 100644
--- a/template/en/default/global/code-error.html.tmpl
+++ b/template/en/default/global/code-error.html.tmpl
@@ -40,6 +40,10 @@
     to any [% parameters %] which you may have set before calling
     ThrowCodeError.
 
+  [% ELSIF error == "attachment_already_obsolete" %]
+    Attachment #[% attachid FILTER html %] ([% description FILTER html %]) 
+    is already obsolete.
+    
   [% ELSIF error == "field_type_mismatch" %]
     Cannot seem to handle <code>[% field %]</code>
     and <code>[% type %]</code> together.
@@ -47,6 +51,13 @@
   [% ELSIF error == "group_bit_invalid" %]
     One of the group bits submitted was invalid.
 
+  [% ELSIF error == "illegal_content_type_method" %]
+    Your form submission got corrupted somehow.  The <em>content
+    method</em> field, which specifies how the content type gets determined,
+    should have been either <em>autodetect</em>, <em>list</em>, 
+    or <em>manual</em>, but was instead 
+    <em>[% contenttypemethod FILTER html %]</em>.
+        
   [% ELSIF error == "illegal_field" %]
     A legal [% field FILTER html %] was not set.
     
@@ -54,6 +65,16 @@
     Attempted to add bug to an inactive group, identified by the bit 
     '[% bit FILTER html %]'.
     
+  [% ELSIF error == "invalid_attach_id_to_obsolete" %]
+     The attachment number of one of the attachments you wanted to obsolete,
+     [% attach_id FILTER html %], is invalid.
+          
+  [% ELSIF error == "mismatched_bug_ids_on_obsolete" %]
+    Attachment [% attach_id FILTER html %] ([% description FILTER html %]) 
+    is attached to bug [% attach_bug_id FILTER html %], but you tried to 
+    flag it as obsolete while creating a new attachment to bug 
+    [% my_bug_id FILTER html %].
+        
   [% ELSIF error == "no_bug_data" %]
     No data when fetching bug [% bug_id %].
     
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index 6c0b37021..c6f970df3 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -116,11 +116,28 @@
     really make sense to mark a bug as a duplicate of itself, 
     does it?  
     
+  [% ELSIF error == "file_not_specified" %]
+    [% title = "No File Specified" %]
+    You did not specify a file to attach.
+    
+  [% ELSIF error == "file_too_large" %]
+    [% title = "File Too Large" %]
+    The file you are trying to attach is [% filesize %] kilobytes (KB) in size.
+    Non-patch attachments cannot be more than [% Param('maxattachmentsize') %]
+    KB.
+    If your attachment is an image, try converting it to a compressable
+    format like JPG or PNG, or put it elsewhere on the web and
+    link to it from the bug's URL field or in a comment on the bug.
+      
   [% ELSIF error == "illegal_at_least_x_votes" %]
     [% title = "Your Query Makes No Sense" %]
     The <em>At least ___ votes</em> field must be a simple number. 
     You entered <tt>[% value FILTER html %]</tt>, which isn't.
     
+  [% ELSIF error == "illegal_attachment_edit" %]
+    [% title = "Unauthorised Action" %]
+     You are not authorised to edit attachment [% attach_id %].
+         
   [% ELSIF error == "illegal_attachment_is_patch" %]
     [% title = "Your Query Makes No Sense" %]
      The only legal values for the <em>Attachment is patch</em> field are
@@ -155,6 +172,21 @@
      The only legal values for the <em>Attachment is obsolete</em> field are
      0 and 1.
          
+  [% ELSIF error == "invalid_attach_id" %]
+    [% title = "Invalid Attachment ID" %]
+    The attachment id [% attach_id FILTER html %] is invalid.
+
+  [% ELSIF error == "invalid_attach_status" %]
+    [% title = "Invalid Attachment Status" %]
+    One of the statuses you entered is not a valid status for this attachment.
+    
+  [% ELSIF error == "invalid_content_type" %]
+    [% title = "Invalid Content-Type" %]
+    The content type <em>[% contenttype FILTER html %]</em> is invalid.
+    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_bug_id" %]
     [% title = "Invalid Bug ID" %]
     The bug id [% bug_id FILTER html %] is invalid.
@@ -175,6 +207,22 @@
     if you are going to accept it.  Part of accepting 
     a bug is giving an estimate of when it will be fixed.
 
+  [% ELSIF error == "missing_attachment_description" %]
+    [% title = "Missing Attachment Description" %]
+    You must enter a description for the attachment.
+    
+  [% ELSIF error == "missing_content_type" %]
+    [% title = "Missing Content-Type" %]
+     You asked Bugzilla to auto-detect the content type, but
+     your browser did not specify a content type when uploading the file, 
+     so you must enter a content type manually.
+                
+  [% ELSIF error == "missing_content_type_method" %]
+    [% title = "Missing Content-Type Determination Method" %]
+    You must choose a method for determining the content type,
+    either <em>auto-detect</em>, <em>select from list</em>, or <em>enter 
+    manually</em>.
+        
   [% ELSIF error == "missing_email_type" %]
     [% title = "Your Query Makes No Sense" %]
     You must specify one or more fields in which to search for
@@ -221,6 +269,12 @@
     [% title = "No Page Specified" %]
     You did not specify the id of a page to display.
 
+  [% ELSIF error == "patch_too_large" %]
+    [% title = "File Too Large" %]
+    The file you are trying to attach is [% filesize %] kilobytes (KB) in size.
+    Patches cannot be more than [% Param('maxpatchsize') %] KB in size.
+    Try breaking your patch into several pieces.  
+
   [% ELSIF error == "reassign_to_empty" %]
     [% title = "Illegal Reassignment" %]
     You cannot reassign to a bug to nobody. Unless you
@@ -236,6 +290,10 @@
     [% title = "Unknown Tab" %]
     <code>[% current_tab_name FILTER html %]</code> is not a legal tab name.    
     
+  [% ELSIF error == "zero_length_file" %]
+    [% title = "File Is Empty" %]
+    The file you are trying to attach is empty!    
+    
   [% ELSE %]
     [%# Cope with legacy calling convention, where "error" was the string
       # to print.