134743 - Turbo on, mailing list entries duplicated after an exit/restart

Status: VERIFIED FIXED

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

Description

[Ninoschka Baca]

Trunk build 2002-04-01: WinMe

Overview: With Turbo on, mailing list entries are duplicated after an
exit/restart. So a list with 2 entries will have 4 entries after an exit/restart. 

Steps to reproduce:
I. mailing list entries duplicated without opening the list:
1. Create a list with 2 entries
2. Exit/restart
3. Open Address Book
4. Select the mailing list in the Results Pane

Actual Results: The Card pane shows 4 entries. Then exit/restart and 6 entries
display so the pattern after each exit/restart is (ab, abab, ababab etc...)
Expected Results: It should only show 2 entries

II. mailing list entries duplicated with opening the list and a new base is
established:
1. Create a list with 2 entries
2. Double click onto the list so it opens in a list dialog (I think this creates
a new base so in this case it's 4)
3. Exit/restart
4. Open Address Book
5. Select the mailing list in the Results Pane

Actual Results: 
a. The Card Pane shows 4 entries (don't open the list)
- Exit/Restart, now 8 entries 
- Exit/Restart, now 12 entries so it adds in increments of 4 (aka the base is 4)
b. If I open the list with 4 entries (open the list)
- Exit/restart then it displays 8 entries (open the list)
- Exit/restart, now it dispays 16 entries

Expected Results: It should only show 2 entries

Additional Information: 
- Closing/reopening the Address Book window does not display the above problems.

Comment 1

[Ninoschka Baca]

Marking nsbeta1 because this should be fixed for this release. Otherwise the
mailing lists will be unusable due to all the dupes in a list.

Comment 2

[Michael Buckland]

Discussed in Mail News bug meeting.  Decided to ADT2 and plus this bug.

Comment 3

[scottputterman]

reassigning to Cavin.

Comment 4

[Cavin Song]

I found the cause of the problem. 'm_AddressList' in nsAbMDBDirProperty object 
is not reset in turbo mode. Now I need to figure out what addrbook method is 
invoked in turbo mode so that it can be reset.

Comment 5

[Conrad Carlen (not reading bugmail)]

See my comment on bug 124208. Sounds like the same problem. We just need to
decide if the addressbook service as a whole should observe profile changes,
just nsAbMDBDirProperty, or what.

Comment 6

[Cavin Song]

Ok great. Let's work on this together. I think there are other things that need 
to be cleaned up too.

Comment 7

[Conrad Carlen (not reading bugmail)]

> Ok great. Let's work on this together.

How's your knowlege of the address book code? What I'm trying to figure out is
what exactly has to be reset when a profile switches. For reference on that,
see: http://lxr.mozilla.org/mozilla/source/profile/public/nsIProfileChangeStatus.idl

I'm thinking that on a "profile-before-change" notification, the addressbook DB
which was read from the profile dir should be closed. Question is, how does this
DB closing affect consumers of that data. Will nsIAbListener take care of this?

Comment 8

[Cavin Song]

Ccing Seth for Conrad's question.

Comment 9

[Cavin Song]

Seems like 'mSubDirectories' in nsAbMDBDirectory object is not cleaned up 
either. This may be the cause of other related turbo mode addrbook bugs.

Comment 10

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

taking.

Comment 11

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

accepting.  I have a feeling this will be fixed by my fix for #124208

Comment 12

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

not fixed by the other bug, but that fix is needed.

I've got the fix for this, though.  patch coming.

Comment 13

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

Attachment: patch

Comment 14

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

Attachment: complete patch.

Comment 15

[racham]

Comment on attachment 79732 [details] [diff] [review]
complete patch.

r=bhuvan

Comment 16

[David :Bienvenu]

Comment on attachment 79732 [details] [diff] [review]
complete patch.

sr=bienvenu

Comment 17

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

fixed on trunk.

Comment 18

[Jaime Rodriguez, Jr.]

nbaca - Can you verify this one today, and chekc to see if this causes any
regressions?

Comment 19

[Ninoschka Baca]

Trunk build 2002-04-19-03: WinMe
Verified Fixed. With turbo on, the lists retain the same number of addresses
even after an exit restart (although there is a different list problem which
appears in  today's build which is logged in bug# 138647)

Comment 20

[scottputterman]

Seth, is the new bug that Ninoschka mentions related to this checkin?

Comment 21

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

> Seth, is the new bug that Ninoschka mentions related to this checkin?

I don't know yet.  I'll let you know as soon as I get to bug #138647

Comment 22

[Jaime Rodriguez, Jr.]

adt1.0.0+ (on ADT's behalf) for checkin to 1.0. Pls check this in today, after
you recieve a=, then add the fixed1.0.0 keyword.

Comment 23

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

It looks likely that my fix for this caused regression bug #138647.

I'll fix the regression tomorrow, and have something suitable for the branch by
then.

Comment 24

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

this change:

 nsAbDirProperty::~nsAbDirProperty(void)
 {
+  if (m_AddressList) {
+    PRUint32 count;
+    nsresult rv;
+    rv = m_AddressList->Count(&count);
+    NS_ASSERTION(NS_SUCCEEDED(rv), "Count failed");
+    PRInt32 i;
+    for (i = count - 1; i >= 0; i--)
+      m_AddressList->RemoveElementAt(i);
+  }
 }

caused the regression.  looking into it.

Comment 25

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

Attachment: patch, minus the regression causing code.

Comment 26

[racham]

Comment on attachment 81200 [details] [diff] [review]
patch, minus the regression causing code.

r=bhuvan

Comment 27

[Scott MacGregor]

Comment on attachment 81200 [details] [diff] [review]
patch, minus the regression causing code.

sr=mscott

Comment 28

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 81200 [details] [diff] [review]
patch, minus the regression causing code.

a=dbaron for 1.0 branch checkin

Comment 29

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

fixed on branch as well.

Comment 30

[Ninoschka Baca]

Branch build 2002-04-29-06: WinMe
Reopening, sorry I'm seeing the same problem on the branch.

Comment 31

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

accepting.

Comment 32

[Ninoschka Baca]

To clarify: The 4/29 branch build displays the problem the 4/29 trunk build is ok.

Comment 33

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

here's an update:

nbaca's shown me this problem on her winME machine, with the 4-29 commercial 
branch bits, but we don't see it on my win2k machine or sheelar's win98 box.

some possible solutions:

1)  fix the code to be more aggressive with respect to resetting state during 
shutdown, with turbo.
2)  fix the code to never add duplicates (side effects, if you try to create a 
mailing list with duplicates, the duplicates will be ignored)
3)  have nbaca try again after jan's fix for #135002 lands on the branch.  (off 
chance, it might be related?)
4)  remove the nsVoidArray of addresses, and just have the entries in memory 
once.

