Closed Bug 181239 Opened 17 years ago Closed 13 years ago

flag table's header shown even if all flags are disabled


(Bugzilla :: Attachments & Requests, defect)

Not set



Bugzilla 3.0


(Reporter: bbaetz, Assigned: bugzilla-mozilla)



(Keywords: polish)


(1 file, 1 obsolete file)

Hixie pointed this out to me

If all flags are disabled for a bug, then we still show the headers, even if the
flag is not set (if it is set, then we should show the header)

The stuff in needs to search for non-disabled flags, or disabled
flags which are set when setting flag types.

I'm not sure if attachment flags have the same issue, but they probably do
any_flag_requesteeable has the same issue.
*** Bug 313447 has been marked as a duplicate of this bug. ***
*** Bug 331555 has been marked as a duplicate of this bug. ***
QA Contact: mattyt-bugzilla → default-qa
Assignee: myk → bugzilla-mozilla
OS: Linux → All
Hardware: PC → All
Target Milestone: --- → Bugzilla 3.0
Attached patch Patch v1 (obsolete) — Splinter Review
Also fixes the edit.html.tmpl one.
Attachment #243248 - Flags: review?(LpSolit)
Comment on attachment 243248 [details] [diff] [review]
Patch v1

>Index: template/en/default/attachment/list.html.tmpl

>+[%# Only show flags if at least one of the visible attachments has a flag %]

I disagree with this change. If there are flag types or flags available for these attachments, display the "Flags" column anyway. This column is not editable anyway. In other words: no need to edit this template.

>Index: template/en/default/bug/edit.html.tmpl

>+        [% FOREACH type = bug.flag_types %]
>+          [% IF type.flags.size || ( && (!type.flags || type.flags.size == 0) && type.is_active) %]

You should first make sure that type.flags is defined:
  [% IF type.flags && type.flags.size > 0 ...

I remember having filed my apache error log because I was in some cases trying to find the size of an undefined object (such as when editing several bugs at once, as flags are undefined in this case). Moreover, if you enter the test in parens, then you already know there are no flag set for this type, else you would have returned earlier, so the test in the 2nd parens is useless.
Attachment #243248 - Flags: review?(LpSolit) → review-
The fieldset shouldn't be displayed if there is no flag to show -> blocker.
Flags: blocking3.0?
Yeah, agreed. This is an important UI polish, so this should block. Otherwise we look unprofessional.
Flags: blocking3.0? → blocking3.0+
Attached patch Patch v2Splinter Review
Attachment #243248 - Attachment is obsolete: true
Attachment #248186 - Flags: review?(LpSolit)
Comment on attachment 248186 [details] [diff] [review]
Patch v2

Works fine. r=LpSolit
Attachment #248186 - Flags: review?(LpSolit) → review+
Flags: approval?
Flags: approval? → approval+
Checking in edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--  edit.html.tmpl
new revision: 1.92; previous revision: 1.91
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.