Closed
Bug 33543
Opened 25 years ago
Closed 24 years ago
Addresses from reading a newsgroup posting shouldn't be added to collected addresses
Categories
(SeaMonkey :: MailNews: Address Book & Contacts, defect, P3)
SeaMonkey
MailNews: Address Book & Contacts
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla0.8
People
(Reporter: lchiang, Assigned: chuang)
References
(Blocks 1 open bug)
Details
(Whiteboard: relnote-user [nsbeta1+])
Attachments
(6 files)
3.35 KB,
patch
|
Details | Diff | Splinter Review | |
15.50 KB,
image/gif
|
Details | |
36.97 KB,
image/gif
|
Details | |
7.92 KB,
patch
|
Details | Diff | Splinter Review | |
8.48 KB,
patch
|
Details | Diff | Splinter Review | |
731 bytes,
patch
|
Details | Diff | Splinter Review |
Addresses from reading a newsgroup posting shouldn't be added to collected
addresses
Win32 2000-03-27-09-m15 build
1) Open a newsgroup
2) Select a posting by someone who you don't have in your collected address
book
3) Open your collected address book
Actual results: The sender of the newsgroup posting is added to your collected
address book
Expected results: I don't think that the sender of a newsgroup posting should
be added to the collected address book. Otherwise, my collected address book
will fill up with names of people that I do not even know simply because I am
browsing or reading through a newsgroup. In most cases, this is different than
reading emails that may be addressed to me or cc'd to me.
cc: jglick - what do you think, Jennifer?
Very good point. Most users probably won't want authors of newgroup postings
added to their Collected Addresses AB.
I say we don't add authors/recipients from newsgroups.
If time permits we could make it a Preference (with default turned off).
Jennifer - can you add this to your ab spec?
Keywords: beta2
Comment 5•25 years ago
|
||
Mass moving M16 to M17 - look for nsbeta2 before anything else.
Target Milestone: M16 → M17
Reassign to David. Address book can't tell if the email addres is from mail or
newsgroups.
Assignee: chuang → bienvenu
Putting on [nsbeta2-] radar. Missed Netscape 6 train. Move to MFuture
milestone.
Whiteboard: [nsbeta2+][5/16] → [nsbeta2-]
Comment 10•25 years ago
|
||
Based on leger's comments 5-30-00 I'm changing target milestone to Future.
David, if you disagree, please change accordingly.
Target Milestone: M20 → Future
Keywords: relnote3,
relnoteRTM
Comment 11•24 years ago
|
||
Doesn't this make the Collected Addresses function pretty useless for anyone who
reads news, then? :-(
Gerv
Comment 12•24 years ago
|
||
Removing nsbeta2 keyword and adding mail5 keyword so this get nominated for 6.5
release. Having a preference that allows the user to turn off the CAB (Collected
Address Book) for newsgroup postings would be a big plus. It would also prevent
a lot of users from reaching the upper CAB limit.
Comment 13•24 years ago
|
||
Correction, should be "mail3", important features.
Comment 14•24 years ago
|
||
This is not going into the release notes, since it's covered in the online help.
ccing myself.
Comment 18•24 years ago
|
||
marking nsbeta1+ and moving to mozilla0.8
This includes adding a pref for this.
Keywords: nsbeta1
Priority: P2 → P3
Whiteboard: relnote-user → relnote-user [nsbeta1+]
Target Milestone: Future → mozilla0.8
Comment 19•24 years ago
|
||
reassigning to chuang. Candice, could you look at this in the mail code, then.
Assignee: bienvenu → chuang
Status: ASSIGNED → NEW
Assignee | ||
Comment 20•24 years ago
|
||
Assignee | ||
Comment 21•24 years ago
|
||
The patch is to check if "Newsgroups" is in the msg header, then won't add teh
sender email into collected address book.
Jennifer, I need the UI instruction on adding the pref.
Status: NEW → ASSIGNED
Comment 22•24 years ago
|
||
+ if ( (!headerInfo) || (!headerInfo->name) || (!(*headerInfo->name)) ||
+ (!headerInfo->value) || (!(*headerInfo->value)))
you don't need most of those ()s
simplification1:
if ( !headerInfo || !headerInfo->name || !*headerInfo->name ||
!headerInfo->value || !*headerInfo->value)
logical collection:
if (!(headerInfo && headerInfo->name && *headerInfo->name && headerInfo->value
&& *headerInfo->value))
use this^^
+ if (nsCRT::strcasecmp("Newsgroups", headerInfo->name) == 0)
+ {
don't explicitly compare with zero.
instead do this:
if (!nsCRT::strcasecmp("Newsgroups", headerInfo->name)) {
Comment 23•24 years ago
|
||
Comment 24•24 years ago
|
||
Note some wording changes. In the describe text at the beginning, "mail" is
replaced with "messages" since news is now included. "my" is removed.
Comment 25•24 years ago
|
||
It looks like if someone posts a message to a newsgroups and cc's me, and I
read the message in my Inbox, I won't collect their address. (this is different
than reading the newsgroup posting.)
is that the desired behaviour? (I'm ok with it, I just want to bring it up.)
timeless provided some good suggestions. can you try them out, and attach a new
patch, so I can review the final form of the patch?
Comment 26•24 years ago
|
||
adding myself to the cc list.
Comment 27•24 years ago
|
||
Let me know if the screen shot accurately captures the actual behavior as
implemented. Thanks.
Comment 28•24 years ago
|
||
aren't we supposed to check a pref to see if we should collect addresses from
the newsgroup? I don't see us checking a pref in the patch - did I miss something?
Reporter | ||
Comment 29•24 years ago
|
||
re: "It looks like if someone posts a message to a newsgroups and cc's me, and
I read the message in my Inbox, I won't collect their address. (this is
different than reading the newsgroup posting."
I think in this case we should be consistent in that any message which passes
through the user's Inbox should be subject to the same "address collection"
preferences regardless of the origin of the message.
Assignee | ||
Comment 30•24 years ago
|
||
1. I haven't done the pref part, I like to check in this patch first. I'll pst
a final one when I get the No.2 taken care.
2. Regarding on Seth's comment, I'll try it out. But since I can only detect
it from the msg header, there has to be enough information for me to tell it's a
mail msg but origin from a news server.
Assignee | ||
Comment 31•24 years ago
|
||
I tried to post a message to a newsgroup and cc to myself. It looks like in
message header, the newsgroup msg header will have "NNTP-Posting-Host" and the
cc msg header doesn't. Instead of checking "Newsgroups", I'll check for
"NNTP-Posting-Host". Since I'm not that familiar with msg header, anyone has
objections?
Comment 32•24 years ago
|
||
aren't all messages supposed to be identical? ie unless i'm wrong, don't rely
on a bug to distinguish context.
Comment 33•24 years ago
|
||
If someone knows of a way to better detect the origin of where a message comes
from then I think we should do that.
But if we can't, then I think it's very acceptable to say that we don't collect
addresses for messages addressed to newsgroups even if they show up in a mail
folder. People who spend enough of their time reading messages like this that it
will be an issue probably aren't going to want to turn off collection for
newsgroups anyway.
Assignee | ||
Comment 34•24 years ago
|
||
Thanks to Seth's email for the link
http://www.vex.net/yarn/list/199901/0094.html. "NNTP-Posting-Host" is not a
required header in newsgroup msg header. So I'll use "Newsgroups". This means
the email address from a mail message which is originated from a newsgroup will
not be collected if the collected newsgroup pref is not checked.
Comment 35•24 years ago
|
||
Comment 36•24 years ago
|
||
Before "Incoming messages" and "Outgoing messages" meant both mail and news.
Since we are now separating out mail from newsgroups, we should probably
change "Incoming messages" to "Incoming mail messages" and "Outgoing mail
messages" so it is more clear.
Assignee | ||
Comment 37•24 years ago
|
||
Comment 38•24 years ago
|
||
can we cache this pref value so we only read them once when msgHdrViewOverlay.js
first gets intialized?
If we were worried about false hits when reading a message you were cc'ed on
that happened to be posting to a newsgroup we can always try to be fancy and
figure out the current account in the folder pane. If it's a news account then
we know to disable the collected address book. If it's non news, then go ahead
and add entries....that may be more trouble than it's worth, I just wanted to
throw it out there.
Assignee | ||
Comment 39•24 years ago
|
||
I'm not sure about cache the pref, what if the pref get changed after
msgHdrViewOverlay.js initialized?
The place where we add the email address in the collected address is in mime
code. I'm not familiar with that part of code, if someone can point out the way
to get the current account in the folder pane in mime, yes, that's the right way
to do it.
Comment 40•24 years ago
|
||
the bracing in @@ -167,6 +167,20 @@ is very messy, please use something normal
+ for (PRInt32 j=0; j<mHeaderArray->Count(); j++) {
+ headerInfoType *headerInfo = (headerInfoType *)mHeaderArray->ElementAt(j);
+ if (!(headerInfo && headerInfo->name && *headerInfo->name))
+ continue;
+
+ if (!nsCRT::strcasecmp("Newsgroups", headerInfo->name)) {
+ bFromNewsgroups = PR_TRUE;
+ break;
+ }
+ }
Keywords: review
Comment 41•24 years ago
|
||
chuang wrote: "what if the pref get changed after msgHdrViewOverlay.js
initialized?"
if that is an issue (I don't know enough about msgHdrViewOverlay.js), then you
can use pref callbacks from JS to get notified when a pref changes. check out
nsIPrefListener.
so, if it isn't an issue, cache the pref value like mscott suggests. if it is
an issue do the pref callback thing.
also, instead of
handleHeader: function(headerName, headerValue, fromNewsgroups)
is should be generalized to
handleHeader: function(headerName, headerValue, dontCollectAddress)
fix the implementation of handleHeader().
the caller (from mime/emitters/src/nsMimeHtmlEmitter.cpp) is fine.
please address those issues and attach a new patch.
Assignee | ||
Comment 42•24 years ago
|
||
Assignee | ||
Comment 43•24 years ago
|
||
Assignee | ||
Comment 44•24 years ago
|
||
Hopefully he 2 patch are the final. The two prefs (for Incoming and newsgroup)
have cached in msgHdrViewOverlay.js as mscott suggested. The name in
msgHdrViewOverlay.js has chnaged as Seth suggested.
Since we cached the prefs as globals, if user changed these two prefs, it won't
be working until user close and reopen the Mail window or restart the app. A
bug 65613 has filed to state the problem.
Comment 45•24 years ago
|
||
silly thought,
@@ -32,6 +33,7 @@
if( emailCollection.checked ){
could we have an <emailCollectionBroadcaster> and have everything that is
getting disabled toggled observe it?
this thought is not required for current patch, but if a new patch is needed,
please consider.
+ ((gCollectIncoming && !dontCollectAddress) || (gCollectNewsgroup &&
dontCollectAddress)))
looks messy
+ dontCollectAddress ? gCollectIncoming : gCollectNewsgroup)
should work, and is hopefuly clearer
+ for (PRInt32 j=0; j<mHeaderArray->Count(); j++)
+ {
it's a counter, so PRUInt32 would make more sense
+ for (PRUInt32 j=0; j<mHeaderArray->Count(); j++) {
+ if (!nsCRT::strcasecmp("Newsgroups", headerInfo->name))
+ {
+ bFromNewsgroups = PR_TRUE;
+ break;
+ }
I hope that isn't a known bracing style
+ if (!nsCRT::strcasecmp("Newsgroups", headerInfo->name)) {
+ bFromNewsgroups = PR_TRUE;
+ break;
+ }
please consider following these sugggestions.
22667: r=timeless
Comment 46•24 years ago
|
||
the bracing is probably screwy because of tabs.
Is there a bug filed for the fact that we are just looking at the newsgroups:
header to determine if the posting was read in a newsgroup?
sr=bienvenu
Assignee | ||
Comment 47•24 years ago
|
||
I'll file a bug for it later.
Comment 48•24 years ago
|
||
r=sspitzer
sorry for the delay (on vacation)
Assignee | ||
Comment 49•24 years ago
|
||
Fix check in 1/17/01.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
VERIFIED FIXED on Windows 2000 build 2001020504, Linux build 2001020506 and Mac
build 2001020513. Browsing a newsgroup's posting doesn't add any additional
entries into the Collected Address Book.
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: Browser → Seamonkey
Comment 51•16 years ago
|
||
(In reply to comment #45)
> + ((gCollectIncoming && !dontCollectAddress) || (gCollectNewsgroup &&
> dontCollectAddress)))
> looks messy
> + dontCollectAddress ? gCollectIncoming : gCollectNewsgroup)
> should work, and is hopefuly clearer
I filed bug 498215.
QA Contact: pmock → addressbook
You need to log in
before you can comment on or make changes to this bug.
Description
•