Last Comment Bug 664917 - Add Preferences API for getting default pref values
: Add Preferences API for getting default pref values
Product: Core
Classification: Components
Component: Preferences: Backend (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla7
Assigned To: Masayuki Nakano [:masayuki] (Mozilla Japan)
: Benjamin Smedberg [:bsmedberg]
Depends on:
Blocks: 548734
  Show dependency treegraph
Reported: 2011-06-16 20:22 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2011-06-22 10:21 PDT (History)
5 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1.0 (19.27 KB, patch)
2011-06-20 18:00 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
bzbarsky: feedback-
Details | Diff | Splinter Review
Patch v2.0 (20.82 KB, patch)
2011-06-20 21:51 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
roc: review+
bzbarsky: feedback+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2011-06-16 20:22:04 PDT
I need to do some default value getting of prefs (the sort that getDefaultBranch on the prefservice would let me do), but there seems to not be Preferences API for it...
Comment 1 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-06-16 20:28:55 PDT
What do you need actually? Only Preferences::GetDefaultBranch()?

Or GetDefaultBool(), GetDefaultInt() and others? If many places use the default value, I think the latter is better.
Comment 2 Boris Zbarsky [:bz] (still a bit busy) 2011-06-16 20:38:03 PDT
For my current thing, I'd need to replace all uses of GetBool("ui.use_native_popup_windows") with something that uses only default prefs.

So yes, the latter would be nicer.  ;)
Comment 3 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-06-16 20:40:35 PDT
Okay, I'll try to write the APIs.
Comment 4 Boris Zbarsky [:bz] (still a bit busy) 2011-06-16 22:17:20 PDT
For what it's worth, it looks like I may be removing the pref usage altogether instead, so this is a lot less urgent...
Comment 5 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-06-20 18:00:24 PDT
Created attachment 540641 [details] [diff] [review]
Patch v1.0

How about these APIs? I don't have better idea for GetDefaultBool()...
Comment 6 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-06-20 18:01:04 PDT
Comment on attachment 540641 [details] [diff] [review]
Patch v1.0

And requesting review to roc.
Comment 7 Boris Zbarsky [:bz] (still a bit busy) 2011-06-20 20:26:44 PDT
Comment on attachment 540641 [details] [diff] [review]
Patch v1.0

I really don't think we should add instances of values other than PR_FALSE and PR_TRUE being passed through PRBool, since the plan is to move to using bool at some point.

Can we require that GetDefaultBool take an argument that indicates what to return if the value is not set?
Comment 8 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-06-20 21:51:41 PDT
Created attachment 540676 [details] [diff] [review]
Patch v2.0

How about this?
Comment 9 Robert O'Callahan (:roc) (email my personal email if necessary) 2011-06-21 22:05:02 PDT
Comment on attachment 540676 [details] [diff] [review]
Patch v2.0

Review of attachment 540676 [details] [diff] [review]:
Comment 10 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-06-21 23:40:35 PDT
Comment 11 Matt Brubeck (:mbrubeck) 2011-06-22 10:21:04 PDT

Note You need to log in before you can comment on or make changes to this bug.