34488 - Add a checkbox to add self to CC list

Status: RESOLVED FIXED

(Bugzilla :: Creating/Changing Bugs, enhancement, P3)

Description

[Stephen Rasku]

There should be a button to add/remove me from the CC list instead of having to
type in my e-mail address in over and over again.  Red Hat's version of bugzilla
http://bugzilla.redhat.com/bugzilla/ supports this.

Comment 1

[Terry Weissman]

[e-mail] is the new owner of Bugzilla and Bonsai.  (For details,
see my posting in netscape.public.mozilla.webtools,
news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)

Comment 2

[Dave Miller [:justdave]]

Possibly related to bug 42851, which was just marked FIXED.  Not exactly the same 
thing, but does make adding/removing from the CC easier.

Comment 3

[Sebastian Späth]

I like the way the CC list is handled now.
However it would still be easier to have the "CC me", button which would do 
nothing more than writing my address (which is known when I am logged in) in the 
'CC me' textfield. This would avoid typos and be much faster. 

Comment 4

[Chris Yeh]

it's still important to have a text field though if you want to add multiple 
cc's to the same line. having a button (add me) and having an editable text 
field may be confusing and somewhat redundant.

Comment 5

[Sebastian Späth]

Although being redundant, I still think it would be a neat solution. Maybe 
checking the "add me" box, would just add the account e-mail address in the text 
field (via JavaScript). This way a user would get visible feedback.
 
Just a thought, dunno if it's a good one myself

Comment 6

[Dave Miller [:justdave]]

And if they don't have JavaScript active, the cgi should add them anyway if the 

box is checked.  That way it still works without JavaScript, it just looks better 

if they do have it.

Comment 7

[Stephen Rasku]

The way that Red Hat Bugzilla works is that it shows you a button if you are
only allowed to add yourself.  It will display a text field if you have
permissions to add other people.

Comment 8

[Jesse Ruderman]

How about letting the user type "me" in the "Add to cc field" as a special 
value?  I'm not sure how users would find out about this, though, and clicking 
a button is easier.

Comment 9

[Ben Bucksch (:BenB)]

I am tired of typing my email address!

See also bug 28849.

Comment 10

[Ben Bucksch (:BenB)]

Nominating for 2.12.

Comment 11

[Gervase Markham [:gerv]]

Ben - if you are tired of typing your email address, bind it to a hotkey - 
that's what I do. I can recommend software for Windows, and as everyone knows 
Linux is better than Windows, I'm sure there's some for it too.

Tara - is this going into 2.12? Seems like UI clutter to me...

Gerv

Comment 12

[Matthew Tuck [:CodeMachine]]

I think the best solution would to have a checkbox for the user and the text box
for other users, and _never_ show the user in the text field.

The only tricky bit I see with this is how to handle users that aren't logged
in, when I guess we should fall back to just a checkbox.

Comment 13

[Jesse Ruderman]

I like the button idea better.  I often decide whether I want to cc myself on a 
bug after I write a comment, but then I sometimes forget that I haven't already 
typed my address in and end up spamming everyone when I go back and add myself.

Comment 14

[Ben Bucksch (:BenB)]

my vote for the checkbox, but anything is better than today.

Comment 15

[Matthew Tuck [:CodeMachine]]

A button that doesn't itself cause spam would be a totally inconsistent kludge. 
It would be much better to stop cc spam to those who don't want it rather than
inserting kludges into the product.  Or implement a proper BCC.

Comment 16

[Stephen Rasku]

I know I don't want to get a bug mailed to me if the only change is in the CC line.  Does anyone want that?

Comment 17

[Ben Bucksch (:BenB)]

Yes, I do, depending on who ccs him-/herself. Since votes are limited and the
same for everybody, ccing is similar to voting. If I see that people I like cc
themselves, I am more likely to implement it sooner (it is rare that it actually
make a difference, but sometimes, it does). Extreme example: if there are 30
people cced on a bug, this is a strong hint that the bug is important.

