546854 - Preference Manager should be able to get/set file preferences.

Status: RESOLVED FIXED

(Camino Graveyard :: Preferences, defect)

Description

[Christopher Henderson]

Bug 546853 requires getting and setting a complex pref (of type nsILocalFile). It makes sense to add this functionality to the Preference Manager.

Comment 1

[Stuart Morgan]

I'd recommend the API be get/setFilePref, have it deal in NSURLs, and do the conversion to and from nsILocalFile internally. That's consistent with the way the rest of the prefs API works, and doesn't force clients to deal in Gecko types to get completely generic support we may never need.

Comment 2

[Christopher Henderson]

Attachment: Fix v1.0

Here's my initial attempt.

Comment 3

[Christopher Henderson]

Attachment: Fix v1.1

This is a bit nicer.

Comment 4

[Christopher Henderson]

Attachment: Fix v1.1

Sorry, this is the real Fix v1.1.

Comment 5

[Christopher Henderson]

Attachment: Fix v1.2

Now with support for spaces in directory names!

Comment 6

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

(I suppose this can actually land in cvs now, but it's not actually needed until 1.9.2; not quite sure how I want to track those cases yet.)

Comment 7

[Stuart Morgan]

Comment on attachment 428328 [details] [diff] [review]
Fix v1.2

r=me.

However... the more I think about it the more I think the default return value for the getter should be nil. I know that's not consistent with the string method, but a URL based on an empty string isn't a meaningful object in the same way that @"" is. I couldn't even tell you for sure what URL that would construct (and I'm not sure an empty string is a valid init argument, since I don't know if the RFC allows empty string URL).

So I think the first line should be changed to 
... = nil;

Comment 8

[Christopher Henderson]

Attachment: Fix v1.3

Same as version 1.2, but with nil initializer for prefValue.

Comment 9

[Mike Pinkerton (not reading bugmail)]

+- (NSURL*)getFilePref: (const char*)prefName withSuccess:(BOOL*)outSuccess;
 - (NSString*)getStringPref: (const char*)prefName withSuccess:(BOOL*)outSuccess;
 - (NSColor*)getColorPref: (const char*)prefName withSuccess:(BOOL*)outSuccess;
 - (BOOL)getBooleanPref: (const char*)prefName withSuccess:(BOOL*)outSuccess;
 - (int)getIntPref: (const char*)prefName withSuccess:(BOOL*)outSuccess;

why the space after the colon? Can we just fix that? The definition doesn't have the space.


+    (void)mPrefs->SetComplexValue(prefName, NS_GET_IID(nsILocalFile), filePref);

Why the (void)? Do we really do this elsewhere?

sr=pink

Comment 10

[Christopher Henderson]

Attachment: Fix v1.4, for checkin

Those two bits of code in the sr review were done to maintain consistency with similar code in the same source file. Changing them doesn't affect the resulting behaviour, so the patch for checkin does that.

Comment 11

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

http://hg.mozilla.org/camino/rev/330bb58041d9