17464 - user preferences for notification mail

Status: VERIFIED FIXED

(Bugzilla :: Bugzilla-General, enhancement, P1)

Description

[Akkana Peck]

My mailbox is completely flooded with bugzilla mail about bugs which are
fixed/verified, where the only chnage is a change in state of a dependant or
depending bug.  This is completely useless information to me, but there's no way
to filter it, even with body filters, much less with the much faster header
filters, because the status of the bug isn't included in the messages.

Is it possible that bugzilla messages could include more information in the
subject line, for instance, the status of the bug and whether the only change is
to a dependant bug?  So instead of saying:

Subject: [Bug 14042] Changed - {sink} JS script output dropped

it might say:

Subject: [Bug 14042 (RESOLVED)] Dependency changed - {sink} JS script output
dropped

or, in a case where the state is changing, so I don't accidentally filter those
out,

Subject: [Bug 14042 (ASSIGNED->RESOLVED)] changed - {sink} JS script output
dropped

Either the status in the subject or having "Dependency changed" when that's all
that's changing would solve the problem, because in either case I could filter
the messages.  Of course, the more information that's there, the better.

(Alternately, if bugzilla just didn't send these messages about
resolved/verified bugs where only a dependency is changing, that would also fix
the problem, but perhaps there's someone out there who actually wants to get
these ...)

Comment 1

[Matthew Tuck [:CodeMachine]]

I think a better solution would be preferences about what changes you want to
trigger a notification.

Comment 2

[Simon Fraser [no longer active]]

I agree. The signal to noise ratio in bugzilla mail is now so low that I'm sure
almost everyone is ignoring all bugzilla mail. This is counter-productive. We
should tune what kind of bug changes generate mail.

Comment 3

[Simon Fraser [no longer active]]

cc:ing myself so I can get lots of lovely spurious bugzilla mail

Comment 4

[Matthew Tuck [:CodeMachine]]

I've put a comment about differing change triggers on bug #14137.

Comment 5

[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 6

[Chris Yeh]

this doesn't seem too hard, but these are famous last words. going to consider 
this for 2.12

Comment 7

[leger]

Updating QA Contact

Comment 8

[Chris Yeh]

changing summary to more accurately reflect the request: need a way for users to 
specify on what actions they receive notification mail. this involves a fair bit 
of work, since we'll need to carve out space in the profiles table to store 
this. the best thing to do is change the 'emailnotification' field of the 
profiles table to something other than an enum, which would easily support this. 

removing the 2.12 marker from the whiteboard, this is too much work to fit in 
that release.

Comment 9

[Dave Miller [:justdave]]

not to mention we'd have to hack up processmail pretty bad to get it to honor the 
prefs correctly.

Comment 10

[Zach Lipton [:zach]]

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

Comment 11

[Zach Lipton [:zach]]

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

Comment 12

[Zach Lipton [:zach]]

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

Comment 13

[Zach Lipton [:zach]]

Don't you not want to get this bugmail about me ccing myself?

Comment 14

[Jesse Ruderman]

See also bug 7345, which asks for a BCC field, which would act like voting in 
that you could add/remove yourself spamming, but wouldn't have a limit of 5 
bugs per user.

Comment 15

[David Krause]

I think a BCC field would be very useful.  Many times I am interested in
following the changes to a bug but don't want to spam everyone with my CC
addition.  I would use voting for this except you are limited to 5 bugs.

Comment 16

[Chris Yeh]

I am _extremely_ reluctant to go create another field in the database 
or another form field to facilitate BCC as it has been described. The better way 
to do this is to create personal notification preferences that has a setting for 
"notify me if the cc field changes". People that want BCC like behavior can turn 
it off, people that want CC behavior can turn it on.

Comment 17

[Matthew Tuck [:CodeMachine]]

This was filed primarily as a privacy measure.  Everything else was just a
side-effect.

Comment 18

[Matthew Tuck [:CodeMachine]]

Whoa, wrong bug.  I was referring to BCC.

Comment 19

[Zach Lipton [:zach]]

This bug blocks 53044, which is a tracking bug for email changes. Please 
discuss this there instead of on this bug. I am putting these cc's on that 
bug.

Comment 20

[alanr]

Attachment: modified version of userprefs.cgi

Comment 21

[alanr]

Attachment: modified processmail script

Comment 22

[alanr]

The above two attachments contain the modified scripts.

I added email preferences to the bugzilla we are using here at Scitech 
Software. It changes userprefs.cgi and processmail, and also requires
a new database table (at least that's how I implemented it - really belongs in 
the user profile table, but I was warned not to change existing tables). 

Users set preferences to get email for each of 4 recipient fields they could 
be on: owner, reporter, QA contact, CC list. 

For each of these groups, you have 5 checkboxes to accept email based on the 
type of change: 'new comments', 'new attachments', 'status and priority 
changes', 'marked Resolved' and 'other changes'. This simply looks at the text 
of the change and what fields have been modified in processmail. New email 
categories and different logic to determine the flags would be easy to add and 
change. 

So right now each user has a total of 20 checkbox choices. 

This was implemented by adding a new table with 'userEmailFlags' to 
checksetup.pl: 

$table{scitech} =
'userid mediumint not null primary key,
userEmailFlags mediumtext not null,
     index (userid)
 ';

Note we use the shadow database here and I had to run both checksetup.pl and 
"syncshadowdb -all" to get everything working. 

So each time an email is sent, processmail will look at the user preferences 
of all the people getting email and then decide if they really want that type 
of email (one with new comments, etc). 

Basically I parse all the preferences as a single text string using Perl 
rather than trying to enumerate flags in the database. I can make an argument 
that this uses the stored preferences even when new flags are added (new flags 
do not conflict with older database preferences), but I really did it this way 
because I have more confidence in my Perl than in my SQL. 

Some other stuff that I changed: now filtering based on new email tech and old 
email tech are completely (?) separate, and the userprefs has an 
enable/disable button: when newEmailTech is enabled, the additional options 
show, otherwise they remain hidden. The emailnotification field from profiles 
(which had three options) is now ignored if newEmailTech is enabled, and is 
used if oldEmailTech is enabled.

To account for users who globally do not want email they send, I added
a flag for this into the new 'userEmailFlags'. 

So I've been debugging this for a couple weeks on a test server and we've been 
using it a few days on our main server. It seems to basically work. 

Comment 23

[Dan Mosedale (:dmosedale, :dmose)]

alanr: this looks like good stuff... unfortunately, given how fast the cvs trunk
of bugzilla moves, it's hard for us to look at just the changed files.  Any
chance you could attach patches (preferably made with "diff -u" or "cvs diff -u")?

Comment 24

[Dan Mosedale (:dmosedale, :dmose)]

I didn't realize that alanr wasn't already on the CC list for this bug.  Alan,
please see my above comments.  Thanks.

Comment 25

[alanr]

So what I've done is sync up to the latest CVS versions of these files and
merged my changes into them. I also fixed some bugs and cleaned up the code
a little. There is one bug in userprefs.cgi that has baffled me for hours:
my enable/disable newEmailTech will show the login name email in the "email
to watch" form when newEmailTech is reenabled... very strange... but easily
reproduceable.

I also added a global flag in processmail to either enable or disable the 
'advanced filtering'. So I'll work on submitting diffs right now.

Comment 26

[alanr]

Attachment: diff of userprefs.cgi against latest mozilla cvs version

Comment 27

[alanr]

Attachment: diff of processmail against latest mozilla cvs version

Comment 28

[Hixie (not reading bugmail)]

Alan: Dude, you rock. This is going to save my life! :-)

