Closed Bug 645207 Opened 13 years ago Closed 13 years ago

Implement high-level Preferences (Settings) API

Categories

(Add-on SDK Graveyard :: General, enhancement, P1)

Product:
Add-on SDK Graveyard
Component:
General
Type:
enhancement

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: myk, Assigned: erikvvold)

References

Details

The SDK should implement a high-level Preferences API that uses the Simple Storage API to store preference values but presents a high-level declarative interface for specifying preferences to make available to users.

For prior art, see the Settings API JEP 24 that formed the basis of the jQuery UI-based Settings API implementation in the Jetpack Prototype:

  https://wiki.mozilla.org/Labs/Jetpack/JEP/24
  https://hg.mozilla.org/labs/jetpack/file/55679b68ec4a/extension/content/js/settings-view.js
  https://hg.mozilla.org/labs/jetpack/file/55679b68ec4a/extension/content/js/settings-store.js

It should be possible for users to access the preferences interface via the Preferences button in an addon's Add-ons Manager entry, and it should also be possible for an addon to open the interface programmatically in a panel anchored to a widget, so addons can open it when users click a widget.

In the prototype, the Settings API used a special, separate simple store to store preference values.  In the SDK, the Preferences API should store them in the addon's regular simple store, in a "prefs" top-level property (perhaps making this configurable for addons that have an unusual need to change this location).
We might land something like this as an experimental API for 1.0.
Priority: -- → P3
Target Milestone: --- → 1.0
> It should be possible for users to access the preferences interface via the
> Preferences button in an addon's Add-ons Manager entry
note bug 653637 / https://developer.mozilla.org/en/Extensions/Inline_Options
(In reply to comment #2)
> > It should be possible for users to access the preferences interface via the
> > Preferences button in an addon's Add-ons Manager entry
> note bug 653637 / https://developer.mozilla.org/en/Extensions/Inline_Options

Yes, I'm about to integrate that into the SDK, hopefully in the next week or so as that has already landed in Nightly.
(In reply to comment #2)
> > It should be possible for users to access the preferences interface via the
> > Preferences button in an addon's Add-ons Manager entry
> note bug 653637 / https://developer.mozilla.org/en/Extensions/Inline_Options

The pull request is ready: https://github.com/mozilla/addon-sdk/pull/187/
Assignee: nobody → colmeiro
(automatic reprioritization of 1.0 bugs)
Priority: P3 → P2
Target Milestone: 1.0 → 1.1
If I understand how the code in the pull request works, then overall it looks like a fine API for simple preferences that fits well into the SDK's high-level API set.

As I understand it, the packaging code looks for a top-level preferences file that declares a set of simple preferences to display in the addon's Details view.

If that file is available, it generates a XUL representation of it when packaging the addon, and the Add-ons Manager then displays the XUL representation in the Details view, writes changes to preferences to Firefox's global preferences store, and sends notifications to observers about button presses.

The module then exposes preferences via a proxy object, giving the addon the ability not only to read preferences but also write them (including those not declared in the preferences file, although only within its namespace).

The module also lets addons listen for pref changes and button presses via event registration, with event names being the names of the preferences being changed/buttons being pressed.


A few thoughts:

1. Instead of making addon developers create another top-level JSON file, it seems better to integrate the preferences into the existing top-level JSON file, i.e. package.json.  We haven't used package.json this way before, but I don't see why we couldn't start now.

cc:ing Brian in case there's a reason I'm not seeing.


2. Because pref change notifications are sent to addons via the observer service, they can theoretically be intercepted by other addons that access the observer service directly.  I don't think this is a problem, since an addon has to explicitly request access to the observer service, after which it can access all sorts of sensitive notifications.


3. It sure would be helpful to have some documentation in the patch and some tests of the new Python script and the new API to help understand exactly how this API works!


I'm also interested in Drew's thoughts on the API here, especially since he has experience implementing a simple persistent datastore for addon data, and there is some (necessary, I think) duplication of that functionality here; cc:ing him as well.
(In reply to comment #6)
> If I understand how the code in the pull request works, then overall it
> looks like a fine API for simple preferences that fits well into the SDK's
> high-level API set.
> 
> As I understand it, the packaging code looks for a top-level preferences
> file that declares a set of simple preferences to display in the addon's
> Details view.
> 
> If that file is available, it generates a XUL representation of it when
> packaging the addon, and the Add-ons Manager then displays the XUL
> representation in the Details view, writes changes to preferences to
> Firefox's global preferences store, and sends notifications to observers
> about button presses.
> 

You got it almost correct. The SDK really checks that a `preferences` property is present in the packages.json file. If so, it creates a new XUL file that is packaged in the root folder of the XPI, and firefox takes care of the rest.

> The module then exposes preferences via a proxy object, giving the addon the
> ability not only to read preferences but also write them (including those
> not declared in the preferences file, although only within its namespace).
> 
> The module also lets addons listen for pref changes and button presses via
> event registration, with event names being the names of the preferences
> being changed/buttons being pressed.
> 

Yes all that is correct.

> 
> A few thoughts:
> 
> 1. Instead of making addon developers create another top-level JSON file, it
> seems better to integrate the preferences into the existing top-level JSON
> file, i.e. package.json.  We haven't used package.json this way before, but
> I don't see why we couldn't start now.
> 
> cc:ing Brian in case there's a reason I'm not seeing.
> 

As I explained above, that is the actual behavior.

> 
> 2. Because pref change notifications are sent to addons via the observer
> service, they can theoretically be intercepted by other addons that access
> the observer service directly.  I don't think this is a problem, since an
> addon has to explicitly request access to the observer service, after which
> it can access all sorts of sensitive notifications.
>

That's true, anyway an addon can listen to preferences change in any branch of the firefox preferences.
 
> 
> 3. It sure would be helpful to have some documentation in the patch and some
> tests of the new Python script and the new API to help understand exactly
> how this API works!
>

Yup, indeed. I was waiting to have green light in the current code before starting documenting it.
 
> 
> I'm also interested in Drew's thoughts on the API here, especially since he
> has experience implementing a simple persistent datastore for addon data,
> and there is some (necessary, I think) duplication of that functionality
> here; cc:ing him as well.

Will wait for his input :)
I like the idea of a simple object as the interface.  I know Brian has much more experience than me with object serialization and persistence, so his input would be valuable!

But it looks like the API doesn't support persisting objects, so there's actually not a lot of overlap with simple-storage.  (I'm curious why it doesn't.  I can imagine people wanting to store things like "list of subscribed feed URLs" that ought to be user-facing rather than placed in simple-storage.)

One thing that can be tricky for people to remember is to use the `simple` object rather than the module object itself.  (BTW, why's it called "simple"?  It could use a better name, like "storage" or "store" or "data".)  For simple-storage, we've talked about emitting warnings for undefined property accesses on the module object.

Other random thoughts:

Do we really want two APIs for preferences, one for my add-on and one for everything else?  I think it's worth considering extending preferences-service to provide easy access to my add-on's prefs and promoting it to addon-kit.

For convenience, _prefObserver should maybe emit the value of the pref in addition to the name.  It would be nice to emit the old value too.

How about hidden prefs, i.e., prefs that don't automatically have UI created for them?  They can be useful in situations where you want users to be able to tweak a setting if necessary, or if they'd like, but only in some extraordinary case.  Or in cases where you don't want to hardcode some magic value, like a network timeout.
Oh, one more thought:  It shouldn't be hard to rip the JsonStore class out of simple-storage.js, rename it ObjectStore, and extend it so clients could provide a back-end rather than reading and writing a file.  Then you could use it here with a preferences-service back-end and get object persistence (and preference cleanup on uninstall, modulo bug 571049).  Might be more work than it's worth, I dunno.
(In reply to comment #8)
> I like the idea of a simple object as the interface.  I know Brian has much
> more experience than me with object serialization and persistence, so his
> input would be valuable!
> 
> But it looks like the API doesn't support persisting objects, so there's
> actually not a lot of overlap with simple-storage.  (I'm curious why it
> doesn't.  I can imagine people wanting to store things like "list of
> subscribed feed URLs" that ought to be user-facing rather than placed in
> simple-storage.)
> 

The idea behind this API was a wrapper around the new inline preferences that are included in FF7. That UI is implemented with the firefox preferences service, so we can only store what can be stored in there.

We didn't think about creating new components that use a combination of the foundational blocks added to FF7, like a list of URLs that would use a certain amount of string input fields. 
I personally would go with a minimalistic API and grow as users demand it, for example that list of URLs could be a text input with a comma separated URL list.

> One thing that can be tricky for people to remember is to use the `simple`
> object rather than the module object itself.  (BTW, why's it called
> "simple"?  It could use a better name, like "storage" or "store" or "data".)

The idea is that in the future there will be a "complex" or another namespaced API to create a full fledged preferences window. It would hook to the [Preferences] button in the addon manager and open a panel with a more complex set of preferences. This is just for the new basic inline options, hence the "simple" name.


> For simple-storage, we've talked about emitting warnings for undefined
> property accesses on the module object.
> 

If it's worth it it might be added.

> Other random thoughts:
> 
> Do we really want two APIs for preferences, one for my add-on and one for
> everything else?  I think it's worth considering extending
> preferences-service to provide easy access to my add-on's prefs and
> promoting it to addon-kit.
>

The preferences-service is a low level package, and is a thin wrapper around the preferences-service in firefox itself. I view this new preferences module being something more UI-oriented, intended mostly for having a UI to display preferences that users can tweak. 
Also, the idea is that this module abstracts the author from where the values end up being stored, whereas preferences-service is a way to get hold of the low level firefox preferences, that you could use to get other things not related to the addon.

 
> For convenience, _prefObserver should maybe emit the value of the pref in
> addition to the name.  It would be nice to emit the old value too.
>

I thought that .emit() only accepted the name of the event its broadcasting, but if it's possible sending also the value of the preference would be useful.

> How about hidden prefs, i.e., prefs that don't automatically have UI created
> for them?  They can be useful in situations where you want users to be able
> to tweak a setting if necessary, or if they'd like, but only in some
> extraordinary case.  Or in cases where you don't want to hardcode some magic
> value, like a network timeout.

That seems like a nice idea! Maybe naming your pref like "_SomeHiddenPref", with the first underscore meaning it won't show up in the inline preferences, could be a useful addition.
(In reply to comment #10)
> The idea behind this API was a wrapper around the new inline preferences
> that are included in FF7.

Oh, so this won't work for 5 or 6?

> That UI is implemented with the firefox preferences service, so
> we can only store what can be stored in there.

True, but people store objects in the prefs all the time, they just serialize them first.

> We didn't think about creating new components that use a combination of the
> foundational blocks added to FF7, like a list of URLs that would use a
> certain amount of string input fields. 
> I personally would go with a minimalistic API and grow as users demand it,
> for example that list of URLs could be a text input with a comma separated
> URL list.

Yeah, but that's not an ideal UX.  I think there will be many requests to store arrays at least.

I'd like to point out that there's a difference between the data that the preferences API can store and the UI created to display that data.  Just because the new inline prefs UI doesn't support lists or whatever doesn't mean we shouldn't allow lists to be stored in the data.  Maybe in the next SDK release we'll decide to move the UI someplace else, or support a wider variety of automatic UI, or the next version of Firefox will support a wider variety.  And people are free to make their own UIs too.

> The idea is that in the future there will be a "complex" or another
> namespaced API to create a full fledged preferences window. It would hook to
> the [Preferences] button in the addon manager and open a panel with a more
> complex set of preferences. This is just for the new basic inline options,
> hence the "simple" name.

Hmm, again, there's a difference between data and UI, and I don't see why there should be a separate data part of the API for a different UI.  We should handle that for the developer.

> The preferences-service is a low level package, and is a thin wrapper around
> the preferences-service in firefox itself.

Right, I'm suggesting we might want to extend it.  Again, there's no reason a UI couldn't automatically be created from the preferences-service API.

I'd also like to suggest that this API guarantee that Firefox's preferences service is used as the back-end, as opposed to some file or something.  Firefox users are accustomed to accessing add-on prefs via about:config and the preferences service.
(In reply to comment #11)
> (In reply to comment #10)
> > The idea behind this API was a wrapper around the new inline preferences
> > that are included in FF7.
> 
> Oh, so this won't work for 5 or 6?
> 

No, it won't work on anything prior to FF7, and that is a concern about who is responsible for determining what to do in case the addon is installed in FF5 or 6. Although chances are that for the time this API get's to the SDK there won't be much people running 5, as it'll be 2 releases behind.

> > That UI is implemented with the firefox preferences service, so
> > we can only store what can be stored in there.
> 
> True, but people store objects in the prefs all the time, they just
> serialize them first.
> 
> > We didn't think about creating new components that use a combination of the
> > foundational blocks added to FF7, like a list of URLs that would use a
> > certain amount of string input fields. 
> > I personally would go with a minimalistic API and grow as users demand it,
> > for example that list of URLs could be a text input with a comma separated
> > URL list.
> 
> Yeah, but that's not an ideal UX.  I think there will be many requests to
> store arrays at least.
> 
> I'd like to point out that there's a difference between the data that the
> preferences API can store and the UI created to display that data.  Just
> because the new inline prefs UI doesn't support lists or whatever doesn't
> mean we shouldn't allow lists to be stored in the data.  Maybe in the next
> SDK release we'll decide to move the UI someplace else, or support a wider
> variety of automatic UI, or the next version of Firefox will support a wider
> variety.  And people are free to make their own UIs too.
>

I think we are getting a bit confused here. The UI that is displayed to the end user in the addon manager IS tied to the type of data being stored. The addon developer (even without using the SDK, going with the original approach) just defines a preference name and a type of value he wants to store. Then the addons manager takes that info and displays the UI according to the type of data indicated by the author: https://developer.mozilla.org/en/Extensions/Inline_Options

 
> > The idea is that in the future there will be a "complex" or another
> > namespaced API to create a full fledged preferences window. It would hook to
> > the [Preferences] button in the addon manager and open a panel with a more
> > complex set of preferences. This is just for the new basic inline options,
> > hence the "simple" name.
> 
> Hmm, again, there's a difference between data and UI, and I don't see why
> there should be a separate data part of the API for a different UI.  We
> should handle that for the developer.
> 

In Firefox itself the approach will be "inline preferences OR full preferences". As I pointed out above this particular type of UI for preferences is tied to the type of data you store, and you can't have both inline preferences and normal preferences in an addon. So the way to reflect that on the SDK was to add a "simple" prefix in the API.
For the full preferences, we will handle the rendering and everything so we can choose how to display things and how to store them, but for the inline ones we are kind of tied and without much space to move.

Hope this helps clarify some things.
Oh.  I was confused because the title of this bug is "implement high-level Preferences (Settings) API", not "implement an Inline Preferences wrapper".

In that case, I think the API should still allow arbitrary data to be stored -- with the option of using Inline Preferences for simple values.  It could be as simple as having an "isInline" flag per pref in the JSON, or an "inline" chunk of prefs.  If/when we implement a fuller, automatically generated prefs window, it would just ignore the "inline" prefs.

The important part is that there's no segregation of inline prefs from normal prefs via the API.  I should be able to ask the same function or object for the value of my inline pref as I do my other prefs.  If the distinction between inline and non-inline is important to reflect via the API, there could be an isInline(prefName) function.

Look at it this way: Inline Preferences are backed by the normal preferences service, right?  If I'm writing a non-SDK add-on, I don't have to ask some different API, service, object, or function what the value of my inline pref is.
Marking anything that potentially looks like a feature request as "enhancement", sorry for the bugspam. :)
Severity: normal → enhancement
Re-prioritizing all 1.1-targeted feature requests to 1.2.
Target Milestone: 1.1 → 1.2
(Pushing all open bugs to the --- milestone for the new triage system)
Target Milestone: 1.2 → ---
Depends on: 564675
Priority: P2 → P1
Assignee: colmeiro → erikvvold
Summary: implement high-level Preferences (Settings) API → Implement high-level Preferences (Settings) API
Status: NEW → ASSIGNED
Depends on: 703489
Commit pushed to https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/5e5abf56321539af169b7d3639da63507e47eb60
Merge pull request #270 from erikvold/prefs

fix bug 645207 - implement high-level preferences API
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Blocks: 706377
You need to log in before you can comment on or make changes to this bug.