Closed Bug 134590 Opened 22 years ago Closed 16 years ago

(Temporarily) Duplicated Address Listing

Categories

(MailNews Core :: Address Book, defect)

Product:
MailNews Core
Component:
Address Book
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla1.9

People

(Reporter: youying, Assigned: standard8)

References

Details

Attachments

(3 files, 3 obsolete files)

Wrong display in address book, lots of duplication address.
84.33 KB, image/jpeg
Details
Comment 39 result (image)
29.50 KB, image/jpeg
Details
[checked in] Fix for case in c39
1.24 KB, patch
standard8
: review+
neil
: superreview+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:0.9.9+) Gecko/20020331
BuildID:    2002033109

After making Mail List in AddressBook, lots of duplicated lists.

Reproducible: Always
Steps to Reproduce:
1.Make an address book
2.In the same address book, make some mail lists and each list has the same
email address.
3.See the address book you made, and lot duplication occures.

Actual Results:  No duplication.
*** Bug 141902 has been marked as a duplicate of this bug. ***
Detail reproduce way:

Steps to Reproduce:
1. Open AddressBook
2. Create 2 mail-lists in the same address book, with the same people,
   for example, in "Collected Address"
   mail-list-a: aaa@xxx.com
                bbb@xxx.com
   mail-list-b: aaa@xxx.com
                bbb@xxx.com
                ccc@xxx.com
3. click on "Collected Address" 
   you will see the deplicated display address listed.
   aaa@xxx.com
   aaa@xxx.com
   bbb@xxx.com
   bbb@xxx.com
   ccc@xxx.com
Now RC3 is out.
Will this bug be fixed ????

*** This bug has been confirmed by popular vote. ***
Status: UNCONFIRMED → NEW
Ever confirmed: true
I have not been able to reproduce the problem if I type the addresses in the
list dialog. I can reproduce if I perform a Quick Search, then drag-n-drop the
card to a  list. If this is the case then it is a duplicate of bug# 150692.
This still seems to occur in 1.4a.
mass re-assign.
Assignee: racham → sspitzer
On my systems it appears this regression occured between 2003-02-27 and the
2003-02-28 builds. This problem appears on all platforms.

Netscape 7.02: ok
build 2003-02-27: ok
build 2003-02-28: bad

Steps to reproduce:
1. add two cards to an address book (i.e. 1@test.com, 2@test.com)
2. create a list and add the two cards to the list (via autocomplete or typing
the addresses)
3. close the list

Actual Results: For each entry in the list, a duplicate card appears:

1@test.com
1@test.com
2@test.com
3@test.com

Resizing the window doesn't appear to force a repaint. Change folders or perform
a quick search/clear quick search and now I see the original cards and the list
so the duplicate entries no longer appear.
Mozilla.org looks like not care MailNews.
This bug exists for long long long time.
Still no patch...

M$ does not release any formal version with such trivial user interface bug.
Alas...
*** Bug 162934 has been marked as a duplicate of this bug. ***
*** Bug 248329 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
*** Bug 271523 has been marked as a duplicate of this bug. ***
Workaround: After creating lists, close and restart mozilla, duplication goes away.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8a6) Gecko/20041129
*** Bug 239144 has been marked as a duplicate of this bug. ***
I'm looking into this at the moment, therefore taking bug for time being, cc'ing
seth as the original owner.
Assignee: sspitzer → mark
*** Bug 273717 has been marked as a duplicate of this bug. ***
Blocks: 35837
This bug is still present in version 1.0 (20041206) Windows.
This bug centres around the fact that when we add a card to a mailing list we
need to add it to the view - if it is showing a mailing list that is. If it is
showing an address book, then we shouldn't be adding it, and ideally sending
just an item property changed notification round.

However I can't see a way of easily getting that to work, except by checking
that what is added isn't in the view already which is what this patch does.

