Bug 301764: The file size indicates "0" when editing local attachments - Patch by Frédéric Buclin <LpSolit@gmail.com> r=myk a=justdave

4 files changed, 86 insertions, 79 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index bb40e74c9..76067bbbc 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -236,6 +236,21 @@ sub ispatch {
 
 =over
 
+=item C<isurl>
+
+whether or not the attachment is a URL
+
+=back
+
+=cut
+
+sub isurl {
+    my $self = shift;
+    return $self->{isurl};
+}
+
+=over
+
 =item C<isobsolete>
 
 whether or not the attachment is obsolete
diff --git a/attachment.cgi b/attachment.cgi
index 0e03e45ce..dbd1e82d4 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -25,6 +25,7 @@
 #                 Alexander J. Vincent <ajvincent@juno.com>
 #                 Max Kanat-Alexander <mkanat@bugzilla.org>
 #                 Greg Hendricks <ghendricks@novell.com>
+#                 Frédéric Buclin <LpSolit@gmail.com>
 
 ################################################################################
 # Script Initialization
@@ -37,9 +38,9 @@ use lib qw(.);
 
 # Include the Bugzilla CGI and general utility library.
 require "globals.pl";
-use Bugzilla::Config qw(:locations);
 
-# Use these modules to handle flags.
+use Bugzilla;
+use Bugzilla::Config qw(:locations);
 use Bugzilla::Constants;
 use Bugzilla::Flag; 
 use Bugzilla::FlagType; 
@@ -47,10 +48,12 @@ use Bugzilla::User;
 use Bugzilla::Util;
 use Bugzilla::Bug;
 use Bugzilla::Field;
+use Bugzilla::Attachment;
 
 Bugzilla->login();
 
 my $cgi = Bugzilla->cgi;
+my $dbh = Bugzilla->dbh;
 my $template = Bugzilla->template;
 my $vars = {};
 
@@ -1107,64 +1110,40 @@ sub insert
 # Any user is allowed to access this page, unless the attachment
 # is private and the user does not belong to the insider group.
 # Validations are done later when the user submits changes.
-sub edit
-{
-  # Retrieve and validate parameters
+sub edit {
   my ($attach_id) = validateID();
+  my $dbh = Bugzilla->dbh;
 
-  # Retrieve the attachment from the database.
-  SendSQL("SELECT description, mimetype, filename, bug_id, ispatch, isurl,
-                  isobsolete, isprivate, LENGTH(thedata)
-           FROM attachments
-           INNER JOIN attach_data
-           ON id = attach_id
-           WHERE attach_id = $attach_id");
-  my ($description, $contenttype, $filename, $bugid, $ispatch, $isurl, $isobsolete, $isprivate, $datasize) = FetchSQLData();
-
-  my $isviewable = !$isurl && isViewable($contenttype);
-  my $thedata;
-  if ($isurl) {
-      SendSQL("SELECT thedata FROM attach_data WHERE id = $attach_id");
-      ($thedata) = FetchSQLData();
-  }
+  my $attachment = Bugzilla::Attachment->get($attach_id);
+  my $isviewable = !$attachment->isurl && isViewable($attachment->contenttype);
 
   # Retrieve a list of attachments for this bug as well as a summary of the bug
   # to use in a navigation bar across the top of the screen.
-  SendSQL("SELECT attach_id FROM attachments WHERE bug_id = $bugid ORDER BY attach_id");
-  my @bugattachments;
-  push(@bugattachments, FetchSQLData()) while (MoreSQLData());
-  SendSQL("SELECT short_desc FROM bugs WHERE bug_id = $bugid");
-  my ($bugsummary) = FetchSQLData();
-  
+  my $bugattachments =
+      $dbh->selectcol_arrayref('SELECT attach_id FROM attachments
+                                WHERE bug_id = ? ORDER BY attach_id',
+                                undef, $attachment->bug_id);
+
+  my ($bugsummary, $product_id, $component_id) =
+      $dbh->selectrow_array('SELECT short_desc, product_id, component_id
+                               FROM bugs
+                              WHERE bug_id = ?', undef, $attachment->bug_id);
+
   # Get a list of flag types that can be set for this attachment.
-  SendSQL("SELECT product_id, component_id FROM bugs WHERE bug_id = $bugid");
-  my ($product_id, $component_id) = FetchSQLData();
   my $flag_types = Bugzilla::FlagType::match({ 'target_type'  => 'attachment' ,
                                                'product_id'   => $product_id ,
                                                'component_id' => $component_id });
   foreach my $flag_type (@$flag_types) {
     $flag_type->{'flags'} = Bugzilla::Flag::match({ 'type_id'   => $flag_type->{'id'},
-                                                    'attach_id' => $attach_id,
+                                                    'attach_id' => $attachment->id,
                                                     'is_active' => 1 });
   }
   $vars->{'flag_types'} = $flag_types;
   $vars->{'any_flags_requesteeble'} = grep($_->{'is_requesteeble'}, @$flag_types);
-  
-  # Define the variables and functions that will be passed to the UI template.
-  $vars->{'attachid'} = $attach_id; 
-  $vars->{'description'} = $description; 
-  $vars->{'contenttype'} = $contenttype; 
-  $vars->{'filename'} = $filename;
-  $vars->{'bugid'} = $bugid; 
+  $vars->{'attachment'} = $attachment;
   $vars->{'bugsummary'} = $bugsummary; 
-  $vars->{'ispatch'} = $ispatch; 
-  $vars->{'isurl'} = $isurl; 
-  $vars->{'isobsolete'} = $isobsolete; 
-  $vars->{'isprivate'} = $isprivate; 
-  $vars->{'datasize'} = $datasize;
-  $vars->{'thedata'} = $thedata;
   $vars->{'isviewable'} = $isviewable; 
-  $vars->{'attachments'} = \@bugattachments; 
+  $vars->{'attachments'} = $bugattachments; 
   $vars->{'GetBugLink'} = \&GetBugLink;
 
   # Determine if PatchReader is installed
diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl
index 65dd5092e..747675f01 100644
--- a/template/en/default/attachment/edit.html.tmpl
+++ b/template/en/default/attachment/edit.html.tmpl
@@ -17,14 +17,19 @@
   # Rights Reserved.
   #
   # Contributor(s): Myk Melez <myk@mozilla.org>
+  #                 Frédéric Buclin <LpSolit@gmail.com>
   #%]
 
 [% PROCESS global/variables.none.tmpl %]
 
 [%# Define strings that will serve as the title and header of this page %]
-[% title = BLOCK %]Edit Attachment #[% attachid %] for [% terms.Bug %] #[% bugid %][% END %]
-[% h1 = BLOCK %]Edit Attachment #[% attachid %] for
-  [%+ GetBugLink(bugid, "$terms.Bug $bugid") %][% END %]
+[% title = BLOCK %]
+  Edit Attachment [% attachment.id %] for [% terms.Bug %] [%+ attachment.bug_id %]
+[% END %]
+[% h1 = BLOCK %]
+  Edit Attachment [% attachment.id %] for
+  [%+ GetBugLink(attachment.bug_id, "$terms.Bug ${attachment.bug_id}") %]
+[% END %]
 [% h2 = BLOCK %][% bugsummary FILTER html %][% END %]
 
 [% PROCESS global/header.html.tmpl
@@ -99,7 +104,7 @@
       if (!has_viewed_as_diff) {
         var viewDiffFrame = document.getElementById('viewDiffFrame');
         viewDiffFrame.src =
-            'attachment.cgi?id=[% attachid %]&action=diff&headers=0';
+            'attachment.cgi?id=[% attachment.id %]&action=diff&headers=0';
         has_viewed_as_diff = 1;
       }
     }
@@ -197,7 +202,7 @@
 </script>
 
 <form method="post" action="attachment.cgi" onsubmit="normalizeComments();">
-  <input type="hidden" name="id" value="[% attachid %]">
+  <input type="hidden" name="id" value="[% attachment.id %]">
   <input type="hidden" name="action" value="update">
   <input type="hidden" name="contenttypemethod" value="manual">
 
@@ -207,33 +212,40 @@
       <td width="25%">
         <small>
         <b>Description:</b><br>
-          <textarea rows="3" cols="25" name="description" wrap="soft">[% description FILTER html %]</textarea><br>
-
-        [% IF isurl %]
-            <input type="hidden" name="filename" value="[% filename FILTER html %]"><br>
-            <input type="hidden" name="contenttypeentry" value="[% contenttype FILTER html %]"><br>
+          <textarea rows="3" cols="25" name="description" wrap="soft">
+            [%- attachment.description FILTER html %]</textarea><br>
+
+        [% IF attachment.isurl %]
+            <input type="hidden" name="filename"
+                   value="[% attachment.filename FILTER html %]"><br>
+            <input type="hidden" name="contenttypeentry"
+                   value="[% attachment.contenttype FILTER html %]"><br>
         [% ELSE %]
           <b>Filename:</b><br>
-            <input type="text" size="20" name="filename" value="[% filename FILTER html %]"><br>
-          <b>Size: </b>[% datasize FILTER unitconvert %]<br>
+            <input type="text" size="20" name="filename"
+                   value="[% attachment.filename FILTER html %]"><br>
+          <b>Size: </b>[% attachment.datasize FILTER unitconvert %]<br>
 
           <b>MIME Type:</b><br>
-            <input type="text" size="20" name="contenttypeentry" value="[% contenttype FILTER html %]"><br>
+            <input type="text" size="20" name="contenttypeentry"
+                   value="[% attachment.contenttype FILTER html %]"><br>
 
           <input type="checkbox" id="ispatch" name="ispatch" value="1"
-                 [% 'checked="checked"' IF ispatch %]>
+                 [% 'checked="checked"' IF attachment.ispatch %]>
           <label for="ispatch">patch</label>
         [% END %]
           <input type="checkbox" id="isobsolete" name="isobsolete" value="1"
-                 [% 'checked="checked"' IF isobsolete %]>
+                 [% 'checked="checked"' IF attachment.isobsolete %]>
           <label for="isobsolete">obsolete</label><br>
           [% IF (Param("insidergroup") && UserInGroup(Param("insidergroup"))) %]
-            <input type="checkbox" name="isprivate" value="1"[% " checked" IF isprivate %]> private<br><br>
+            <input type="checkbox" name="isprivate" value="1"
+                   [% " checked" IF attachment.isprivate %]> private<br><br>
           [% ELSE %]<br>
           [% END %]
 
         [% IF flag_types.size > 0 %]
-          [% PROCESS "flag/list.html.tmpl" bug_id=bugid attach_id=attachid %]<br>
+          [% PROCESS "flag/list.html.tmpl" bug_id = attachment.bug_id
+                                           attach_id = attachment.id %]<br>
         [% END %]
 
         <div id="smallCommentFrame">
@@ -243,19 +255,20 @@
 
         <input type="submit" value="Submit"><br><br>
         <strong>Actions:</strong>
-        <a href="attachment.cgi?id=[% attachid %]">View</a>
-        [% IF ispatch && patchviewerinstalled %]
-         | <a href="attachment.cgi?id=[% attachid %]&action=diff">Diff</a>
+        <a href="attachment.cgi?id=[% attachment.id %]">View</a>
+        [% IF attachment.ispatch && patchviewerinstalled %]
+         | <a href="attachment.cgi?id=[% attachment.id %]&action=diff">Diff</a>
         [% END %]
         </small>
       </td>
 
       [% IF isviewable %]
         <td width="75%">
-          <textarea id="editFrame" name="comment" style="height: 400px; width: 100%; display: none;" cols="80" wrap="soft"></textarea>
-          <iframe id="viewFrame" src="attachment.cgi?id=[% attachid %]" style="height: 400px; width: 100%;">
+          <textarea id="editFrame" name="comment" wrap="soft" cols="80"
+                    style="height: 400px; width: 100%; display: none;"></textarea>
+          <iframe id="viewFrame" src="attachment.cgi?id=[% attachment.id %]" style="height: 400px; width: 100%;">
             <b>You cannot view the attachment while editing it because your browser does not support IFRAMEs.
-            <a href="attachment.cgi?id=[% attachid %]">View the attachment on a separate page</a>.</b>
+            <a href="attachment.cgi?id=[% attachment.id %]">View the attachment on a separate page</a>.</b>
           </iframe>
           <script type="text/javascript">
             <!--
@@ -274,15 +287,15 @@
             //-->
           </script>
         </td>
-      [% ELSIF isurl %]
+      [% ELSIF attachment.isurl %]
         <td width="75%">
-          <a href="[% thedata FILTER html %]">
-            [% IF datasize < 120 %]
-              [% thedata FILTER html %]
+          <a href="[% attachment.data FILTER html %]">
+            [% IF attachment.datasize < 120 %]
+              [% attachment.data FILTER html %]
             [% ELSE %]
-              [% thedata FILTER truncate(80) FILTER html %]
+              [% attachment.data FILTER truncate(80) FILTER html %]
               &nbsp;...
-              [% thedata.match(".*(.{20})$").0 FILTER html %]
+              [% attachment.data.match(".*(.{20})$").0 FILTER html %]
             [% END %]
           </a>
         </td>
@@ -290,11 +303,11 @@
         <td id="noview" width="50%">
           <p><b>
             Attachment is not viewable in your browser because its MIME type 
-            ([% contenttype FILTER html %]) is not one that your browser is 
+            ([% attachment.contenttype FILTER html %]) is not one that your browser is 
             able to display.
           </b></p>
           <p><b>
-            <a href="attachment.cgi?id=[% attachid %]">Download the attachment</a>.
+            <a href="attachment.cgi?id=[% attachment.id %]">Download the attachment</a>.
           </b></p>
         </td>
       [% END %]
@@ -305,10 +318,10 @@
 
   Attachments on this [% terms.Bug %]:
   [% FOREACH a = attachments %]
-    [% IF a == attachid %]
-      #[% a %]
+    [% IF a == attachment.id %]
+      [% a %]
     [% ELSE %]
-      <a href="attachment.cgi?id=[% a %]&amp;action=edit">#[% a %]</a>
+      <a href="attachment.cgi?id=[% a %]&amp;action=edit">[% a %]</a>
     [% END %]
     [% " |" UNLESS loop.last() %]
   [% END %]
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index c5334c343..a2cf6c0d3 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -451,8 +451,8 @@
 ],
 
 'attachment/edit.html.tmpl' => [
-  'attachid', 
-  'bugid', 
+  'attachment.id', 
+  'attachment.bug_id', 
   'a', 
 ],