Last Comment Bug 149961 - Mail list addresses are not imported from Outlook
: Mail list addresses are not imported from Outlook
Status: VERIFIED FIXED
[ish]
:
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Address Book & Contacts (show other bugs)
: Trunk
: x86 Windows XP
: -- normal with 3 votes (vote)
: ---
Assigned To: Cavin Song
: Ninoschka Baca
:
Mentors:
: 148801 187095 200469 205663 (view as bug list)
Depends on:
Blocks: 213274 165296 222774
  Show dependency treegraph
 
Reported: 2002-06-07 08:46 PDT by Dalen Kruse
Modified: 2009-11-12 03:50 PST (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch for 1.5b to add outlook mailist support (9.88 KB, patch)
2003-09-14 04:27 PDT, wind li
mozilla: review-
Details | Diff | Splinter Review
updated (11.68 KB, patch)
2003-10-20 06:53 PDT, wind li
no flags Details | Diff | Splinter Review
update patch to trunk (11.68 KB, patch)
2003-10-20 17:35 PDT, wind li
no flags Details | Diff | Splinter Review
New patch incorporating David's suggestions (12.60 KB, patch)
2003-10-20 20:58 PDT, wind li
no flags Details | Diff | Splinter Review
update (11.76 KB, patch)
2003-10-24 20:13 PDT, wind li
mozilla: review+
mscott: superreview+
Details | Diff | Splinter Review

Description Dalen Kruse 2002-06-07 08:46:54 PDT
When importing an address book from Outlook, the addresses for mailing lists are
not imported.  An entry is created for the list, but no e-mail addresses for the
list is displayed.

If requested, I can send the Outlook address book file.
Comment 1 Ninoschka Baca 2002-07-03 13:14:40 PDT
Branch build 2002-07-01: WinMe
Reproduced the problem. 
Marking nsbeta1 so this can be fixed for this release.
Comment 2 Cavin Song 2002-07-03 14:16:57 PDT
Yes, we need to add code to extract members of the lists from MAPI in order to 
fix the problem (see nsOutlookMail::CreateList()).
Comment 3 Ninoschka Baca 2002-07-03 17:01:13 PDT
Cavin, bug# 82791/comment# 9 states "...Members of lists are not imported due to 
not being able to retrieve the member values." This is the same issue, right?
Comment 4 Cavin Song 2002-07-03 17:27:48 PDT
Right. So it's good that we have this bug to keep track of the member issue.
Comment 5 Samir Gehani 2003-01-14 02:08:15 PST
Mail triage team: nsbeta1-
Comment 6 Daniel Wang 2003-03-23 05:29:25 PST
*** Bug 187095 has been marked as a duplicate of this bug. ***
Comment 7 Jason M. Abels 2003-07-01 11:48:34 PDT
*** Bug 148801 has been marked as a duplicate of this bug. ***
Comment 8 Jason M. Abels 2003-07-01 11:48:59 PDT
*** Bug 205663 has been marked as a duplicate of this bug. ***
Comment 9 Jason M. Abels 2003-07-01 11:49:09 PDT
*** Bug 200469 has been marked as a duplicate of this bug. ***
Comment 10 wind li 2003-09-08 01:35:26 PDT
Add myself to the cc list.I am working on OpenOffice Address Book Integration
Project.And meet the same problem.
Comment 11 wind li 2003-09-14 04:27:17 PDT
Created attachment 131429 [details] [diff] [review]
patch for 1.5b to add outlook mailist support

This is a work through patch.I test it with mozilla 1.5b and outlook 2002.
Comment 12 wind li 2003-09-25 19:37:59 PDT
[tracking] StarOffice/OpenOffice AddressBook integration
Comment 13 wind li 2003-10-13 20:31:08 PDT
Comment on attachment 131429 [details] [diff] [review]
patch for 1.5b to add outlook mailist support

any one review it?
Comment 14 wind li 2003-10-15 03:08:08 PDT
Comment on attachment 131429 [details] [diff] [review]
patch for 1.5b to add outlook mailist support

David can you review this?
Comment 15 David :Bienvenu 2003-10-15 08:08:49 PDT
Comment on attachment 131429 [details] [diff] [review]
patch for 1.5b to add outlook mailist support

+	[noscript] void AddListCardColumnsToRow(in nsIAbCard pCard, in
nsIMdbRow pListRow, in PRUint32 pos,out nsIAbCard pNewCard, in PRBool
aInMailingList);
+	[noscript] void GetCardFromDB(in nsIAbCard newCard, in nsIMdbRow
cardRow);
+	[noscript] void SetListAddressTotal(in nsIMdbRow listRow, in PRUint32
total);
+	[noscript] boolean FindRow(in nsIAbCard card, out nsIMdbRow oldRow);
why are these [noscript]? they could be scriptable, right? You should also use
idl types when possible, so  unsigned long instead of PRUint32, boolean instead
of PRBool.

also, all the argument names should begin with a (for argument), so it would be
"in nsIAbCard aPCard".

FindRow could return an nsIMdbRow instead of boolean, and return a null row in
case it couldn't find the row. And, why is the arg called "oldRow"? It looks
like it's the returned row, not "oldRow". Also, there seem to be a lot of tabs
in the patch - those need to be replaced with two space indents.

you can use nsCOMPtr's here:
					nsIMdbRow* newRow = nsnull;
+					nsIMdbRow* oldRow = nsnull;
. That simplifies the ref-counting code.

The GetCardFromDB method is not named correctly (I realize you didn't name it,
but you're making it a public interface, so it's more important that its name
reflect what it does) I suggest something like InitCardFromRow.

Fix these things and I'll review a new patch as quickly as I can. Thx for
working on this!
Comment 16 wind li 2003-10-20 06:53:14 PDT
Created attachment 133684 [details] [diff] [review]
updated
Comment 17 wind li 2003-10-20 07:41:17 PDT
->why are these [noscript]? they could be scriptable, right? 
No.The whole interface is definded noscript.
->The GetCardFromDB method is not named correctly 
GetCardFromDB is the origin name of this function.And a nsIMdbRow is a db row.So
I think matbe GetCardFromDB.
OK?
Comment 18 wind li 2003-10-20 09:47:16 PDT
Comment on attachment 133684 [details] [diff] [review]
updated

>Index: mailnews/addrbook/public/nsIAddrDatabase.idl
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/addrbook/public/nsIAddrDatabase.idl,v
>retrieving revision 1.37
>diff -u -w -b -i -r1.37 nsIAddrDatabase.idl
>--- mailnews/addrbook/public/nsIAddrDatabase.idl	18 Mar 2003 18:39:32 -0000	1.37
>+++ mailnews/addrbook/public/nsIAddrDatabase.idl	20 Oct 2003 13:21:10 -0000
>@@ -257,4 +257,14 @@
>      *        The column value (example: jabroni316)
>      */
>     [noscript] void addRowValue(in nsIMdbRow aRow, in ACString aLDIFAttributeName, in AString aColValue);
>+
>+    [noscript] void AddListCardColumnsToRow(in nsIAbCard aPCard,
>+                                            in nsIMdbRow aPListRow,
>+                                            in unsigned long aPos,
>+                                            out nsIAbCard aPNewCard,
>+                                            in boolean aInMailingList);
>+    [noscript] void GetCardFromDB(in nsIAbCard aNewCard,in nsIMdbRow aCardRow);
>+    [noscript] void SetListAddressTotal(in nsIMdbRow aListRow, in PRUint32 aTotal);
>+    [noscript] nsIMdbRow FindRow(in nsIAbCard aCard);
>+
> };
>Index: mailnews/addrbook/src/nsAddrDatabase.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/addrbook/src/nsAddrDatabase.cpp,v
>retrieving revision 1.116
>diff -u -w -b -i -r1.116 nsAddrDatabase.cpp
>--- mailnews/addrbook/src/nsAddrDatabase.cpp	23 May 2003 21:33:47 -0000	1.116
>+++ mailnews/addrbook/src/nsAddrDatabase.cpp	20 Oct 2003 13:22:43 -0000
>@@ -1618,7 +1618,7 @@
>     return rv;
> }
> 
>-nsresult nsAddrDatabase::AddListCardColumnsToRow
>+NS_IMETHODIMP nsAddrDatabase::AddListCardColumnsToRow
> (nsIAbCard *pCard, nsIMdbRow *pListRow, PRUint32 pos, nsIAbCard** pNewCard, PRBool aInMailingList)
> {
>   if (!pCard && !pListRow )
>@@ -1782,9 +1782,20 @@
>     return count;
> }
> 
>-nsresult nsAddrDatabase::SetListAddressTotal(nsIMdbRow* listRow, PRUint32 total)
>+NS_IMETHODIMP nsAddrDatabase::SetListAddressTotal(nsIMdbRow* aListRow, PRUint32 aTotal)
> {
>-    return AddIntColumn(listRow, m_ListTotalColumnToken, total);
>+    return AddIntColumn(aListRow, m_ListTotalColumnToken, aTotal);
>+}
>+
>+NS_IMETHODIMP nsAddrDatabase::FindRow(nsIAbCard * aCard,nsIMdbRow **_retval)
>+{
>+    PRUnichar *aPrimaryEmail;
>+    nsIMdbRow *newRow=nsnull;
>+    aCard->GetPrimaryEmail(&aPrimaryEmail);
>+    nsresult rv=GetRowForCharColumn (aPrimaryEmail,m_PriEmailColumnToken,PR_TRUE,&newRow);
>+
>+    *_retval=newRow;
>+    return rv;
> }
> 
> 
>@@ -2607,7 +2618,7 @@
>   return rv;
> }
> 
>-nsresult nsAddrDatabase::GetCardFromDB(nsIAbCard *newCard, nsIMdbRow* cardRow)
>+NS_IMETHODIMP nsAddrDatabase::GetCardFromDB(nsIAbCard *newCard, nsIMdbRow* cardRow)
> {
>     nsresult    err = NS_OK;
>     if (!newCard || !cardRow)
>Index: mailnews/addrbooksrc/nsAddrDatabase.h
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/addrbook/src/nsAddrDatabase.h,v
>retrieving revision 1.44
>diff -u -w -b -i -r1.44 nsAddrDatabase.h
>--- mailnews/addrbook/src/nsAddrDatabase.h	25 Apr 2003 03:05:12 -0000	1.44
>+++ mailnews/addrbook/src/nsAddrDatabase.h	20 Oct 2003 13:22:47 -0000
>@@ -322,6 +322,12 @@
> 
>   static void PRTime2Seconds(PRTime prTime, PRUint32 *seconds);
>   
>+
>+    NS_IMETHOD AddListCardColumnsToRow(nsIAbCard *aPCard, nsIMdbRow *aPListRow, PRUint32 aPos, nsIAbCard** aPNewCard, PRBool aInMailingList);
>+    NS_IMETHOD GetCardFromDB(nsIAbCard *aNewCard, nsIMdbRow* aCardRow);
>+    NS_IMETHOD SetListAddressTotal(nsIMdbRow* aListRow, PRUint32 aTotal);
>+    NS_IMETHOD FindRow(nsIAbCard * card,nsIMdbRow **_retval);
>+
> protected:
>   
>   static void		AddToCache(nsAddrDatabase* pAddrDB) {GetDBCache()->AppendElement(pAddrDB);}
>@@ -346,16 +352,13 @@
> 	nsresult GetIntColumn(nsIMdbRow *cardRow, mdb_token outToken, 
> 							PRUint32* pValue, PRUint32 defaultValue);
> 	nsresult GetBoolColumn(nsIMdbRow *cardRow, mdb_token outToken, PRBool* pValue);
>-	nsresult GetCardFromDB(nsIAbCard *newCard, nsIMdbRow* cardRow);
> 	nsresult GetListCardFromDB(nsIAbCard *listCard, nsIMdbRow* listRow);
> 	nsresult GetListFromDB(nsIAbDirectory *newCard, nsIMdbRow* listRow);
> 	nsresult AddRecordKeyColumnToRow(nsIMdbRow *pRow);
> 	nsresult AddAttributeColumnsToRow(nsIAbCard *card, nsIMdbRow *cardRow);
>-	nsresult AddListCardColumnsToRow(nsIAbCard *pCard, nsIMdbRow *pListRow, PRUint32 pos, nsIAbCard** pNewCard, PRBool aInMailingList);
> 	nsresult AddListAttributeColumnsToRow(nsIAbDirectory *list, nsIMdbRow *listRow);
> 	nsresult CreateCard(nsIMdbRow* cardRow, mdb_id listRowID, nsIAbCard **result);
> 	nsresult CreateCardFromDeletedCardsTable(nsIMdbRow* cardRow, mdb_id listRowID, nsIAbCard **result);
>-	nsresult SetListAddressTotal(nsIMdbRow* listRow, PRUint32 total);
> 	nsresult DeleteCardFromListRow(nsIMdbRow* pListRow, mdb_id cardRowID);
> 	void DeleteCardFromAllMailLists(mdb_id cardRowID);
> 	nsresult NotifyListEntryChange(PRUint32 abCode, nsIAbDirectory *dir, nsIAddrDBListener *instigator);
>Index: mailnews/import/outlook/src/MapiApi.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/import/outlook/src/MapiApi.cpp,v
>retrieving revision 1.23
>diff -u -w -b -i -r1.23 MapiApi.cpp
>--- mailnews/import/outlook/src/MapiApi.cpp	19 Feb 2002 22:49:08 -0000	1.23
>+++ mailnews/import/outlook/src/MapiApi.cpp	20 Oct 2003 13:24:47 -0000
>@@ -505,13 +505,12 @@
> {	
> 	ULONG		ulObjType;
> 	HRESULT		hr;
>-	hr =	lpMdb->OpenEntry(	cbEntry,
>+    hr = m_lpSession->OpenEntry(cbEntry,
> 								pEntryId,
> 								NULL,
> 								0,
> 								&ulObjType,
> 								(LPUNKNOWN *) ppOpen);
>-
> 	if (FAILED(hr)) {
> 		MAPI_TRACE2( "OpenMdbEntry failed: 0x%lx, %d\n", (long)hr, (int)hr);
> 		return( FALSE);
>Index: mailnews/import/outlook/src/nsOutlookMail.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/import/outlook/src/nsOutlookMail.cpp,v
>retrieving revision 1.29
>diff -u -w -b -i -r1.29 nsOutlookMail.cpp
>--- mailnews/import/outlook/src/nsOutlookMail.cpp	6 Nov 2002 02:58:04 -0000	1.29
>+++ mailnews/import/outlook/src/nsOutlookMail.cpp	20 Oct 2003 13:25:06 -0000
>@@ -1005,7 +1005,7 @@
>           pVal = m_mapi.GetMapiProperty( lpMsg, PR_SUBJECT);
>           if (pVal)
>             m_mapi.GetStringFromProp( pVal, subject);
>-          CreateList(subject.get(), pDb);
>+          CreateList(subject.get(), pDb, lpMsg, pFieldMap);
>         }
> 			}			
> 
>@@ -1017,8 +1017,10 @@
>   rv = pDb->Commit(nsAddrDBCommitType::kLargeCommit);
> 	return rv;
> }
>-
>-nsresult nsOutlookMail::CreateList( const PRUnichar * pName, nsIAddrDatabase *pDb)
>+nsresult nsOutlookMail::CreateList( const PRUnichar * pName,
>+                                   nsIAddrDatabase *pDb,
>+                                   LPMAPIPROP pUserList,
>+                                   nsIImportFieldMap *pFieldMap)
> {
>   // If no name provided then we're done.
>   if (!pName || !(*pName))
>@@ -1029,17 +1031,105 @@
>   if (!pDb)
>     return rv;
> 
>-  nsCOMPtr <nsIMdbRow> newRow;
>-  rv = pDb->GetNewListRow(getter_AddRefs(newRow));
>+  nsCOMPtr <nsIMdbRow> newListRow;
>+  rv = pDb->GetNewListRow(getter_AddRefs(newListRow));
>+  NS_ENSURE_SUCCESS(rv, rv);
>+  nsCAutoString column;
>+  column.AssignWithConversion(pName);
>+  rv = pDb->AddListName(newListRow, column.get());
>   NS_ENSURE_SUCCESS(rv, rv);
> 
>-  rv = pDb->AddListName(newRow, NS_ConvertUCS2toUTF8(pName).get());
>+    HRESULT             hr;
>+    LPSPropValue aValue = NULL ;
>+    ULONG aValueCount = 0 ;
>+
>+    LPSPropTagArray properties = NULL ;
>+    m_mapi.MAPIAllocateBuffer(CbNewSPropTagArray(1),
>+                       (void **)&properties) ;
>+    properties->cValues = 1;
>+    properties->aulPropTag [0] = m_mapi.GetEmailPropertyTag(pUserList, 0x8054);
>+    hr = pUserList->GetProps(properties, 0, &aValueCount, &aValue) ;
>+
>+    SBinaryArray *sa=(SBinaryArray *)&aValue->Value.bin;
>+
>+    LPENTRYID    lpEid;
>+    ULONG        cbEid;
>+    PRInt32        idx;
>+    LPMESSAGE        lpMsg;
>+    nsCString        type;
>+    LPSPropValue    pVal;
>+    nsString        subject;
>+    PRUint32 total;
>+    
>+
>+    total=sa->cValues;
>+    for (idx=0;idx<sa->cValues ;idx++)
>+    {
>+        lpEid= (LPENTRYID) sa->lpbin[idx].lpb;
>+        cbEid = sa->lpbin[idx].cb;
>+
>+
>+        if (!m_mapi.OpenEntry(cbEid, lpEid, (LPUNKNOWN *) &lpMsg))
>+        {
>+            
>+            IMPORT_LOG1( "*** Error opening messages in mailbox: %S\n", pName);
>+            return( NS_ERROR_FAILURE);
>+        }
>+        {
>+            {
>+                // This is a contact, add it to the address book!
>+                subject.Truncate( 0);
>+                pVal = m_mapi.GetMapiProperty( lpMsg, PR_SUBJECT);
>+                if (pVal)
>+                    m_mapi.GetStringFromProp( pVal, subject);
>+                
>+                    nsIMdbRow* newRow = nsnull;
>+                    nsIMdbRow* oldRow = nsnull;
>+                    pDb->GetNewRow( &newRow);
>+                    if (newRow) {
>+                        if (BuildCard( subject.get(), pDb, newRow, lpMsg, pFieldMap)) 
>+                        {
>+                            nsCOMPtr <nsIAbCard> userCard;
>+                            nsCOMPtr <nsIAbCard> newCard;
>+                            userCard = do_CreateInstance(NS_ABMDBCARD_CONTRACTID, &rv);
>   NS_ENSURE_SUCCESS(rv, rv);
>-  rv = pDb->AddCardRowToDB(newRow);
>+                            pDb->GetCardFromDB(userCard,newRow);
>+                            
>+                            //add card to db
>+                            PRBool bl=PR_FALSE;
>+                            pDb->FindRow(userCard,&oldRow);
>+                            if (oldRow)
>+                            {
>+                                NS_RELEASE(newRow);
>+                                newRow = oldRow;
>+                            }
>+                            else
>+                            {
>+                                pDb->AddCardRowToDB( newRow);
>+                            }
>+                            
>+                            //add card list
>+                            pDb->AddListCardColumnsToRow(userCard,
>+                                              newListRow,idx+1,
>+                                              getter_AddRefs(newCard),PR_TRUE);
>+
>+                            NS_RELEASE(newRow);
>+                        }
>+                    }
>+
>+
>+            }
>+        }
>+    }
>+  
>+  rv = pDb->AddCardRowToDB(newListRow);
>   NS_ENSURE_SUCCESS(rv, rv);
>-  rv = pDb->AddListDirNode(newRow);
>+
>+  rv = pDb->SetListAddressTotal(newListRow, total);
>+  rv = pDb->AddListDirNode(newListRow);
>   return rv;
> }
>+
> 
> void nsOutlookMail::SanitizeValue( nsString& val)
> {
>Index: mailnews/import/outlook/src/nsOutlookMail.h
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/import/outlook/src/nsOutlookMail.h,v
>retrieving revision 1.11
>diff -u -w -b -i -r1.11 nsOutlookMail.h
>--- mailnews/import/outlook/src/nsOutlookMail.h	6 Nov 2002 02:58:05 -0000	1.11
>+++ mailnews/import/outlook/src/nsOutlookMail.h	20 Oct 2003 13:25:06 -0000
>@@ -78,7 +78,7 @@
> 	void		SanitizeValue( nsString& val);
> 	void		SplitString( nsString& val1, nsString& val2);
> 	PRBool		BuildCard( const PRUnichar *pName, nsIAddrDatabase *pDb, nsIMdbRow *newRow, LPMAPIPROP pUser, nsIImportFieldMap *pFieldMap);
>-  nsresult  CreateList( const PRUnichar * pName, nsIAddrDatabase *pDb);
>+  nsresult  CreateList( const PRUnichar * pName, nsIAddrDatabase *pDb, LPMAPIPROP pUserList, nsIImportFieldMap *pFieldMap);
>   void SetDefaultContentType(CMapiMessage &msg, nsCString &cType);
> 
> private:
Comment 19 wind li 2003-10-20 17:35:48 PDT
Created attachment 133720 [details] [diff] [review]
update patch to trunk
Comment 20 wind li 2003-10-20 17:37:29 PDT
Comment on attachment 133720 [details] [diff] [review]
update patch to trunk

->why are these [noscript]? they could be scriptable, right? 
No.The whole interface is definded noscript.
->The GetCardFromDB method is not named correctly 
GetCardFromDB is the origin name of this function.And a nsIMdbRow is a db
row.So
I think matbe GetCardFromDB.
OK?
Comment 21 David :Bienvenu 2003-10-20 19:30:06 PDT
As I said before, "the GetCardFromDB method is not named correctly (I realize
you didn't name it, but you're making it a public interface, so it's more
important that its name reflect what it does) I suggest something like
InitCardFromRow."

>No.The whole interface is definded noscript.

Not sure what you mean - there are many methods in nsIAddrDatabase.idl that are
scriptable. Your new ones should also be scriptable.
Comment 22 wind li 2003-10-20 20:58:17 PDT
Created attachment 133731 [details] [diff] [review]
New patch incorporating David's suggestions
Comment 23 David :Bienvenu 2003-10-22 09:17:06 PDT
Thx for fixing those things. Here are the rest of my comments:

You can use nsCOMPtrs in this code - I think I didn't give you enough context in
my previous comment about this. Sorry about that. So, this code:

+                    nsIMdbRow* newRow = nsnull;
+                    nsIMdbRow* oldRow = nsnull;
+                    pDb->GetNewRow( &newRow);
+                    if (newRow) {
+                        if (BuildCard( subject.get(), pDb, newRow, lpMsg,
pFieldMap)) 
+                        {
+                            nsCOMPtr <nsIAbCard> userCard;
+                            nsCOMPtr <nsIAbCard> newCard;
+                            userCard =
do_CreateInstance(NS_ABMDBCARD_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = pDb->AddCardRowToDB(newRow);
+                            pDb->InitCardFromRow(userCard,newRow);
+                            
+                            //add card to db
+                            PRBool bl=PR_FALSE;
+                            pDb->FindRow(userCard,&oldRow);
+                            if (oldRow)
+                            {
+                                NS_RELEASE(newRow);
+                                newRow = oldRow;
+                            }
+                            else
+                            {
+                                pDb->AddCardRowToDB( newRow);
+                            }
+                            
+                            //add card list
+                            pDb->AddListCardColumnsToRow(userCard,
+                                              newListRow,idx+1,
+                                              getter_AddRefs(newCard),PR_TRUE);
+
+                            NS_RELEASE(newRow);
+                        }

can look like this code below instead.  However, I'm confused by this code.
After CreateInstance of userCard, you add the new row to the db with ths line:

  rv = pDb->AddCardRowToDB(newRow);

then, later, if you don't find the row, you add the new row to the db again. Am
I misunderstanding something?

                           pDb->FindRow(userCard,getter_AddRefs(oldRow));
                            if (oldRow)
                                newRow = oldRow;
                            else
                                pDb->AddCardRowToDB( newRow);

So, here's what the code would look like with comptrs.

                    nsCOMPtr <nsIMdbRow> newRow;
                    nsCOMPtr <nsIMdbRow> oldRow;
                    pDb->GetNewRow( getter_AddRefs(newRow));
                    if (newRow) {
                        if (BuildCard( subject.get(), pDb, newRow, lpMsg,
pFieldMap)) 
                        {
                            nsCOMPtr <nsIAbCard> userCard;
                            nsCOMPtr <nsIAbCard> newCard;
                            userCard =
do_CreateInstance(NS_ABMDBCARD_CONTRACTID, &rv);
   NS_ENSURE_SUCCESS(rv, rv);
  rv = pDb->AddCardRowToDB(newRow);
                            pDb->InitCardFromRow(userCard,newRow);
                            
                            //add card to db
                            PRBool bl=PR_FALSE;
                            pDb->FindRow(userCard,getter_AddRefs(oldRow));
                            if (oldRow)
                                newRow = oldRow;
                            else
                                pDb->AddCardRowToDB( newRow);
                            
                            //add card list
                            pDb->AddListCardColumnsToRow(userCard,
                                              newListRow,idx+1,
                                              getter_AddRefs(newCard),PR_TRUE);

                        }

this code doesn't follow the argument naming standard for the arg, but uses it a
for a local var. and it also looks like it leaks the primary e-mail string.
+NS_IMETHODIMP nsAddrDatabase::FindRow(nsIAbCard * aCard,nsIMdbRow **_retval)
+{
+    PRUnichar *aPrimaryEmail;
+    nsIMdbRow *newRow=nsnull;
+    aCard->GetPrimaryEmail(&aPrimaryEmail);
+    nsresult rv=GetRowForCharColumn
(aPrimaryEmail,m_PriEmailColumnToken,PR_TRUE,&newRow);
+
+    *_retval=newRow;
+    return rv;
 }
 

so, this code should look more like this:

NS_IMETHODIMP nsAddrDatabase::FindRow(nsIAbCard * aCard,nsIMdbRow **aRow)
{
    nsXPIDLString primaryEmail;
    aCard->GetPrimaryEmail(getter_Copies(primaryEmail));
    return GetRowForCharColumn(aPrimaryEmail, m_PriEmailColumnToken, PR_TRUE, aRow);
}
 
+    NS_IMETHOD FindRow(nsIAbCard * card,nsIMdbRow **_retval);

should be:
+    NS_IMETHOD FindRow(nsIAbCard * card,nsIMdbRow **aRow);


Again, thx for doing this.
Comment 24 wind li 2003-10-24 20:13:40 PDT
Created attachment 134089 [details] [diff] [review]
update

David , thanks for your particular comments.I have got many benefits from your
kindness.I am working on other an outlook interrelated bug. You can find it
here http://bugzilla.mozilla.org/show_bug.cgi?id=219559 .
Comment 25 David :Bienvenu 2003-10-25 09:12:46 PDT
Comment on attachment 134089 [details] [diff] [review]
update

looks good, r=bienvenu, thx.
Comment 26 wind li 2003-10-25 20:43:29 PDT
Can you suggest a sr David?I am a beginner to Mozilla community.Thx.
Comment 27 wind li 2003-10-26 01:17:00 PDT
Comment on attachment 134089 [details] [diff] [review]
update

Seth,Can you have a look at this?
Comment 28 David :Bienvenu 2003-10-29 11:35:09 PST
yes, wind li, this can be checked in. Do you want me to check it in for you when
the tree opens for 1.6b?
Comment 29 wind li 2003-10-29 18:15:56 PST
Yes,I do.Thanks,David.
Comment 30 David :Bienvenu 2003-11-04 14:48:21 PST
fix checked in. thx again wind li
Comment 31 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2004-01-31 02:12:04 PST
Fix doesn't work for Outlook Express. Testing with OE6 and Mozilla 1.6 still
results in an empty list but all entries were imported. Result is a broken
mailing list.

-> Reopen because bugs marked as dupes refers also to other products as Outlook.
Comment 32 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2004-01-31 02:16:10 PST
Damn, bug 222774 depends on that bug. I'm very sorry.

-> Resetting to Fixed.

Note You need to log in before you can comment on or make changes to this bug.