Closed Bug 203927 Opened 21 years ago Closed 17 years ago

Add ability to read from Mac OS X system Address Book

Categories

(MailNews Core :: Address Book, enhancement)

All
macOS
enhancement
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.9alpha8

People

(Reporter: jpd, Assigned: Bienvenu)

References

(Depends on 1 open bug, Blocks 2 open bugs, )

Details

Attachments

(4 files, 18 obsolete files)

78.22 KB, patch
standard8
: review-
Details | Diff | Splinter Review
76.59 KB, patch
Details | Diff | Splinter Review
1.34 KB, patch
mscott
: superreview+
Details | Diff | Splinter Review
9.23 KB, patch
Bienvenu
: superreview+
Details | Diff | Splinter Review
User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4b) Gecko/20030423
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.4b) Gecko/20030423

Mac OS X provides a system-wide shared address book  that any Mac OS X
application can access. It would be nifty if Mozilla, in addition to it's own
address book and LDAP could search and maybe list the items in the Mac OS X
shared address book. I'm envisioning an option in the current Mozilla address
book that works much the same way that LDAP does now.

See
http://developer.apple.com/techpubs/macosx/AdditionalTechnologies/AddressBook/ 
for details. 

Reproducible: Always

Steps to Reproduce:
Confirming. I'm surprised this isn't a dup.
Summary: Add ability to read from Mac OS X system address book → Add ability to read from Mac OS X system Address Book
Confirming since Greg forgot to do it. Similar bug for Camino is bug 188666.
Status: UNCONFIRMED → NEW
Ever confirmed: true
mass re-assign.
Assignee: racham → sspitzer
I would really, really like this as well.  Not working with the OS X Address
Book is one of the remaining things which keeps Mozilla from being
well-integrated with the OS.

This would also solve the issue of Palm sync - instead of re-inventing the wheel
to achieve Mozilla <--> Palm address sync on OS X, just use the system Address
Book and everything else which uses the Address Book will 'just work' (Palm sync
included, via iSync).

Just for comparison, I contacted a commercial vendor of Palm conduits for OS X,
and they quoted $5000 - $20000 to write a Mozilla conduit for OS X.  Let's show
them how the community can do it better!
A vote for this one. The integration with iSync makes Apple's address book
really interesting.
This is _the_ feature that will turn many apple mail users in thunderbird ones 
(including me ;))
It's a pity this isn't going to happen soon, as Entourage also fails to use the
built in address book limiting it's usefullness also.

I should be very excited to see this become a reality.
Does TB support applescript? Or is there any external method of accessing and manipulating TB 
address book data? I'd be willing to take a crack at writing an importer for Apple's Address book if 
there's some kind of accessible method for doing so - I'm not a very strong programmer, but I 
could do something w/ applescript.

I /really/ want to see some kind of import mechanism at the very least for Apple address book data. 
Obviously it would be best if TB actually had an option to use apple's address book instead of its 
own, but because of TB's cross-platform nature, I'd be just as happy with some easy way to keep 
the information synced between the two apps.
(In reply to comment #8)
> Does TB support applescript? Or is there any external method of accessing 
> and manipulating TB address book data?

If you look in the preferences folder of your home folder, you will see a
'Thunderbird' folder. Inside of a subfolder of the Thunderbird preferences
folder you will find a file called:

  abook.mab

This appears to be the address book for a profile. As near as I can tell, it is
a plain text file. If you could obtain the details on this file format, it
should be pretty easy to add addresses from the built-in address book to it.

I did a quick google search to try to find the details on this file format, but
I didn't turn up anything useful. Perhaps someone else can provide the relevant
information.

One you had this working, it would then make it far easier for someone else to
complete the process by writing an iSync Conduit.
(In reply to comment #9) 
> This appears to be the address book for a profile. As near as I can tell, it is
> a plain text file. If you could obtain the details on this file format, it
> should be pretty easy to add addresses from the built-in address book to it.

IMO, this would be the wrong approach and certainly it is not what I had in mind
when I filed the bug. It's a great short-term workaround, though. I don't mind
syncing files between devices (palm <--> Mac) but I certainly don't want to be
locked into syncing files on the *same* device (Mac address book <--> Moz
address book).

The Apple Address Book is designed to be queried, the Mozilla address Book has
the ability to do queries (as with LDAP), so let's just connect the two. I think
a good portion of the work has already been done to make Bug 188666 happen.
(In reply to comment #10)
> (In reply to comment #9) 
> > This appears to be the address book for a profile. As near as I can tell, it is
> > a plain text file. If you could obtain the details on this file format, it
> > should be pretty easy to add addresses from the built-in address book to it.
> 
> IMO, this would be the wrong approach and certainly it is not what I had in mind
> when I filed the bug. It's a great short-term workaround, though. 

Well, I never intended to suggest that it would be the final solution...just a
solution that a 'not a very strong programmer' could probably implement
relatively quickly since the strong programmers don't apparently have the time
to spend on it right now...and I would love to be able to do something like this
sooner rather then later.

I do think it is important, however, to make sure the potential for integration
with iSync is there...not sure if you were suggesting that this was a bad idea.
See also Thunderbird bug 218145.
I'm placing a US$100.00 (one hundred US dollars) bounty on this feature.  

The conditions are:
1.  Mozilla queries the AddressBook.app database on-the-fly, for a seamless user
experience.
2.  A patch is provided against 1.7 so I can roll my own  build ASAP
3.  The feature is submitted for incorporation into 1.8

First with an accepted patch that works & meets the critera gets a $100.00 bill.

There are 24 votes for this bug....  anyone else who needs this feel free to
chip in and up the ante.  Let's get this resolved, please!  This solves the PDA
sync issue as well (PDAs sync with address book already, so this simply taps
into an existing solution).

Any takers?
That ought to add some incentive!  (Too bad I don't know how to do this.) :-)  Just wondering... would 
the patch also apply to Thunderbird?
I will also put my wallet where my mouth is (I opened bug #218145 for Thunderbird) and contribute 
$40 US to the pot. I really want to see this functionality added to Thunderbird / the Mozilla suite. 
The stipulations noted in comment #13 hold true; this really needs to be addressed (pardon the 
pun) by someone!
I'll also add $20 to the pot for having apple's addressbook access as an option
for thunderbird's addressbook
Attached patch v1 (obsolete) — Splinter Review
This patch needs attachment 151911 [details] [diff] [review] to land first. After applying both
attachment 151911 [details] [diff] [review] and and adding the following lines to prefs.js you should be
able to see the cards in Mozilla's address book.

user_pref("ldap_2.servers.osx.description", "OS X");
user_pref("ldap_2.servers.osx.dirType", 1);
user_pref("ldap_2.servers.osx.uri", "moz-abOSXdirectory://foo/");


There's more to be done (lists and queries and liveness).
Assignee: sspitzer → peterv
Status: NEW → ASSIGNED
Comment on attachment 151911 [details] [diff] [review]
Switch to nsISimpleEnumerator

+nsListAddressEnumerator::GetNext(nsISupports **aResult)
 {
+    *aResult = nsnull;
...
+    if (++mAddressPos <= mAddressTotal) {
...
+	 return CallQueryInterface(resultCard, aResult);
+    }
+
+    return NS_OK;
the return value is wrong:

 76    *	 NS_ERROR_FAILURE if there are no more elements
 77    *			  to enumerate.
 78    * @return the next element in the enumeration.
 79    */
 80   nsISupports getNext();
Comment on attachment 151912 [details] [diff] [review]
v1

>+static void
>+MapAddress(CFDictionaryRef aDictionary, nsString &aAddress, nsString &aCity,
>+           nsString &aState, nsString &aZipCode, nsString &aCountry)
>+{
>+#define MAP_ADDRESS_MEMBER(value, dictionary, key, member) \
>+    value = NS_STATIC_CAST(CFStringRef, \
>+                           CFDictionaryGetValue(dictionary, key)); \
>+    if (value) { \
>+        ConvertCFString(value, member); \
>+    }
>+
>+    CFStringRef value;
>+    MAP_ADDRESS_MEMBER(value, aDictionary, kAbAddressStreetKey, aAddress)
>+    MAP_ADDRESS_MEMBER(value, aDictionary, kAbAddressCityKey, aCity)
>+    MAP_ADDRESS_MEMBER(value, aDictionary, kAbAddressStateKey, aState)
>+    MAP_ADDRESS_MEMBER(value, aDictionary, kAbAddressZIPKey, aZipCode)
>+    MAP_ADDRESS_MEMBER(value, aDictionary, kAbAddressCountryKey, aCountry)

I think it'd be best if you #undef'd MAP_ADDRESS_MEMBER -here- so that it acts
like a function scoped creature.
>+}

>+NS_IMETHODIMP
>+nsAbOSXCard::Init(const char *aUri)
>+{
...
>+#define MAP_STRING_VALUE(card, osxName, member) \
>+    MapStringProperty(card, osxName, member);
>+
>+    MAP_STRING_VALUE(card, kAbFirstNameProperty, m_FirstName);
...

>+#define MAP_PHONE_VALUE(value, label, constant, member) \
>+    if (CFStringCompare(label, constant, 0) == kCFCompareEqualTo) { \
>+        CFStringRef phone = \
>+            NS_STATIC_CAST(CFStringRef, \
>+                           abMultiValueCopyValueAtIndex(value, i)); \
>+        ConvertCFString(phone, member); \
>+    } else
>+#define MAP_PHONE_VALUE_END(value, label, constant, member) \
>+    MAP_PHONE_VALUE(value, label, constant, member) \
>+        ;
>+
>+    value = GetMultiValue(card, kAbPhoneProperty);
>+    if (value) {
>+        unsigned count = abMultiValueCount(value);
>+        unsigned i;
>+        for (i = 0; i < count; ++i) {
>+            CFStringRef label = abMultiValueCopyLabelAtIndex(value, i);
>+            MAP_PHONE_VALUE(value, label, kAbPhoneWorkLabel, m_WorkPhone)
>+            MAP_PHONE_VALUE(value, label, kAbPhoneHomeLabel, m_HomePhone)
>+            MAP_PHONE_VALUE(value, label, kAbPhoneWorkFAXLabel, m_FaxNumber)
>+            MAP_PHONE_VALUE(value, label, kAbPhonePagerLabel, m_PagerNumber)
>+            MAP_PHONE_VALUE_END(value, label, kAbPhoneMobileLabel, m_CellularNumber)
>+            CFRelease(label);
>+        }
>+    }

here for phone value (end)

...

and here for string value

>+    return NS_OK;
>+}

>Index: addrbook/src/nsAbOSXCard.h
>+ * The Initial Developer of the Original Code is
>+ * Netscape Communications Corporation.
>+ * Portions created by the Initial Developer are Copyright (C) 2002
>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Contributor(s):
>+ *   Peter Van der Beken <peterv@netscape.com> (original author)

You wrote this for netscape in 2002?

>Index: addrbook/src/nsAbOSXDirFactory.h
>+ * The Initial Developer of the Original Code is
>+ * Peter Van der Beken.
>+ * Portions created by the Initial Developer are Copyright (C) 2002
>+ * the Initial Developer. All Rights Reserved.
>+ *
>+ * Contributor(s):
>+ *   Peter Van der Beken <peterv@propagandism.org> (original author)

And you wrote this independently in 2002?

>+//NS_IMETHODIMP
>+//nsAbOSXDirectory::Init(const char *aUri)
>+//{
>+//    return nsAbDirectoryRDFResource::Init(aUri);
>+//}

Do you plan to commit that ^^?

>+NS_IMETHODIMP
>+nsAbOSXDirectory::GetChildNodes(nsISimpleEnumerator **aNodes)
>+{
>+    NS_ENSURE_ARG_POINTER(aNodes);
>+
>+    nsCOMPtr<nsISupportsArray> array;
>+    NS_NewISupportsArray(getter_AddRefs(array));
>+    if (!array) {
>+        return NS_ERROR_OUT_OF_MEMORY;
>+    }
>+
>+    return NS_NewArrayEnumerator(aNodes, array);

Why not use EmptyEnumerator?
>+}

>Index: addrbook/src/nsAbOSXDirectory.h
>+    // nsAbDirectoryRDFResource method
>+    //NS_IMETHOD Init(const char *aUri);
>+protected:
>+//    static void ConvertArrayToSupportsArray(const void* aVal, void* aContext);

>+#endif // nsAbOSXDirectory_h___

>Index: addrbook/src/nsAbOSXLibrary.cpp
>\ No newline at end of file

^^ fix
I'll add $20 as well (I'll pay up when this shows up as an extension or in an
official Thunderbird build).
Attached patch Switch to nsISimpleEnumerator (obsolete) — Splinter Review
Attachment #151911 - Attachment is obsolete: true
Attachment #155196 - Flags: superreview?(mscott)
Attachment #155196 - Flags: review?(bienvenu)
Comment on attachment 155196 [details] [diff] [review]
Switch to nsISimpleEnumerator

could you lose the k&r braces style and use the prevailing style? :

if (a)
{
}
else
{
}
....

also, could you do a -uw diff so I don't see the whitespace changes? Other than
that, it looks ok so far...
Attachment #155196 - Attachment is obsolete: true
Attachment #155196 - Flags: superreview?(mscott)
Attachment #155196 - Flags: review?(bienvenu)
Attachment #155208 - Flags: superreview?(mscott)
Attachment #155208 - Flags: review?(bienvenu)
Comment on attachment 155208 [details] [diff] [review]
Switch to nsISimpleEnumerator (diff -w)

you should rev the cid of nsIAbDirectory.idl and nsIAddrDatabase.idl to reflect
that you've changed the interfaces.
Attachment #155208 - Flags: review?(bienvenu) → review+
(In reply to comment #21)
> I'll add $20 as well (I'll pay up when this shows up as an extension or in an
> official Thunderbird build).

Let me vote on this as well - and I'll addd $20 to the kitty when this gets
implemented! :)
First, THANK YOU PETER, for taking this on!  As I've said before, lack of integration with the system 
Address Book is the main thing stopping me from becoming a Mozilla Mail/Thunderbird user (and I do 
really *want* to be one).  However, I'm concerned about the pseudo-LDAP approach because query is 
only half of the problem!  If the new e-mail addresses I add while in Moz don't end up in the Apple 
address book then I'm once again stuck having to manually synchronize the two.  What we need is a 
way to say "use the system Address Book *as* the Mozilla address book."  The point is, we only want 
*one* address book, not one per app.

If I've misunderstood something and your approach will solve this problem, then I apologize for wasting 
your time and owe you an even deeper debt of gratitude.  In any case, thanks again for your time and 
effort.
If Thunderbird had Mac OS X Address Book integration, I'd leave Apple's Mail.app
behind in a second.
Blocks: 218145
So can anybody elaborate on the status of this? Is there any chance to see addressbook synced in TB 
1.0?
The patch is waiting for mscott's sr.
Casting my vote here as well!
Product: Browser → Seamonkey
Flags: blocking-aviary1.0?
Whiteboard: [have patch]
Unfortunately this will not make the 1.0 train.  There is simply not enough
time, but we will still happily accept patches for the next release. Minusing
for 1.0.
Flags: blocking-aviary1.0? → blocking-aviary1.0-
Any chance it'll make it for Mozilla Suite 1.8 then?  The patch has been
submitted months ago, and has been out for superreview since AUGUST!  I placed a
bounty on this bug in June conditional on 1.8, and others have upped the ante
since then.  It seems like it's the review process holding up the developers in
this case.
on a related tangent, see bug 187454, for an address book related issue in camino.
(In reply to comment #34)
> The patch has been submitted months ago, and has been out for superreview since
> AUGUST!

Please everyone remain calm. The patch that I put up for review ("Switch to
nsISimpleEnumerator") is just preparation for the real patch (currently "v1").
While the difficulties of getting mscott's attention are certainly frustrating I
haven't been pushing this very hard because I still need to finish the real patch.
I am only a volunteer and these days I'm mostly taking care of Mozilla Europe's
site and server. Now that the pressure from Firefox 1.0 is ebbing away I'm going
to get back to development, but I don't know when I'll be able to work on this
one. I know it's very important to many of you, but getting it into Thunderbird
1.0 is simply impossible. We'll see about 1.8 but I can't promise that.
Flags: blocking-aviary1.1?
(In reply to comment #36)
> Please everyone remain calm. The patch that I put up for review ("Switch to
> nsISimpleEnumerator") is just preparation for the real patch (currently "v1").
> While the difficulties of getting mscott's attention are certainly frustrating I
> haven't been pushing this very hard because I still need to finish the real patch.
> I am only a volunteer and these days I'm mostly taking care of Mozilla Europe's
> site and server. Now that the pressure from Firefox 1.0 is ebbing away I'm going
> to get back to development, but I don't know when I'll be able to work on this
> one. I know it's very important to many of you, but getting it into Thunderbird
> 1.0 is simply impossible. We'll see about 1.8 but I can't promise that.

Has there been any movement on this? It is by far the single reason I do not use
Thunderbird on the Mac. THE single reason. Mail.app is junk in comparison, save
this feature. Whoever can move this up the priority ladder, please do so. It's a
very important missing component of Thunderbird.
Let me add another voice requesting this be addressed (no pun intended!). I also
agree with other comments that the goal ought to be to have TB use the Apple
abook.mab as THE address book, not just read from it (tho I confess I'd settle
for that at this point).

(In reply to comment #37)
> (In reply to comment #36)
> > Please everyone remain calm. The patch that I put up for review ("Switch to
> > nsISimpleEnumerator") is just preparation for the real patch (currently "v1").
> > While the difficulties of getting mscott's attention are certainly frustrating I
> > haven't been pushing this very hard because I still need to finish the real
patch.
> > I am only a volunteer and these days I'm mostly taking care of Mozilla Europe's
> > site and server. Now that the pressure from Firefox 1.0 is ebbing away I'm going
> > to get back to development, but I don't know when I'll be able to work on this
> > one. I know it's very important to many of you, but getting it into Thunderbird
> > 1.0 is simply impossible. We'll see about 1.8 but I can't promise that.
> 
> Has there been any movement on this? It is by far the single reason I do not use
> Thunderbird on the Mac. THE single reason. Mail.app is junk in comparison, save
> this feature. Whoever can move this up the priority ladder, please do so. It's a
> very important missing component of Thunderbird.

Adding pointless comments is *not* going to make this bug get fixed faster, it
just adds to the bugspam I have to go through every day. Please read
http://bugzilla.mozilla.org/page.cgi?id=etiquette.html, thanks.
No longer blocks: majorbugs
Keywords: helpwanted
QA Contact: nbaca → addressbook
Carrying the r=bienvenu forward.
Attachment #155208 - Attachment is obsolete: true
Attachment #192210 - Flags: superreview?(dmose)
Attachment #192210 - Flags: review+
Attached patch Switch to nsISimpleEnumerator (obsolete) — Splinter Review
Attachment #155209 - Attachment is obsolete: true
Attachment #155208 - Flags: superreview?(mscott)
Comment on attachment 192210 [details] [diff] [review]
Switch to nsISimpleEnumerator (diff -w)

Looks good; just a few minor nits.

>Index: mailnews/addrbook/public/nsIAbDirectory.idl
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/addrbook/public/nsIAbDirectory.idl,v
>retrieving revision 1.32
>diff -u -p -w -r1.32 nsIAbDirectory.idl
>--- mailnews/addrbook/public/nsIAbDirectory.idl	17 Apr 2004 18:32:13 -0000	1.32
>+++ mailnews/addrbook/public/nsIAbDirectory.idl	10 Aug 2005 15:00:29 -0000
>@@ -63,7 +63,7 @@ interface nsIAbDirectoryProperties : nsI
>   attribute long position;
> };
> 
>-[scriptable, uuid(AA920C90-1DD1-11B2-96D3-AA81268ADAFC)]
>+[scriptable, uuid(2edfc712-e6c5-11d8-9a39-000a95dc234c)]
> interface nsIAbDirectory : nsISupports {
> 
> 	// Types of operation
>@@ -100,7 +100,7 @@ interface nsIAbDirectory : nsISupports {
> 	// Get the cards associated with the directory
> 	// This will return the cards associated with
> 	// the mailing lists too
>-	readonly attribute nsIEnumerator childCards;
>+	readonly attribute nsISimpleEnumerator childCards;

As long as you're here, can you turn the above comment into a doxygen-style
one?

>   // Modifies a top level directory, 
>   // which also updates the preferences
>Index: mailnews/addrbook/public/nsIAddrDatabase.idl
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/addrbook/public/nsIAddrDatabase.idl,v
>retrieving revision 1.42
>diff -u -p -w -r1.42 nsIAddrDatabase.idl
>--- mailnews/addrbook/public/nsIAddrDatabase.idl	23 Mar 2005 18:05:48 -0000	1.42
>+++ mailnews/addrbook/public/nsIAddrDatabase.idl	10 Aug 2005 15:00:29 -0000
>@@ -128,7 +128,7 @@ interface nsIMdbRow;
> 
> %}
> 
>-[scriptable, uuid(ca536e0e-1dd1-11b2-951a-e02b86e4f60e)]
>+[scriptable, uuid(14d89995-e6c5-11d8-9a39-000a95dc234c)]
> interface nsAddrDBCommitType 
> {
>   const long kSmallCommit = 0;
>@@ -155,8 +155,8 @@ interface nsIAddrDatabase : nsIAddrDBAnn
> 	void createNewListCardAndAddToDB(in nsIAbDirectory list, in unsigned long listRowID, in nsIAbCard newCard, in boolean aNotify);
> 	void createMailListAndAddToDB(in nsIAbDirectory newList, in boolean aNotify);
>   void createMailListAndAddToDBWithKey(in nsIAbDirectory newList, in boolean aNotify, out PRUint32 key);
>-	nsIEnumerator enumerateCards(in nsIAbDirectory directory);
>-	nsIEnumerator enumerateListAddresses(in nsIAbDirectory directory);
>+	nsISimpleEnumerator enumerateCards(in nsIAbDirectory directory);
>+	nsISimpleEnumerator enumerateListAddresses(in nsIAbDirectory directory);

And since you're touching them, if you could give the above two attributes
doxygen comments too, that'd be great.


>Index: mailnews/addrbook/src/nsAddrDatabase.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/addrbook/src/nsAddrDatabase.cpp,v
>retrieving revision 1.131
>diff -u -p -w -r1.131 nsAddrDatabase.cpp
>--- mailnews/addrbook/src/nsAddrDatabase.cpp	28 Jul 2005 16:42:57 -0000	1.131
>+++ mailnews/addrbook/src/nsAddrDatabase.cpp	10 Aug 2005 15:00:30 -0000
>@@ -3098,233 +3098,221 @@ nsresult nsAddrDatabase::GetListFromDB(n
>     return err;
> }
> 
>-class nsAddrDBEnumerator : public nsIEnumerator 
>+class nsAddrDBEnumerator : public nsISimpleEnumerator 
> {
> public:
>     NS_DECL_ISUPPORTS
> 
>-    // nsIEnumerator methods:
>-    NS_DECL_NSIENUMERATOR
>+    // nsISimpleEnumerator methods:
>+    NS_DECL_NSISIMPLEENUMERATOR
> 
>     // nsAddrDBEnumerator methods:
> 
>-    nsAddrDBEnumerator(nsAddrDatabase* db);
>-    virtual ~nsAddrDBEnumerator();
>+    nsAddrDBEnumerator(nsAddrDatabase* aDb);

I don't know if anyone inherits from this class (or would even want to), but
losing the virtual destructor here will make that impossible, I think.	Fix at
your disgression.


sr=dmose with the above nits addressed.
Attachment #192210 - Flags: superreview?(dmose) → superreview+
Attached patch fix for regression (obsolete) — Splinter Review
standard8 reported a problem with opening address books when the fix for this
bug was checked in. This should fix that. But I suspect mailing lists are still
broken. I'll look at that next while standard8 tries this patch.
Attachment #193084 - Flags: superreview?(mscott)
Attachment #193084 - Flags: review?(bugzilla)
Comment on attachment 193084 [details] [diff] [review]
cumulative fix, including fix for lists

Just one nit:

+  if (aDb && mDbTable)
+  {
+    mDbTable->GetTableRowCursor(mDb->GetEnv(), mRowPos,

Can we make that if (mDb please? it follows better.

r=me with that changed.
Attachment #193084 - Flags: review?(bugzilla) → review+
Attachment #193084 - Flags: superreview?(mscott) → superreview+
Comment on attachment 193084 [details] [diff] [review]
cumulative fix, including fix for lists

This patch is wrong, it makes HasMoreElements have side-effects. Better patch
coming up.
Attachment #193084 - Flags: superreview-
Attachment #193084 - Flags: superreview+
Attachment #193084 - Flags: review-
Attachment #193084 - Flags: review+
Attached patch Fix regression (obsolete) — Splinter Review
I copied the checks for mDb from the original code, but they're not needed. If
mDb is null we'd crash when creating the iterator (the constructor calls
mDB->GetPabTable()).
There doesn't seem to be a way to clone a nsIMdbTableRowCursor, so
nsAddrDBEnumerator::HasMoreElements gets a new one every time it's called,
ideally we'd just clone mRowCursor and use that one.
I've also switched nsAddrDBEnumerator::HasMoreElements to NextRowOid instead of
NextRow + GetOid, we don't need the row, just the Oid.
Attachment #193082 - Attachment is obsolete: true
Attachment #193084 - Attachment is obsolete: true
Attachment #193173 - Flags: superreview?(bienvenu)
Attachment #193173 - Flags: review?(bugzilla)
Comment on attachment 193173 [details] [diff] [review]
Fix regression

This seems to work ok for the cases I've tested & looks reasonable.
Attachment #193173 - Flags: review?(bugzilla) → review+
Comment on attachment 193173 [details] [diff] [review]
Fix regression

ok, that works, thx.
Attachment #193173 - Flags: superreview?(bienvenu) → superreview+
Attached file Search Regression stack. (obsolete) —
The previous enumerator patches appear to have caused further regressions in
address book.

The major one I have found so far is to go into Advanced Address Book search,
enter a query & select search. SeaMonkey locks up completely (I am assuming
Thunderbird does the same as it's the same back end).

The attached stack is the last 60 frames from the stack when I interrupted it.
Backing out locally the two patches that have gone in so far fixed the problem.
(In reply to comment #50)
> The previous enumerator patches appear to have caused further regressions in
> address book.
> 
> The major one I have found so far is to go into Advanced Address Book search,
> enter a query & select search. SeaMonkey locks up completely (I am assuming
> Thunderbird does the same as it's the same back end).

Peter has now fixed this and I haven't found any further problems. Thanks Peter.
rehtruht yna dnuof 
Asking for blocking Seamonkey 1.0a, as this enhancement is highly visible on Mac
platform and it has r/sr+.
Flags: blocking-seamonkey1.0a?
It doesn't have r/sr+, there's not even a complete patch yet. Some preliminary
work went in, that's all.
Flags: blocking-seamonkey1.0a?
Whiteboard: [have patch]
If it gets r/sr and approval, it surely can go in also on branch, but if it
doesn't, we surely won't hold any branch release release for a feature (like
that), please don't request blocking-* flags for it.
Sorry, I didn't read the comments as carefully as I should, and thus thought the
patch is fixing the whole bug.
Moving to core address book now that we have one in place (with Peter's agreement)
Component: Address Book → MailNews: Address Book
Product: Mozilla Application Suite → Core
Sorry for the spam, bugzilla messed up I think.
*** Bug 218145 has been marked as a duplicate of this bug. ***
Blocks: 249341
No longer blocks: 218145
What is the latest on this enhancement?
Thunderbird v1.5 is out and still no Apple AB support... Is this feature given up ? Is there anyone assigned and working on it ? 

I know it's easy to ask and not to do but Is it worth waitting for this feature or must I revert to AppleMail ?
I was trying to work on it a bit as at least getting it to build in the first place with the patch applied.
(see http://vafer.org/blog/20060113170355) unfortunately the patch does not apply to trunk/1.6a (anymore) ...so I've tried to contact the author of the patch - no luck so far. Anyone else having a clue? I want this really badly ...and happy to work on it a bit - but I would need at least a semi-working patch to guide me.
I bet there are millions of silent OS X Thunderbird users who are dying for this feature. Without it, Thunderbird loses half its attraction.
One more vote for this patch.  This also is what keeps me from switching to thunderbird.  
I was finally able to apply the patch and compile it with trunk from a few days ago. Unfortunately I cannot work out how to use the OSX addressbook. I've added 

user_pref("ldap_2.servers.osx.description", "OS X");
user_pref("ldap_2.servers.osx.dirType", 1);
user_pref("ldap_2.servers.osx.uri", "moz-abOSXdirectory://mymachinename/");

And then set preferences to use "OS X" as a LDAP server. Is that how it should work?
(In reply to comment #60)
> Is there anyone assigned and working on it ? 

Yes, me. See the "Assigned To" field.
(In reply to comment #65)
> (In reply to comment #60)
> > Is there anyone assigned and working on it ? 
> 
> Yes, me. See the "Assigned To" field.
> 

Cool ! ;-)
I feel better now :-) ! So it's not dead, that's what I wanted to know.

Can you give us a status ?

Just by my side I know at least 25 people that have not subscribed to this bug but that are waiting for this patch to definitely switch to Thundebird.
just unzip it in the mozilla/mozilla dir and then apply the included addressbook.diff
Although I was able to compile with the patch applied I am still not getting it to work. I've created an OSX Directory of type "moz-abOSXdirectory" but I don't get any results when I type in the "To:". It just turns red.

If anyone wants to play with the compiled version drop me an email and I can make the compiled version available somewhere.

Peter, I've tried to contact you ...didn't you get my email?
Attached patch v2 (obsolete) — Splinter Review
With mailing lists, searching (so autocomplete works) and some basic editing. Editing is not completely implemented so danger for dataloss. Still need to update after editing so we show the commited data, implement complex editing (adding, deleting, mailing lists).

Adding the following to prefs.js should do the trick:

user_pref("ldap_2.servers.osx.description", "OS X");
user_pref("ldap_2.servers.osx.dirType", 3);
user_pref("ldap_2.servers.osx.uri", "moz-abOSXdirectory:///");
Attachment #151912 - Attachment is obsolete: true
Attachment #192210 - Attachment is obsolete: true
Attachment #192212 - Attachment is obsolete: true
Attachment #193173 - Attachment is obsolete: true
Attachment #193263 - Attachment is obsolete: true
Attachment #209565 - Attachment is obsolete: true
Works like charm! Many thanks for your time in this project(part).
Any hints on building it static?

I started several tries with different configs, but i had no luck.
Expect it has something to do with the makefiles.

Greetings from Russia.
I think this would be a pretty cool new feature for Thunderbird 2. Peter, let me know if you need anything from me to help with this.
Flags: blocking-thunderbird2?
(In reply to comment #71)
> I think this would be a pretty cool new feature for Thunderbird 2. Peter, let
> me know if you need anything from me to help with this.
> 

Just a comment, if you want this on Thunderbird 2, you're going to have to take the nsISimpleEnumerator patch over to the branch, that means you're going to be changing idls, or rather creating new versions of them specifically for the branch.
Attached patch fixes the static build (obsolete) — Splinter Review
The system address book also stores a "middle name" property, but Mozilla doesn't support middle names (see bug 144134). Perhaps this code could import the middle name property and attach it to the end of the first name in Mozilla so the middle name isn't totally disregarded.
AddressBook is read-only

Having the addressbook.app is really a great enhancement. Unfortunately I'm not able to store anything to the addressbook, just reading works...

When will it make the way into official TB?

Cheers,

     Gernot
This should definitely find it's way into next TB release. (2.x?)

The fix by Peter van der Beken is already working, pity it isn't available in any official release but only through some developers releases as <a href="http://vafer.org/blog/tag/thunderbird">the one</a> by Torsten.
*** Bug 337894 has been marked as a duplicate of this bug. ***
*** Bug 338939 has been marked as a duplicate of this bug. ***
Camino has it, and it works very well. Other applications have it too, and the source code is open... Is it possible to have a plug-in for it, instead of waiting
until v2.0?
I didn't know Camino had it.  Are you sure?  I'm going to look into that....

If it does, it's probably because Camino uses Cocoa, which is some kind of language Apple provides people to develop applications for Mac OS X.  Since Thunderbird uses XUL and not Cocoa, I would imagine the solution is not particularly transferrable.  

But then again I'm just being logical.  I'm not a developer. :)  (Although XUL sounds very interesting.  I'd like to learn more on it in the future... got links? :)
Cocoa is a set of Objective-C APIs (descendants of the old NeXTSTEP APIs).

The Address Book documentation:

    http://developer.apple.com/documentation/UserExperience/Conceptual/AddressBook/index.html

indicates that both Objective-C *and* C APIs exist for it.

Can, and should, there be a generic way for Mozilla (and Thunderbird) to plug into a desktop environment's address book mechanism, such as the Windows Address Book:

    http://windowssdk.msdn.microsoft.com/library/default.asp?url=/library/en-us/wab/wab/wabentry.asp

or the KDE/GNOME/etc. mechanisms?  KDE and GNOME are trickier, as Mozilla/Thunderbird for UNIX+X11 don't know whether they'll be running under KDE, GNOME, or "none of the above", so they'd have to figure it out at run time or be built in different versions.  See also the Portland project's APIs:

    http://webcvs.freedesktop.org/*checkout*/portland/portland/dapi/doc/API.txt

for abstracting away stuff such as that, so you can have applications using desktop toolkit X running under desktop Y and looking as much like a native desktop Y app as possible.
An alternative approach is to keep TB as it is, and insist on OSX/XP/Linux to have a local LDAP for their address books. The solution would be more in line with the need for standardization and reliable service.
Please stop. This isn't a discussion forum, most of the comments are either redundant or off-topic. This bug treats with one specific issue, integrating the OS X address book in Thunderbird using the Cocoa API (see the bug's URL field for a clue).

Unless you have a comment about the patch, please refrain from commenting. Thanks.
> Please stop. This isn't a discussion forum, most of the comments are either
redundant or off-topic. This bug treats with one specific issue, integrating
the OS X address book in Thunderbird using the Cocoa API (see the bug's URL
field for a clue). Unless you have a comment about the patch, please refrain from commenting. Thanks.

Then you should stop reindexing bugs into other bugs that have a less general scoop. The problem is not the one you stated. The problem is how to interface TB with local address books, whether in OSX, XP, or Linux, or whatever your OS.  Given the breadth of the topic, my comment is relevant. Please stop diminishing the time that people spend in giving good feedback, because time is money, and I am not here to waste mine. My comments are expensive. If you do not show respect for user's feedback, this project will die. Enough of resentment now, and open your mind.
We have a patch here ...that works. Instead of starting a general discussion you should rather nag Peter to commit it ;-P (sorry, peter ...couldn't resist) Everything else should be handled on the mailing lists/newsgroups or IRC.
*** Bug 345304 has been marked as a duplicate of this bug. ***
Using the dmg from Torsten's site <http://vafer.org/pub/unofficial-thunderbird-3.0a1-with-addressbook-patch.dmg>
as I don't have a build environment.  Added the requisite 3 lines to prefs.js and did some basic testing:

Lookup works great, and it sees new Address Book entries as soon as you exit the editing mode (cool!). 

Writing to Address Book via Tbird doesn't work. I know writing isn't part of the spec, but unfortunately it lets me do all the things that look like I should write--I can select OSX in the pop-up for where to create a new address.  It should either actually write there, or remove this option.

Header backgrounds in message pane are gone; funky. 

Couldn't make it crash :-)

I miss my extensions too much to spend all day in this environment but will keep testing.  This is an important enhancement for Mac users; thanks Torsten!
Flags: blocking-thunderbird2?
Ethan, did you mean to drop the "blocking-thunderbird2?" flag?
oops.  No.  Sorry.
Flags: blocking-thunderbird2?
Peter Van der Beken, how about we ask review for the current patch which is confirmed to add major functionality to Mozilla products and open new bugs for the remaining issues such as write access? :)

Great work :)
Any chance to get this in now? ...it has been reported to be working for a while now. So at least getting it into trunk would be great!
I love TB, but the lack of integration with OS X's Address Book creates several inconveniences, such as synchronization with PDAs and other apps.  Recently I accidentally discovered that the only way to have my Address Book currently imported into TB was to have it copied from Eudora (Eudora provides this integration with Address Book with one single check mark in its preferences).  I am excited to see the possibility of integrating OS X's Address Book with Thunderbird.
I love TB, but the lack of integration with OS X's Address Book creates several inconveniences, such as synchronization with PDAs and other apps.  Recently I accidentally discovered that the only way to have my Address Book currently imported into TB was to have it copied from Eudora (Eudora provides this integration with Address Book with one single check mark in its preferences).  I am excited to see the possibility of integrating OS X's Address Book with Thunderbird.
Add another "me, too" to the stack of people who can't (and didn't) wait for this, and another report that Torsten's build including this patch seems to be quite stable.

(In reply to comment #90)
> Peter Van der Beken, how about we ask review for the current patch which is
> confirmed to add major functionality to Mozilla products and open new bugs for
> the remaining issues such as write access? :)

+1, esp. if it helps this patch make it into < 2.0.

> Great work :)

Understatement. Incredible work, and great persistence.

Thanks, Peter!

--j

As far as I can tell, this patch has been stale for several months - I haven't been able to build Thunderbird with it for quite a while. Are you using an old build, or have you been able to apply this patch and build on the current trunk?

From a security standpoint it's probably not wise to be using a several-month-old version of Thunderbird.
> As far as I can tell, this patch has been stale for several months - I haven't
> been able to build Thunderbird with it for quite a while. Are you using an old
> build, or have you been able to apply this patch and build on the current
> trunk?

I have just tried recently as I wanted to build an universal/intel version of Thunderbird with it ...but a few things have changed in trunk. I don't think it's a big deal - but it requires some work. 

That's exactly why maintaining a patch is a PITA and it should finally go into trunk. We are wasting people's energy here that we could use for bug fixes instead.

I am failing to understand why it's such a big deal to get this applied, guys.
We are talking about trunk - not one of the stable branches.
So, the state of the patch is that it is bitrotted. Someone needs to step up and update it, and possibly fix any errors in its current state.

Please, do not comment in this bug unless you want to help out with that work.

General discussion about this bug can continue in the newsgroup, or other fora. Thanks!
Attached patch v2.1 (obsolete) — Splinter Review
Attachment #214730 - Attachment is obsolete: true
Attachment #218627 - Attachment is obsolete: true
Comment on attachment 240331 [details] [diff] [review]
v2.1

Asking for review. This patch adds major functionality on the Mac. 

Remaining issues (like read access) can be dealt with in follow up bugs.
Attachment #240331 - Flags: review?
Comment on attachment 240331 [details] [diff] [review]
v2.1

Don't touch my patches.
Attachment #240331 - Flags: review?
Many of us have been waiting for a good long time for a) this to be moved to trunk or b) an explanation of why it's staying where it is. What else can we do to make this happen?
It'll be checked in when it's ready. I don't know of any outstanding major issues, but I'll ask review when I feel confident about the patch. I have a lot of other (more important) things on my plate. And I've said this before but generating useless bugmail doesn't make my workload go down.

To actually add something useful, I've been discussing this with david and mscott, so here are the known issues for getting this into Thunderbird 2:

* enumerator differences between trunk and branch
* patch assumes minimum version of OS X 10.3, need to verify if branch still
  supports 10.2 or not

Write-support will be moved to another bug, we'll probably avoid the round-trip issues (street/number vs address etc) by only supporting editing of specific fields.
Attached patch Update patch for 3.0a trunk (obsolete) — Splinter Review
I updated the patch to work on the current trunk (12/5/06). Mostly it was bit rot, but my compilation did find a mistake in nsAbOSXCard.h: NS_DEFINE_STATIC_IID_ACCESSOR(nsIAbOSXCard, NS_IABOSXCARD_IID)
was missing.

Also, for others who might try a naive build with fink:
ensure that autoconf2.13 is installed, not autoconf
ensure that neither gsed nor ssed are installed
ensure that you have no flags pointing into fink (such as CPPFLAGS...)

NB: My patch is a plain diff patch, not a cvs diff patch like its source.
Blocks: 359277
This missing feature is the only thing stopping me from using Thunderbird over AppleMail. I have a great deal of contact management that requires the use of the OSX Addresbook, why should I constantly have to reimport my addressbook everytime I need an address thats out of sync.
It seems that both the v2.1 and update 3.0a patches do not work with the latest 1.8 branch code.
This is something I am really missing in TB. It has been a very long wait and I was contemplating shifting over to AppleMail. Should I wait any longer?
i'm"just an end user"; i'd honetsly rather NOT be here ...

but, at the apparent risk of being flamed,

  > Please stop. This isn't a discussion forum, most of the comments are
  > either redundant or off-topic.

let me point out that when asking about this issue 'elsewhere' (lists,m forums, irc, etc), we're *repeatedly sent here*, to this bug.

which, afaict, was created on: 2003-04-30, and, as of 2007-02-13 (really? 4 years ?!) is still is an open/unresolved issue in , at least, TBird/nightly.

is someone working on this with the intent of AddressBook integration happening sometime 'soon' (hint: not another 4 years, please ...)?

if so, or frankly, if not, could someone kindly post an update here, or point to other information?

thanks.
(In reply to comment #107)
> is someone working on this with the intent of AddressBook integration happening
> sometime 'soon' (hint: not another 4 years, please ...)?

Yeah, I was. But comments like yours have finally made me decide that I'd rather not read more bugmail from this bug.
Assignee: peterv → nobody
Status: ASSIGNED → NEW
wow. who ran over your dog?

"please" & "kindly" not enough for you? from me, AND others?

and _you're_ the one flaming folks?  paragon of politeness, you are ...

i'm soooo releived that it was my simply inquiry -- not any of the OTHER g'zillion participants' -- that drove you over the edge.  hey, look at it this way, now you have someone to blame!

likewise, "comments like yours have finally made me decide that I'd
rather not read more bugmail from" ... you.
Wait, wait wait,

I just voted on this issue and I'm glad to be on this list.   Please continue to peruse this bug!

The Mac community is a demanding one, and Mail.app is a tough application to go up against - but please keep at it.

OS X/Address Book.app/Mail.app and Thuderbird handle contacts very differently.  Frankly most other OSs/E-Mail Clients do not support centralizing contact information in a different application like Mail.app does.  But I think it's worth doing, and potentially where future integration with iPods/iPhones etc. on all platforms may come from.

In summary, please keep working on this bug, and I offer my encouragement and my testing in lieu of code.

@snowcrash+mozilla:   Tone counts more than text.
Snowcrash (I agree your comment wasn't that offensive) and others: clearly, any code that is contributed into Mozilla by volunteers is a gift horse, so not much complaint is legitimate. Perhaps some alternatives may be worth exploring: such as sync'ing AddressBook into an LDAP server running on localhost and then having Thunderbird perform address lookup on that server. It should be possible to code this up fairly easily (and the change of status of this bug is a good motivator), but is perhaps not very user-friendly since it involves setting up and managing an LDAP server. There is also the issue that currently, AFAIK, Thunderbird only supports one directory server lookup at a time. This would not help with adding addresses from Thunderbird (such as senders and recipients of messages, using context menu options), but I believe that was not slated to happen even in the proper solution to this bug.

This is not the best forum to discuss such alternatives. We may need to return to the Mozilla mailing lists to investigate further.

(Of course Apple AddressBook can read from an LDAP directory, but seems to have certain problems, such as when trying to iSync AddressBook to an external device).
I am not really using Thunderbird on OSX much these days, but rather than act desperate, constantly being given the run-around, or extremely childish, I'll offer one suggestion.

If any other people besides Peter feel like working on this bug/patch/improvement, simply finish up the patch, give it enough polish to catch rough edges, and then hang around #developers on irc.mozilla.org, get on the right mailing lists, and talk to people that understand why this is important to you, and that have commit access.

The page to start all that is here: http://developer.mozilla.org/en/docs/Developing_Mozilla

@Peter: you worked on this bug/patch for 2 1/2 years -- I don't know you at all Peter, maybe you're a really busy guy, maybe it was really low on your priority list.  However, your last response was childish, and only serves to irritate normal end users that have been waiting on this bug/patch for many /years/.  

@Normal end users: sorry, you'll have to wait longer, use a better piece of software that better serves your individual needs, or learn how to program.

@Everyone else: if you have the desire and the time, take ownership of this bug/patch.  Scratch your own itch, and you'll be a hero to many people waiting for this bug/patch to be closed.

What is the process for getting a patch added to the tree? I updated this patch, have been running it for months for 3.0a, and there's been a guy who's done a few builds: http://icecube.co.nz/2006/12/7/new-thunderbird-build-with-address-book-patch. But I don't see what the formal process is for getting it into the trunk. It works read-only. That's what these people want. As it currently sits, on 3.0a it fulfills my needs, and hasn't caused any bugs distinguishable from the general problems of sitting on the trunk. But if it's not integrated, it will continue to suffer from bit-rot, and never be ready for release, requiring endless tweaking until a major interface change occurs, requiring the patch to go back to zero.
What is the process for getting a patch added to the tree? I updated this patch, have been running it for months for 3.0a, and there's been a guy who's done a few builds: http://icecube.co.nz/2006/12/7/new-thunderbird-build-with-address-book-patch. But I don't see what the formal process is for getting it into the trunk. It works read-only. That's what these people want. As it currently sits, on 3.0a it fulfills my needs, and hasn't caused any bugs distinguishable from the general problems of sitting on the trunk. But if it's not integrated, it will continue to suffer from bit-rot, and never be ready for release, requiring endless tweaking until a major interface change occurs, requiring the patch to go back to zero.
Read-only is *not* what "these people" want.  We want integration.  We want *one* address book system-wide.  I would prefer not to see a read-only patch integrated because it would probably blunt the push for real integration.
Attached patch v2.2Splinter Review
Here's the latest state for trunk. And yes, sorry to all the end-users who patiently and quietly waited for this bug to be fixed. Hopefully someone else will step up.
Attachment #240331 - Attachment is obsolete: true
Attachment #247714 - Attachment is obsolete: true
@Peter:
Thank you a lot for what you donated to the community so far

@Nathaniel Gray:
Read only *is* what "these people" want.
It would be a nice start and its already working with Peters patch.
The concept of apples central adressbook is, that you use IT to make changes to your contacts or add new contacts. why the heck should thunderbird duplicate this functionality in its own gui?

bringing read only apple adressbook support to TB will give it a *major* boost in use on the osx platform.
trust me!

greetings from russia.
I guess I would concur with Moz-Russia -- Read-only is better than nothing.  Right now, Thunderbird is not even an option on my Mac, due to the total absence of address-book integration.  While it would be nice to have addressees automatically added to the system address book, I could probably live without it.

While Nathaniel's point about blunting the push for real integration is well-taken, I would hope for limited system address book integration leading to greater use of Thunderbird on Macs, which then leads to a push for tighter integration with OS X services (Address Book, Keychain, etc.)
@Moz-Russia:
Read-only is what *some* people are willing to live with, but some of us want true integration.  The centralized address book concept is simple -- there's just *one* address book.  Contacts belong to the *user*, not the application.  It doesn't take a visionary to understand why your email client should be able to add contacts to your address book.
Adding contacts is nice but the second step. Read-only *would* already be a major step forward. What is not required is a GUI for editing the contacts from within TB. So this is not rocket science. LDAP an other solutions are just wrong workarounds.

Of course a "thank you" to Peter for helping to keep this patch up-to-date. But the fact that a working patch did not made it into *trunk* (not stable - but trunk!) within all these years draws a bad picture of the TB team (sorry guys!) With this attitude you cannot expect people to contribute code. (Do you intentionally try to keep the TB team small?) And Peter: as an experienced OSS developer you should know better than reacting like this ...unfortunately this was not the first time.

Anyway, I am giving up on TB (on OSX) and decided to stop begging or spending more of my precious time on helping (http://vafer.org/blog/tag/thunderbird) if this just does not get committed anyway.

This seems less to be a technical problem. Maybe someone should do a fork. Anyway this is not the right forum for this - I am out of here...
Here's the branch patch that I was working on, no idea if it compiles or works.

Note that I'm not part of the Thunderbird team, I only did this patch in my spare time. As for the people complaining about my attitude: please carefully look at everything that has been posted and done in this bug, and consider the countless private emails I've received on this bug, then carefully read https://bugzilla.mozilla.org/page.cgi?id=etiquette.html
(In reply to comment #114)
> What is the process for getting a patch added to the tree?

-- Disclaimer: I have no association with TB. The following is not an official statement of any kind, just a general observation. --

It would need to go through the usual review process.

I'm not familiar with the code this would fit into, but skimming just the Objective-C code I have some concerns about the scope of the global autorelease pool potentially causing memory problems over time, and there's at least one leak. In other words, it's very unlikely to just be a case of rubber-stamping it and putting it in the tree; someone is probably going to have to step up and take ownership of this bug, driving the patch through the usual process and making tweaks to the patch as necessary.

If at some point someone does this and they or one of the module owners would like a review of the Cocoa/Obj-C portion of the patch, feel free to ping me.

(In reply to comment #120)
> This seems less to be a technical problem. Maybe someone should do a fork.

No, it's a manpower problem. If there is someone with enough technical ability and interest in this to fork TB, they would be better served to take over this bug and get the patch finalized and landed. There haven't been any barriers to that other than no-one with sufficient time actually doing so; Peter's patch wasn't landed because, as he said several times, he simply didn't have the time to finish it.
Hi all!
I just would like to understand how is going on..

I mean, will this feature be included into TB2?

If not, what's the way to build a TB with this feature?

Thanks in advance for the attention!
(In reply to comment #123)
> Hi all!
> I just would like to understand how is going on..
> 
> I mean, will this feature be included into TB2?
> 
> If not, what's the way to build a TB with this feature?
> 
> Thanks in advance for the attention!
> 

It is not clear exactly how this will be done. If the Penelope team adds this feature, it will likely be in the mail/news code. If the Thunderbird people want to include the change they should be able to; that is up to them.

Matt
Most of the discussion going on in here should probably be taking place in a newsgroup - can someone start one and link to it here so we can try and minimize future noise?
(In reply to comment #125)
> Most of the discussion going on in here should probably be taking place in a
> newsgroup - can someone start one and link to it here so we can try and
> minimize future noise?
> 

While not a newsgroup, feel free to use the Penelope section of the Eudora Forums for discussion. See http://eudorabb.qualcomm.com/showthread.php?t=11740

Matt
this just takes part of Mark's fix for bug 336491 and puts it on the branch - it has the effect of stopping the ldap properties dialog from coming up on the os/x address book. The properties dialog doesn't work correctly on the os/x address book, but it's better not to bring up the ldap directory props...

I'll try to figure out why we think the type is ldap, but this is a quick fix.
Attachment #258592 - Flags: superreview?(mscott)
right diff this time
Attachment #258592 - Attachment is obsolete: true
Attachment #258592 - Flags: superreview?(mscott)
Attachment #258594 - Flags: superreview?(mscott)
Attachment #258594 - Flags: superreview?(mscott) → superreview+
Comment on attachment 255002 [details] [diff] [review]
v2.2

David (one of the thunderbird devs) and I have been discussing progressing this patch. As part of the progressing, I've got the task of reviewing it. I think David will be updating the patch with my comments.

David: I've still got to go through this in a bit more detail (I may not get to that till tomorrow or after the weekend), but I think there's enough here to do a first pass at updating the patch.

General:

+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */

All of these should specify a tab-width of 2, and a c-basic-offset of 2.

All indentation should be changed to 2 spaces as well.

+        if (!propertyMap.mOSXProperty) {
+            continue;
+        }

There are a lot of these single lines where {} are not required.


Index: mail/app/Makefile.in

-LIBS	+= -framework QuickTime -framework IOKit -lcups
+LIBS	+= -framework QuickTime -framework IOKit -lcups -framework AddressBook

Please confirm if this change is definitely required. If so, a similar change needs adding to suite/app/Makefile.in and 

xpfe/bootstrap/Makefile.in otherwise the patch will break SeaMonkey builds.

Index: mailnews/build/Makefile.in

We're going to need a similar change in mailnews/addrbook/build/Makefile.in - (for SeaMonkey builds)

Index: mail/config/basemail-os2
Index: mail/config/basemail-unix
Index: mail/installer/windows/packages-static

I don't see that these changes have anything to do with this patch, please remove.

+#define NS_ABOSXDIRECTORY_PREFIX "moz-abOSXdirectory"
+#define NS_ABOSXCARD_PREFIX "moz-abOSXcard"

All the rest of the prefixes are in completely in lower case, I think these should be as well.

+EXPORTS		+= \
+		nsAbOSXCard.h \
+		nsAbOSXDirFactory.h \
+		nsAbOSXDirectory.h \
+		$(NULL)

We should not be exporting these files.

Index: mailnews/addrbook/src/nsAbOSXCard.h

+struct nsStringPropertyMap;
+
I see no reason for this to be in this file.

+class nsIAbOSXCard : public nsISupports

Not sure I really like this being defined in just c++. Would it be better to do it from idl so that js can access it?

+    friend class nsAbOSXUtils;

The only items in nsAbOSXUtils are public - so this doesn't need to be a friend (besides I prefer not to use friend 

options anyway).


Index: mailnews/addrbook/src/nsAbOSXCard.mm


+NS_IMPL_ISUPPORTS_INHERITED2(nsAbOSXCard,
+                             nsRDFResource,
+                             nsIAbCard,
+                             nsIAbOSXCard)

Shouldn't that be inherited3?

Index: mailnews/addrbook/src/nsAbOSXCard.mm
+static void
+SetStringProperty(nsAbOSXCard *aCard, nsString &aValue, nsString &aMember,

The static functions in this file should be private functions in nsAbOSXCard.

+            PRBool found = MapMultiValue(this, card, propertyMap, aNotify,
+                                         abSession);
+
+            if (found && propertyMap.mOSXProperty == kABAddressProperty) {

Should just be one if statement.

+#define SET_STRING(_value, _name, _notify, _session) \
+    SetStringProperty(this, _value, m_##_name, #_name, _notify, _session)
+#define MAP_DATE(_date, _name, _notify, _session) \
+    MapDate(this, _date, m_##_name##Year, #_name"Year", m_##_name##Month, \
+            #_name"Month", m_##_name##Day, #_name"Day", _notify, _session)

I think it'd be clearer if these were at the start of the file.

+        // Use the order used in the OS X address book to set DisplayName.
+        int order = flags & kABNameOrderingMask;
+        if (order == kABDefaultNameOrdering) {
+           order = [addressBook defaultNameOrdering];
+        }

This is wrong. We should be using the display name format that is used in the rest of the address book to set the order - 

so that we are consistent in the application. 

+        nsAutoString displayName;
+        if (order == kABFirstNameFirst) {
...
+        else {
+            displayName.Append(m_LastName);
+            displayName.Append(' ');
+            displayName.Append(m_FirstName);
+        }

Typically I believe this should have a comma. I've got a feeling there is a function around that does this anyway.

+            unsigned int j, count = [value count];
+            j = [value indexForIdentifier:[value primaryIdentifier]];
+
+            if (j < count) {

Seems a bit unnecessary to separate these into two variables. Though if os x requires it I'd prefer to do:

unsigned int count = [...
unsigned int j = [...

Index: mailnews/addrbook/src/nsAbOSXDirectory.h

+class nsIAbCardHashKey : public PLDHashEntryHdr

I think this should just be nsAbCardHashKey - its not an interface.

+class nsIAbOSXDirectory : public nsISupports

Same comment as above about nsIAbOSXCard

class nsAbOSXDirectory

I think this should really implement GetURI as well.

Index: mailnews/addrbook/src/nsAbOSXDirectory.cpp

+        rv = rdfService->GetResource(NS_LITERAL_CSTRING("moz-abOSXdirectory:///"),
+                                     getter_AddRefs(resource));

There's a #define around that should be used for most of that string.

+static nsresult
+MapConditionString(nsIAbBooleanConditionString *aCondition, PRBool aNegate,
+                   PRBool &aCanHandle, ABSearchElement **aResult)
+static nsresult
+BuildSearchElements(nsIAbBooleanExpression *aExpression,
+                    PRBool &aCanHandle,
+                    ABSearchElement **aResult)
+static PRBool
+Search(nsIAbBooleanExpression *aExpression, NSArray **aResult)
+static PRUint32 sObserverCount = 0;
+static ABChangedMonitor *sObserver = nsnull;

Again, I'd prefer it if these were member functions/varables.

+NS_IMPL_ISUPPORTS_INHERITED2(nsAbOSXDirectory,
+                             nsAbDirectoryRDFResource,
+                             nsIAbDirectory,
+                             nsIAbOSXDirectory)

3?

+NS_IMETHODIMP
+nsAbOSXDirectory::Init(const char *aUri)
+{

This should be defining/setting up m_DirPrefId like we've recently done in the Init functions of the other directory types.

+    if (mURINoQuery.Length() > 22) {
+        nsCAutoString uid(Substring(mURINoQuery, 21));

We should be using constants for those numbers (and other places in this file). Additionally, the 21/22 difference looks strange.
Attachment #255002 - Flags: review-
Comment on attachment 255002 [details] [diff] [review]
v2.2

If someone is updating this, a couple of comments on the Cocoa side:

>+    [[aNotification object] release];

This shouldn't be here at all, as this method doesn't own that object.

>+        array = [[NSMutableArray alloc] init];

This should be
  array = [NSMutableArray arrayWithCapacity:count];
(it leaks the array as it is now).

This version of the patch removed the static autorelease pool entirely. Is there guaranteed to be an autorelease pool already in place when this code is used? If not, everything will leak.
thx, Mark, I'll try to address your comments as soon as I can.
+class nsIAbCardHashKey : public PLDHashEntryHdr

I think this should just be nsAbCardHashKey - its not an interface.

No, but it is a hashkey for nsIAbCards, so nsAbCardHashKey isn't right either. So perhaps something ugly like nsHashKeyFornsIAbCards. I'm inclined to leave it the way it is unless we can think of something better. It's not used outside of nsAbOSXDirectory.h

the bad news is that this didn't build on our tinderbox, so unless we can get an updated sdk, this is a non-starter for 2.0...(unless I screwed something up landing this, but I suspect it's the sdk version)

Assignee: nobody → bienvenu
+NS_IMPL_ISUPPORTS_INHERITED2(nsAbOSXCard,
+                             nsRDFResource,
+                             nsIAbCard,
+                             nsIAbOSXCard)

this is correct - it's only implementing 2 interfaces, and inheriting from a concrete base class (nsRDFResource).

Mark, I'm going through and trying to address your other comments.
Hello,

This issue about a global addressbook is not Mac specific.

I'm happy to see that other people share my views about the necessity of a system-wide addressbook.

Please see these also:   (they were brutally closed  ;-)   )
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=221028
http://forums.mozillazine.org/viewtopic.php?t=508632&highlight
http://bugs.freestandards.org/show_bug.cgi?id=104


Regards,
Răzvan


Hello,

This issue about a global addressbook is not Mac specific.

I'm happy to see that other people share my views about the necessity of a system-wide addressbook.

Please see these also:   (they were brutally closed  ;-)   )
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=221028
http://forums.mozillazine.org/viewtopic.php?t=508632&highlight
http://bugs.freestandards.org/show_bug.cgi?id=104


Regards,
Răzvan
(In reply to comment #69)
> user_pref("ldap_2.servers.osx.description", "OS X");
> user_pref("ldap_2.servers.osx.dirType", 3);
> user_pref("ldap_2.servers.osx.uri", "moz-abOSXdirectory:///");

David, I'm concerned by the use of dirType = 3 here, http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/mailnews/addrbook/src/nsDirPrefs.h&rev=1.43&mark=63#57

Basically it implies its a MAPI directory, which I think is wrong. Whilst I think we'll get away with it from an internal AB perspective (having had a quick glace at the code), I think it would be better for this to be changed, or at least re-named.
Attachment #255032 - Attachment description: v2.2 → v2.2 (1.8 branch version)
I've thought about this comment and I tend to disagree:
"
+        // Use the order used in the OS X address book to set DisplayName.
+        int order = flags & kABNameOrderingMask;
+        if (order == kABDefaultNameOrdering) {
+           order = [addressBook defaultNameOrdering];
+        }

This is wrong. We should be using the display name format that is used in the
rest of the address book to set the order - 

so that we are consistent in the application. "

I think if the user tells us they want to use the OSX Address Book, to me that implies that's their primary address book, and we should try to respect its settings - instead of taking a Mozilla-app centric view, we should try to take a   wider OS/desktop-wide view.

Re this comment:

"+    friend class nsAbOSXUtils;

The only items in nsAbOSXUtils are public - so this doesn't need to be a friend
(besides I prefer not to use friend options anyway)."

That's required for the class to compile - iirc, the friend class needs access to some of the base class properties, or something like that. I'm not crazy about friend either, but it has its place.
(In reply to comment #137)
> I've thought about this comment and I tend to disagree:
...
> I think if the user tells us they want to use the OSX Address Book, to me that
> implies that's their primary address book, and we should try to respect its
> settings - instead of taking a Mozilla-app centric view, we should try to take
> a   wider OS/desktop-wide view.

I'm happy if you want to do that, but if we do, then View->Show Name As menu option should be disabled for OSx book display (which may be confusing should we ever display results from more than one book at the same time), or possibly given another option - use default for specified ab.

We would probably also have to work out what to do with mail.addr_book.displayName.lastnamefirst in the OSx case if/when we implement writing to the OSX ab.
(In reply to comment #138)

I've been waiting for this feature to be in the stable release for a long time.  Now this discussion about primary addressbooks is a good time to mention something that I think has been dogging the approach used in this patch.

Thunderbird only needs to know how to read address data out of the address book, it doesn't need to display or edit it.  If someone uses the OSX Address Book, then when they click to bring up an entry, Thunderbird should launch the OSX Address Book application and let it do the viewing/editing.  I'm not exactly sure how this would look in the Thunderbird GUI, but it should be much easier to code, and it would be much more consistent with the whole concept behind using a single unifying Address Book application.

How to handle a mixed environment might be a little complicated, but is there really a large mixed environment target audience?
(In reply to comment #139)

> Thunderbird only needs to know how to read address data out of the address
> book, it doesn't need to display or edit it.  If someone uses the OSX Address
> Book, then when they click to bring up an entry, Thunderbird should launch the
> OSX Address Book application and let it do the viewing/editing.

This is essentially how Apple's Mail.app interacts w/ Address Book.app and it seems to work just fine.
I compiled on the latest 2.0 branch, and believe I found what the problem was: in mozilla/mail/app/Makefile.in, a -framework AddressBook has been dropped. It was in the patches, but didn't seem to make it/stay in the proper version. 

Appears to work fine with that change. Since this will probably take a while to make it into a release, I'm going to add my patch on the buglist, which includes commenting out the #if 0 and the commented out Makefile parts, so that folks who want it can build it for their own uses.

This is the patch just for the missing library:
RCS file: /cvsroot/mozilla/mail/app/Makefile.in,v
retrieving revision 1.46.2.5
diff -a -u -r1.46.2.5 Makefile.in
--- mail/app/Makefile.in	28 Feb 2007 06:52:59 -0000	1.46.2.5
+++ mail/app/Makefile.in	12 Jun 2007 19:51:08 -0000
@@ -159,7 +159,7 @@
 
 ifneq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT)))
 ifdef BUILD_STATIC_LIBS
-LIBS	+= -framework QuickTime -framework IOKit
+LIBS	+= -framework QuickTime -framework IOKit -framework AddressBook
 endif
 endif
 
At the risk of using these comments for the "wrong" purpose, and writing from a position of ignorance...

Would it be possible to make a change that adds some sort of "hook" by which an extension developer could launch another address book (e.g., Mac OS X's Address book) and get addresses there? That puts the onus on the putative extension developer(s) to work out the details.

Obviously, if the turnaround time for something like this is as long as that for the change above, then it's not worth it. But if it's simple to expose address book lookups for extensions, then it may be worthwhile.
I found a second problem which may have resulted in the tinderbox failure: Thunderbird is still being compiled on ppc's with SDK 10.2.8. Some of the addressbook symbols aren't defined there. The "Applish" way to handle this would be to compile with sdk=10.3 and target=10.2 and check for null pointers of the weakly linked symbols. Since it'll probably take until Thunderbird 3 to start compiling with SDK 10.3, I've added code to check for the compilation SDK and define unknown symbols with nils or defaults to avoid using non-existent features. I've also set up the code so that it'll compile with 10.3, and properly test for nulls if it is actually run under 10.2.
Attachment #268125 - Attachment is obsolete: true
a.peyser, thx a lot for this patch! I'll try to work this into the trunk when I get a chance.
Flags: blocking-thunderbird2?
Hardware: Macintosh → All
After quite a long time not checking the status of this bug I am back with some input.
Maybe we have to rethink this thing a bit.

The major thing which makes software good software is: not reinventing the wheel.
the non osx mozilla people always cry when talking about this bug as it is non standard (different from the other platforms).

every major operating system has its own "standard" adressbook manager. so why not just use the default system adressbook on EVERY system (not only osx)?

personally (as an osx user) i really dont get the point why i should use thunderbirds internal adressbook (or at least the ui with my system adressbook data).

the way mail.app (the default osx mail application) handles the adressbook is the way to go (in my opinion). when clicking "adressbook" in thunderbird, launch the default system adressbook (as said before, not only on osx, also on windows, linux, ....). when i write an email.. hook up the system adressbook via its API and do autocompletion... and thats it.

why replicating features which are already in the system?
even windows has its own system adressbook - why not use it?
(In reply to comment #146)
> Maybe we have to rethink this thing a bit.

This is not a discussion for this bug. The discussion you wish to have would be best suited to the newsgroups (probably mozilla.dev.apps.thunderbird or mozilla.dev.apps.seamonkey).
The original problem was to "Add ability to read from Mac OS X system Address Book". The solution must take into account the underlying standard used by Apple to store the data. This standard is described at the following address:

http://en.wikipedia.org/wiki/VCard

In a different post, I proposed to merge Mozilla Sunbird with the address book in Thunderbird. Sunbird is similar to Apple's iCal and happily manages the iCalendar standard:

http://en.wikipedia.org/wiki/ICalendar

Now, if Sunbird is extended to the VCard standard, Thunderbird could easily "use" Sunbird to store and retrieve the addresses, and Firefox could similarly use Sunbird to store and retrieve the bookmarks (similarly to Apple's browser Safari). Not nearly enough, bookmark/address management would finally receive proper management. Finally, back to the original problem of how to "Add ability to read from Mac OS X system Address Book", Sunbird is already able to import the data from Apple's iCal, due to its compliance with the iCalendar standard; it could similarly import the data from Apple's Address Book, if compliance with the vCard standard is added.

This would solve not only the original problem, but also a stream of others, including the possibility to finally have Sunbird as an open-source and cross-platform Personal Organizer. In summary, you get a stream of pigeons with one stone.
This is not what this bug is about. Import/Export is not enough. And pushing people to use Sunbird is not different from pushing people to use Thunderbird's addressbook. The key is integration with the underlying system ...and that is what this patch is meant to bring - at least for OSX.
This enhancement already has a brilliant solution (thanks a.peyser, its working great!) which just waits for being commited to the trunk. Ideas on a general solution for more platforms should be discussed elsewhere.

For clarification (In reply to comment #148):

This bug is not talking about importing address book data from Mac OS address book. It about adding the address book as a directory to Mailnews. Sunbird, in contrast, is _importing_ data from iCal.
Does anybody have both the patch and the compile commands?
Re: Daniel Küstner

> This enhancement already has a brilliant solution
> which just waits for being commited to the trunk.

I've seen this type of comments for the good part of two years now.
If any such solution truly exist, then an add-on for it would solve
the problem about waiting commitment to trunk.
(In reply to comment #152)
> If any such solution truly exist

It exists, it works brilliantly and you can download it, see comments above. Did you try it out? And did you read and _understand_ our remarks on the difference between Import/Export and real integration?
(In reply to comment #144)
> Created an attachment (id=268949) [details]
> Patch to compile with SDKs 10.2,10.3 and 10.4 (for 2.0.0.4)

Applying the patch to the current trunk sources results in a lot of rejects. Seems that there were a lot of changes within the last month.

patching file mail/app/Makefile.in
Hunk #1 FAILED at 159.
1 out of 1 hunk FAILED -- saving rejects to file mail/app/Makefile.in.rej
patching file mailnews/addrbook/build/nsAbFactory.cpp
Hunk #1 FAILED at 135.
Hunk #2 FAILED at 334.
Hunk #3 FAILED at 351.
3 out of 3 hunks FAILED -- saving rejects to file mailnews/addrbook/build/nsAbFactory.cpp.rej
patching file mailnews/addrbook/src/Makefile.in
Hunk #1 FAILED at 157.
1 out of 1 hunk FAILED -- saving rejects to file mailnews/addrbook/src/Makefile.in.rej
patching file mailnews/addrbook/src/nsAbOSXCard.mm
Hunk #2 FAILED at 253.
1 out of 2 hunks FAILED -- saving rejects to file mailnews/addrbook/src/nsAbOSXCard.mm.rej
patching file mailnews/addrbook/src/nsAbOSXDirectory.mm
Hunk #2 FAILED at 204.
1 out of 2 hunks FAILED -- saving rejects to file mailnews/addrbook/src/nsAbOSXDirectory.mm.rej
patching file mailnews/addrbook/src/nsAbOSXUtils.mm
patching file mailnews/build/Makefile.in
Hunk #1 FAILED at 149.
1 out of 1 hunk FAILED -- saving rejects to file mailnews/build/Makefile.in.rej
patching file mailnews/build/nsMailModule.cpp
Hunk #1 FAILED at 407.
Hunk #2 FAILED at 989.
Hunk #3 FAILED at 998.
3 out of 3 hunks FAILED -- saving rejects to file mailnews/build/nsMailModule.cpp.rej
I have this patch in my trunk tree, and it compiles - I'll try to find some time to try to land it on the trunk.
(In reply to comment #153)
> It exists, it works brilliantly and you can download it, see comments above.
> Did you try it out? And did you read and _understand_ our remarks on the
> difference between Import/Export and real integration?

I am trying to try it out, but without any knowledge whatsoever on how to apply the above patch or to compile with it, I'm not getting any further.

I'd love to see David Bienvenu's trunk version!

(In reply to comment #156)
> I am trying to try it out, but without any knowledge whatsoever on how to apply
> the above patch or to compile with it, I'm not getting any further.
> 
> I'd love to see David Bienvenu's trunk version!

From comment 113: A compiled version can be found here:

http://icecube.co.nz/downloads/thunderbird/latest/
Comment on attachment 268949 [details] [diff] [review]
Patch to compile with SDKs 10.2,10.3 and 10.4 (for 2.0.0.4)

I've tried this on the trunk - it still builds on my machine (thought the mail/app/Makefile.in part doesn't apply).

I'm going to check this in, and then try turning this on on the trunk, one more time. Thx very much for the patch.
Attachment #268949 - Flags: superreview+
I can hardly believe it, but it seems that the Thunderbird and Seamonkey Tinderboxes both claim to have built successfully with OSX Addressbook integration turned on. So it would be worth downloading tomorrow's trunk builds, setting the relevant prefs, and trying it out.
David, I have rebuilt my trunk debug version 3.0a1pre (2007080501) and added following prefs to prefs.js:

user_pref("ldap_2.servers.osx.description", "OS X");
user_pref("ldap_2.servers.osx.dirType", 3);
user_pref("ldap_2.servers.osx.uri", "moz-abOSXdirectory:///");

The system address book isn't visible in the list of address books. Even the autocomplete feature doesn't work within the composer window. Is there anything more I have to enter to get it running?
Status: NEW → ASSIGNED
Same here. Downloaded the nightly, but no OSX address book.
try changing this:

user_pref("ldap_2.servers.osx.uri", "moz-abOSXdirectory:///");

to

user_pref("ldap_2.servers.osx.uri", "moz-abosxdirectory:///");

I changed the code to have a lower case uri. That may have been the wrong thing to do, but that's the URI I had set, and when I was trying to get the code to work again (it had bitrotted quite a bit), I made it expect a lower case uri value (like LDAP does).
Dude! Sweet! That was it! I see OS X Addresses in my address book. Hope to look at further after a celebratory beer.
Does it also work with 2.0.0.6pre or 2.0.0.7pre? I don't seem to get it to work there yet. I'm using a tinderbox build from: ftp://ftp.mozilla.org/pub/mozilla.org/thunderbird/tinderbox-builds/latest-mozilla1.8-l10n
(In reply to comment #162)
> user_pref("ldap_2.servers.osx.uri", "moz-abosxdirectory:///");

That's it! Thanks David. It's so great to have the systems address book in Thunderbird! Thanks for all the work, also to Peterv and all other contributors.

Having some tests shows me a really unstable Thunderbird. I'm not able to get any debug info and for the official build the crashreporter opens but can't find its settings. It's bad. I will file this as a new bug.

David, I think we can close this bug as fixed?

(In reply to comment #164)
> Does it also work with 2.0.0.6pre or 2.0.0.7pre? I don't seem to get it to 

No, it doesn't. The patch was only checked-in to trunk.
No longer blocks: 391020
well, I'm sure there's more we'd like to do with this feature, but we can open new bugs for those. marking fixed. Thx to everyone who helped, and a special thanks to Peter for coding this.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Now up to Bug 391057 :-D
Uhm, sorry, but it doesn’t work at all for me. I tried a lot of things but there are no address book entries available in thunderbird.
Is there something special to do?

Which version has to be used? Do we still have to patch it or is there a build/binary?

What settings do I have to use?

Sorry, but I really don’t get it to work…

Thx
Hi Thorsten,

with latest trunk builds (version 3.0a1pre (2007080503) onwards) it worked for me after setting the following prefs:

user_pref("ldap_2.servers.osx.description", "OS X");
user_pref("ldap_2.servers.osx.dirType", 3);
user_pref("ldap_2.servers.osx.uri", "moz-abosxdirectory:///");

They can be added via the about:config dialogue (advanced -> general -> config editor). First and third pref have to be a string, second is an integer.

I hope that helps.
Blocks: 391057
Yes, thanks, that helps.
I used the latest trunk and that crashed all the time while rendering the initial screen.
Well, actually that one crashes too, but "just" if I try to open a message… ;-/
So, it’s fine to see the light at the end of the tunnel though it is useless at this moment.

Thx
Keywords: helpwanted
Target Milestone: --- → mozilla1.9 M8
Status: RESOLVED → VERIFIED
Sorry for the newbie question, but in which official Thunderbird release will this fix be included?
Thunderbird 3.0. The release should be in 2008.
I am currently building a 2.0.0.6 Universal build, patched, and hope it will work. If it works, I will upload it to my web site and put the address right here, so anybody interested can download it (since it's more stable than the trunk version, which is 3.0a1).
In reply to comment #173, that would be a great idea.  I have tried to use the trunk version 3.0 twice now, but I was not successful.  TBird would start, but as soon as I opened about conf, it would crash.  

Thanks to all who worked really hard to make this happen!

Egídio
It does work. I have a Universal 2.0.0.6 build, en-US, only with a slightly different icon (I will try to change this in the 2.0.0.7 build, or a possible en-GB build of 2.0.0.6). Add the following lines to your prefs.js:

user_pref("ldap_2.servers.osx.description", "OS X");
user_pref("ldap_2.servers.osx.dirType", 3);
user_pref("ldap_2.servers.osx.uri", "moz-abOSXdirectory:///");

Make use OSX is capitalised, unlike in the trunk edition. Here is the download, I only have limited bandwith (so if somebody has an idea where I could otherwise host this, please e-mail me):

http://files.mistermartin75.net/thunderbird-2.0.0.6.en-US.mac.dmg
Please get in contact with Robert Coleman who already built Thunderbird 2.0.0.4 as an Universal Binary:

http://icecube.co.nz/2007/7/12/at-last-a-new-thunderbird-universal-binary-with-apple-addressbook

Also be aware of the Mozilla Trademark Policy for Distribution Partners:
http://www.mozilla.org/foundation/trademarks/distribution-policy.html
See it as a community edition, and, thanks for pointing it out Henrik, I will not change the icons in any distribution, since this is not allowed (so either put them in yourself or wait for 3.0). Furthermore, nothing changes, and it just works.

Henrik, Robert Coleman is, as you may have noticed, a little behind on the builds (we're at 2.0.0.6 already), hence why I built that one.
To save martin's bandwidth, feel free to download it from:
http://lowbatteries.com/files/thunderbird-2.0.0.6.en-US.mac.dmg
I get this error with martin's build, when sending an email:
Unable to load address book file steven. It may be read-only, or locked by another application. Please try again later.
As others have commented, this is great to finally have this functionality.
Implementation question - is there some reason that names come out last name
first, first name last?

For example, entry in my Address Book is John Smith for john.smith@someglorp.com
But when I compose a message, it gets filled in as Smith John <john.smith@someglorp.com>
@lowbatteries: I also had this error, but only once. After I have set my Accounts to look into 'local address book' only (Preferences - Composition - Addressing) - the error vanished.

@James Hsieh: When composing a message I get 'Firstname Lastname <emailaddress>'


@tom - toggling that setting did fix my problem
is there any possibility to patch the build to de-DE?
I am trying to find out how to build localised builds, but I haven't had much time yet. I'll check that out later and will build it for you. In case I find out and anybody else is interested in localised builds, please send me an e-mail (this prevents this to become a submission form).
(In reply to comment #185)
> I am trying to find out how to build localised builds, but I haven't had much
> time yet. I'll check that out later and will build it for you. In case I find
> out and anybody else is interested in localised builds, please send me an
> e-mail (this prevents this to become a submission form).
> 

I made the localized build for NVU (mozilla's program to make web pages). The steps for that are listed on http://www.patrickhendriks.com/nvu.html

Thunderbird can be localized using the same approach.
Guys, this bug is fixed and verified. Could we please move chatter about acquiring builds out of Bugzilla? For instance, the mozillazine forums seem like a good place. I started a thread there, summarizing recent posts to this bug.

http://forums.mozillazine.org/viewtopic.php?p=3003235
Hello Guys:
First of all: You did a great job. Now I will use Thunderbird and nothing else.

But I have discoved something. As I don't know if I can change this through settings I post it here:
I'm using lists within my thunderbird addresses. I can normally drag-and-drop addresses from within my addressbooks to the lists. This is not working with the OSX address book which seems to be not dragable.
Any ideas? Can I change this by myself?

thanks for any ideas.
gb5256
(In reply to comment #180)
> As others have commented, this is great to finally have this functionality.
> Implementation question - is there some reason that names come out last name
> first, first name last?
> 
> For example, entry in my Address Book is John Smith for
> john.smith@someglorp.com
> But when I compose a message, it gets filled in as Smith John
> <john.smith@someglorp.com>
> 

I have the same issue. I have no idea why. I have tried a number of things to resolve this, but they have no effect: switching first/last on entries in my Address Book changing the Address Book "Display Order" preference, the AB "Sort by" preference, and looking at the Thunderbird (About:Config) preferences called "mail.addr_book.displayName.lastnamefirst" (false) and "mail.addr_book.lastnamefirst" (0). Any ideas? Is anyone else having this problem with the fix?
(In reply to comment #189)
> Any ideas? Is anyone else having this
> problem with the fix?

I don't have this problem with the build from icecube. But maybe you should open another bug for this?

(In reply to comment #188)
> I'm using lists within my thunderbird addresses. 
> This is not working with
> the OSX address book

Workaround: You can create lists in the OS X address book and use them in Tb.
(In reply to comment #190)
> (In reply to comment #189)
> > Any ideas? Is anyone else having this
> > problem with the fix?
> 
> I don't have this problem with the build from icecube. But maybe you should
> open another bug for this?
> 

I already opened bug 391478 for this issue.
For german users of the patched Thunderbird-Build for OSX just copy the files de.jar, de.manifest and the folder de.lproj from the german binary package to the adequate directories in the patched build and use the locale switcher add-on to activate the german localisation
Great work on this bug - are the three prefs lines going to be included by default in the OSX release?

Thanks
Noel
(In reply to comment #194)
> Great work on this bug - are the three prefs lines going to be included by
> default in the OSX release?

I filed bug 397811 for that issue.
Blocks: 397811
... Please keep an open mind. Others do...

http://www.opensync.org

Try to contribute to it, if you can.

Bob
(In reply to comment #179)
> I get this error with martin's build, when sending an email:
> Unable to load address book file steven. It may be read-only, or locked by
> another application. Please try again later.

I applied the patch to 2.0.0.9 on Tiger 10.4.11 (Intel) and I have the same error. I can reproduce it with a translated and original english version, also in Leopard (10.5.1). Everytime I type an email address and it will be autocompleted by Thunderbird, the error described in comment #179 appears. Even when I set up 'local address book only' as described in #182 the bug still exists when addresses got autocompleted.

It is very annoying. Has anyone another solution than completely turning off the  autocompletion feature?!
If you can reproduce with latest trunk, file a new bug about it (and you can't find one filed already).
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-trunk/
I think that Jason only talks about 1.8 branch builds. I cannot see any error on trunk. Due to this bug only handles the system address book integration on trunk any problems and questions about 2.0.0.x should be asked in the forum. This bug is fixed. Thanks.
Product: Core → MailNews Core
Perhaps a quick-and-dirty (and easy?) solution to allowing users to edit their Apple address book would be to put a button, menu item, and/or context menu in the Thunderbird address book UI that loads apple's address book externally.
Peter: interesting idea.  File a new bug for that?
OK, I just filed bug 489300 to "put a button, menu item, and/or context menu in
the Thunderbird address book UI that loads apple's address book externally".
> I think that Jason only talks about 1.8 branch builds. I cannot see any error
> on trunk. Due to this bug only handles the system address book integration on
> trunk any problems and questions about 2.0.0.x should be asked in the forum.
> This bug is fixed. Thanks.

Correct. I also tried trunk a few weeks ago. There the addressbook feature was implemented but unfortunately only readonly.
Depends on: 642549
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: