Closed Bug 790374 Opened 12 years ago Closed 12 years ago

Add ability to retrieve floats from libpref

Categories

(Core :: Preferences: Backend, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: jwir3, Assigned: jwir3)

References

Details

(Keywords: dev-doc-needed)

Attachments

(1 file)

Floating point preferences can be stored as strings (e.g. "1.0", "1.9"...) in the backend, but we should add a new GetFloat() API to libpref, so we can retrieve a floating point number from libpref in circumstances where this is desirable. It's possible that, in the future, we could transition to an actual floating point storage mechanism, but, for now, doing automatic translation from string->float is acceptable.
OS: Linux → All
Hardware: x86_64 → All
Attached patch b790374 (deleted) — Splinter Review
Added initial implementation that allows retrieval of string preferences as floating-point numbers.
Attachment #664156 - Flags: review?(benjamin)
Comment on attachment 664156 [details] [diff] [review] b790374 ISTR that I did another review for something very similar to this where somebody added "AddFloatVarCache". But I can't find the bug# and I don't remember whether I marked r+ or r- on that bug. Could you spend a minute looking for that and coordinating if appropriate? This looks fine: in the test, please add failure checking: check that getFloatPref throws if the value is "" and "1.4a1" and other values which aren't full floats.
Attachment #664156 - Flags: review?(benjamin) → review+
(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > Comment on attachment 664156 [details] [diff] [review] > b790374 > > ISTR that I did another review for something very similar to this where > somebody added "AddFloatVarCache". But I can't find the bug# and I don't > remember whether I marked r+ or r- on that bug. Could you spend a minute > looking for that and coordinating if appropriate? > I did a number of searches, and spent about twenty minutes looking for this, including searching google for "AddFloat bugzilla", "AddFloatVarCache", and searching bugzilla for attachment data containing the words"AddFloatVarCache" or "AddFloat". Unfortunately, I was unable to find anything. :| > This looks fine: in the test, please add failure checking: check that > getFloatPref throws if the value is "" and "1.4a1" and other values which > aren't full floats. Sure, I'll add that.
Target Milestone: --- → mozilla18
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Depends on: 828296
> ISTR that I did another review for something very similar to this where somebody added > "AddFloatVarCache". But I can't find the bug# and I don't remember whether I marked r+ > or r- on that bug. Could you spend a minute looking for that and coordinating if > appropriate? That would be Bug 826586 - Add a AddFloatVarCache() API to libpref.
No actually, it was definitely not that bug because that one is very new. But in any case, it has landed.
Depends on: 831751
I think in nsIPrefBranch.idl the inserting of GetFloatPref procedure between the earlier interface procedures is not correct because it makes the interface NOT backward compatible. It shifts the offset of procedures by 4, that means different procs will be called in plugins created before this change. The proc should be appended at the end, or a new interface created. It gives hard time to the developers. By the way it makes almost impossible to be backward compatible with the earlier browsers.
Binary compatibility of interfaces is *not* a goal in Firefox. It is expected that if you are using binary extensions, they will need to be recompiled for each release. Thus use of binary XPCOM is strongly discouraged.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: