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

VERIFIED FIXED in mozilla1.0

Status

SeaMonkey
MailNews: Address Book & Contacts
VERIFIED FIXED
16 years ago
13 years ago

People

(Reporter: Ninoschka Baca, Assigned: (not reading, please use seth@sspitzer.org instead))

Tracking

Trunk
mozilla1.0
x86
Windows ME
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ADT1] [eta 05/03])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

16 years ago
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.
(Reporter)

Comment 1

16 years ago
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.
Keywords: nsbeta1

Comment 2

16 years ago
Discussed in Mail News bug meeting.  Decided to ADT2 and plus this bug.
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [ADT2]
Target Milestone: --- → mozilla1.0

Comment 3

16 years ago
reassigning to Cavin.
Assignee: racham → cavin

Comment 4

16 years ago
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.
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

16 years ago
Ok great. Let's work on this together. I think there are other things that need 
to be cleaned up too.
(Reporter)

Updated

16 years ago
Blocks: 136757
(Reporter)

Updated

16 years ago
Blocks: 128661
> 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

16 years ago
Ccing Seth for Conrad's question.

Comment 9

16 years ago
Seems like 'mSubDirectories' in nsAbMDBDirectory object is not cleaned up 
either. This may be the cause of other related turbo mode addrbook bugs.
taking.
Assignee: cavin → sspitzer
accepting.  I have a feeling this will be fixed by my fix for #124208
Status: NEW → ASSIGNED
Depends on: 124208
not fixed by the other bug, but that fix is needed.

I've got the fix for this, though.  patch coming.
Whiteboard: [ADT2] → [ADT2] [fix in hand]
Created attachment 79732 [details] [diff] [review]
complete patch.
Attachment #79729 - Attachment is obsolete: true

Comment 15

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

r=bhuvan
Attachment #79732 - Flags: review+
Whiteboard: [ADT2] [fix in hand] → [ADT2] [fix in hand, have r=, awaiting sr=]

Comment 16

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

sr=bienvenu
Attachment #79732 - Flags: superreview+
Whiteboard: [ADT2] [fix in hand, have r=, awaiting sr=] → [ADT2] [fix in hand, have r=, sr=, will land on trunk today]
fixed on trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Keywords: adt1.0.0
Resolution: --- → FIXED
Whiteboard: [ADT2] [fix in hand, have r=, sr=, will land on trunk today] → [ADT2] [fixed on trunk]

Comment 18

16 years ago
nbaca - Can you verify this one today, and chekc to see if this causes any
regressions?
(Reporter)

Comment 19

16 years ago
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)
Status: RESOLVED → VERIFIED

Comment 20

16 years ago
Seth, is the new bug that Ninoschka mentions related to this checkin?
> 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

16 years ago
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.
Keywords: adt1.0.0 → adt1.0.0+, approval
Whiteboard: [ADT2] [fixed on trunk] → [ADT2] [fixed on trunk] [Needs a=]
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.
Whiteboard: [ADT2] [fixed on trunk] [Needs a=] → [ADT2] [fixed on trunk] [eta for branch 4/27]
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.
Created attachment 81200 [details] [diff] [review]
patch, minus the regression causing code.
Attachment #79732 - Attachment is obsolete: true

Comment 26

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

r=bhuvan
Attachment #81200 - Flags: review+

Comment 27

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

sr=mscott
Attachment #81200 - Flags: superreview+
Comment on attachment 81200 [details] [diff] [review]
patch, minus the regression causing code.

a=dbaron for 1.0 branch checkin
Attachment #81200 - Flags: approval+
fixed on branch as well.
Keywords: fixed1.0.0
Whiteboard: [ADT2] [fixed on trunk] [eta for branch 4/27] → [ADT2]
(Reporter)

Comment 30

16 years ago
Branch build 2002-04-29-06: WinMe
Reopening, sorry I'm seeing the same problem on the branch.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
accepting.
Status: REOPENED → ASSIGNED
(Reporter)

Comment 32

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

Updated

16 years ago
Keywords: adt1.0.0+, fixed1.0.0 → adt1.0.0
Whiteboard: [ADT2] → [ADT1]
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
Whiteboard: [ADT1] → [ADT1] [eta 5/1]
(Reporter)

Updated

16 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago16 years ago
Resolution: --- → FIXED
(Reporter)

Comment 34

16 years ago
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.
(Reporter)

Comment 35

16 years ago
Verified Fixed for now since I can't reproduce on the latest branch build and I
don't see the problem on the trunk.
Status: RESOLVED → VERIFIED
Keywords: verified1.0.0

Comment 36

16 years ago
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 ...
Whiteboard: [ADT1] [eta 5/1] → [ADT1] [eta 05/03]

Comment 37

16 years ago
Did this ever happen with only one profile?

Comment 38

16 years ago
Marking fixed per Ninoschka verification.
Keywords: fixed1.0.0

Updated

16 years ago
Keywords: adt1.0.0 → adt1.0.0+
(Reporter)

Comment 39

16 years ago
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.
Keywords: fixed1.0.0
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.