394781 - An address database (nsIAddrDatabase) should be got from the directory not from the address book service via uri.

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Mark Banner (:standard8)]

Attachment: The fix v1

I started working on this for bug 387404, but I've spun it off to reduce the size of that patch.

In nsIAddressBook we currently have:

nsIAddrDatabase getAbDatabaseFromURI(in string URI);

This returns a database for the given directory URI. We should really be getting the database from the nsIAbMDBDirectory (or nsIAbLDAPDirectory for replicated ABs).

Therefore I'm proposing we implement some new attributes (databaseFile and database under nsIAbMDBDirectory and similar ones for nsIAbLDAPDirectory) to do this.

This will simplify some of the code, and in the cases where we don't already have a directory, we'll just have to get it from the service RDF which is no problem as we already have the URI :-)

Comment 1

[David :Bienvenu]

Comment on attachment 279494 [details] [diff] [review]
The fix v1

this makes so much more sense to me. thx, Mark.

Comment 2

[Scott MacGregor]

Comment on attachment 279494 [details] [diff] [review]
The fix v1

cool!

Comment 3

[Mark Banner (:standard8)]

I checked this in last night with one minor bustage fix on windows.

Fixed.

Comment 4

[jag (Peter Annema)]

I don't seem to be able to add addresses to my address book, either through clicking an e-mail address (the dialog pops up but when I click the OK button I get an exception) or through the "always load remote content from ..." link.

I backed out these changes and it now seems to work again. Let me put them back in and see if I can get the error messages.

Comment 5

[jag (Peter Annema)]

"Click here to always load remote content from newsletter ...":

Error: uncaught exception: [Exception... "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [nsIAbMDBDirectory.cardForEmailAddress]"  nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)"  location: "JS frame :: chrome://messenger/content/mailWindowOverlay.js :: allowRemoteContentForSender :: line 2306"  data: no]


Add to Address Book, click OK button:

Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIAbDirectory.addCard]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://messenger/content/addressbook/abCardOverlay.js :: NewCardOKButton :: line 409"  data: no]
Source File: chrome://messenger/content/addressbook/abCardOverlay.js
Line: 409

Comment 6

[Mark Banner (:standard8)]

Just to let you know I'm looking at this. If I can't see an easy fix then I'll back out in an hour or two.

Is it Thunderbird or SeaMonkey that you're using? I'm not seeing the Add to address book error with SeaMonkey.

Comment 7

[Mark Banner (:standard8)]

I've just backed this out - although I can't see Jag's exact problems, I have found one case where we somehow get into an infinite loop :-(

I'll take another look at it all and see if I can get a revised patch soon.

Comment 8

[jag (Peter Annema)]

It's SeaMonkey, sorry, should've said that before.

Comment 9

[jag (Peter Annema)]

So it looks like GetAbDatabase() is failing. Let me see if I can figure out why.

Comment 10

[jag (Peter Annema)]

mURINoQuery has "moz-abmdbdirectory://abook.mab", so we use "moz-abmdbdirectory:/" are the parentURI to get the resource for.

Comment 11

[Mark Banner (:standard8)]

(In reply to comment #10)
> mURINoQuery has "moz-abmdbdirectory://abook.mab", so we use
> "moz-abmdbdirectory:/" are the parentURI to get the resource for.

Yeah, the mailing list detection is screwed. I fixed that locally, however I've now got the problem that the remote content policy stuff seems to be getting into an infinite loop for some weird reason.

Not sure why your new card ok button didn't work - it seemed to be fine for me.

Comment 12

[jag (Peter Annema)]

I've seen it work and I've seen it fail. I think it fails after I try clicking the link first. But since clicking the ok button ends up calling GetAbDatabase() it should always fail, methinks, unless for some reason it ends up with a different mURINoQuery depending on the order?

Comment 13

[Mark Banner (:standard8)]

Attachment: Revised patch

Jag, any chance you could give this patch a try please?

Its the same as the first, except in nsAbMDBDirectory::Init, I'm making sure that the mIsMailingList member variable is initialised correctly.

It seems to work ok here, but you were seeing some errors I wasn't.

Comment 14

[Mark Banner (:standard8)]

Comment on attachment 280154 [details] [diff] [review]
Revised patch

David, this patch is virtually the same as the original, except that I've fixed the windows compilation problem (that was I hadn't changed one function call), and I've also added a line to nsAbDirProperty::Init which will initialise mIsMailingList depending on the URI supplied.

This means that when we try and get the database it should now be working out the URI right, and therefore getting the right database (not from a broken uri).

I've given this a run and it certainly fixes my problems that I'd found, so I think its worth landing it again and seeing if we get any other problems.

Note that I'm carrying forward sr as the patch is still doing virtually the same thing.

Comment 15

[David :Bienvenu]

Comment on attachment 280154 [details] [diff] [review]
Revised patch

thx, Mark

Comment 16

[Mark Banner (:standard8)]

Patch checked in again, so hopefully this is fixed now ;-)