Closed Bug 99611 Opened 23 years ago Closed 23 years ago

Freeze nsIPrefService and nsIPrefBranch

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.6

People

(Reporter: chak, Assigned: bnesse)

References

()

Details

Attachments

(1 file, 2 obsolete files)

Freeze the following services/interfaces:

nsIPrefService
nsIPrefBranch

Please follow the guidelines outlined in the "How to freeze an interface" at
http://www.mozilla.org/projects/embedding/EmbedInterfaceFreeze.html to freeze
these interfaces.
Blocks: 98417
QA Contact: mdunn → depstein
->0.9.6

Hi Brian : 

If you've done changing this interface, could we get this frozen...

Thanks 
Chak
Target Milestone: --- → mozilla0.9.6
Just finished it up a few minutes ago. Doing a build right now to make sure
nothing broke. :)
changing qa contact to Dharma.
QA Contact: depstein → dsirnapalli
Attached patch Changes for API freeze. (obsolete) — Splinter Review
I have attached a patch which adds documentation to the 2 interfaces we are
freezing, as well as the remainder of the preferences interfaces which just
needed this done anyway.

There are also a couple of minor "correctness" changes that stemmed from the
documentation effort.

Input, reviews please.
Comment on attachment 52253 [details] [diff] [review]
Changes for API freeze.

sr=alecf
Attachment #52253 - Flags: review+
Comment on attachment 52253 [details] [diff] [review]
Changes for API freeze.

I think alec meant to check has-super-review. I checked it for him. has-review = valeski

Looks great!
Attachment #52253 - Flags: superreview+
Attachment #52253 - Attachment is obsolete: true
Comment on attachment 52257 [details] [diff] [review]
New patch file that includes nsIPrefBranch.idl, and not 2 copies of nsIPrefBranchInternal.idl

Sorry about my previous r=, I missed some stuff :-(.

1. Please remove the PREF_INVALID, LOCKED, REMOTE, LI_LOCAL, etc, and the associated comment. Those should live in the impl, not at the iface level.

2. Please use the attribute keyword in idl for the "get" "set" methods, when you can. This should cut down on description as well.

boolean getBoolPref(...) becomes

readonly attribute boolean boolPref(...) for example.

I don't think there's a "writeonly" though, so those you'd have to keep as "setFoo(...)"
Attachment #52257 - Flags: needs-work+
Comment on attachment 52257 [details] [diff] [review]
New patch file that includes nsIPrefBranch.idl, and not 2 copies of nsIPrefBranchInternal.idl

New patch posted based on valeski's comments (the ones that compiled anyway :D).
Attachment #52257 - Attachment is obsolete: true
Attachment #52278 - Flags: review+
looks ok, sr=alecf
On the constant values like PREF_INVALID and PREF_LOCKED, am I correct in
assuming that the values are used by getPrefType()?
getPrefType() returns PREF_STRING, PREF_INT, or PREF_BOOL. It could potentially
return 0 (PREF_INVALID) if the preftype is none of the above, but I'm not really
sure how this could happen. I could leave PREF_INVALID in just in case...

The PREF_LOCKED flag is basically returned via prefIsLocked(), the PREF_USER_SET
value returned via prefHasUserValue(). PREF_CONFIG is set internally, but never
used and PREF_REMOTE & PREF_LI_LOCAL are completely unused.

Checked in.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
verified against Mozilla 1.1a Gecko 20020617 build. Verified patch checkin for
nsIPrefBranch.idl, nsIPrefBranchInternal.idl, nsIPrefService.idl,
nsISecurityPref.idl, nsPref.cpp, nsPrefBranch.cpp (only change is removal of
NS_LITERAL_STRING in observer->Observe()), and nsPrefService.cpp
Status: RESOLVED → VERIFIED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.