nsIAbDirectory.URI problem

RESOLVED FIXED

Status

RESOLVED FIXED
12 years ago
10 years ago

People

(Reporter: hesslow, Assigned: standard8)

Tracking

({fixed1.8.1})

Trunk
fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

12 years ago
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.
(Assignee)

Comment 1

12 years ago
I wonder if this is the cause of some of our initialisation problems. I'll look into it in the next few days.
Component: Address Book → MailNews: Address Book
Product: Thunderbird → Core
QA Contact: address-book → addressbook
(Reporter)

Comment 2

12 years ago
Created attachment 232377 [details] [diff] [review]
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.
Attachment #232377 - Flags: review?(bugzilla)
(Assignee)

Comment 3

12 years ago
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.
(Reporter)

Comment 4

12 years ago
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.
(Assignee)

Comment 5

12 years ago
Created attachment 232604 [details] [diff] [review]
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.
(Assignee)

Comment 6

12 years ago
Created attachment 232605 [details] [diff] [review]
Fixes the default preferences

opps, removed the extra changes from another patch I had in my tree.
Attachment #232604 - Attachment is obsolete: true
(Reporter)

Comment 7

12 years ago
Mark: The last patch solved the problem for me.
(Assignee)

Comment 8

12 years ago
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.
Attachment #232377 - Attachment is obsolete: true
Attachment #232377 - Flags: review?(bugzilla) → review-
(Assignee)

Comment 9

12 years ago
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.
Attachment #232605 - Flags: superreview?(bienvenu)
Attachment #232605 - Flags: review?(bienvenu)

Updated

12 years ago
Attachment #232605 - Flags: superreview?(bienvenu)
Attachment #232605 - Flags: superreview+
Attachment #232605 - Flags: review?(bienvenu)
Attachment #232605 - Flags: review+
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: PC → All
(Assignee)

Comment 10

12 years ago
Patch checked in to trunk. Thanks to Emil for your help with finding and solving this bug.
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED

Comment 11

12 years ago
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

12 years ago
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.

Updated

12 years ago
Depends on: 349073

Comment 13

12 years ago
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

12 years ago
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.
Attachment #232605 - Flags: approval-thunderbird2+
(Assignee)

Comment 15

12 years ago
Patch checked in on branch at Scott's request.
Keywords: fixed1.8.1
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.