Comment 18

[Ben Bucksch (:BenB)]

> ccing is similar to voting

well, sometimes

Comment 19

[Matthew Tuck [:CodeMachine]]

This is not the place for discussing whether we want CC spam.  You should either
take this to bug #17464 or bug #7345.

From the point of view of this bug, we should be consistent and either never
generate it or always generate it.

Comment 20

[Tara Hernandez]

IMHO I like where we're at now.  For Bugzilla 3, I'm hoping to get multiply
selectable popup/drop down/something capabilities for all user related fields,
and realistically this won't be addressed until then.  Removing 2.12 flag.

Comment 21

[GavinS]

Attachment: Patch to add a add-me-to-cc' checkbox

Comment 22

[GavinS]

The patch I just added is against 2.12. I will update it when 2.14 arrives.

This adds a checkbox which says 'Add me to CC list'

With javascript enabled, activating this checkbox adds the login to the newcc
field. (And removes it on un-activation.)

Without javascript, there is a little tweak to process_bug which provides the
same add-me-to-cc list functionality.

I think I still prefer the simple checkbox only approach for a person that can
only add themselves.

Maybe I'll look at that tomorrow...

Comment 23

[Andreas Franke (gone)]

-> Bugzilla product, Changing-Bugs component, reassigning.

Adding patch/review keywords, and thus retargetting to 2.16.

Comment 24

[Christopher Aillon (sabbatical, not receiving bugmail)]

Taking.  Very similar to bug 7710 which is also mine.

Comment 25

[Gervase Markham [:gerv]]

I think this should be done with minimum page bloat and maximum utility by
having the textbox and list, as now, but the list never shows you. Below the
list is a checkbox, thus:

[X] And me
. This is filled correctly when the bug is sent, and checked when it's submitted.

Gerv

Comment 26

[GavinS]

Gerv: I was looking at implementing your suggestion, but I got stuck when the
user wasn't actually logged in, because then you can't check who they are :(

In this case, you could have a little note by the new 'And Me' box which says
'you aren't currently logged in, so I don't know if I should make this checkbox
selected' -- but that is a bit of a mess -- any ideas what should happen in the
not-logged-in case?

Comment 27

[Christopher Aillon (sabbatical, not receiving bugmail)]

Gerv,Gavin:

the way I'm implementing this is similar to what I'm doing for bug 7710.  Yes
there will be a checkbox because it really is best IMO.  And it will involve
both perl and javascript.  One note: if a user has the option to automatically
add themselves to CC (7710), this checkbox should not appear IMO.

Comment 28

[Sebastian Späth]

Just leave the checkbox away if the uesr is not logged in. The less clutter for
unexperienced users the better. You are only likely to use this feature if you
are a regular bugzilla user in which case you probably are logged in via cookies
anyway.

Comment 29

[Christopher Aillon (sabbatical, not receiving bugmail)]

Yes, i forgot to mention that part. already have thought about it and i'll get
it in soon.

Comment 30

[Gervase Markham [:gerv]]

caillon: are you on the case here?

If the user isn't logged in, the interface is exactly as now.
If the user is logged in, they don't appear in the CC list, and there's a
checkbox in the correct state, which they can change to CC or un-CC them.

This solution is simple, effective, and does not require JS, which is good. I
don't think the current patch does it right.

Gerv

Comment 31

[Christopher Aillon (sabbatical, not receiving bugmail)]

Gerv, yes I did have a patch ready but I was stumped as to where to put the
checkbox, which then made me put it aside and I forgot about it.  I discussed
some on IRC and consensus was to put it underneath the comments textarea I'm
typing into right now.

Patch on the way.  There are a few whitespace changes in the patch, but the diff
-u -w is kind of ugly with the way bracketing goes, so I will just attach a
regular diff -u.  The whitespace changes are pretty few (and obvious) so I hope
it's not a problem (though if someone wants a diff -w, let me know...)

I have applied it to http://landfill.tequilarista.org/bz34488/

Comment 32

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Patch v1.0

Comment 33

[Christian Reis]

Let's see if mpt can find us here.

Christoper: thanks a lot for handling this bug, it's a mostwanted for me. So
okay, let me try and hash out the UI ideas.

This checkbox is _very_ important to the end user - one of the most common
mistakes today is forgetting to add self to CC:, and the fact that the box is
really far away from the textarea and status selection are is plain bad
usability --- vertical scroll occurs in most cases, and with < 800 pixel width
browser windows, horizontal scroll occurs. In conclusion, the current way to add
to CC: is very bad.

This checkbox is an important part of fixing that. However, I believe "add self
to cc" isn't the best label to use. So I propose the following:

[ ] Email me when this bug changes

with the following characteristics:

a) The caption is a <LABEL> so we can click on it to toggle the box
b) If we are already owner, QA or CC, the box comes selected
c) If we are owner or QA, the box is also DISABLED since clicking on it would     
   be pointless. If using a UA that does not support DISABLED you can choose 
   either to do nothing or print an error message - I don't care and no error 
   message wouldn't do much harm.
d) If we are on the CC list, clicking on the box toggles status.

Christopher, can you prepare a patch with these changes? I'll do so if you can't
of course :)

Kiko (looking for the [ ] Email me when this bug changes box on _this_ bug) 

Comment 34

[Christopher Aillon (sabbatical, not receiving bugmail)]

I'm still iffy as to the placement of the checkbox.  I can see the benefits of
underneath the textarea, but it seems weird for me to have it so far away from
the other CC controls.

In any case, kiko you know I love <label> controls.  just didn't add it in this
patch because i wanted to keep it simple.
Owner and QA checks I'll add.  However, the control should not be there at all
to select or unselect if the viewer owns or is QA of the bug.
I can add d) also.

I do NOT like your proposed wording though.  You are implying that we are
definitely going to email them when the bug changes which is totally NOT the
case.  We only add them to the CC list.  Their email prefs can prevent them from
recieving email when the bug changes.  Quite frankly, for a user to have a
bugzilla they must have an email account.  And if they have an email account,
they should know what a CC list is (i hope!!).  I see nothing wrong with "Add me
to CC list" or a slight mutation of that.  It is concise and honest.  Unless a
more convincing argument is presented or a better choice offered, I will leave
that alone.

Comment 35

[Christian Reis]

> underneath the textarea, but it seems weird for me to have it so far away from
> the other CC controls.

The other CC controls have very bad usability. Following their example would be
a mistake.

> In any case, kiko you know I love <label> controls.  just didn't add it in 
> this patch because i wanted to keep it simple.

This is a minor change and I think it's very acceptable.

> You are implying that we are definitely going to email them when the bug
> changes which is totally NOT the case.  We only add them to the CC list. 

Christopher, I understand your point. But let me state mine.

You are currently assigned to bug 28849, which proposes to add a checkbox for a
user to ask to receive email notification. This bug is _almost_ a dupe of that
one in the UI sense: both bugs simply refer to an expedited way to ask to
receive emails when the bug changes. In that sense, this bug subject to me is a
misnomer.

The fact that we use CC lists for users that have editbugs is a technicality;
the bottom line is: the user wants to be notified when the bug changes. His task
isn't "being added to the CC list". It's "getting notified when the bug changes".

Furthermore, if bug 28849 is implemented (and it should be, of course) we will
now have two separate controls that can occur on that page. Will they be
positioned in the same place? If so, will they have different semantics? That
would definitely be bad usability in the long run.

My proposal: unify the notification controls into one, simple, 

[ ] Email me if bug changes

Checkbutton. This checkbutton will have two functions:

1) Offer feedback if I am currently receiving email on this bug or not.
2) Allow me to change this state and "subscribe"/"unsubscribe" to this bug.

This will make bug 28849 uniformly fit in.

> Their email prefs can prevent them from recieving email when the bug changes.

There is a simple solution to this: when the user has selected not to receive
email on that bug, print a <SMALL> message substituting the control:

"You have currently chosen not to receive email notification for bugs that change."

And if you like make part of that sentence a phrase to userprefs.cgi

> And if they have an email account, they should know what a CC list is (i >
> hope!!).  I see nothing wrong with "Add me to CC list" or a slight mutation 
> of that.

I beg to differ. My parents are bugzilla users here at Async, since they are
users of one of our Point of Sales application. However, they have no idea of
what CC lists are (nor what all those other wierd controls are up there). I've
been told lately "bugzilla != bmo" and I realize this is quite true.

Kiko

Comment 36

[Dave Miller [:justdave]]

let's go with the "email me when this bug changes" wording.  I'm with Kiko,
there's an increasingly large number of people out there with no clue what a CC
is.  (I used to deal with them all the time when I did tech support).

Comment 37

[Christopher Aillon (sabbatical, not receiving bugmail)]

We should change the wording to what kiko suggested if we query their mail
settings to see if they have CC enabled.  If we check, it it will lead to double
querying whenever process_bug.cgi is called because process_bug calls
processmail which in turn also queries the user's mail settings.  However I
spoke to justdave on IRC about this and he said to query here anyway.

Here's my thinking and justdave seemed to agree with this...

If they have CC mail turned off in prefs, we softly warn them along with a link
to their prefs.  Also, since we are using a checkbox, unchecking it will result
in an "unsucessful" form input and therefore not sent with the form data on
post/get.  So when displaying the checkbox I will need to add an additional
hidden field to make sure that the checkbox was in fact displayed.  (It will not
be displayed if a user views a bug and then changes it.  Without this check, it
might be assumed that the checkbox was unchecked which is an incorrect assumption).

Comment 38

[Jesse Ruderman]

>c) If we are owner or QA, the box is also DISABLED since clicking on it 
>would be pointless. If using a UA that does not support DISABLED you can 
>choose either to do nothing or print an error message - I don't care and no 
>error message wouldn't do much harm.

What if you're giving ownership to someone else, but want to stay CC'ed on the 
bug?  What if you want to add someone else?

Comment 39

[Christian Reis]

> >c) If we are owner or QA, the box is also DISABLED since clicking on it
> >would be pointless. If using a UA that does not support DISABLED you
> >can choose either to do nothing or print an error message - I don't
> >care and no error message wouldn't do much harm.
> 
> What if you're giving ownership to someone else, but want to stay CC'ed
> on the  bug?  What if you want to add someone else?

Okay, this makes things interesting. In that case, I think DISABLED
might be bad, actually. But it also makes us have to handle that case
and add us to the CC list of the bug when we are reassigning. Does this
also apply to QA (and watch owner/QA too?)

It also makes this feature even more likeable, do you agree?

I'd argue for that being a separate bug to ease Christopher's acceptance
of this bug :)

Comment 40

[Gervase Markham [:gerv]]

CCing myk, who is (I believe) templatising the Show Bug page.

Gerv

Comment 41

[Christopher Aillon (sabbatical, not receiving bugmail)]

->2.18

Comment 42

[Jouni Heikniemi]

*** Bug 152683 has been marked as a duplicate of this bug. ***

Comment 43

[Bradley Baetz (:bbaetz)]

*** Bug 156379 has been marked as a duplicate of this bug. ***

Comment 44

[Bradley Baetz (:bbaetz)]

If we don't stop you manually adding yourself to the cclit by typing in your
email (which we don't), then we shouldn't make the checkbox do anything differently.

Comment 45

[Ian Pottinger]

From Comment 34
> You are implying that we are definitely going to email them when the bug
> changes which is totally NOT the case.  We only add them to the CC list. 

Instead of using the CC list metaphor, how about adopting a subscription list
metaphor.  Therefore the "CC List" changes to a "Subscription List" and the
proposed wording for this new feature could be:

[ ] Include me in this bug's subscription list.
    (Note: Your <a>Email Settings</a> govern the effect of subscribing to bugs.)

Just a thought :-)

Comment 46

[Bradley Baetz (:bbaetz)]

OK, I think that people have become sidetracked. This bug is not about
redesigning the cc-list ui - its about adding a checkbox to add someone to the
cclist.

The checkbox is displayed iff the user is not already in the cclist. If you want
to change this to be disabled instead (for a consistent UI) then thats OK. The
'remove' checkbox doesn't show if the cclist is empty, though, so its a question
of what you want to be consistent with. You also have the issue that removing
the check from the box won't remove you (if we choose to show it), combined with
the 'what to show when I'm not logged in' issue, which is why I think not
displaying the checkbox is the best option.

The text of the label is 'Add [% user.login FILTER html %]'. 'Add self' or 'Add
me' didn't seem to flow for me.

The checkbox is shown even if you aren't logged in, because the user will have
to log in before submitting. Adding themselves if they were already added is
harmless, too, since its just the same as typing in the email manually.

Discussion about what to do if the person is already added are irrelevent to
this bug; we should do the exact same thing as if the user typed the name in
themselves.

Comment 47

[Bradley Baetz (:bbaetz)]

Attachment: Try this

The |defined| checks are sort of useless, because textareas will always be
defined. I just added them to be consistent - we really need to clean up
process_bug.

Comment 48

[Jouni Heikniemi]

Comment on attachment 90919 [details] [diff] [review]
Try this

>+            <input type="checkbox" name="addselfcc">
>+            Add [% user.login FILTER html %]
>+          </label>

This part is bad if you're not logged in. A checkbox with the label of "Add"
just doesn't cut it. "Add myself" would be acceptable. :-)

Comment 49

[Christopher Aillon (sabbatical, not receiving bugmail)]

Sigh.  No time to work on this anymore...  ->Default owner.

Comment 50

[Bradley Baetz (:bbaetz)]

-> me

Comment 51

[Bradley Baetz (:bbaetz)]

myself is better than the email address (for long email addresses), but what if
there isn't a user currently logged in?

Comment 52

[Matthew T (active 1999-2002)]

Not logged in                Logged in, not added         Logged in, added

 CC: +---------------+-+      CC: +---------------+-+      CC: +---------------+-+
     |               | |          |               | |          |               | |
     |               | | ==>      |               | | ==>      |               | |
     |               | |          |               | |          |               | |
     |               | |          |               | |          |               | |
     +---------------+-+          +---------------+-+          +---------------+-+
     [/] Remove selected          [/] Remove selected          [/] Remove selected
Add: [                 ]     Add: [                 ]     Add: [[e-mail].u]
                                  [ ] I am CCed                [/] I am CCed

Comment 53

[Boris 'pi' Piwinger]

For comment 51: If you submit the form you have to login, right? If so "myself"
would do it and refers to a uniquely determined user:-))

pi

Comment 54

[Bradley Baetz (:bbaetz)]

I discussed mpt's design withhim, and its not sufficient - hes going to play
withit in the morning.

If you leave the box unchecked, but you're not logged in, _and_ you're on the
list, sould you be removed? I say no, but the UI may be confusing-ish in that case.

Comment 55

[Stephen Rasku]

How about if you don't display the checkbox if you are not logged in.  This
would be equivalent to what Red Hat does.  You would still be able to CC
yourself by the current method.

Comment 56

[Dave Miller [:justdave]]

*** Bug 162179 has been marked as a duplicate of this bug. ***

Comment 57

[Aleksey Nogin]

http://bugzilla.redhat.com/ has this implemented. The checkbox is right next to
the "Additional Comments" text, which aslo helps with bug 7710

Comment 58

[Stephen Rasku]

I agree with comment 57.  I would accept it as a fix for this bug.  I won't
speak to bug 7710 since I am not the reporter for that bug.

Comment 59

[Olivier Massoud]

Attachment: an off-topic attachment

You can even delete, it's of no interest at all

Comment 60

[Robert Pollak]

