Bug 424215 - show_bug.cgi should hide unset flags unless needing to edit them

r/a=LpSolit

https://bugzilla.mozilla.org/show_bug.cgi?id=424125

3 files changed, 108 insertions, 116 deletions

diff --git a/template/en/default/bug/edit.html.tmpl b/template/en/default/bug/edit.html.tmpl
index ecfac6f59..03b713913 100644
--- a/template/en/default/bug/edit.html.tmpl
+++ b/template/en/default/bug/edit.html.tmpl
@@ -842,11 +842,19 @@
 [% BLOCK section_flags %]
   [%# *** Flags *** %]
   [% show_bug_flags = 0 %]
+  [% bug_flags_set = 0 %]
+  [% show_more_flags = 0 %]
   [% FOREACH type = bug.flag_types %]
     [% IF (type.flags && type.flags.size > 0) || (user.id && type.is_active) %]
       [% show_bug_flags = 1 %]
-      [% LAST %]
     [% END %]
+    [% IF type.is_active && (!type.flags || type.is_multiplicable) %]
+      [% show_more_flags = 1 %]
+    [% END %]
+    [% IF type.flags && type.flags.size > 0 %]
+      [% bug_flags_set = 1 %]
+    [% END %]
+    [% LAST IF show_bug_flags && show_more_flags && bug_flags_set %]
   [% END %]
   [% IF show_bug_flags %]
     <tr>
@@ -862,6 +870,27 @@
                                            flag_types = bug.flag_types
                                            any_flags_requesteeble = bug.any_flags_requesteeble %]
         [% END %]
+        [% IF show_more_flags %]
+          <span id="bz_flags_more_container" class="bz_default_hidden">
+            [% IF !bug_flags_set %]<em>None yet set</em>[% END %]
+            (<a href="#" id="bz_flags_more_action">[% IF !bug_flags_set %]set[% ELSE %]more[% END %] flags</a>)
+          </span>
+          <script type="text/javascript">
+            YAHOO.util.Dom.removeClass('bz_flags_more_container', 'bz_default_hidden');
+            var table = YAHOO.util.Dom.get("flags");
+            var rows = YAHOO.util.Dom.getElementsByClassName('bz_flag_type', 'tbody', table);
+            for (var i = 0; i < rows.length; i++) {
+                YAHOO.util.Dom.addClass(rows[i], 'bz_default_hidden');
+            }
+            YAHOO.util.Event.addListener('bz_flags_more_action', 'click', function (e) {
+                YAHOO.util.Dom.addClass('bz_flags_more_container', 'bz_default_hidden');
+                for (var i = 0; i < rows.length; i++) {
+                    YAHOO.util.Dom.removeClass(rows[i], 'bz_default_hidden');
+                }
+                YAHOO.util.Event.preventDefault(e);
+            }); 
+          </script>
+        [% END %]
       </td>
     </tr>
   [% END %]
diff --git a/template/en/default/filterexceptions.pl b/template/en/default/filterexceptions.pl
index ac63b809b..b4986e1f7 100644
--- a/template/en/default/filterexceptions.pl
+++ b/template/en/default/filterexceptions.pl
@@ -52,7 +52,6 @@
 ],
 
 'flag/list.html.tmpl' => [
-  'flag.id', 
   'flag.status', 
   'type.id', 
 ],