As my belief is that this bug should block the drag and drop one until it's
fixed (because otherwise users will be dragging to lists and getting duplicates
appear in the view and get even more confused) what do we think about putting
this one in as wallpaper for the time being? (hence the XXX comment).
Attachment #177176 - Flags: review?(neil.parkwaycc.co.uk)
Comment on attachment 177176 [details] [diff] [review]
Patch to stop duplicate emails being added to the view

Sorry about this I need to reconsider this as it hides the fact of new cards
being added with duplicate email addresses (which is bug 45946).
Attachment #177176 - Flags: review?(neil.parkwaycc.co.uk)
I didn't think it was a good idea anyway, although I can't see a way of fixing
this without adding the new parent as part of the notification.
This has become a strategic bug :
https://bugzilla.mozilla.org/showdependencytree.cgi?id=134590 . Nominating for
1.1 . Also, could someone with the appropriate rights correct the OS to "all" ?

Isn't it possible to mimick the workaround of comment #14 with some sort of reload ?
Flags: blocking-aviary1.1?
Although a reload could be done it's not that simple and would be time consuming
for an address book with lots of cards. It's just not the right fix (I've
already considered it).

The proper fix is probably what Neil was getting at and what I am looking into
when I have time - send the parent who initiated the change around. This may
also resolve some other issues we have when we delete cards if we get it right.
Status: NEW → ASSIGNED
OS: Windows 98 → All
Component: Address Book → MailNews: Address Book
Product: Mozilla Application Suite → Core
QA Contact: nbaca → addressbook
Can reproduce bug with Thunderbird (win) version 1.5 Beta 1 (20050908).
See an additional danger: when I delete one of the dupes, I delete both.
When I switch view to any other and switch back, the dupes are gone.
A reload seems not necessary. A refresh of the view or something the like 
should do it. 


I am not a code wizard, but my thougts are as follows:
There are two concepts to solve such problem:

(a) the strictly hierarchichal, secure but not very elegant way
(b) the quick, but somehow more complex way

Way (a) goes like this:

1. Addition of address to mailing list causes addition of address to address 
book, if not already existing. As far as I see, this is the case.
2. View is not directly manipulated by any manipulation of mailing list, but is 
dependent only on address book plus filtering by mailing list, if necessary. So 
if any of both is changed, a signal to refresh the view has to be sent. Data 
stock is always consistent. View is always consistent with data stock.

Way (b) goes like:

1. As in (a).
2. View is directly changed by change in mailing list, but conditionally: a new 
address is only added, if it belongs to the view and if it is not already 
there. This is the way mentioned by Mark in comment #19.

This patch only notifies of an update to the existing card in the database - in case we decide to add mailing list info to the existing card or something to that effect.  This fixes the GUI issue of seeing two cards, even though only one is present in the database.
Please also attach a cvs diff -u -w
fixing whitespace diff problem w/ first patch.
Attachment #203297 - Flags: review+
Attachment #203297 - Flags: review+ → review?(bugzilla)
Attachment #177176 - Attachment is obsolete: true
Attachment #203296 - Attachment is obsolete: true
Comment on attachment 203297 [details] [diff] [review]
Better patch file for no insert notification - no whitespace differences

Sorry, but this doesn't actually improve matters much. I've put together some test cases (see below) and with this patch we're basically shifting the problem from one area to another :-(

Also:
+    else if (!aInMailingList) { //we didn't perform an insert here, mailing list notifications will get sent too
+      NotifyCardEntryChange(AB_NotifyPropertyChanged, newCard);

Please ensure comments are on a new line as its clearer and cuts down on the line length.

Test cases:
1) Create new card in address book
2) Create new card via mailing list with address book selected
3) Create new card via mailing list with mailing list selected
4) Add card to mailing list with address book selected
5) Add card to mailing list with mailing list selected
6) Add card to address book via sidepanel with address book selected in address book window.
7) Add card to address book via sidepanel with mailing list selected in address book window.
8) Add card to mailing list via sidepanel with address book selected in address book window.
9) Add card to mailing list via sidepanel with mailing list selected in address book window.