Comment on attachment 116081 [details]
an off-topic attachment

Please use landfill.bugzilla.org for testing bugzilla.

Comment 61

[Jouni Heikniemi]

Comment on attachment 90919 [details] [diff] [review]
Try this

Per my comment 48 and further comments by mpt & bbaetz, marking r-. Besides,
it's bitrotten.

Comment 62

[Oleg Sidletskiy]

I think the user should have an option to have this "checkbox" checked by
default (prefs).

Comment 63

[alanjstr]

I have to scroll all the way to the end of the bug page to get the email address
that I use for Bugzilla, copy it, then scroll all the way up to paste it in the
CC list.

If someone wants to add themselves to a bug but not submit a comment, then they
might get confused by clicking the Commit button thinking it would submit an
empty comment.  

I'm lazy and I just want to click a button to add myself.

Comment 64

[Michael Lefevre]

Alan - I don't know how often you're changing your sneakemail address, but
there's an easier way than copy and paste. For example, with your current
address, if you just type "quaw" in to the cc field, bugzilla gives you the
option of adding yourself or some guy at squawk.com. If you type in "quawb",
it'll add your address.

Comment 65

[alanjstr]

Why is it that "people without power" are able to add/remove people who are not
themselves?  This just became a problem 
http://bugzilla.mozilla.org/show_activity.cgi?id=171082

Comment 66

[timeless]

alanjstr, that's bug 207895 please don't randomly spam bugs

Comment 67

[Bill McGonigle (not currently reading bugmail; please contact directly)]

over in bug 7710 Christian Reis suggested a UI like:

|                                       |
|                                       |
+---------------------------------------+
[ ] Add me to the CC list.

< > Leave as ASSIGNED
< > Resolve bug, changing resolution to [ FIXED     [v]

which seems really straightforward and minimizes the edge cases that need to be
considered.  To the user it would work exactly the same as typing your e-mail
address into the cc: list box.  So to answer most of the questions on this bug
the answer would be, 'what happens if you add your e-mail address to the cc list?".

Sure, there might be some better terminology than cc: for the whole system but
that's too much to tackle here.  Sure there might be some nifty things that
could be done with javascript.  Sure there might be some fancy things that could
be done to optimize the button's state in any number of conditions.

But I respectfully submit that trying to overengineer the problem has lead to 4
years of people having to type in their e-mail addresses. :)

Comment 68

[Lyle Greg Lisle]

I can't believe this still hasn't been fixed!  It's been FOUR years!

I thought OSS was supposed to be fast response.

Comment 69

[Joel Peshkin]

Now that we have user wildcards, it only takes a few keystrokes to add yourself
to the CC list.  I suggest we WONTFIX this.

Comment 70

[alanjstr]

There's an autocomplete in my browser, too.  So what?  This would make it so
that I wouldn't have to touch the keyboard.

Comment 71

[Christian Reis]

Joel's teasing. I still want to see this implemented, am just a bit hogged to do
it right now.

Comment 72

[Lyle Greg Lisle]

Sorry,

    I'm a greenhorn with Mozilla and OSS, I just started wandering around in
Bugzilla and couldn't find a way to mark bugs so that I could go back to them
easily.  I thought putting myself on the CC list would work, but it's work to
do.  I have also discovered the drawbacks of being on the list.  :-)

   Joel, where do I find the auto-complete function? I think I saw it once, but
. . . ?

   I assume that Bugzilla is smart enough to prevent duplicate entries in the CC
list so that absent minded old men don't get dumped on. 

   Anyway, thanks for your efforts. 

Comment 73

[Lyle Greg Lisle]

One more time,

   A major irritant here in Moz world is the realization that something can be
done, but there is no simple way to find out how.

   I have looked high, low, and under rocks, and can find nothing about "wild
cards" or "auto-complete" that seems to apply here.

Comment 74

[Jesse Ruderman]

"Wild cards" is a Bugzilla feature.  If your address is the only one containing
"l-squared", you can just type "l-squared" into the cc list instead of your
entire address.

