Closed
Bug 198303
Opened 21 years ago
Closed 21 years ago
Deleted cards still show up for junk mail whitelisting, people I know views.
Categories
(SeaMonkey :: MailNews: Address Book & Contacts, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
mozilla1.4beta
People
(Reporter: esther, Assigned: sspitzer)
References
Details
(Keywords: regression, Whiteboard: [adt2])
Attachments
(1 file, 7 obsolete files)
20.84 KB,
patch
|
Details | Diff | Splinter Review |
Using trunk build 20030318 on winxp, macosx and linux if you delete a card from the Personal Address book, it is still in the abook.mab file when viewing with notepad. This breaks the Junk Mail whitelisting against the personal Address book in some cases. Bug 196777 may have broken this. 1. create a new profile 2. add a mail account 3. add your address to the Personal Address book 4. Exit and relaunch 5. Delete your card from the Personal Address Book. Exit 6. View your abook.mab file with an editor, see that your email address is in that file. Expected: if you delete your card from the address book your email address should be deleted from that file too.
nominating and reassigning
Comment 2•21 years ago
|
||
Mail triage team: nsbeta1+/adt2
Assignee | ||
Comment 3•21 years ago
|
||
accepting, but it isn't a regression. (or it is, but it's not from my checkin.) here's the problem: we keep track of deleted cards in the mab when we do autocomplete, or mail views, or whitelisting, we ask the PAB: "do you have a row for email == xyz@foo.com" rows are not bound to a table, and a mab can have multiple tables (say "cards" and "deleted cards"), and a row can be in both. the fix is to make sure the card is not in the deleted table, if it is, pretend it isn't there.
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•21 years ago
|
||
Assignee | ||
Comment 5•21 years ago
|
||
here's the "fix" part of this bug: + // fix for bug #198303 + // here's the problem: + // for palm and absync, we keep track of deleted cards in the mab + // when we do autocomplete, or mail views, or whitelisting, we ask the PAB: + // "do you have a row for email == xyz@foo.com?" + // rows are not bound to a table, and a mab can have multiple tables (say "cards" + // and "deleted cards"), and a row can be in both. + // the fix is to make sure the card is not in the deleted table, + // if it is, act like we didn't find it. + if (mdbRow) + { + nsresult rv; + // we might not have loaded the "delete cards" table yet + // so do that (but don't create it, if we don't have one), + // so we can see if the row is really a delete card. + if (!m_mdbDeletedCardsTable) + rv = InitDeletedCardsTable(PR_FALSE); + mdb_bool hasRow = PR_FALSE; + rv = m_mdbDeletedCardsTable->HasRow(env, mdbRow, &hasRow); + if (NS_SUCCEEDED(rv) && hasRow) + *aFindRow = nsnull; + else + NS_IF_ADDREF(*aFindRow = mdbRow); + } but I'll ask cavin to review it all.
Assignee | ||
Updated•21 years ago
|
Attachment #117966 -
Flags: review?(cavin)
Assignee | ||
Comment 6•21 years ago
|
||
whoops, I've removed that spurious comment: + //m_mdbPabTable
Assignee | ||
Comment 7•21 years ago
|
||
Attachment #117966 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #117971 -
Flags: review?(cavin)
Assignee | ||
Updated•21 years ago
|
Attachment #117966 -
Flags: review?(cavin)
Assignee | ||
Comment 8•21 years ago
|
||
Attachment #117971 -
Attachment is obsolete: true
Assignee | ||
Updated•21 years ago
|
Attachment #117971 -
Flags: review?(cavin)
Assignee | ||
Updated•21 years ago
|
Attachment #117989 -
Flags: review?(cavin)
Assignee | ||
Updated•21 years ago
|
Attachment #117989 -
Attachment is obsolete: true
Attachment #117989 -
Flags: review?(cavin)
Assignee | ||
Comment 9•21 years ago
|
||
Assignee | ||
Updated•21 years ago
|
Attachment #117995 -
Flags: review?
Assignee | ||
Updated•21 years ago
|
Target Milestone: --- → mozilla1.4alpha
Assignee | ||
Comment 10•21 years ago
|
||
bugs I should test: #146482, #183416, #73814 there could be many places where we were getting deleted data.
Comment 11•21 years ago
|
||
Comment on attachment 117995 [details] [diff] [review] updated patch, fixes a crash when no deleted table. r=cavin. One minor nit: - nsresult err = NS_OK; - nsIMdbRow *cardRow; + nsresult rv = NS_OK; + nsCOMPtr <nsIMdbRow> cardRow; if (!newCard || !m_mdbPabTable) return NS_ERROR_NULL_POINTER; - err = GetNewRow(&cardRow); + rv = GetNewRow(getter_AddRefs(cardRow)); we can just do: nsresult rv = GetNewRow(getter_AddRefs(cardRow));
Attachment #117995 -
Flags: review? → review+
Assignee | ||
Comment 12•21 years ago
|
||
good catch, I'll fix that before sr and checkin.
Assignee | ||
Comment 13•21 years ago
|
||
fixed.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Summary: [junk] Deleting address cards from Personal address book doesn't delete the entry in abook.mab file, causing junk mail whitelisting to break → Deleted cards still show up for junk mail whitelisting, people I know views.
Reporter | ||
Comment 14•21 years ago
|
||
Testing this on winxp with build 20030327, this is partially fixed. If I remove a card from the address book we do the right thing for White Listing and Mail Views "People I Know" but if I add that same card back into my Personal Address Book it won't recognize it as "People I Know" or for White Lisitng Junk Mail. Reopening
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 15•21 years ago
|
||
good catch. right, a card can be in both tables (if you add, delete, and add), so the right fix is to check the good table, instead of the bad table. here comes a patch...
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 16•21 years ago
|
||
Assignee | ||
Comment 17•21 years ago
|
||
Attachment #118782 -
Attachment is obsolete: true
Assignee | ||
Comment 18•21 years ago
|
||
so, the add / delete / add scenario that esther found points out more issues with the addrbook on top of mork. I've got a patch that addresses it, but this deleted table (for palm sync) has really caused some problems. I'll continue to test, and look over the mailing list side of this fix, and report back.
Attachment #118786 -
Attachment is obsolete: true
Assignee | ||
Comment 19•21 years ago
|
||
Attachment #117995 -
Attachment is obsolete: true
Attachment #118833 -
Attachment is obsolete: true
Assignee | ||
Comment 20•21 years ago
|
||
fixed. this patch also touches some code that can be hit when you try to create mailing lists. to test it, I did this: 1) create a mailing list called "foo" 2) try to create a second mailing list called "foo" (should get alert, can't have two of the same name) 3) delete "foo" 4) try to create a mailing list called "foo" again (shouldn't get alert) esther already knows the other tests, involving cards.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago → 21 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.4alpha → mozilla1.4beta
Comment 21•21 years ago
|
||
the checkin here busted builds without LDAP support. nsAddrDatabase.cpp now requires nsIPromptService.h, which is in the windowwatcher include directory. But the Makefile only grabs stuff from that directory when LDAP is enabled.
Assignee | ||
Comment 22•21 years ago
|
||
I'll fix the build bustage today. Index: Makefile.in =================================================================== RCS file: /cvsroot/mozilla/mailnews/addrbook/src/Makefile.in,v retrieving revision 1.43 diff -u -w -r1.43 Makefile.in --- Makefile.in 19 Apr 2003 05:13:31 -0000 1.43 +++ Makefile.in 25 Apr 2003 15:13:22 -0000 @@ -52,6 +52,7 @@ appcomps \ intl \ import \ + windowwatcher \ $(NULL) CPPSRCS = \ @@ -136,7 +137,6 @@ ifdef MOZ_LDAP_XPCOM REQUIRES += mozldap \ uriloader \ - windowwatcher \ $(NULL) EXPORTS += nsAbLDAPDirectoryQuery.h \
Assignee | ||
Comment 23•21 years ago
|
||
build bustage should be fixed now.
Reporter | ||
Comment 24•21 years ago
|
||
using trunk build 20030428 on winxp, macosx and linux the origianl test scenario and the scenario mentioned in comment 20 this is verified fixed.
Status: RESOLVED → VERIFIED
Updated•20 years ago
|
Product: Browser → Seamonkey
You need to log in
before you can comment on or make changes to this bug.
Description
•