Closed Bug 315277 Opened 14 years ago Closed 8 years ago

Convert Toolkit Preference Calls to Fuel

Categories

(Toolkit :: Preferences, defect)

defect
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: mconnor, Assigned: jeresig)

References

()

Details

Attachments

(1 file)

JSLib and other bits and pieces around the tree reimplement wrappers to pref code (and other code, but that's a second step).  We should reuse some of this work to clean up our code, and provide something that extensions can reuse safely for the forseeable future.
Attached patch first cutSplinter Review
this doesn't address XBL bindings, that's the next step
This seems a promising start. Still, some aspects to think about:
* Is XPCU really necessary? If it is, shouldn't it belong somewhere more general and get a more descriptive name (at least gXPCUtils)?
* Wouldn't it be possible to derive the type for setPref from aValue if aType is undefined and the pref doesn't exist yet (see GreaseMonkey's prefmanager.js)? This would also make the not-so-beautiful nsIPrefBranch2.PREF_* values less needed.
* What about !!aValue and parseInt(aValue) for boolean and integer prefs in setPref? Alternatively or additionally, I'd rather have the exception passed on to the caller - otherwise this might cost many programmers several hours of sleep.
Is it really necessary to call if (!this.mPrefs) this.init(); in every methods?
Should be better to make a static function called at module load time
Agreed about XPCU with Simon. If we decide to use this everywhere, it should be put in a more general place and more thought should be put into it. (And btw: "aURL" should be "aContractID" or something, shouldn't it?)

In my opinion trying to guess the type of a preference is a bad idea, because prefs backend has the concept of 'complex' types which doesn't map well on the simple types as returned by getPrefType, nor on JS variable types.

The previous issue would be more obvious if you tried to implement file prefs, which I think should be done.

>+  addObserver: function(aDomain, aFunction)
It's not a function, but an object implementing nsIObserver, right?

+const nsIPrefBranch2 = Components.interfaces.nsIPrefBranch2;
Wouldn't this cause problems in case this is already declared in parent window, for example when the helper is included in multiple overlays for the same document (think extensions).

I wrote a similar wrapper a while ago: http://mozilla.doslash.org/prefutils/prefutils.js
Apart from the dynamic creation of methods (get*PrefDef), I believe it's a nice wrapper, and something similar (in terms of functionality) should be implemented in toolkit.
could be useful to move this into the FUEL js library
Mark, if you guys can pull this into FUEL I would love that, I'm a long way from having time to work on this, but I'd love to convert our chrome code to using the wrappers.  There's a dozen different wrappers floating around, so we should just ship one and use it ourselves.
Mark and I are going to take the work presented in this ticket and move it over to the new Fuel library (since Fuel is included with Firefox now). This should be on track for inclusion in Firefox 3.0a6.
Assignee: mconnor → jresig
Product: Toolkit → Firefox
Target Milestone: --- → Firefox 3 alpha6
Version: unspecified → Trunk
(In reply to comment #8)
> Mark and I are going to take the work presented in this ticket and move it over
> to the new Fuel library (since Fuel is included with Firefox now). This should
> be on track for inclusion in Firefox 3.0a6.

This bug was filed for pref wrappers accessible to all toolkit apps, so I don't think it's appropriate to morph this bug to be Firefox-specific, even if for the moment FUEL is Firefox-only. I suggest filing a new bug for pref wrappers in FUEL, and making this bug depend on it, perhaps.
I don't see why a wrapper like fuel should be part of firefox, but not the toolkit. Where was it discussed?
(In reply to comment #10)
> I don't see why a wrapper like fuel should be part of firefox, but not the
> toolkit. Where was it discussed?

It's certainly going to be a part of both; but to start with, we're just focusing on making it work for Firefox Extension development. This was discussed in our planning of Fuel and our team agreed that we should tackle this goal first.
Depends on: 380288
Ok - so things have been reorganized now. There are two separate goals: Converting the toolkit to using the Fuel preference calls (this bug), and converting Firefox to use the calls (bug 380288). That means that this bug is no longer on track for any specific deadline (only at some point after the Firefox conversion is complete).
Depends on: 372069
Product: Firefox → Toolkit
Summary: Provide pref system wrappers in toolkit → Convert Toolkit Preference Calls to Fuel
Target Milestone: Firefox 3 alpha6 → ---
QA Contact: nobody → preferences
(See bug 644016)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.