"Auto-complete" is a browser feature in IE and Firefox.  If you've ever CC'ed
yourself, typing the first few letters of your address into the CC field will
show an auto-complete dropdown, and pressing the Down arrow key or clicking the
item will complete your address.

Comment 75

[Dave Miller [:justdave]]

Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.

Comment 76

[Ilya N.]

Changing Blocking2.20 flag to '?' because it seems that this is a good idea, and
the patch needs to be reviewed (the one titled 'try this') but we need someone
to fix this 4 YEAR OLD request

Comment 77

[Jouni Heikniemi]

I reviewed it 26 months ago (with the apparent negative result), and it's
probably bitrotten anyway. Many of us agree that this would be a nice feature,
but it won't land unless somebody is ready to expend the energy needed to get
this in.

Comment 78

[Tobias Sager]

Attachment: cc-addself.patch

Adapted to current cvs.

Comment 79

[Tobias Sager]

Attachment: cc-addself2.patch

Alternative implementation suggested in comment 67.

Comment 80

[Tobias Sager]

Attachment: cc-addself2-v1.patch

Comment 81

[Dave Miller [:justdave]]

Comment on attachment 159894 [details] [diff] [review]
cc-addself2-v1.patch

This looks nice on the face (I haven't tested it yet).	A couple minor things
to fix:

$::userid is deprecated (as are all global variables).	When new alternatives
exist we should avoid using them in new code.  For the current user's ID, use
Bugzilla->user->id (or just $user->id if $user has already had Bugzilla->user
assigned to it somewhere).

DBID_to_name should never be used on the currect user because it causes a
database lookup, and we already know who the current user is.  Use
Bugzilla->user->login for this.

I'm not going to argue about use of $::FORM because that's an ongoing
conversion and this file hasn't been converted yet anyway :)

Comment 82

[Frédéric Buclin]

(In reply to comment #80)
> Created an attachment (id=159894)
> cc-addself2-v1.patch
> 

Tobias, are you still working on it?

Comment 83

[Dave Miller [:justdave]]

Tobias hasn't responded in over a month, so I guess if anyone else wants to pick
up where he left off and try to get this in, you can go for it.

Comment 84

[GavinS]

Attachment: updated previous patch to HEAD, and fixed justdave's issues with $::userid

This fixes the issues justdave had with the previous patch (hopefully). It also
fixes the couple of other places where $::userid is used in process_bug.cgi
(well, why not? :)

I've tested this a bit, and it seems to work, but I'd personally prefer the
checkbox to be in another place. But, in the interests of landing this, I'll
leave it it where it is...

Comment 85

[Frédéric Buclin]

Comment on attachment 168729 [details] [diff] [review]
updated previous patch to HEAD, and fixed justdave's issues with $::userid

Your patch works fine. Except I would prefer to see this checkbox between the
"Add CC" and the CC list. I did not read all the comments here so I don't know
what is the "final" decision about its position. But when I applied your patch,
I thought something went wrong because I could not find it (it was out of my
screen). Indeed, I expected to find this checkbox near the CC list, as would
certainly any common user.

If a user doesn't want to add a comment, he will miss this checkbox and will
add his email address in the "Add CC" textfield, for sure!

What I suggest is either to move it between the "Add CC" and the CC list or to
add a second checkbox there, so that a user can choose to add himself to the CC
list either while writing a comment or from the top of the page, as ususal.



>Index: process_bug.cgi
>===================================================================
>+if (defined $::FORM{newcc} || defined $::FORM{'addselfcc'} || defined $::FORM{removecc} || defined $::FORM{masscc}) {

Nit: split this line; it's too long.


>@@ -863,6 +863,9 @@
>             $cc_add{$pid} = $person;
>         }
>     }
>+    if ($::FORM{'addselfcc'}) {
>+        $cc_add{$whoid} = $user->login;
>+    }

Nit: I would add a comment here to explain why $cc_add{$whoid} is not validated
first as we do for other email addresses (because the user is logged in and
consequently authentified).


>+    if (defined $::FORM{newcc} || defined $::FORM{'addselfcc'} || defined $::FORM{removecc} || defined $::FORM{masscc}) {

Nit: again a too long line.



>Index: template/en/default/bug/edit.html.tmpl
>===================================================================
>+  [% IF NOT bug.cc || NOT bug.cc.contains(user.login) %]
>+    <br>
>+    <label>
>+      <input type="checkbox" name="addselfcc">
>+      Add [% user.login FILTER html %] to CC list
>+    </label>
>+  [% END %]

Nit: If the user is not logged in, he sees "Add to CC list". Maybe something
like "Add me to the CC list" would be better, using:
Add [% IF user.login %] [% user.login FILTER html %] [% ELSE %] me [% END %] to
the CC list

Comment 86

[Frédéric Buclin]

KaiRo: LpSolit: talking about the CC checkbox? I'd love to see it near the CC
box as well :)
avatraxiom: I'd also like to see it near the CC box.
avatraxiom: In Red Hat's Bugzilla, it's above the comment box, and it's not very
useful, there.

Comment 87

[Ilya N.]

This bug is five years old, I'm not even bothering anymore!

Comment 88

[GavinS]

I too prefer it somewhere else, but I'm not the one who approves things, so
until an approver says move it, I'm going for approval on the 5th most voted for
Bugzilla bug....

If an approver needs it moved to get approval, then I will update the patch (and
fix the nits at the same time), but it would be soooo nice to get this in now we
have an r+

Comment 89

[Frédéric Buclin]

Comment on attachment 168729 [details] [diff] [review]
updated previous patch to HEAD, and fixed justdave's issues with $::userid

Who said you needed to put this checkbox here?

myk is the UI owner, so asking him to decide.

Comment 90

[GavinS]

(In reply to comment #89)
> Who said you needed to put this checkbox here?

All I did was pick up the ball on an orphaned patch someone else had produced,
apply Dave's review comments, and submit it. I don't care where the chackbox
goes....

Please feel free to update the patch and put the checkbox whereever you want - I
just want to see something checked in :)

Comment 91

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 168729 [details] [diff] [review]
updated previous patch to HEAD, and fixed justdave's issues with $::userid

The reason it's by the comments field is because users tend to forget to cc
themselves on bugs when commenting on them.  That's a reasonable location for
the control, therefore, although arguably it makes sense to put another button
up with the other cc controls as well.	But that can be another bug.

Comment 92

[Aleksey Nogin]

Red Hat's bugzilla (http://bugizlla.redhat.com/) had the CC box right by the
"comment" textbox for quite a while. At first I also considered it to be a
strage place to put it, however my experience convinced me otherwise - this
layout makes it very easy to remember to add yourself to CC list when you add a
comment. So at least until bug 7710 RFE ("Pref to automatically put me on the
CC: list of bugs I change") is implemented, this placements is IMO a good idea.

Comment 93

[Frédéric Buclin]

Comment on attachment 168729 [details] [diff] [review]
updated previous patch to HEAD, and fixed justdave's issues with $::userid

myk is the boss! Removing my review. ;)

Then per myk's comment, I would really like to see a new bug opened about an
additional checkbox near the CC list.

Comment 94

[GavinS]

(In reply to comment #93)
> Then per myk's comment, I would really like to see a new bug opened about an
> additional checkbox near the CC list.
> 
Frédéric, I've created bug#279243 for that.

Myk: Any chance of approval to go alongside your review???

Comment 95

[Shane H. W. Travis]

Checking in process_bug.cgi;
/cvsroot/mozilla/webtools/bugzilla/process_bug.cgi,v  <--  process_bug.cgi
new revision: 1.226; previous revision: 1.225
done
Checking in template/en/default/bug/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/edit.html.tmpl,v  <--
  edit.html.tmpl
new revision: 1.50; previous revision: 1.49
done