335389 - flags should use '', '?', '+', '-' as option order

Status: RESOLVED FIXED

(Bugzilla :: Attachments & Requests, defect)

Description

[Axel Hecht [:Pike]]

It seems that newbie users just click on the first available choice when supposed
to request approval or other flags. Thus, '?' should be the first.
Whether '-' or '+' should be last, is likely a question of how often we approve
and deny requests, as the last item is easier to pick than the one in the middle.

Comment 1

[Neil Deakin]

The + and - options shouldn't be there at all, since one can't accept or reject a patch created by themselves.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #1)
> The + and - options shouldn't be there at all, since one can't accept or reject
> a patch created by themselves.

Well, some people post new patches and "carry-over" review flags from previous patches, but you're right that people without editbugs or canconfirm probably shouldn't be able to do that. There are obviously a lot of improvements that could be made to the flag system, but I think it might be best to keep this bug focused on it's summary :).

Comment 3

[Kadir Topal [:atopal]]

I don't think this has anything to do with the flag order (when people really choose to set a flag). For new users it's not obvious at all, what "?" means. They then choose "+" because most even don't know that they are not allowed to set that flag.

Comment 4

[James Graham]

This may be discussed elsewhere but if "+", "-" and "?" read "approved" "rejected" and "requested" respectively, this wouldn't be an issue (though it would still be a nice optimization to get the order right)

Comment 5

[Uri Bernstein]

(In reply to comment #4)
> This may be discussed elsewhere but if "+", "-" and "?" read "approved"
> "rejected" and "requested" respectively, this wouldn't be an issue (though it
> would still be a nice optimization to get the order right)
> 

This is bug 240763, duped to bug 178852.

Comment 6

[Dave Miller [:justdave]]

FYI, flags *can* be restricted so that only certain people can + or - them.  We have the approval flags on the Bugzilla product set up that way, mostly for this same reason (people would set them instead of requesting them).

I do like the idea of changing the order though, it's nice and simple, and might actually work. :)

I'd love for a UE person to figure out a real solution though.

Comment 7

[Mike Beltzner [:beltzner, not reading bugmail]]

A UE person's opinion using the constraints of the existing system:

Re-ordering would help, using full text like "? Requested" (or "? Request from:" in cases where the user must pick a requestee), "+ Approved", "- Denied" in the drop downs seems like an easy way to get the full meaning across and teach users about the quite-useful shorthand.

A UE person's opinion using slightly more than is currently available:

A "real" solution would involve fixing some other things that are hard about flaging, including:

 - understanding what the flag is for (tooltip, onclick/hover DHTML layer?)
 - understanding who can set the flag
 - understanding who can be requested for approval/rejection
 - adding some colour so that a person can quickly scan down the list of flags and see the status
 - indicate which (if any) flags require a user's attention
 - hiding obsolete/irrelevant flags

Comment 8

[Daniel Veditz [:dveditz]]

A bugzilla group l10n-drivers already exists (looks like no-one's on it, just inherited bugzilla admins). Add Axel and maybe a couple others, then restrict the setting of the approval-l10n + and - flags the way blocking and core patch approval flags are restricted to the mozilla-drivers group.

Marcia can do this (and if she doesn't know how she can ask Asa).

That doesn't help the UE any, but will quickly help in practice.

Comment 9

[timeless]

dveditz: if someone wants to change a group they should use mozilla.org:bugzilla keywords&components. let's leave this bug for the Bugzilla product.

Comment 10

[Reed Loden [:reed]]

Attachment: patch - v1

Used "", "?", "+", "-" as the order, as that is the more logical choice.

Comment 11

[Frédéric Buclin]

Comment on attachment 248999 [details] [diff] [review]
patch - v1

r=LpSolit

Comment 12

[Reed Loden [:reed]]

Checking in template/en/default/flag/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/flag/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.25; previous revision: 1.24
done

Comment 13

[Frédéric Buclin]

I backed out a change which has been checked in but which is not part of the patch I reviewed:

-[% NEXT UNLESS type.flags && type.flags.size > 0 && type.is_multiplicable && type.is_active %]
+[% NEXT UNLESS type.is_multiplicable && type.is_active %]

This change is obviously wrong!


Checking in template/en/default/flag/list.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/flag/list.html.tmpl,v  <--  list.html.tmpl
new revision: 1.26; previous revision: 1.25
done

Comment 14

[Reed Loden [:reed]]

(In reply to comment #13)
> I backed out a change which has been checked in but which is not part of the
> patch I reviewed:

Oops, that was part of another patch I had worked on several months ago. My fault! Thanks for catching that. ;)