346880 - nsIAbDirectory.URI problem

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Emil Hesslow]

My extension loops through all addressbooks that exists and I found a problem.

nsIAbDirectory.URI is "moz-abmdbdirectory://". The filename is missing. The same problem is on nsIAbDirectory.directoryProperties.URI.

I talked to Standard8 on IRC so he knows what I mean.

Comment 1

[Mark Banner (:standard8)]

I wonder if this is the cause of some of our initialisation problems. I'll look into it in the next few days.

Comment 2

[Emil Hesslow]

Attachment: Patch #1

I must say that I have no knowledge of this is the right way to do. But after a lot of trial and error this seams to work for me. I must also say that I can not program C++. I mostly copy and paste and cross my fingers.

I'm not sure if I just can remove the part where the filename is found before appending it to the dbPath.

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 232377 [details] [diff] [review]
Patch #1

This patch looks reasonable, though I'd like to be able to reproduce it myself first before I can accept it.

However one change, if you left:

   // The filename for address books within this directory.
-  readonly attribute ACString fileName;
+  attribute ACString fileName;

as it was, drop the SetFileName functions and call and replace that (and the current aProperties->GetFileName) with just a directory->GetFileName would that still work?

I'm a bit concerned about adding a SetFileName in at that level at the moment as that will alter prefs and may interfere with other things at the moment.

Though its also suspect as to why the uri is wrong.

Comment 4

[Emil Hesslow]

The problem I had was that the filename never got set which mean that directory->GetFileName would just return null. To call it the filename needs to be set first and I couldn't find any place were it was set. That why I set it. You probably know the code much better so you could tell if I'm wrong.

The first thing I tried was after the line "rv = listDatabase->GetMailingListsFromDB(directory);" I used "directory->GetFileName" and outputed the filename. But is was empty.

Comment 5

[Mark Banner (:standard8)]

Attachment: Fixes the default preferences

This patch sets the default filename preferences for the personal & collected address book. Now that we have started moving away from using nsDirPrefs, directory.URI requires the filename preference to be set correctly (as we don't store a separate URI for mork address books). 

The older directory.directoryProperties.URI doesn't need that as the nsDirPrefs backend sets it internally within the address book but never saves the preference :-( .

So this patch sets the filename preferences within the default preferences and thus fixes the problem.

I think we can probably loose these lines as well (as the default preferences will do the same job):
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/mailnews/addrbook/src/nsDirPrefs.cpp&rev=1.116&mark=1579-1582,1599#1579
though I'm still considering that and if there are any other items that can be removed.

Comment 6

[Mark Banner (:standard8)]

Attachment: Fixes the default preferences

opps, removed the extra changes from another patch I had in my tree.

Comment 7

[Emil Hesslow]

Mark: The last patch solved the problem for me.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 232377 [details] [diff] [review]
Patch #1

Although this patch works, I think the other version is better as it gets to the heart of the problem.

Comment 9

[Mark Banner (:standard8)]

Comment on attachment 232605 [details] [diff] [review]
Fixes the default preferences

Mork address books normally have a filename set in prefs - I see no reason why we shouldn't do the same for the personal/collected abs, although of course I think we do overwrite these at the moment (its what we would do anyway). 

I can't see any ill effects of setting these prefs and it fixes the bug as well.

Comment 10

[Mark Banner (:standard8)]

Patch checked in to trunk. Thanks to Emil for your help with finding and solving this bug.

Comment 11

[Scott MacGregor]

Mark, I'm seeing something weird after updating m trunk build with this patch. Somehow I ended up with a .pab.filename value of "pab.mab" in my prefs.js file when I ran with a trunk build. But all of my personal address book data is in "abook.mab". So I'm now getting a blank address book. Anyone else report similar issues? 

Comment 12

[Scott MacGregor]

It gets even stranger. Every time I run a branch build, it actually overrides the pref with a value of pab.mab. If I reset the pref using a trunk build and restart the trunk build, the value is abook.mab and my address book looks good. If I then run a branch nightly, the pref gets overwritten with a user pref valueof "pab.mab" and my address book no longer works in th trunk or the branch until I reset the pref again. 

lxr shows no references to pab.mab on the 1.8 branch. Hmmm.

Comment 13

[Scott MacGregor]

This caused 349073 I think which effects users who use the trunk and 1.5 or 1.8.1, see https://bugzilla.mozilla.org/show_bug.cgi?id=349073#c3

Comment 14

[Scott MacGregor]

Comment on attachment 232605 [details] [diff] [review]
Fixes the default preferences

approving for the 1.8.1 branch for seamonkey and thunderbird as this helps fix some of the issues in Bug 349073.

Comment 15

[Mark Banner (:standard8)]

Patch checked in on branch at Scott's request.