Chris: 2.12? Pleeeeeeeeeeease?

BTW, once I have some more time I would like to hack this a little to add a few
more options -- personally I'm interested in the status/resolution fields
changing, but the severity, priority, milestone, platform, os and version
fields are much less interesting. Similarly, changes in the AssignedTo,
QAContact, and CC fields are of no interest to me with one exception: if the
change involves me (e.g., bug is assiged to me, or I am cc'ed, or some such).
This will be much easier to implement now that Alan has done the main part of
the work! :-)

Comment 29

[Jesse Ruderman]

Should bug 55108, "Create a status-update field that's separate from CC", be 
marked as a dup of this bug?

Comment 30

[Alan Raetz]

I'm adding my home email address to the cc list (I no longer work at Scitech 
Software). I do see one problem in my processmail code: it uses a file global 
to determine whether to use this new method of email filtering, and that's hard-
coded to be enabled... well, that worked at Scitech, but if you wanted it to 
respect a users old email tech preferences, it would have to look at the 
newemailtech flag in the user profile table. So I think you could replace lines 
like this:

>  if ($useAdvancedEmailFilter==1) {

with, (after getting the flag from the db),

>  if ($newemailtech==1) {

Or something like that... 

Comment 31

[Dan Mosedale (:dmosedale, :dmose)]

Alan: who warned you not to change existing tables?  I agree that this really
belongs in profiles, and I'd like to put it there.

Comment 32

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v3

Comment 33

[Dan Mosedale (:dmosedale, :dmose)]

I've just uploaded a not-yet-well-tested new version of this patch, this one
changes from using a separate scitech table to adding an "emailflags" column to
the profiles table.

It seems to me that in the interest of keeping our tables small, this really
wants to be an int8 of binary flags.  So here's my take on some revisions that
we probably want to see before checking this in:

* switch from a string to an int8

* add new database and upgraded database support for profiles.emailflags to
checksetup.pl

* tweak the indentation style of the patch to more or less match that of the
existing code, and, as much as possible, wrap at column 78.  Having different
styles in different parts of the same code makes for difficult reading.

* in the current code, unselecting checkboxes causes "perl -w" warnings to show
up in the httpd log.  whatever gets checked in should not have this problem.

* Al's proposed fix about respecting the oldemailtech preferences

* Rather than hiding the newemailtech preferences if newemailtech is not in use,
how about just making the text be grey and unselectable?  Hiding them makes it
too difficult for users of the oldemailtech to discover the advantages of
switching to the new.

Al & Tom, any objections to any of this, and/or alternate suggestions?  Do
either of you have time to help out with any of these revisions?

Hixie, any chance I can convince you to do the string -> int8 conversion, since
that would be an opportunity for you to get all the flags in that you want?

Comment 34

[Alan Raetz]

All these changes seem to make sense. It doesn't look like I'm going to have 
more time to work on this, though. It's possible, but unlikely. Yea, I agree,
userprefs.cgi form should always show all the options, because I know users 
won't actually read the fine print... even in red bold letters. Both the code 
and the interface are convoluted and need a better solution.

There was another idea I had... One shortcoming of this scheme is there might 
be a single bug where I want all the emails sent, regardless of the email 
preferences I set. So I thought of how this can be implemented: you add a 
string field to the user profile table and it's simply a list of all the bug 
numbers that you always want all email on. The processmail script reads this 
field for each person on the mailing list and if the bug number matches any bug 
number on the list, you are put on the 'send' list, overriding your global 
preferences. The show bug form would have a button "always email me changes to 
this bug" which would add the bug number to the users profile table. Then 
somewhere else (?), when the bug is resolved (?) that bug is removed from the 
user's list.

Comment 35

[Blake Ross]

This would be extremely useful to me and many other contributors.  Let's not 
let this patch rot, please...

Comment 36

[Dan Mosedale (:dmosedale, :dmose)]

Not to worry; it's not rotting.  There's a thread still in progress in .webtools
about the Right Thing regarding the usage of binary flags as it relates to this
bug in particular.  Once that gets sorted out, we'll be one step closer.

Comment 37

[Matthew Tuck [:CodeMachine]]

Can we get more checkboxes?  This really doesn't provide all the customisation a
user would want.  As far as I can see this does nothing to prevent cc and dep
change spam if you still want to see the important changes.  I would expect a
checkbox for pretty much every field.

So if we get this patch in I would not expect a RESOLVED FIXED to be
appropriate.

Also, I'm not exactly sure what this would entail, but in the context of any
design issues could you keep in mind bug #14137 and "notification classes".

The essential idea is that your change policy should eventually be able to be
chosen based on the bug, for example what component the bug is in.  Choosing
policy based on what list you're on is a good idea I hadn't though of, but it's
not the only way you'd want to choose.

As described you always have four policies when you might not care what list
you're on (which is not so serious for 20 checkboxes but we really need more),
and you can't have more policies based, for example, on what component the bug
is in.

Comment 38

[Tara Hernandez]

dmose, get on the stick!

:)

This bug will hold the 2.12 release, so upping the priority.

Comment 39

[Tom Ryan]

WARNING: We have had problems with this patch when some users are on old mail 
tech and some users are on new mail tech. In those cases, mail is sent out 
inconsistently (it was a tough problem to track down).

Suggested resolution: Allow the system administrator to force new mail tech 
globally. Also, if new mail tech remains a user option, editusers.cgi should be 
updated so that the administrator can change the user to old or new email tech 
manually. Currently the administrator has no way to do this, except to "become" 
the user in question and edit the users preferences via userprefs.cgi - this 
would be tedious or impossible with large user bases.

Comment 40

[Jacob Steenhagen]

Tom, remember that everything in bugzilla is nothing more than an SQL database
entry.  If you want to flip the switch for all your users to have the
'newemailtech' options turned on, simply issue the following SQL command:

update profiles set newemailtech = '1';

See http://lxr.mozilla.org/mozilla/source/webtools/bugzilla/docs/DATABASE.txt
for more info on using the mysql client.

Also, see bug 62832 for a patch to make this optionally the default setting for
new users.

Comment 41

[Dan Mosedale (:dmosedale, :dmose)]

OK, I've made a some progress on this patch.  I've fixed it to work when QA
contacts are not in use, and fixed "processmail regenerate" to not spit out
HTML.  I've also added logic to checksetup.pl to update the profiles table, as
well as removed all the hard tabs and done a bunch of line-wrapping.  In the
interest of getting this checked in for 2.12, I'm probably not going to write
new bitfield code for this, and just check it in as is with the string-based flags.

Assuming that we decide to keep oldemailtech around for another release (bug
63272), my inclination is to simply punt on the two oldemailtech bugs mentioned
here, and relnote this feature as unsupported if oldemailtech is in use.

matty: agreed, this is definitely not the be-all, end-all of mail filtering
features, but it's a fine start.  And we do want to get it into the 2.12
release, so there's not a lot more time for tweaking.

What remains to be done, I think:

* possibly add one more set of checkboxes for keyword changes

* in the current code, unselecting checkboxes causes "perl -w" warnings to show
up in the httpd log.

* finish my own reviewing/testing the code and getting it up for testing on landfill

Comment 42

[Zach Lipton [:zach]]

landfill? ugh! Landfill is such a mess right now that I don't think it will really 
be useful for this. I screwed it up too much with my <label> tag stuff. Also, 
it doesn't output email to most users, so we can't really test this patch with 
it. Landfill needs to be clbred, but in the meen time, I have my own 
personal install that we could test this on. Let me know what you want up 
and I'll post it at nuevaschool.org/cgi-bin/zach/bugzilla (note that the 
index.html page will not display so you will need to go to nuevaschool.org/
cgi-bin/zach/bugzilla/query.cgi (or something like that) for things to work.

Comment 43

[Zach Lipton [:zach]]

EEK. I tried to use the patch and got some errors. Looks like no-one has 
edited checksetup.pl to fix the tables for this. Here is what I get:

Software error:
SELECT emailflags FROM profiles WHERE userid = 1: Unknown column 
'emailflags' in 'field list' at globals.pl line 152. 

It looks like checksetup.pl hasn't been edited to add emailflags to the 
profiles table. Yes, I am userid 1 in this case, not a bug!

Comment 44

[Matthew Tuck [:CodeMachine]]

Dan, I would have expected the extra fields to be nearly trivial to add.

Regarding the flexible policies, I was mainly concerned with having to change
the schema rather than extend it, which is what extra fields would be for
example.

Given 3.0 is going to have a schema redesign, putting this patch in 2.12
probably is not a problem, my main concern would be that 3.0 considers a more
general proposal at least in terms of putting in future support into the design
and schema.

However, this is another feature that will have to be reimplemented in 3.0 to
achieve feature parity.  But given the time we're probably away from the 3.0
release, I'll agree leaving this patch rotting would probably not be a good
idea.

Comment 45

[Dan Mosedale (:dmosedale, :dmose)]

Zach: I haven't posted my most recent changes to this bug yet; I'll do that
tommorrow.

Comment 46

[Tara Hernandez]

I agree with dmose.  The one niggly I suggest is detecting oldmailtech and
throwing a warning if possible, just because I know how well people read release
notes and other forms of documentation.

Comment 47

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v4

Comment 48

[Dan Mosedale (:dmosedale, :dmose)]

OK, this version of the patch has fixed a bunch of perl warnings, re-indented
more stuff, protoized the subroutines, and added checkboxes for keyword field
changes.

Still to do: review/test, deal with the old emailtech related stuff.

Comment 49

[Zach Lipton [:zach]]

Since landfill was messed up beyond quick repair. I posted this patch at 
http://nuevaschool.org/cgi-bin/zach/bugzilla/index.cgi (note that the 
index.html page does not work, so you will need to access it with 
index.cgi). It looks fine, but there are not enough bugs in the database to 
test things now so you may want to submit a few (just use the test 
product, anything of any note is in private groups for internal use). If mysql 
goes down, I will reboot it ASAP. If you need anything else, just let me 
know. I will also have the patch for 58436 up on the same installation, but 
that should not be a problem.

Also, when I ran checksetup.pl after patching, I got some errors and perl 
warnings. Dmose: you may want to take a look at these:

bash$ patch <email.patch
patching file `checksetup.pl'
Hunk #1 succeeded at 749 (offset 1 line).
Hunk #2 succeeded at 931 with fuzz 1 (offset -863 lines).
patching file `processmail'
patching file `userprefs.cgi'
bash$ ./checksetup.pl
main::GetFieldDef() called too early to check prototype at ./checksetup.pl 
line
        939 (#1)
    
    (W prototype) You've called a function that has a prototype before the 
parser saw a
    definition or declaration for it, and Perl could not check that the call
    conforms to the prototype.  You need to either add an early prototype
    declaration for the subroutine in question, or move the subroutine
    definition ahead of the call to get proper prototype checking.  
Alternatively,
    if you are certain that you're calling the function correctly, you may put
    an ampersand before the name to avoid the warning.  See perlsub.
    
main::AddField() called too early to check prototype at ./checksetup.pl line
        939 (#1)
Checking perl modules ...
If you you want to see graphical bug dependency charts, you may install
the optional libgd and the Perl modules GD-1.19 and Chart::Base-0.99b, 
e.g. by
running (as root)

   perl -MCPAN -eshell
   install LDS/GD-1.19.tar.gz
   install N/NI/NINJAZ/Chart-0.99b.tar.gz

Checking user setup ...
Adding new field emailflags to table profiles ...
Populating duplicates table...
Now regenerating the shadow database for all bugs.
Use of uninitialized value in pattern match (m//) at ./processmail line 722 
(#1)
    
    (W uninitialized) An undefined value was used as if it were already 
defined.  It was
    interpreted as a "" or a 0, but maybe it was a mistake.  To suppress this
    warning assign a defined value to your variables.
    
[Thu Dec 21 18:31:16 2000] processmail: Use of uninitialized value in 
pattern match (m//) at ./processmail line 722.
[Thu Dec 21 18:31:16 2000] processmail: Use of uninitialized value in 
pattern match (m//) at ./processmail line 722.
1 [Thu Dec 21 18:31:16 2000] processmail: Use of uninitialized value in 
pattern match (m//) at ./processmail line 722.
[Thu Dec 21 18:31:16 2000] processmail: Use of uninitialized value in 
pattern match (m//) at ./processmail line 722.
2 
Reminder: Bugzilla now requires version 3.22.5 or later of MySQL.
Reminder: Bugzilla now requires version 8.7 or later of sendmail.
bash$

Zach

Comment 50

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v5

Comment 51

[Dan Mosedale (:dmosedale, :dmose)]

I've just updated the patch to be applicable to the latest cvs tip, now that the
fix for bug 62832 has been checked in.

Zach, the weird prototype warnings and the huge offset output by patch make me
think that your checksetup.pl file was truncated before you applied the last
patch.  Can you try removing the three files touched by this patch, updating to
the latest CVS tip, and then applying the new patch that I just posted?

Comment 52

[Zach Lipton [:zach]]

I clbr'd my install and repatched. All is fine now. Test away!

Comment 53

[Zach Lipton [:zach]]

Also, why is CC not an option?

Comment 54

[Matthew Tuck [:CodeMachine]]

It will also be eventually necessary to make it so that only certain keyword
changes trigger notifications.  Typically, people are only interested in a
specific small subset of keywords.

I had a significant thought the other day.  Once we get this up and running, I
expect everyone will want to receive "new comments".  But what happens when
someone makes a change, but wants to explain that change?  Often, people do this
spuriously, but there are often good reasons for doing so.

I suggest we'll need a "Comment Is Purely Explanatory" checkbox or some such to
not trigger notifications in this instance.

Comment 55

[Hixie (not reading bugmail)]

We should just educate people not to say the redundant things like "-> evang"
and "->0.8" and "FIXED" and "cc'ing self" and so on.

Comment 56

[Jesse Ruderman]

Hixie: "-> evang", "->0.8", and "FIXED" may be redundant when you first receive 
the bugmail, but they make the bug easier to read later.

Comment 57

[Matthew Tuck [:CodeMachine]]

I agree with Hixie.  We could block the notifications using what I suggested but
we'd still clutter the report.  People should definitely not put redundant
comments.

But I wasn't making my point clear enough.  It wasn't about redundant comments,
it was about something like "Marking X because Y.", where the Y is not redundant
and might be important.

Comment 58

[Matthew Tuck [:CodeMachine]]

Oh, and "for making the bug screen easier to read", see bug #11368.

Comment 59

[Hixie (not reading bugmail)]

Jesse: It only makes it easier to read because Bugzilla is not ideal in that
respect -- it would be quite possible, for instance, to interleave the activity
log with the comments, thus making it just as easy to read as with the current
redundant comments.

Matty: Absolutely. If there is a "because Y" then I am all for it. However,
often there is not, and that is noise, as you said.

Comment 60

[Dan Mosedale (:dmosedale, :dmose)]

Zach: FWIW, your test server appears to be torqued in some way at the moment.
Regarding a separate checkbox for CC, do you think it's really warranted?
That's already covered by the "Any field not mentioned above" box...

Matty: yes, there have been other request for "keyword-specific" keyword
notifications, especially by triage folks.  It's a good idea, but the goal for
2.12 is just to get basic filtering under control... I think that'll have to
come a bit later.

Comment 61

[Zach Lipton [:zach]]

Odd. I'm on it now.

Comment 62

[Zach Lipton [:zach]]

Dmose: What is wrong with my installation? The edit users page doesn't 
show you with an account, did you have a problem making one? Note to 
all: I CAN see your password (not that I try) so make it something 
unsecure (not your super-duper everything, root password to the world 
password, mysql premissions are not very good on the system). Oh, now 
I see the problem :) The index page is down. Just start at http://
nuevaschool.org/cgi-bin/zach/bugzilla/query.cgi and make an account 
from there :)

Comment 63

[Jacob Steenhagen]

Dan, on of the first things I noticed was the lack of CC in the options.  IMHO,
it should be an option... of everything that is possible to turn off, that's
really the only one I want to.  I'd still like to recieve mail for any other
changes, but if I select "Any field not mentioned above" I'd be turning off more
than I want to.

I agree that keyword specific should not make it to this patch for time sake.
Of course, I wouldn't be opposed to it making it into 2.13 early on.

Comment 64

[Jacob Steenhagen]

Two other things I've noticed...
 1) The option of All Bugs, All but my changes, Only CC isn't available if
    newemailtech is turned on.  Is this intentional?
 2) When you click the link to "Enable New Email Tech" this setting isn't saved
    until you click the submit at the bottom of that page.  The word Enable in
    the link gives the impression that simply clicking that link will enable the
    feature.

Comment 65

[Dan Mosedale (:dmosedale, :dmose)]

Jake: re point 1, I think that's OK.  It's still possible to do all those things
by using the Advanced Filtering checkboxes.

I've got a new version of the patch in my tree, but I haven't posted it here yet
because it's still got a couple of bugs related to old/new email tech
interoperability.

Zach: it fixes those processmail warnings you saw when you first installed the patch

Jake: it adds a separate CC checkbox, and it adds verbiage to warn people about
your point 2

Comment 66

[Dave Miller [:justdave]]

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

Comment 67

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v6

Comment 68

[Dan Mosedale (:dmosedale, :dmose)]

OK, this iteration of the patch fixes all the problems I know about.  It fixes
the problems mentioned in my previous comment.  It also fixes the stuff that Al
and Tara mentioned about interacting reasonably with the oldemailtech code.

Now so far, this is minimally tested with newemailtech, but seems to work.  It's
not actually tested with oldemailtech at all.

So now this baby needs serious testing and review.  I'm gonna do some of that,
but I'd like to see some other review as well.  I've hacked on the patch
significantly, so I definitely shouldn't be the only reviewer.  Perhaps Al and
someone else (volunteers?) could review it as well.

Comment 69

[(not reading, please use [e-mail] instead)]

bless you dmose for working on this!

Comment 70

[Dave Miller [:justdave]]

I applied this patch to my system and started playing around with it.  First 
thing I noticed is that when you hit submit on the email settings page, it takes 
you to the Account Settings tab, and it has cleared your Real Name out of the 
database.  (If I leave the error check in SqlQuote commented out like it is in 
the current CVS).

If I uncomment the error check in SqlQuote (which I actually had done, because 
I'd been noticing undefined concatenation errors related to it in my error log), 
I get this:

Undefined passed to SqlQuote at globals.pl line 893
main::SqlQuote(undef) called at /home/httpd/html/bugzilla/userprefs.cgi line 150
main::SaveAccount called at /home/httpd/html/bugzilla/userprefs.cgi line 672

Line 150 is here:
    SendSQL("UPDATE profiles SET " .
->          "realname = " . SqlQuote($::FORM{'realname'}) .
            " WHERE userid = $userid");

Line 672 is here:
    if ($::FORM{'dosave'}) {
->      &$savefunc;
        print "Your changes have been saved.";
    }

Sounds like it's getting the wrong $savefunc for that tab...

Comment 71

[Zach Lipton [:zach]]

patch is up!

Comment 72

[Dave Miller [:justdave]]

Zach:  Server error: can't connect to database server

trying to get to the query.cgi page.

Comment 73

[Zach Lipton [:zach]]

My installation is fixed now. Someone with root access decided to 
shutdown mysql :) Fixed.

Comment 74

[Dave Miller [:justdave]]

The error noted in my comment on 01-13 13:38 is reproducable on your system, too, 
Zach.  It's getting the wrong savefunction for that tab.  When I try to change my 
email notificaton prefs, I get taken to the Account settings tab and my real name 
is cleared, and my notification preferences are not updated.

Comment 75

[Zach Lipton [:zach]]

Huh. I am not seing this problem with my account. Is this only a problem 
on non-god accounts? Also, a big problem: When accessing the edit 
users screen, the options for old email tech filtering are still there, but no 
new options. 

Comment 76

[Dave Miller [:justdave]]

Attachment: patch v7

Comment 77

[Dave Miller [:justdave]]

found what was causing the problem.  Mozilla is the only browser that was immune 
to it as far as I could tell, everything else choked.  The <INPUT> item for the 
"savedEmailTech" hidden item was missing the closing >.  The hidden <INPUT> that 
had the "bank" variable in it was the next one right after it.  So most of the 
browsers were interpretting it as part of the same tag.

As long as I was running it through an HTML checker, I cleaned up a whole mess
of other HTML errors as well (mostly &nbsp with no ; after them, swapped <TD>
and <TR> tags, extra </TABLES> and </CENTERS>, and unescaped &'s (should be
"&amp;")

Comment 78

[Zach Lipton [:zach]]

zachfill has been patched!

Comment 79

[Dan Mosedale (:dmosedale, :dmose)]

Dave: nice work on tracking down the HTML lossage and cleaning up!

Zach: you're right, it sucks that the admin page doesn't have the ability to
edit users' new email tech stuff.  I don't see myself getting time to fix that
real soon, so unless someone else wants to contribute that, I don't expect it
will make 2.12.

Everybody: I think it would be a good idea to make the "don't send me mail on
bugs that I change" bit default to off, so that people don't inadvertantly miss
messages at the time admins upgrade to this code.  Any thoughts?

Comment 80

[timeless]

i use don't send me mail. otherwise my webmail box would overflow. 

Comment 81

[Zach Lipton [:zach]]

I think that most users who know what they are doing, turn that option off. It 
just generates too much spam.

Comment 82

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v8

Comment 83

[Dan Mosedale (:dmosedale, :dmose)]

Latest patch has two more changes:

* fixed attribution for dave in the patch

* made userprefs.cgi automagically migrate ExcludeSelfChanges forward rather
  than using a static default.

I still need to tweak editusers.cgi to disable email notification editing
widgetry for users who have new emailtech.  I'll do that tommorrow.

Comment 84

[Stephan Niemz]

Zach, I disagree. Thanks to procmail, my bugzilla mail automatically lands in
a seperate folder. I find it often handy to have bug information on my local
disk, sometimes it's faster to search in that mailbox than to query bugzilla.

Comment 85

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v9 w/editusers changes

Comment 86

[Dan Mosedale (:dmosedale, :dmose)]

In this latest patch, I've tweaked edituser.cgi as follows:

* worked around some perl warnings

* made the edit and delete user screens not show "emailnotification" info if the
user has newemailtech set to 1, since in that case it's no longer used

* removed the setting for emailnotification from the adduser screen, on the
assumption that most sites are going to be adding users with newemailtech

This fixes all the bugs that I currently know about in this patch; more testing
and review requested.

Comment 87

[Zach Lipton [:zach]]

My installation is now updated. Someone decided to deleate mysql, but all 
is well now. Patch v9 w/editusers changes is up!

Comment 88

[Dan Mosedale (:dmosedale, :dmose)]

Patch v9 has now been checked into the CVS tip.  Marking fixed.

Comment 89

[lchiang]

When will this version be applied to Bugzilla?  Do I need to file a different
bug on that?

Thanks.

Comment 90

[Zach Lipton [:zach]]

This is in bugzilla right now. If you mean b.m.o, it will be when 2.12 comes 
out...Which SHOULD be RSN!!!

Comment 91

[Dave Miller [:justdave]]

After installing this patch on my system, anyone whose name is removed from owner 
or QA gets an oldemailtech mailing, even though they have newemailtech selected 
in their preferences.  I haven't been able to test CC list yet to see what it 
does to the person being removed when you remove a CC.

Comment 92

[Brian 'netdragon' Bober]

I might be in the wrong bug, but since this is earmarked for 2.12, I think 
there is one other thing that should be done. Especially if dave is going to be 
fishing around processmail anyway. 

I think that there should be a user pref to let the mail sit for X (the pref) 
amount of time, and then all get sent out in a large email combined together, 
with all the changes to all the bugs in their list in one email. If X is 0 (the 
default) then it would be normal.

Comment 93

[Dave Miller [:justdave]]

Brian: see bug 26943

Comment 94

[Brian 'netdragon' Bober]

Here is an example of what I think should be done in the emails. Note that if 
you had a mail reader that could show links, then they would change color, 
showing you that you have gone to the bug.



Changes Update -  1/24/00 11:34PM to 1/25/00 12:34PM - Duration: 60 min
                    
                      Numbers                     |         Summaries
--------------------------------------------------------------------------------

http://bugzilla.mozilla.org/show_bug.cgi?id=64179 | "K" should be "KB"
http://bugzilla.mozilla.org/show_bug.cgi?id=29346 | Prevent repeating popup win
http://bugzilla.mozilla.org/show_bug.cgi?id=24689 | XUL Reference Manual
http://bugzilla.mozilla.org/show_bug.cgi?id=16775 | Query confusing for newbies








--------------------------------------------------------------------------------
http://bugzilla.mozilla.org/show_bug.cgi?id=64179 | "K" should be "KB"
--------------------------------------------------------------------------------

[e-mail] changed:


           What    |Old Value                   |New Value
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |66480



------- Additional Comments From [e-mail]  2001-01-24 17:11 -------
Patch will be attached to bug 66480 to eliminate duplicating patch attachments.








--------------------------------------------------------------------------------
http://bugzilla.mozilla.org/show_bug.cgi?id=29346 | Prevent repeating popup 
window
--------------------------------------------------------------------------------


------- Additional Comments From [e-mail]  2001-01-24 18:22 -------
OK, I've just tested the following and it works just fine: to block popups from
a specific site or group of sites, add these lines to bin/defaults/pref/all.js
(turns out you have to add these to all.js, not prefs.js, I think):

pref("capability.policy.popupsites.sites", "www.annoyingsite1.com
www.popupsite2.com [and so on, space-separated list of URLs]");
pref("capability.policy.popupsites.windowinternal.open","noAccess");

The first line defines a group (the equivalent of a "security zone" in IE)
consisting of a list of sites, and the second line forbids these sites from
opening new windows. Try it, it works!



------- Additional Comments From [e-mail]  2001-01-24 18:27 -------
Actually, the URLs in the above example have to start with http:// (or ftp or
whatever). Don't leave off the protocol.






--------------------------------------------------------------------------------
http://bugzilla.mozilla.org/show_bug.cgi?id=24689 | XUL Reference Manual
--------------------------------------------------------------------------------

[e-mail] changed:

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
                 CC|[e-mail],axel@pi|[e-mail],axel@p
                   |ke.org,biswapesh_chatterjee@|ike.org,biswapesh_chatterje
                   |tcscal.co.in,[e-mail],|[e-mail],boberb@rpi.e
                   |[e-mail],gary_|du,[e-mail],g
                   |[e-mail],mozilla@david|[e-mail],maolson@
                   |krause.com,rudman@netscape.c|earthlink.net,mozilla@david
                   |om                          |krause.com,rudman@netscape.
                   |                            |com








--------------------------------------------------------------------------------
http://bugzilla.mozilla.org/show_bug.cgi?id=16775 | Query confusing for newbies
--------------------------------------------------------------------------------

[e-mail] changed:

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
  Status Whiteboard|                            |2.12



------- Additional Comments From [e-mail]  2001-01-24 22:34 -------
now have a viable patch here... nominating for 2.12 to get it on the radar.  
There's a few other things going on with query.cgi, so this may or may not 
actually go in, depending on the others.  This just gets it on the map so we 
can 
compare.

Comment 95

[Brian 'netdragon' Bober]

How the heck did my browser get on this bug? Sorry for that long error post.

Comment 96

[Jacob Steenhagen]

I can confirm that this also happens w/the CC list.

Comment 97

[Dan Mosedale (:dmosedale, :dmose)]

Dave/Jake: for some reason, I'm unable to reproduce this problem.  Can you walk
me through the exact steps that you can cause this with?

Comment 98

[Stephan Niemz]

See also bug 62751. There seems to be one thing missing from what's currently
in 2.11: "When I'm a voter for a bug, email me ..." similar to the other new
pref settings.

Comment 99

[Dan Mosedale (:dmosedale, :dmose)]

Stephan: I agree that this sounds like a good idea.  It's probably too late to
make the 2.12 train, however.  With luck, I'll get my final fix for the mail
stuff in today, and then I'll hopefully be done with it.

Comment 100

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: patch v10, fixes oldemailtech spam bug

Comment 101

[Dan Mosedale (:dmosedale, :dmose)]

Despite the attachment name, it's actually not patch V10.  It's bugfix V1.

Alright, this latest patch has seen only minimal testing, so I'd appreciate it
if folks could hammer on this.  In addition to the usual testing with
newemailtech, I'd appreciate it if someone could verify that I haven't busted
anything with oldemailtech.

This patch adds a new field to each group called Removeme ("If that changes")
for lack of a more creative name on my part.  This allows a user to select
whether they get mail when they are removed from the CC list, or their capacity
as QA Contact or Owner.  The astute reader will notice that there is a checkbox
for this option in Reporter as well, even though it can't change.  It's a dummy,
and kept this patch from being even more of a nasty hack than it already is.

If someone felt inclined to review that patch, that'd be spiffy too.

Comment 102

[Dan Mosedale (:dmosedale, :dmose)]

Jake found a problem that I didn't spot because of the accounts I was using to
test was set up wrong.  Investigating...

Comment 103

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: bugfix v2, fixes problem found by jake

Comment 104

[Dan Mosedale (:dmosedale, :dmose)]

here's a second try at the bugfix.  as before, testing and review desired.

Comment 105

[Jacob Steenhagen]

My only remaining comment is that the text "If that changes" implies addition or
removal but the code only seems to fire on removal (at least in the brief
testing I did w/CC).

Comment 106

[Dan Mosedale (:dmosedale, :dmose)]

Jake: you're right, but that's the intended behavior.  Would it be clearer if
the text changed from "If that changes" to "If I am removed from that capacitcy"

Comment 107

[Dan Mosedale (:dmosedale, :dmose)]

Fix checked in, with the wording change that I proposed above. 
r=[e-mail].

Comment 108

[Jacob Steenhagen]

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

Comment 109

[Matthew Tuck [:CodeMachine]]

Verify Note: I need to examine this and work out what missing functionality to
file bugs on.

Comment 110

[Tara Hernandez]

Sorry for the spam, but I needed to be able to query for all of these correctly.

Comment 111

[Dave Miller [:justdave]]

The setting for "all qualifying bugs/only bugs which I change/bugs on which I'm 
CC'd" is missing if you are using oldemailtech with this patch.  It should be 
missing if you're on newemailtech, but it should still be there if you're on 
oldemailtech.  In order to get 2.12 in a release state, this needs to either come 
back or we need to eliminate oldemailtech altogether.  I think restoring this 
setting will be the easier of the two in the required timeframe.

Comment 112

[Dave Miller [:justdave]]

taking this, since dmose is off on other projects now.

Comment 113

[Dave Miller [:justdave]]

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

Comment 114

[Jacob Steenhagen]

Interestingly enough, it appears that if you have the Param() off in editparams,
the dropdown doesn't appear.  If the Param() is on but your pref isn't the
dropdown is there.

Comment 115

[Dave Miller [:justdave]]

Attachment: Bugfix 2 v1

Comment 116

[Dave Miller [:justdave]]

Turns out the oldemailtech preferences were getting printed, but inside table 
cells, and there was no table to put the cells in.  Added <TABLE> and </TABLE> 
tags and that fixed it.

Comment 117

[Dave Miller [:justdave]]

checked in.

Comment 118

[Jacob Steenhagen]

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

Comment 119

[Dave Miller [:justdave]]

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

Comment 120

[lchiang]

I don't feel comfortable verifying this bug since I don't use this feature.  I 
leave my prefs at default.  change QA contact to Asa.

Comment 121

[Jacob Steenhagen]

VERIFY.  Any new problems/requestes for expanded functionality should be filed
as new bugs (as has already been done a time or three).

Comment 122

[Dave Miller [:justdave]]

Moving closed bugs to Bugzilla product

Comment 123

[Pulsebot]

Pushed by ffxbld-merge:
https://hg.mozilla.org/releases/mozilla-beta/rev/a21e9ff0e1c8
[fenix] For https://github.com/mozilla-mobile/fenix/issues/17464: Handle selected item background programmatically.