Bug 365267: attachment.cgi should not be editable when the user is not logged in - Patch by Frédéric Buclin <LpSolit@gmail.com> r=pyrzak a=LpSolit

7 files changed, 158 insertions, 72 deletions

diff --git a/skins/standard/create_attachment.css b/skins/standard/create_attachment.css
index 9ed515178..0d8a2b886 100644
--- a/skins/standard/create_attachment.css
+++ b/skins/standard/create_attachment.css
@@ -104,3 +104,42 @@ tbody.file pre:empty {
 .warning {
   color: red
 }
+
+table.attachment_info th {
+    text-align: right;
+    vertical-align: top;
+}
+
+table.attachment_info td {
+    text-align: left;
+    vertical-align: top;
+}
+
+/* Text displayed when the attachment is not viewable by the web browser */
+#noview {
+    text-align: left;
+    vertical-align: middle;
+}
+
+#attachment_attributes {
+    width: 25%;
+}
+
+#attachment_attributes div {
+    padding-bottom: 0.4em;
+}
+
+#attachment_attributes label,
+#attachment_attributes span.label,
+#attachment_actions span.label
+{
+    font-weight: bold;
+}
+
+#attachment_attributes .block {
+    display: block;
+}
+
+#attachment_attributes table#flags {
+    padding-top: 1em;
+}
diff --git a/skins/standard/global.css b/skins/standard/global.css
index 1fe8d41ce..64c73c3d5 100644
--- a/skins/standard/global.css
+++ b/skins/standard/global.css
@@ -401,22 +401,6 @@ dl dl > dt {
     padding-left: 1em;
 }
 
-table.attachment_info th {
-    text-align: right;
-    vertical-align: top;
-}
-
-table.attachment_info td {
-    text-align: left;
-    vertical-align: top;
-}
-
-/* Text displayed when the attachment is not viewable by the web browser */
-#noview {
-    text-align: left;
-    vertical-align: middle;
-}
-
 /* For bug fields */
 .uneditable_textarea {
     width: 30em;
diff --git a/template/en/default/attachment/edit.html.tmpl b/template/en/default/attachment/edit.html.tmpl
index bbdf24866..823131d64 100644
--- a/template/en/default/attachment/edit.html.tmpl
+++ b/template/en/default/attachment/edit.html.tmpl
@@ -36,11 +36,14 @@
   header = header
   subheader = subheader
   doc_section = "attachments.html"
-  javascript_urls = ['js/attachment.js'];
+  javascript_urls = ['js/attachment.js']
+  style_urls = ['skins/standard/create_attachment.css']
 %]
 
 [%# No need to display the Diff button and iframe if the attachment is not a patch. %]
 [% use_patchviewer = (feature_enabled('patch_viewer') && attachment.ispatch) %]
+[% can_edit = attachment.validate_can_edit %]
+[% editable_or_hide = can_edit ? "" : " bz_hidden_option" %]
 
 <form method="post" action="attachment.cgi" onsubmit="normalizeComments();">
   <input type="hidden" name="id" value="[% attachment.id %]">
@@ -54,17 +57,22 @@
   <table class="attachment_info" width="100%">
 
     <tr>
-      <td width="25%">
-        <small>
-        <b><label for="description">Description</label>:</b><br>
+      <td id="attachment_attributes">
+        <div id="attachment_description">
+          <label for="description">Description:</label>
           [% INCLUDE global/textarea.html.tmpl
             id             = 'description'
             name           = 'description'
             minrows        = 3
             cols           = 25
             wrap           = 'soft'
+            classes        = 'block' _ editable_or_hide
             defaultcontent = attachment.description
-          %]<br>
+          %]
+          [% IF !can_edit %]
+            [%+ attachment.description FILTER wrap_comment(25) FILTER html %]
+          [% END %]
+        </div>
 
         [% IF attachment.isurl %]
             <input type="hidden" name="filename"
@@ -72,68 +80,104 @@
             <input type="hidden" name="contenttypeentry"
                    value="[% attachment.contenttype FILTER html %]">
         [% ELSE %]
-          <b><label for="filename">Filename</label>:</b><br>
-            <input type="text" size="20" id="filename" name="filename"
-                   value="[% attachment.filename FILTER html %]"><br>
-          <b>Size:</b>
-          [% IF attachment.datasize %]
-            [%+ attachment.datasize FILTER unitconvert %]
-          [% ELSE %]
-            <em>deleted</em>
-          [% END %]<br>
+          <div id="attachment_filename">
+            <label for="filename">Filename:</label>
+            <input type="text" size="20"  class="block[% editable_or_hide %]"
+                   id="filename" name="filename"
+                   value="[% attachment.filename FILTER html %]">
+            [% IF !can_edit %]
+              [%+ attachment.filename FILTER truncate(25) FILTER html %]
+            [% END %]
+          </div>
 
