User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.3b) Gecko/20030103 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.3b) Gecko/20030103 when choosing "Junk Mail Controls..." the "Do not mark messages as junk if the sender is in my address book:" should allow a set of checkboxes for all address books, not just a menu choice of one address book. or, at the very least, add the option of "all". Reproducible: Always Steps to Reproduce: 1. create multiple address books 2. choose "Junk Mail Controls..." from the Tools menu Actual Results: a menu allowing the choice of one address book Expected Results: checkboxes beside each potential address book the user may want to allow mail to appear from as "not junk".

The junk mail controls feature is still being revised... there are plans to implement selection of multiple Address Books. Spec (not final yet) in progress at http://www.mozilla.org/mailnews/specs/spam/

Mail triage team: nsbeta1-

mass re-assign.

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

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

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

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

You may wish to re-think the change in the summary - it might not be a bad idea to allow the mail controls to *blacklist* on an address book, as well.

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

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

(David wrote in comment #8) > You may wish to re-think the change in the summary - it might not be a bad idea > to allow the mail controls to *blacklist* on an address book, as well. not a bad idea, but shouldn't that be done with a seperate bug request I fail to see how this blocks bug 180012

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

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

there's no blocking relationship that I can see here.

same issue in tb 1.5.0, i do agree with topic starter.

This patch: * Changes am-junk UI to allow for multiple local address books to be used for whitelisting. * Changes back end to look through each selected local address book to see if email address has been whitelisted.

Changes since v0.1: * use OnSave rather than changing using onClick (which didn't capture changes made via keyboard) for junk UI * backend now caches all valid directories into an array

Comment on attachment 257882 [details] [diff] [review] Using onSave patch v0.2 >+ var currentArray = wlValue.split(","); I'm not sure that you can guarantee that URIs don't contain commas? Maybe spaces would be safer. >+ var wlNode; >+ for (var i = 0; i < wList.childNodes.length; i++) >+ { >+ wlNode = wList.childNodes[i]; Nit: might as well declare var wlNode inside the loop. >+ } Nit: indentation wrong. >+ var newArray = new Array(); Nit: var wlArray = []; (newArray is an awful name for a variable). >+ <vbox> >+ <hbox flex="1"> Nit: any reason not to just use <hbox> (no flex) here? >+ PRInt32 index = -1; >+ while (++index < whiteListArray.Count()) Use a for loop please. >+ nsCOMPtr<nsIRDFResource> resource; >+ rv = rdfService->GetResource(*whiteListArray[index], getter_AddRefs(resource)); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ nsCOMPtr<nsIAbMDBDirectory> whiteListDirectory = do_QueryInterface(resource, &rv); >+ NS_ENSURE_SUCCESS(rv, rv); >+ if (whiteListDirectory) >+ whiteListDirArray.AppendObject(whiteListDirectory); No need to ENSURE these. If something goes wrong then we just don't append. >- // if we can't get the db, we probably want to continue firing spam filters. Put that comment back in to remind you ;-) >+ if (useWhiteList || !trustedMailDomains.IsEmpty()) I think we should use whiteListDirArray.Count() != 0 instead of useWhiteList. >+ PRInt32 index = -1; >+ while (++index < whiteListDirArray.Count() && !cardExists) >+ rv = whiteListDirArray[index]->HasCardForEmailAddress(authorEmailAddress, &cardExists); Another for loop please. Also, you should not exit the loop if an address book returns a failure code while claiming the card exists.

Comment on attachment 257882 [details] [diff] [review] Using onSave patch v0.2 Oh, and the list doesn't refresh properly when switching between junk settings for two different accounts.

Changes since v0.2: * Use comma to space as the separator * Process value even when it's empty so checkboxes get set correctly.

(In reply to comment #23) > Created an attachment (id=262816) [details] > Space instead of comma patch v0.3 > > Changes since v0.2: > * Use comma to space as the separator Of course I meant: * Use space instead of comma as the separator

Comment on attachment 262816 [details] [diff] [review] Space instead of comma patch v0.3 > PRBool cardExists = PR_FALSE; > // don't want to abort the rest of the scoring. > if (!authorEmailAddress.IsEmpty()) >- rv = whiteListDirectory->HasCardForEmailAddress(authorEmailAddress, &cardExists); >+ { >+ for (PRInt32 index = 0; index < whiteListDirArray.Count() && !cardExists; index++) >+ rv = whiteListDirArray[index]->HasCardForEmailAddress(authorEmailAddress, &cardExists); >+ if (NS_FAILED(rv)) >+ cardExists = PR_FALSE; >+ } > if (NS_SUCCEEDED(rv) && cardExists) Nit: don't need the NS_SUCCEEDED any more.

Comment on attachment 262816 [details] [diff] [review] Space instead of comma patch v0.3 >+ var wlValue = ""; >+ if (document.getElementById("server.useWhiteList").checked) >+ wlValue = document.getElementById("server.whiteListAbURI").value; >+ >+ var currentArray = wlValue.split(" "); I think this should say var currentArray = []; if (...checked) currentArray = ...value.split(" "); >+ for (var i = 0; i < wList.childNodes.length; i++) >+ { >+ var wlNode = wList.childNodes[i]; >+ if (wlNode.localName == "listitem") >+ wlNode.checked = (currentArray.indexOf(wlNode.id) > -1); >+ } You could alternatively use wList.getRowCount() and wList.getItemAtIndex(i) thus saving you the listitem check. >Index: suite/locales/en-US/chrome/mailnews/pref/am-junk.dtd You forgot the TB file in this patch ;-)

Changes from v0.3: * use getRowCount/getItemAtIndex instead of listitem checking * included missed TB dtd diff Carrying forward r= and requesting sr=

Hi Ian, can you attach a screen shot?

This is how the new pref panel looks for Junk Settings

Comment on attachment 262929 [details] [diff] [review] listitem checkless patch v0.3a the patch looks good. One UI thought.. What do you think about removing the tree col that says "Address books"? I don't think it provides any extra information.

Changes from v0.3a: * Removed listheader (and associated entities) as suggested by mscott Carried forward r/sr Checking in (trunk) mailnews/base/prefs/resources/content/am-junk.js; new revision: 1.11; previous revision: 1.10 mailnews/base/prefs/resources/content/am-junk.xul; new revision: 1.5; previous revision: 1.4 mailnews/base/util/nsMsgDBFolder.cpp; new revision: 1.302; previous revision: 1.301 done

What's the status on this one now? I have Thunderbird 2.0.0.12, and the issue isn't solved there. Do I have to install the patch manually? Don't know how this works? - Please explain somebody - thanks!! :-)

This will be fixed in Thunderbird 3 (or an alpha of it).