I'll discuss these four options with ADT tomorrow.

nbaca plans to try on other winME systems, and also try the test builds for 
#135002

Comment 34

[Ninoschka Baca]

We can mark this bug fixed on the branch for now since I can only reproduce the
problem on my WinMe system using the 04/29 build. I cannot reproduce it in a new
4/29 installation on my system, a WinMe system in the lab, a Win98 or Win2k
system. I must have done something to trigger the problem but not sure what that
is right now. 

Here are more details if interested:

Branch build 2002-04-30-06: WinMe, ok.
I renamed my Mozilla directory, created new profile, opened AB window, created a
list, close/reopen AB window to see the expand widget in dir pane, expanded the
widget, turned on Turbo, exit/restart and now it's ok...I see the correct number
of entries in the list (they are not duplicated). 

Branch build 2002-04-29 original installation: WinMe, problem occurs.
In my original installation of 4/29 I performed the above steps and was able to
reproduce the problem easily. Tried another WinMe system in the lab using the
same build and could not reproduce the problem.

Branch build 2002-04-29, reinstalled in a new directory: WinMe
Tried same scenario and could not reproduce the problem.

Comment 35

[Ninoschka Baca]

Verified Fixed for now since I can't reproduce on the latest branch build and I
don't see the problem on the trunk.

Comment 36

[Jaime Rodriguez, Jr.]

This says verified1.0.0. Did this get on the branch without a adt1.0.0+? It
appears to be fixed per Ninoschka last comment. hmmmmmmmmmm ...

Comment 37

[Peter Trudelle]

Did this ever happen with only one profile?

Comment 38

[Jaime Rodriguez, Jr.]

Marking fixed per Ninoschka verification.

Comment 39

[Ninoschka Baca]

Checked Branch build 2002-05-20 on WinMe and the problem still no longer
appears. Removing "fixed1.0.0" and keeping "verified1.0.0" for Keywords.