diff --git a/template/en/default/flag/list.html.tmpl b/template/en/default/flag/list.html.tmpl
index 4467e81ce..2f0e1481a 100644
--- a/template/en/default/flag/list.html.tmpl
+++ b/template/en/default/flag/list.html.tmpl
@@ -51,73 +51,13 @@
 
     [%-# Step 1a: Display existing flag(s). %]
     [% FOREACH flag = type.flags %]
-      <tr>
-        <td>
-          <span title="[% flag.setter.identity FILTER html %]">[% flag.setter.nick FILTER html %]</span>:
-        </td>
-        <td>
-          <label title="[% type.description FILTER html %]"
-                 for="flag-[% flag.id %]">
-            [%- type.name FILTER html FILTER no_break -%]</label>
-        </td>
-        <td>
-          <select id="flag-[% flag.id %]" name="flag-[% flag.id %]" 
-                  title="[% type.description FILTER html %]"
-                  onchange="toggleRequesteeField(this);"
-                  class="flag_select flag_type-[% type.id %]">
-            [%# Only display statuses the user is allowed to set. %]
-            [% IF user.can_request_flag(type) || flag.setter_id == user.id %]
-              <option value="X"></option>
-            [% END %]
-            [% IF type.is_active %]
-              [% IF (type.is_requestable && user.can_request_flag(type)) || flag.status == "?" %]
-                <option value="?" [% "selected" IF flag.status == "?" %]>?</option>
-              [% END %]
-              [% IF user.can_set_flag(type) || flag.status == "+" %]
-                <option value="+" [% "selected" IF flag.status == "+" %]>+</option>
-              [% END %]
-              [% IF user.can_set_flag(type) || flag.status == "-" %]
-                <option value="-" [% "selected" IF flag.status == "-" %]>-</option>
-              [% END %]
-            [% ELSE %]
-              <option value="[% flag.status %]" selected="selected">[% flag.status %]</option>
-            [% END %]
-          </select>
-        </td>
-        [% IF any_flags_requesteeble %]
-          <td>
-            [% IF (type.is_active && type.is_requestable && type.is_requesteeble) || flag.requestee %]
-              <span style="white-space: nowrap;">
-                [% SET flag_custom_list = [] %]
-                [% IF Param('usemenuforusers') %]
-                  [% flag_custom_list = flag.type.grant_list %]
-                  [% IF !(type.is_active && type.is_requestable && type.is_requesteeble) %]
-                    [%# We are here only because there was already a requestee. In this case,
-                        the only valid action is to remove the requestee or leave it alone;
-                        nothing else. %]
-                    [% flag_custom_list = [flag.requestee] %]
-                  [% END %]
-                [% END %]
-                [% INCLUDE global/userselect.html.tmpl
-                           name     => "requestee-$flag.id"
-                           id       => "requestee-$flag.id"
-                           value    => flag.requestee.login
-                           multiple => 0
-                           emptyok  => 1
-                           classes => ["requestee"]
-                           custom_userlist => flag_custom_list
-                %]
-              </span>
-            [% END %]
-          </td>
-        [% END %]
-      </tr>
+      [% PROCESS flag_row flag = flag type = type %]
     [% END -%]
+    [% SET flag = "" %]
 
     [%-# Step 1b: Display UI for setting flag. %]
     [% IF (!type.flags || type.flags.size == 0) && type.is_active %]
-
-      [% PROCESS flag_row first_cell_empty = 1 addl_text = "" %]
+      [% PROCESS flag_row type = type %]
     [% END %]
   [% END %]
 
@@ -125,11 +65,12 @@
   [% FOREACH type = flag_types %]
     [% NEXT UNLESS type.flags && type.flags.size > 0 && type.is_multiplicable && type.is_active %]
     [% IF !separator_displayed %]
+      <tbody class="bz_flag_type">
         <tr><td colspan="3"><hr></td></tr>
-        [% separator_displayed = 1 %]
+      </tbody>
+      [% separator_displayed = 1 %]
     [% END %]
-
-    [% PROCESS flag_row first_cell_empty = 0 addl_text = "addl." %]
+    [% PROCESS flag_row type = type addl_text = "addl." %]
   [% END %]
 </table>
 
@@ -159,58 +100,81 @@
   [% END %]
 [% END %]
 
-[%# Display a table row for unset flags %]
+[%# Display a table row for flags %]
 
 [% BLOCK flag_row %]
-  <tr>
-  [% IF first_cell_empty %]
-    <td>&nbsp;</td>
-    <td>
-  [% ELSE %]
-    <td colspan="2">
-  [% END %]
-
-      [% addl_text FILTER html %]
-      <label title="[% type.description FILTER html %]" for="flag_type-[% type.id %]">
-        [%- type.name FILTER html FILTER no_break %]</label>
-    </td>
-    <td>
-      <select id="flag_type-[% type.id %]" name="flag_type-[% type.id %]"
-              title="[% type.description FILTER html %]"
-              [% " disabled=\"disabled\"" UNLESS (type.is_requestable && user.can_request_flag(type)) || user.can_set_flag(type) %]
-              onchange="toggleRequesteeField(this);"
-              class="flag_select flag_type-[% type.id %]">
-        <option value="X"></option>
-        [% IF type.is_requestable && user.can_request_flag(type) %]
-          <option value="?">?</option>
-        [% END %]
-        [% IF user.can_set_flag(type) %]
-          <option value="+">+</option>
-          <option value="-">-</option>
+  [% SET fid = flag ? "flag-$flag.id" : "flag_type-$type.id" %]
+  <tbody[% ' class="bz_flag_type"' IF !flag %]>
+    <tr>
+      <td>
+        [% IF flag %]
+          <span title="[% flag.setter.identity FILTER html %]">[% flag.setter.nick FILTER html %]</span>:
+        [% ELSE %]
+          [% addl_text FILTER html %]
         [% END %]
-      </select>
-    </td>
-    [% IF any_flags_requesteeble %]
+      </td>
       <td>
-        [% IF type.is_requestable && type.is_requesteeble %]
-          <span style="white-space: nowrap;">
-            [% SET grant_list = [] %]
-            [% IF Param('usemenuforusers') %]
-              [% grant_list = type.grant_list %]
-            [% END %]
-            [% INCLUDE  global/userselect.html.tmpl
-                        name     => "requestee_type-$type.id"
-                        id       => "requestee_type-$type.id"
-                        multiple => type.is_multiplicable * 3
-                        emptyok  => !type.is_multiplicable
-                        value    => ""
-                        custom_userlist => grant_list
-                        classes => ["requestee"]
-            %]            
-            
-          </span>
+        <label title="[% type.description FILTER html %]" for="[% fid FILTER html %]">
+          [%- type.name FILTER html FILTER no_break -%]</label>
+      </td>
+      <td>
+        <select id="[% fid FILTER html %]" name="[% fid FILTER html %]"
+                [% IF !flag && !((type.is_requestable && user.can_request_flag(type)) || user.can_set_flag(type)) %]
+                  disabled="disabled"
+                [% END %]
+                title="[% type.description FILTER html %]"
+                onchange="toggleRequesteeField(this);"
+                class="flag_select flag_type-[% type.id %]">
+        [%# Only display statuses the user is allowed to set. %]
+        [% IF !flag || user.can_request_flag(type) || flag.setter_id == user.id %]
+          <option value="X"></option>
+        [% END %]
+        [% IF type.is_active %]
+          [% IF (type.is_requestable && user.can_request_flag(type)) || (flag && flag.status == "?") %]
+            <option value="?" [% "selected" IF flag && flag.status == "?" %]>?</option>
+          [% END %]
+          [% IF user.can_set_flag(type) || (flag && flag.status == "+") %]
+            <option value="+" [% "selected" IF flag && flag.status == "+" %]>+</option>
+          [% END %]
+          [% IF user.can_set_flag(type) || (flag && flag.status == "-") %]
+            <option value="-" [% "selected" IF flag && flag.status == "-" %]>-</option>
+          [% END %]
+        [% ELSE %]
+          <option value="[% flag.status %]" selected="selected">[% flag.status %]</option>
         [% END %]
+        </select>
       </td>
-    [% END %]
-  </tr>
+      [% IF any_flags_requesteeble %]
+        <td>
+          [% IF (type.is_active && type.is_requestable && type.is_requesteeble) || (flag && flag.requestee) %]
+            <span style="white-space: nowrap;">
+              [% SET grant_list = [] %]
+              [% IF Param('usemenuforusers') %]
+                [% grant_list = type.grant_list %]
+                [% IF flag && !(type.is_active && type.is_requestable && type.is_requesteeble) %]
+                  [%# We are here only because there was already a requestee. In this case,
+                      the only valid action is to remove the requestee or leave it alone;
+                      nothing else. %]
+                  [% grant_list = [flag.requestee] %]
+                [% END %]
+              [% END %]
+              [% SET flag_name = flag ? "requestee-$flag.id" : "requestee_type-$type.id" %]
+              [% SET flag_requestee = (flag && flag.requestee) ? flag.requestee.login : '' %]
+              [% SET flag_multiple = flag ? 0 : type.is_multiplicable * 3 %]
+              [% SET flag_empty_ok = flag ? 1 : !type.is_multiplicable %]
+              [% INCLUDE global/userselect.html.tmpl
+                         name     => flag_name
+                         id       => flag_name
+                         value    => flag_requestee
+                         multiple => flag_multiple
+                         emptyok  => flag_empty_ok
+                         classes  => ["requestee"]
+                         custom_userlist => grant_list
+              %]
+            </span>
+          [% END %]
+        </td>
+      [% END %]
+    </tr>
+  </tbody>
 [% END %]