203859 - Deleted address book reappear after an exit/restart

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Address Book & Contacts, defect)

Description

[Ninoschka Baca]

Trunk build 2003-04-29: WinXP, Mac 10.1.5

Overview: Delete a user defined address book and after an exit/restart the
address book reappears.

Steps to reproduce:
1. Create an address book (i.e. AB1)
2. Add cards, but it is not necessary
3. Exit/restart and the address book is still present (AB1)
4. Delete the address book
5. Exit/restart

Actual Results: 
- Address Books with cards: results in the address book name reappearing in the
left pane but the cards are gone.
- Address Book without cards: results in the address book name reappearing.

- In one session create an address book, then delete the address book. After an
exit/restart then this address book Is deleted.

Expected Results: The Address Book should not reappear.

Additional Information:
Bug 173430 most likely caused this regression according to Seth.

Comment 1

[Ninoschka Baca]

This was working in the 4/28 builds but regressed in the 4/29 builds.

Seth says this is a 1.4b blocker.

Comment 2

[Christopher Aillon (sabbatical, not receiving bugmail)]

Looking.

Comment 3

[Asa Dotzler [:asa]]

assuming nbaca meant to nominate and setting the correct flag for that.

Comment 4

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

blocking.  

the fix for bug #173430 has caused at least two regression, both we should fix
before shipping 1.4 beta.

Comment 5

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: Fix

This fixes both this bug and bug 203945.  The problem was an oversight on my
part.  nsDirPrefs relied on dupes being removed from the child list after the
trailing branch got chomped off and I forgot to account for that in my patch to
173430.  So this puts in equivalent code by adding a helper function here.  I
also did a little bit of cleanup since the code was pretty unreadable in a few
places.  diff -w to follow.

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Attachment: diff -w

Comment 7

[Asa Dotzler [:asa]]

Who can review this? Time is short on 1.4b.

Comment 8

[Benjamin Smedberg]

Comment on attachment 122194 [details] [diff] [review]
diff -w

>+    for (PRUint32 i = *aCount; i--;) {

Is this right? I don't see an exit from this loop anywhere.

caillon: timeless mentioned that you're looking for a hashtable to do the
dup-removal instead of that quicksort. It looks simple to me, I'm working on a
little demo code.

Comment 9

[Benjamin Smedberg]

m'apologies, I read the code wrong, it is of course correct.

Comment 10

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

assume a=sspitzer for 1.4 beta, once you get reviews.

chris and I have an agreement, that if there are more problems (once this fix
lands), that this and the original fix for bug #173430, will be backed out, and
will re-land in 1.5

thanks chris.

Comment 11

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

testing / reviewing that patch now.

Comment 12

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

Comment on attachment 122194 [details] [diff] [review]
diff -w

r/sr/a=sspitzer

Comment 13

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

note, chris tells me he's re-run flawfinder on this code, too.

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

Fix is in.

Comment 15

[esther]

Tested with build 200305002 on winxp, macosx and linux this is fixed. 
Note I tested deleting an LDAP directory too since I found that it had the same
 problem as address books.  Verified.

Comment 16

[[e-mail]]

Attachment: small optimization

this is a small optimization to the function dir_GetChildList we created. as it
stands, for a "typical end-user" (in this case, me ;) addrbook setup, the
number of entries returned by nsIPrefBranch::GetChildList is 60-odd, but the
number of unique entries is only 15. so memmoving for every non-unique entry is
quite slow - better approach is to move only unique entries (which is a simple
ptr assignment). yeah, i know this isn't really important in the scheme of
things, but i felt guilty about writing slow code. ;)

i've tested this and it looks to work just fine. it's not a biggie so it can
wait 'til whenever is convenient to land.

Comment 17

[[e-mail]]

Comment on attachment 122478 [details] [diff] [review]
small optimization

(i know this bug is v/fixed, but this patch really doesn't deserve its own bug,
so i'm just chucking it here.)

Comment 18

[[e-mail]]

ok, caillon thinks it does deserve its own bug. ;)

opened bug 204753 for it.