Doing GetResource(abUri).QueryInterface(Components.interfaces.nsIAbDirectory) or GetResource(abUri).QueryInterface(Components.interfaces.nsIAbLDAPDirectory) on an RDF Service soon after startup returns objects that don't have the dirPrefId correctly set, even though functions that return constants work fine. This implies they aren't being initialised properly. The problem appears to be that nsAbDirectoryRDFResource defines an Init function to initialise the RDF resource. Out of the three directory types we currently have, only LDAP & Outlook extend the Init function, and neither of those set dirPrefId. So getting the RDF resource means you're getting something without dirPrefId set - which we now rely on a lot more. It should be noted that once address book is loaded, then dirPrefId is correctly set. This also could do with an explanation.

Ian, I think this initialises the m_DirPrefId in LDAP directories correctly. Can you test and see if you think its working right and fixes your problem? If so, then I'll work on changes for the other directory types as well.

(In reply to comment #1) > Created an attachment (id=248179) [edit] > Possible fix for ldap case > > Ian, I think this initialises the m_DirPrefId in LDAP directories correctly. > Can you test and see if you think its working right and fixes your problem? > Yes, it seems to get my patch working okay and everything seems to be initialised correctly now.

Fixes the ldap case, also adds comment to nsAbDirProperty in case we create new classes from it. I've a WIP patch for the local case, though its a bit nastier than this one.

Comment on attachment 251683 [details] [diff] [review] LDAP fix v1 (checked in) one nit - you don't need to assign uri.Length to length - you only use it in one place.

This is the WIP patch for the MDB case. I think it works ok, though any comments would be appreciated. I'm a bit concerned about having to search all the ldap_2.servers.* prefs for the filename, but its the best way I could think of. Ideally we'd rewrite the prefs backend and have better uris and separate pref bases for the different types of ab.

This seems to work for the mdb case. I really don't like having to iterate through the prefs, but as I said before, I think its about the only way we have to do it at the moment (and hopefully one day, it'd all be rewritten in a better way ;-) )

Comment on attachment 251683 [details] [diff] [review] LDAP fix v1 (checked in) r=me assuming you fix as per David's comment.

Comment on attachment 251683 [details] [diff] [review] LDAP fix v1 (checked in) LDAP patch checked into trunk with nit fixed.

Comment on attachment 251683 [details] [diff] [review] LDAP fix v1 (checked in) >Index: mailnews/addrbook/src/nsAbDirProperty.h >+ * Note that any derived implementations should ensure that this item >+ * (m_DirPrefId) is correctly initialised correctly correctly correctly

Comment on attachment 251958 [details] [diff] [review] MDB fix v1 You have missed out the header file changes in this version of the patch. The bracket before the return needs to be correctly indented too. With patch applied everything still works with my patch on bug125188 (but it did for local address books already).

Comment on attachment 251958 [details] [diff] [review] MDB fix v1 Updated patch coming in a mo.

Revised patch that fixes Ian's comments

Comment on attachment 252645 [details] [diff] [review] MDB fix v2 (checked in) don't you really want to know if child ends with ".filename", not if it contains "filename"? i.e., StringEndsWith(child, ".filename") sr=bienvenu, but if that optimization makes sense, feel free to make it...

Comment on attachment 252645 [details] [diff] [review] MDB fix v2 (checked in) I checked this in with the StringEndsWith option that David mentioned - should be a better optimisation like that.

This should fix the outlook case. I haven't compiled/tested this as I don't have access to windows dev - Ian could you compile & test for me? For testing I suggest inserting a printf just after the points where 1) uri is shortened and 2) m_DirPrefId is assigned, then starting ab - there should be some output when it starts up, and further output if you do searches and things (Note that an Outlook (or Express) ab needs setting up as per http://kb.mozillazine.org/Using_Outlook_%28Express%29_contacts_with_Thunderbird_or_Mozilla_Mail).

-w diff version of above.

Comment on attachment 253113 [details] [diff] [review] Outlook fix v1 (checked in) I think Ian's a bit busy at the moment, David would you be able to look at this for me please?

Comment on attachment 253113 [details] [diff] [review] Outlook fix v1 (checked in) thx for the patch - a couple nits: if (StringEndsWith(child, NS_LITERAL_CSTRING(".uri"))) + { + if (NS_SUCCEEDED(prefBranch->GetCharPref(child.get(), + getter_Copies(childValue)))) + { + if (childValue == uri) these can be all one big if + // We need to ensure that the m_DirPrefId is initialized properly + nsCAutoString uri = aUri; I prefer nsCAutoString uri(aUri); but I don't really care.

Comment on attachment 253113 [details] [diff] [review] Outlook fix v1 (checked in) I checked this in with David's nits addressed and a couple of bustage fixes. One was to add an extern to nsAbWinHelper.h (there were already some there) - I'd missed it in this version of the patch. The second was to revert: prefix.AssignLiteral(mAbWinType == nsAbWinType_Outlook ? "OP ", "OE "); back to what it was, because it didn't like the , and then it didn't like the method of using ? for assigning to a literal. I don't think we need to follow this up with anything.

All done -> fixed.

Did it want a ':' instead of a ',' ?

(In reply to comment #21) > Did it want a ':' instead of a ',' ? > Yes, but it also didn't like the fact we were using the "? :" notation in the AssignLiteral because I think it was wanting a fixed string, and it wasn't quite getting it.

This broke outlook address books - they don't use dir prefs...

Comment on attachment 323701 [details] [diff] [review] Outlook backout plus nsAbDirProperty tweak r=me, though I haven't got the capability to test this at the moment. I think it should all work.

Outlook changes backed out.