-          <b><label for="contenttypeentry">MIME Type</label>:</b><br>
-            <input type="text" size="20"
+          <div id="attachment_mimetype">
+            <label for="contenttypeentry">MIME Type:</label>
+            <input type="text" size="20" class="block[% editable_or_hide %]"
                    id="contenttypeentry" name="contenttypeentry"
-                   value="[% attachment.contenttype FILTER html %]"><br>
+                   value="[% attachment.contenttype FILTER html %]">
+            [% IF !can_edit %]
+              [%+ attachment.contenttype FILTER truncate(25) FILTER html %]
+            [% END %]
+          </div>
 
-          <input type="checkbox" id="ispatch" name="ispatch" value="1"
-                 [%+ 'checked="checked"' IF attachment.ispatch %]>
-          <label for="ispatch">patch</label>
+          <div id="attachment_size">
+            <span class="label">Size:</span>
+            [% IF attachment.datasize %]
+              [%+ attachment.datasize FILTER unitconvert %]
+            [% ELSE %]
+              <em>deleted</em>
+            [% END %]
+          </div>
+
+          <div id="attachment_creator">
+            <span class="label">Creator:</span>
+            [%+ INCLUDE global/user.html.tmpl who = attachment.attacher %]
+          </div>
+
+          <div id="attachment_ispatch">
+            <input type="checkbox" id="ispatch" name="ispatch" value="1"
+                   [%+ IF !can_edit %]class="bz_hidden_option"[% END %]
+                   [%+ 'checked="checked"' IF attachment.ispatch %]>
+            [% IF can_edit %]
+              <label for="ispatch">patch</label>
+            [% ELSE %]
+              <span class="label">Is Patch:</span>
+              [%+ attachment.ispatch ? "yes" : "no" %]
+            [% END %]
+          </div>
         [% END %]
+
+        <div id="attachment_isobsolete">
           <input type="checkbox" id="isobsolete" name="isobsolete" value="1"
+                 [%+ IF !can_edit %]class="bz_hidden_option"[% END %]
                  [%+ 'checked="checked"' IF attachment.isobsolete %]>
-          <label for="isobsolete">obsolete</label>
-          [% IF user.is_insider %]
-            <br>
-            <input type="checkbox" id="isprivate" name="isprivate" value="1"
-                   [% " checked" IF attachment.isprivate %]>
-            <label for="isprivate">private (only visible to 
-              <strong>[% Param('insidergroup') FILTER html %]</strong>)
-            </label>
+          [% IF can_edit %]
+            <label for="isobsolete">obsolete</label>
+          [% ELSE %]
+            <span class="label">Is Obsolete:</span>
+            [%+ attachment.isobsolete ? "yes" : "no" %]
           [% END %]
-          <br>
-        </small>
+        </div>
+
+        [% IF user.is_insider %]
+          <div id="attachment_isprivate">
+            <input type="checkbox" id="isprivate" name="isprivate" value="1"
+                   [%+ IF !can_edit %]class="bz_hidden_option"[% END %]
+                   [%+ 'checked="checked"' IF attachment.isprivate %]>
+            [% IF can_edit %]
+              <label for="isprivate">private (only visible to
+                <strong>[% Param('insidergroup') FILTER html %]</strong>)
+              </label>
+            [% ELSE %]
+              <span class="label">Is Private:</span>
+              [%+ attachment.isprivate ? "yes" : "no" %]
+            [% END %]
+          </div>
+        [% END %]
 
         [% IF attachment.flag_types.size > 0 %]
-          [% PROCESS "flag/list.html.tmpl" bug_id = attachment.bug_id
-                                           attach_id = attachment.id
-                                           flag_types = attachment.flag_types
-          %]<br>
+          <div id="attachment_flags">
+            [% PROCESS "flag/list.html.tmpl" bug_id = attachment.bug_id
+                                             attach_id = attachment.id
+                                             flag_types = attachment.flag_types
+            %]
+          </div>
         [% END %]
 
-        <div id="smallCommentFrame">
-          <b><small><label for="comment">Comment</label> (on the
-          [%+ terms.bug %]):</small></b><br>
+        [% IF user.id %]
+          <div id="smallCommentFrame">
+            <label for="comment">Comment (on the [% terms.bug %]):</label>
             [% INCLUDE global/textarea.html.tmpl
               id      = 'comment'
               name    = 'comment'
               minrows = 5
               cols    = 25
               wrap    = 'soft'
-            %]<br>
-        </div>
+              classes = 'block'
+            %]
+          </div>
 
-        <input type="submit" value="Submit" id="update"><br><br>
-        <strong>Actions:</strong>
-        <a href="attachment.cgi?id=[% attachment.id %]">View</a>
-        [% IF use_patchviewer %]
-         | <a href="attachment.cgi?id=[% attachment.id %]&amp;action=diff">Diff</a>
-        [% END %]
-        [% IF Param("allow_attachment_deletion")
-              && user.in_group('admin')
-              && attachment.datasize > 0 %]
-          | <a href="attachment.cgi?id=[% attachment.id %]&amp;action=delete">Delete</a>
+          <input type="submit" value="Submit" id="update"><br><br>
         [% END %]
       </td>
 
@@ -210,11 +254,24 @@
           </b></p>
         </td>
       [% END %]
-
     </tr>
-
   </table>
+</form>
+
+<div id="attachment_actions">
+  <span class="label">Actions:</span>
+  <a href="attachment.cgi?id=[% attachment.id %]">View</a>
+  [% IF use_patchviewer %]
+    | <a href="attachment.cgi?id=[% attachment.id %]&amp;action=diff">Diff</a>
+  [% END %]
+  [% IF Param("allow_attachment_deletion")
+        && user.in_group('admin')
+        && attachment.datasize > 0 %]
+    | <a href="attachment.cgi?id=[% attachment.id %]&amp;action=delete">Delete</a>
+  [% END %]
+</div>
 
+<div id="attachment_list">
   Attachments on [% "$terms.bug ${attachment.bug_id}" FILTER bug_link(attachment.bug_id) FILTER none %]:
   [% FOREACH a = attachments %]
     [% IF a == attachment.id %]
@@ -224,9 +281,6 @@
     [% END %]
     [% " |" UNLESS loop.last() %]
   [% END %]
-
-</form>
-
-<br>
+</div>
 
 [% PROCESS global/footer.html.tmpl %]
diff --git a/template/en/default/attachment/show-multiple.html.tmpl b/template/en/default/attachment/show-multiple.html.tmpl
index 1f8cab88a..bcc297713 100644
--- a/template/en/default/attachment/show-multiple.html.tmpl
+++ b/template/en/default/attachment/show-multiple.html.tmpl
@@ -31,6 +31,7 @@
   title = title
   header = header
   subheader = filtered_summary
+  style_urls = ['skins/standard/create_attachment.css']
 %]
 
 <br>
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index 7e0d99c36..9e04b8ef0 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -380,7 +380,8 @@
 'attachment/edit.html.tmpl' => [
   'attachment.id', 
   'attachment.bug_id', 
-  'a', 
+  'a',
+  'editable_or_hide',
 ],
 
 'attachment/list.html.tmpl' => [
diff --git a/template/en/default/flag/list.html.tmpl b/template/en/default/flag/list.html.tmpl
index 2568e7b4b..5c810480f 100644
--- a/template/en/default/flag/list.html.tmpl
+++ b/template/en/default/flag/list.html.tmpl
@@ -192,8 +192,13 @@
 
 [% ELSE %]
   [%# The user is logged out. Display flags as read-only. %]
+  [% header_displayed = 0 %]
   [% FOREACH type = flag_types %]
     [% FOREACH flag = type.flags %]
+      [% IF !flag_no_header AND !header_displayed %]
+        <p><b>Flags:</b></p>
+        [% header_displayed = 1 %]
+      [% END %]
       [% flag.setter.nick FILTER html %]:
       [%+ type.name FILTER html FILTER no_break %][% flag.status %]
       [% IF flag.requestee %]
diff --git a/template/en/default/global/textarea.html.tmpl b/template/en/default/global/textarea.html.tmpl
index 006158b45..b762f1c4f 100644
--- a/template/en/default/global/textarea.html.tmpl
+++ b/template/en/default/global/textarea.html.tmpl
@@ -19,6 +19,7 @@
   # name:           (optional) The "name"-attribute of the textarea.
   # accesskey:      (optional) The "accesskey"-attribute of the textarea.
   # style:          (optional) The "style"-attribute of the textarea.
+  # classes:        (optional) The "class"-attribute of the textarea.
   # wrap:           (deprecated; optional) The "wrap"-attribute of the textarea.
   # minrows:        (required) Number of rows the textarea shall have initially
   #                 and when not having focus.
@@ -36,6 +37,7 @@
           [% IF id %] id="[% id FILTER html %]"[% END %]
           [% IF accesskey %] accesskey="[% accesskey FILTER html %]"[% END %]
           [% IF style %] style="[% style FILTER html %]"[% END %]
+          [% IF classes %]class="[% classes FILTER html %]"[% END %]
           [% IF wrap %] wrap="[% wrap FILTER html %]"[% END %]
           [% IF defaultrows && user.settings.zoom_textareas.value == 'off' %]
             rows="[% defaultrows FILTER html %]"