Closed
Bug 790374
Opened 12 years ago
Closed 12 years ago
Add ability to retrieve floats from libpref
Categories
(Core :: Preferences: Backend, defect)
Core
Preferences: Backend
Tracking
()
RESOLVED
FIXED
mozilla18
People
(Reporter: jwir3, Assigned: jwir3)
References
Details
(Keywords: dev-doc-needed)
Attachments
(1 file)
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Updated•12 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Assignee | ||
Comment 1•12 years ago
|
||
Added initial implementation that allows retrieval of string preferences as floating-point numbers.
Attachment #664156 -
Flags: review?(benjamin)
Comment 2•12 years ago
|
||
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+
Assignee | ||
Comment 3•12 years ago
|
||
(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.
Assignee | ||
Comment 4•12 years ago
|
||
Inbound, after requested changes made:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ac9fdc7330d7
Target Milestone: --- → mozilla18
Comment 5•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Comment 6•12 years ago
|
||
> 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.
Comment 7•12 years ago
|
||
No actually, it was definitely not that bug because that one is very new. But in any case, it has landed.
Updated•12 years ago
|
Keywords: dev-doc-needed
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.
Comment 9•12 years ago
|
||
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.
Description
•