With [https://bugzilla.mozilla.org/attachment.cgi?id=203297&action=edit patch 203297] we fails on 5 (doesn't get added to mailing list view), 7 (card incorrectly gets added to mailing list view), 9 (card doesn't get added to mailing list view).

Without that patch we fail on 4 (duplicated card in address book view), 7 (card incorrectly gets added to mailing list view), 8 (duplicated card in address book view).

Finally, I'm not sure if finding a way to fix this warning may help with this problem, it occurs with & without the patch, and I can see why, but I've not thought of a way of fixing it yet:

WARNING: NS_ENSURE_TRUE(NS_SUCCEEDED(err)) failed, file /opt/mozmaster/mozilla/mailnews/addrbook/src/nsAddrDatabase.cpp, line 2212
Attachment #203297 - Flags: review?(bugzilla) → review-
*** Bug 327117 has been marked as a duplicate of this bug. ***
Not sure this adds anything new, but I can still reproduce this bug in TB 1.5.0.5 (20060719) on Windows, as follows: create a new address book; add some cards to it; make sure the address book is selected; click New List; use the Mailing List dialog to name the new list /and/ to assign one of the cards to it - a duplicate of that card appears in the address book; deselect the address book (e.g. by selecting the list) and reselect it - the duplicate disappears. Assigning cards to the list by reopening the Mailing List dialog or by drag-and-drop causes no duplicates.
Still seeing this bug in Thunderbird version 3.0a1pre (2008022703), OS X. Adding any new entry to an existing list (either by dragging, or typing in list properties view) adds a duplicate card. Deleting the duplicate also deletes entry from list. For what it's worth the list was created in Thunderbird 2 (can't remember if OS X or Win, I use both and copy address book back and forth) and this duplication did not occur in the original, only is appearing now. Regression? This should be fixed.
Depends on: 406921
Update. Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008041603 Thunderbird/3.0a1pre ID:2008041603 nightly seems to have mostly resolved this issue, except that adding an alternate email to a list still duplicates the address book card. This should not happen.
All the test cases mentioned in comment 29 have been fixed by attachment 320906 [details] [diff] [review] on bug 406921 that was just checked in. This will be included in the next release of Thunderbird (3.x)/Shredder alpha 2, or SeaMonkey 2.0 alpha 1. It cannot be put on to the 2.x branch because of the level of changes involved.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008051803 Thunderbird/3.0a2pre] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008051802 SeaMonkey/2.0a1pre] (nightly) (W2Ksp4)

This bug still occurs. (See bug 434227 comment 1 as an example).
The issue is display only:
displaying another address book then coming back fixes it.
(In reply to comment #35)
> This bug still occurs. (See bug 434227 comment 1 as an example).
> The issue is display only:
> displaying another address book then coming back fixes it.

No. Bug 434227 which got duped to bug 189895 (correctly) is NOT this bug. This bug was about adding cards *without* the same email address to a mailing list.

Additionally you say that going to another one and coming back fixes the bug 434227 case - it doesn't, but lets address that in the proper location.
(In reply to comment #36)
> No. Bug 434227 which got duped to bug 189895 (correctly) is NOT this bug. This
> bug was about adding cards *without* the same email address to a mailing list.

Correct. And that's what my comment 35 is about:
Create a new mailing list which includes two cards with different (Display Names and) Email Addresses.

> Additionally you say that going to another one and coming back fixes the bug
> 434227 case - it doesn't, but lets address that in the proper location.

No, I said that bug 434227 steps (still) triggered both bug 189895 and this bug ... in current nightlies.
(In reply to comment #37)
> (In reply to comment #36)
> > No. Bug 434227 which got duped to bug 189895 (correctly) is NOT this bug. This
> > bug was about adding cards *without* the same email address to a mailing list.
> 
> Correct. And that's what my comment 35 is about:
> Create a new mailing list which includes two cards with different (Display
> Names and) Email Addresses.

Well please provide exact steps to reproduce this, because it is working fine for me here.

> > Additionally you say that going to another one and coming back fixes the bug
> > 434227 case - it doesn't, but lets address that in the proper location.
> 
> No, I said that bug 434227 steps (still) triggered both bug 189895 and this bug
> ... in current nightlies.

Bug 189895 is a *completely* different issue to this one.
Attachment #203297 - Attachment is obsolete: true
(In reply to comment #38)
> Well please provide exact steps to reproduce this, because it is working fine
> for me here.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008051803 Thunderbird/3.0a2pre] (nightly) (W2Ksp4)

1. Create (dumb) profile.
2. Create (dumb) email account.
3. Open Address Book.
4. Create a card.
5. Create a list, "autocompleting" it with the card.
5r. This bug !

> Bug 189895 is a *completely* different issue to this one.

*I know*: I'm merely saying that since I'm seeing this bug ... I see it two when I test bug 189895 :-|
(In reply to comment #39)
> 1. Create (dumb) profile.
> 2. Create (dumb) email account.
> 3. Open Address Book.
> 4. Create a card.
> 5. Create a list, "autocompleting" it with the card.
> 5r. This bug !

Ok, I see it now. Not sure why I didn't to begin with.

Siva, any idea how we managed to not fix this with bug 406921, or is this a case we missed?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Yes, it seems I have missed this case.

I found the problem also. It is because the wrong parent was sent to AddListAttributeColumnsToRow() call in nsAddrDatabase::CreateMailListAndAddToDB.
Currently the parent of the list is sent. Instead it should be the list itself.

I tested this one and old cases after this change. Should I add the patch here?
(In reply to comment #41)
> Should I add the patch here?

Yes, please.
Attachment #321618 - Flags: review?(bugzilla)
Comment on attachment 321618 [details] [diff] [review]
[checked in] Fix for case in c39

Yes, that seems to fix the problem. Thanks.
Attachment #321618 - Flags: superreview?(neil)
Attachment #321618 - Flags: review?(bugzilla)
Attachment #321618 - Flags: review+
(In reply to comment #41)
> I found the problem also. It is because the wrong parent was sent to
> AddListAttributeColumnsToRow() call in
> nsAddrDatabase::CreateMailListAndAddToDB.
> Currently the parent of the list is sent. Instead it should be the list itself.
Then should the aParent parameter be removed (as a followup)?
Attachment #321618 - Flags: superreview?(neil) → superreview+
(In reply to comment #45)
> Then should the aParent parameter be removed (as a followup)?

This parameter is (= would be) still used at line 1733...
{{
1715 NS_IMETHODIMP nsAddrDatabase::CreateMailListAndAddToDB(nsIAbDirectory *aNewList, PRBool aNotify /* = FALSE */, nsIAbDirectory *aParent)

1725         AddListAttributeColumnsToRow(aNewList, listRow, aParent);

1733         NotifyCardEntryChange(AB_NotifyInserted, listCard, aParent);
}}
Comment on attachment 321618 [details] [diff] [review]
[checked in] Fix for case in c39

Checking in mailnews/addrbook/src/nsAddrDatabase.cpp;
/cvsroot/mozilla/mailnews/addrbook/src/nsAddrDatabase.cpp,v  <--  nsAddrDatabase.cpp
new revision: 1.166; previous revision: 1.165
Attachment #321618 - Attachment description: Fix for case in c39 → [checked in] Fix for case in c39
(In reply to comment #46)
> (In reply to comment #45)
> > Then should the aParent parameter be removed (as a followup)?
> 
> This parameter is (= would be) still used at line 1733...
> {{
> 1715 NS_IMETHODIMP nsAddrDatabase::CreateMailListAndAddToDB(nsIAbDirectory
> *aNewList, PRBool aNotify /* = FALSE */, nsIAbDirectory *aParent)
> 
> 1725         AddListAttributeColumnsToRow(aNewList, listRow, aParent);
> 
> 1733         NotifyCardEntryChange(AB_NotifyInserted, listCard, aParent);
> }}
> 
That's correct, so I don't think we remove that parameter.

Closing as fixed again, as I believe this time its fixed for real.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
(In reply to comment #46)
> (In reply to comment #45)
> > Then should the aParent parameter be removed (as a followup)?
> This parameter is (= would be) still used at line 1733...
> 1725         AddListAttributeColumnsToRow(aNewList, listRow, aParent);
> 1733         NotifyCardEntryChange(AB_NotifyInserted, listCard, aParent);
But aParent == aNewList so we can just replace it with aNewList everywhere.
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9pre) Gecko/2008052007 SeaMonkey/2.0a1pre] (SEA-WIN32-TBOX-trunk) (W2Ksp4)

V.Fixed.

*****

(In reply to comment #49)
> But aParent == aNewList so we can just replace it with aNewList everywhere.

(I don't know:)
FWIW:
<http://mxr.mozilla.org/mozilla/search?string=CreateMailListAndAddToDB>
{{
/mailnews/addrbook/public/nsIAddrDatabase.idl (View change log or blame annotations)
    * line 198 -- void createMailListAndAddToDB(in nsIAbDirectory aNewList, in boolean aNotify, in nsIAbDirectory aParent);

/mailnews/addrbook/src/nsAbMDBDirectory.cpp (View change log or blame annotations)
    * line 656 -- mDatabase->CreateMailListAndAddToDB(newlist, PR_TRUE, this);
    * line 659 -- mDatabase->CreateMailListAndAddToDB(list, PR_TRUE, this);

/mailnews/addrbook/src/nsAddrDatabase.h (View change log or blame annotations)
    * line 78 -- NS_IMETHOD CreateMailListAndAddToDB(nsIAbDirectory *newList, PRBool notify, nsIAbDirectory *parent);

/mailnews/addrbook/src/nsAddrDatabase.cpp (View change log or blame annotations)
    * line 1714 -- NS_IMETHODIMP nsAddrDatabase::CreateMailListAndAddToDB(nsIAbDirectory *aNewList, PRBool aNotify /* = FALSE */, nsIAbDirectory *aParent)
}}
Status: RESOLVED → VERIFIED
(In reply to comment #50)
> <http://mxr.mozilla.org/mozilla/search?string=CreateMailListAndAddToDB>

NB:
*|aNotify| is unused ... only in this function.
*Should be used (per idl) or removed (per cpp), or the idl documentation updated !?
(In reply to comment #50)
> (In reply to comment #49)
> > But aParent == aNewList so we can just replace it with aNewList everywhere.
> (I don't know:)
> FWIW:
> <http://mxr.mozilla.org/mozilla/search?string=CreateMailListAndAddToDB>
Wrong function, I'm talking about AddListAttributeColumnsToRow
Did this update land in the latest nightly build? [Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008052103 Thunderbird/3.0a2pre ID:2008052103]

If so, the issue is not yet resolved. Adding a person to an email list using the address from the "additional email" field still creates a duplicate entry.

For example, a colleague has both a business and a personal email; personal email address is the primary one in the address book card. One one list I have this address included. On a second list, this person is added using the work email (listed under "additional email" in address book card). As soon as this is done, the contact is duplicated in my Address Book, with the additional email listed on that card as the primary address.
(In reply to comment #53)
> If so, the issue is not yet resolved. Adding a person to an email list using
> the address from the "additional email" field still creates a duplicate entry.

That is bug 254248 and not this bug. They are different cause and effect.

I've updated the summary to try and make it a bit clearer.
Summary: Duplicated Address List → (Temporarily) Duplicated Address Listing
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: