397202 - Error accessing nsIAbDirectory.URI with OS X addressbook

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Phil Ringnalda (:philor)]

STR:
1. Set up OS X ab integration per bug 203927 comment 169
2. Open Thunderbird prefs, Composition pane, Addressing tab
3. Try to find something listed in the menu for "Automatically add outgoing e-mail addresses to my:"

Error console says:

Error: [Exception... "Component returned failure code: 0x80004001 (NS_ERROR_NOT_IMPLEMENTED) [nsIAbDirectory.URI]"  nsresult: "0x80004001 (NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame :: chrome://messenger/content/preferences/compose.js :: anonymous :: line 114"  data: no]
Source File: chrome://global/content/bindings/preferences.xml
Line: 676

which seems odd to me, since I think it's talking about http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mail/components/preferences/compose.js&rev=1.18&mark=148,150#134 where I naively expected .isRemote to be true for the read-only OS X ab. Or perhaps I don't understand the point of .isRemote, and this is actually a Tb prefs bug, that it needs to check one more thing before going in there?

Comment 1

[Mark Banner (:standard8)]

Attachment: Possible Fix

Phil, could you give this patch a try please? (I've not got OSX facilities).

This provides a GetURI function for the OSX directory, makes the LDAP version consistent with the rest of them (it was wrong), and also improves the comment in nsIAbDirectory just to make it clear what (Get)URI should actually return.

I've still gotta test the LDAP part fully though, I think it'll work - I need to rebuild tb first as I haven't built it in a while.

If it doesn't work could you enter the following in the error console and post what comes out from the dump commands:

var enumerator = Components.classes["@mozilla.org/rdf/rdf-service;1"].getService(Components.interfaces.nsIRDFService).GetResource("moz-abdirectory://").QueryInterface(Components.interfaces.nsIAbDirectory).childNodes; while(enumerator.hasMoreElements()) { var ab = enumerator.getNext(); if (ab instanceof Components.interfaces.nsIAbDirectory) dump("abURI is " + ab.URI + "\n"); }

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 284656 [details] [diff] [review]
Possible Fix

I've just checked the LDAP part of this and I'm happy its working correctly, hence requesting reviews. See also my previous comment.

Comment 3

[Phil Ringnalda (:philor)]

Argh, sorry for the delay - something ate the mail for comment 1, so I didn't see your previous comment when I should have.

/Users/philringnalda/moz/mozilla/mailnews/addrbook/src/nsAbOSXDirectory.mm:486: error: no 'nsresult nsAbOSXDirectory::GetURI(nsACString_internal&)' member function declared in class 'nsAbOSXDirectory'
/Users/philringnalda/moz/mozilla/mailnews/addrbook/src/nsAbOSXDirectory.mm: In member function 'nsresult nsAbOSXDirectory::GetURI(nsACString_internal&)':
/Users/philringnalda/moz/mozilla/mailnews/addrbook/src/nsAbOSXDirectory.mm:489: error: no matching function for call to 'nsACString_internal::AppendLiteral(char)'

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #3)
> /Users/philringnalda/moz/mozilla/mailnews/addrbook/src/nsAbOSXDirectory.mm:486:
> error: no 'nsresult nsAbOSXDirectory::GetURI(nsACString_internal&)' member
> function declared in class 'nsAbOSXDirectory'
> /Users/philringnalda/moz/mozilla/mailnews/addrbook/src/nsAbOSXDirectory.mm: In
> member function 'nsresult nsAbOSXDirectory::GetURI(nsACString_internal&)':
> /Users/philringnalda/moz/mozilla/mailnews/addrbook/src/nsAbOSXDirectory.mm:489:
> error: no matching function for call to
> 'nsACString_internal::AppendLiteral(char)'

I've just seen this but can't do a fresh patch till later. So in case you see this before then, can you try adding the following to the nsAbOSXDirectory.h header file (in class nsAbOSXDirectory, just under HasDirectory):

NS_IMETHOD GetURI(nsACString &aURI);

The line:

aURI.AppendLiteral('/');

should be:

aURI.AppendLiteral("/");

Comment 5

[Phil Ringnalda (:philor)]

That works fine, thanks, with the minor exception of offering "OS X" as an option for saving outgoing addresses when it really shouldn't.

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #5)
> That works fine, thanks, with the minor exception of offering "OS X" as an
> option for saving outgoing addresses when it really shouldn't.

Excellent. Thanks for trying that. I'll do an updated patch tonight. I should be able to fix the incorrect offering at the same time.

Comment 7

[Mark Banner (:standard8)]

Attachment: Fix v2

This should fix the compilation and incorrect selection of writeable directory problems that the previous patch had.

Comment 8

[Phil Ringnalda (:philor)]

Comment on attachment 284942 [details] [diff] [review]
Fix v2

Works beautifully, thanks! r=philringnalda, though I don't even pretend to understand AB.

Comment 9

[Mark Banner (:standard8)]

Patch checked in, fixed. Thanks for the help with testing Phil.