implement site-specific preference service

RESOLVED FIXED in Firefox 3 alpha6

Status

()

Firefox
General
--
enhancement
RESOLVED FIXED
11 years ago
5 years ago

People

(Reporter: myk, Assigned: myk)

Tracking

(Depends on: 1 bug, {dev-doc-complete})

Trunk
Firefox 3 alpha6
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
blocking-firefox3 -
wanted-firefox3 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PREF-001a)

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

11 years ago
Implement a site-specific preference service that lets callers get and set arbitrary preferences on a site-specific basis.
(Assignee)

Updated

11 years ago
Blocks: 378549
(Assignee)

Comment 1

11 years ago
Created attachment 262623 [details] [diff] [review]
patch v1: more or less straight from Content Preferences extension

Here's a first version of a patch (created with cvsdo, since it isn't clear where these files should live) to implement this service.  This comes more or less straight from the Content Preferences extension, with just a bit of modification.  It'll need some more, no doubt, plus some unit tests (xpcshell-based, I guess).
Assignee: nobody → myk
Status: NEW → ASSIGNED
Out of curiosity, why browser/ and not toolkit/ where extensions and other apps could use it?
(Assignee)

Comment 3

11 years ago
(In reply to comment #2)
> Out of curiosity, why browser/ and not toolkit/ where extensions and other apps
> could use it?

There's nothing browser-specific about the service.  It could well be in toolkit/ for use by other apps.  Would it be useful outside the browser?
Not sure, since I'm not sure what this service is actually doing...  I'd guess "yes", however.

Comment 5

11 years ago
at the very least i'm sure seamonkey would want it.

but in fact i'm pretty sure *i* want it, and i'm not even xul.

+      // If we don't have a host, then use the entire URI as the group.
+      // This means that URIs like about:mozilla will be considered separate
+      // groups from about:blank, but at least they will have some group.

drop the query and hash.

file:///tmp/x.html, file:///tmp/x.html?1, file:///tmp/x.html#a are the same.

Comment 6

11 years ago
These are my comments from the previous version brought forward for reference (ie I haven't looked too thoroughly at this new code)

+  group: function TLDPlusOneGrouper_group(uri) {
+    var group;
+
+    var host;
+    try {
...
+    }
+    catch(ex) {
...
+      return uri.spec;
+    }

before bug 364129 is fixed you will now need to check for numeric addresses
     if (host_is_a_numeric_address)
       return host;
after it is fixed you will still need something in both the try and catch below.

+
+    try {
+      var eTLDService = Cc["@mozilla.org/network/effective-tld-service;1"].
+                        getService(Ci.nsIEffectiveTLDService);
+      var len = eTLDService.getEffectiveTLDLength(host);
+      var extractor = new RegExp("[^.]+\..{" + len + "}$");
+      var result = extractor.exec(host);

see also bug 367446

+      if (result)
+        group = result[0];
+      else
+        throw("failed to place into group using effective TLD service");
+    }
+    catch(ex) {
...
+      group = host.split(".").reverse().slice(0,2).reverse().join(".");

group = host.split(".").slice(-2).join("."); ???

+    }
+
+    return group;
+  }

Also, unless it is expected there will always be at least one observer, I would like to see an early return from _notifyObservers if _callbacks is empty.

Finally, do records in the other tables (groupers, groups, settings) get expunged? or do the tables accumulate unreferenced records?
(Assignee)

Comment 7

11 years ago
(In reply to comment #5)
> at the very least i'm sure seamonkey would want it.
> 
> but in fact i'm pretty sure *i* want it, and i'm not even xul.

Hmm, sounds like it probably makes more sense in toolkit/ instead, then, presumably in toolkit/components/contentprefs.


> +      // If we don't have a host, then use the entire URI as the group.
> +      // This means that URIs like about:mozilla will be considered separate
> +      // groups from about:blank, but at least they will have some group.
> 
> drop the query and hash.
> 
> file:///tmp/x.html, file:///tmp/x.html?1, file:///tmp/x.html#a are the same.

Good point, I'll do that.


(In reply to comment #6)
> before bug 364129 is fixed you will now need to check for numeric addresses
>      if (host_is_a_numeric_address)
>        return host;
> after it is fixed you will still need something in both the try and catch
> below.

Good point, I'll add some code to catch this condition.

> 
> +
> +    try {
> +      var eTLDService = Cc["@mozilla.org/network/effective-tld-service;1"].
> +                        getService(Ci.nsIEffectiveTLDService);
> +      var len = eTLDService.getEffectiveTLDLength(host);
> +      var extractor = new RegExp("[^.]+\..{" + len + "}$");
> +      var result = extractor.exec(host);
> 
> see also bug 367446

Yup, I'll take advantage of that when it lands (and add a comment to that effect to my code in the meantime).


> Also, unless it is expected there will always be at least one observer, I 
> would like to see an early return from _notifyObservers if _callbacks is 
> empty.

There should always be at least one observer, since each browser window will have a controller that registers itself as a generic pref observer.  I guess things could be different in a non-Firefox context, though.


> Finally, do records in the other tables (groupers, groups, settings) get
> expunged? or do the tables accumulate unreferenced records?

Not yet, but you're right, they should get checked and expunged upon removal of a preference.  I'll add code for that to the next version of the patch.
(Assignee)

Updated

10 years ago
Target Milestone: Firefox 3 → Firefox 3 alpha5
(Assignee)

Comment 8

10 years ago
Created attachment 264346 [details] [diff] [review]
patch v2: addresses issues raised in review meeting

Here's an updated patch that addresses some of the issues raised in last week's review meeting as well as some others I found along the way.

Notable changes:

1. The service now defines a site as a full hostname rather than an ETLD+1, per discussion in the review that this is more likely to be right for the initial consumers of this code (particularly text zoom).  I'll also note that existing site-specific prefs in Firefox (f.e. exception lists for various permissions) use this definition by default.

2. The service now uses a custom observer interface (nsIContentPrefObserver) to notify observers about changes to prefs.  nsIObserver proved to be a poor fit for these notifications, and trying to shoehorn them into that interface was cumbersome, since aData wasn't very useful and I had to stick all data into a property bag passed as aSubject.

The new approach lets me pass each datum in its own argument, which makes the interface clearer.  It's also more consistent with the way annotation observers work (although less consistent with pref branch notifications).

3. I moved the component from browser/components/contentprefs to toolkit/components/contentprefs so that other apps can take advantage of it.  I still only build it for Firefox at the moment (via "#ifdef MOZ_PHOENIX" in toolkit/components/Makefile.in), but now that it's in the right place, turning it on for other apps should be a simple build system change.

4. I backed out my attempt at making the grouping of URIs into sites extensible.  It's still not clear what extensibility would be useful here, and I'm wary of implementing something without concrete use cases for it.  But an extension can still override the default grouper by registering itself with its contract ID, and we can modify how this works in the future once we understand better what kinds of extensibility would be useful.

5. When a consumer calls removePref to remove a preference, the service now also removes dangling group and setting records from the database.

6. I've added a set of xpcshell tests exercising the service's interface and functionality.


Outstanding issues:

1. nsIVariant isn't thread-safe, so we either need to limit use of the service to consumers on the main thread, make nsIVariant thread-safe, or stop using nsIVariant and instead implement type-specific methods for getting and setting prefs like nsIPrefBranch and nsIAnnotationService do.  Making nsIVariant thread-safe would be ideal, but it may not be realistic (still looking into this).

2. Seth and I have been talking about whether or not to use annotations to store site-specific prefs.  There are some advantages to doing so (particularly consolidated schema management and cache preloading), but there are also some disadvantages, primarily around the practical limitations and conceptual mismatch of treating sites as URIs.  See bug 380080 for more discussion.

3. Places uses mozIStorageConnection::preload to preload the database cache on startup to speed up read operations out of the gate.  If we stick with a separate database, we'll need to figure out if this would be a significant-enough win to justify doing it for the site-specific prefs database too.

4. There's some question about whether site-specific prefs are private data that we should allow to be cleared when users clear private data.  I did some investigating, and it looks like we don't clear existing site-specific settings when clearing private data, specifically the exceptions lists for the "block pop-up windows", "load images automatically", "accept cookies from sites", "remember passwords for sites", and "warn me when sites try to install add-ons" settings, so I'm inclined to treat site-specific prefs the same way initially.

5. dveditz and seth suggested it might be useful to namespace settings via a separate "type" attribute.  Currently consumers namespace settings using the same approach as nsIPrefBranch: by prefixing the setting name with its namespace followed by a period.  Creating a separate "type" attribute might make extensions more likely to namespace their settings appropriately, but it also makes the interface more complex.


Future enhancements:

1. A way to cascade prefs like the permission manager does, so if there's no pref for www.mozilla.com, but there's a pref for mozilla.com, we use the pref for mozilla.com when we're on www.mozilla.com.  Since we're not planning any UI at this point to set preferences for anything other than a hostname, there's no value in supporting cascades, but it might prove useful at some point.

2. Support for pref sets, so consumers can get and set multiple values for a given pref for a given site.  There's no concrete use case for this at the moment, and consumers can always serialize multiple values into a single pref to store a set, but this could be useful in the future (f.e. a stylesheet applicator could store a list of stylesheets to apply to a site).

3. ETLD+1-based sites, and non-standard "site" definitions generally (f.e. by content type, or via a regexp/wildcard).  For the uses so far identified (mainly text zoom, but folks have also brought up page style, and dcamp wants to store site-specific DOMStorage quotas), full hostnames seem to be preferable.  But we should keep an eye on other possibilities here and how we might support them.
Attachment #262623 - Attachment is obsolete: true
Attachment #264346 - Flags: review?(mconnor)
Do you envision non-main-thread uses for this code?

Note that the pref service is only safe to use from the main thread, for example.  I realize you're implementing a separate beastie, but if it's not a problem for the pref service right now...
(Assignee)

Comment 10

10 years ago
(In reply to comment #9)
> Do you envision non-main-thread uses for this code?

I don't have (or know of) any non-main-thread uses, so we could just say it isn't thread safe, at the (unknown) cost of not supporting whatever non-main-thread use cases pop up in the future.


> Note that the pref service is only safe to use from the main thread, for
> example.

Ah, I didn't realize that.  Guess I don't know much about thread safety in Mozilla.  I thought these lines in nsPrefBranch.cpp implied it:

NS_IMPL_THREADSAFE_ADDREF(nsPrefBranch)
NS_IMPL_THREADSAFE_RELEASE(nsPrefBranch)

> I realize you're implementing a separate beastie, but if it's not a
> problem for the pref service right now...

Yeah, you're right, it's probably not a problem for the content pref service either.  Ok, let's just leave it as it is and not worry about it.
(Assignee)

Comment 11

10 years ago
I'll also note two other promising ideas that are probably more "phase 2":

dbaron pointed out that what we really want is a more general way of recording how a user configures the browser when visiting various pages and then deducing from that how the browser should be configured on other pages.

Percy Cabello suggested the following algorithm for saving prefs to accommodate cases where foo.example.com and bar.example.com are actually the same site:

Regarding defining "site", what about this behavior:
1- A user visits sub1.domain.com and changes the text size, so, as
suggested so far the setting is saved for the entire host name
2- The user visits sub2.domain.com and changes the text size, the
setting is saved for the entire host name
3- The user visits sub3.domain.com and changes the text size, then
another setting is saved for the ETLD+1 as it seems the user wants the
setting for all subdomains.
4- The user visits sub4.domain.com, has the text automatically resized
because of step 3. Here:
   - If the user keeps the text size, nothing happens
   - If the user resizes the text which will mean he doesn't want it
for all subdomains, the ETLD+1 entry is removed
> I thought these lines in nsPrefBranch.cpp implied it:

Those imply it's ok to call addref/release on it from other threads, nothing more.  See bug 380315.  :(

Comment 13

10 years ago
This is progressing nicely!

(In reply to comment #8)

> 2. The service now uses a custom observer interface (nsIContentPrefObserver) to
> notify observers about changes to prefs.  nsIObserver proved to be a poor fit
> for these notifications, and trying to shoehorn them into that interface was
> cumbersome, since aData wasn't very useful and I had to stick all data into a
> property bag passed as aSubject.
> 
> The new approach lets me pass each datum in its own argument, which makes the
> interface clearer.  It's also more consistent with the way annotation observers
> work (although less consistent with pref branch notifications).

I concur; Getting the information back out was fiddly as well. This is a good time to streamline the code.

> Outstanding issues:

> 4. There's some question about whether site-specific prefs are private data
> that we should allow to be cleared when users clear private data.  I did some
> investigating, and it looks like we don't clear existing site-specific settings
> when clearing private data, specifically the exceptions lists for the "block
> pop-up windows", "load images automatically", "accept cookies from sites",
> "remember passwords for sites", and "warn me when sites try to install add-ons"
> settings, so I'm inclined to treat site-specific prefs the same way initially.

I suspect that this the province of the consumers. In an analogy with places - and nothing should be decided on this independently of places which could have a similar question to answer as it develops:
- a user action to set a pref should be treated like bookmarks are and be passed to the service
- automatic setting of prefs should be treated like history is and, at the very least, should honour historyDisabled (or similar) before requesting that a site be added to the database.

> 5. ... Currently consumers namespace settings using the same approach as nsIPrefBranch: by
> prefixing the setting name with its namespace followed by a period.

I've taken the hint and prefixed 'extensions' to HCT+ prefname.

Did I imagine it? or is there, currently, a limit on the number of databases that can be used? For some reason I have the number six in my head; I make this, if it stays separate, the fifth.

I hate to ask, doesn't the javascript style guide suggest that argument names begin with 'a' - aName, aValue, aURI, ...?

Comment 14

10 years ago
(In reply to comment #8)

> 5. When a consumer calls removePref to remove a preference, the service now
> also removes dangling group and setting records from the database.

I suspect that you will need a way to remove all the preferences for a setting - eg on extension uninstall or to reset all groups back to the default value of a setting - something like removeAllPrefs(name) but I am happy to leave that to an enhancement bug.
Comment on attachment 264346 [details] [diff] [review]
patch v2: addresses issues raised in review meeting

Index: toolkit/components/Makefile.in
 
+# XXX Firefox-only until other apps say they want it.

hmm, we should ask bsmedberg about this, especially since this becomes an issue with Fx-on-xulrunner..

Index: toolkit/components/contentprefs/public/nsIContentPrefService.idl
+  /**
+   * Called when a content pref is removed.
+   * 
+   * @param    group      the group to which the pref belongs, or null
+   *                      if it's a global pref (applies to all URIs)
+   * @param    name       the name of the pref that was set

s/set/removed/ I'd think


+  /**
+   * Check whether or not a pref exists.
+   *
+   * @param    uri        the URI for which to check for the pref
+   * @param    name       the name of the pref to check for
+   */
+  boolean hasPref(in nsIURI uri, in AString name);

is this necessary?  the impl seems like its straightforward enough for consumers to do themselves.  not sure if its worth abstracting away, but its in line with the prefs API

Index: toolkit/components/contentprefs/src/nsContentPrefService.js

+      this.__consoleSvc = Cc["@mozilla.org/consoleservice;1"].
+                           getService(Ci.nsIConsoleService);

nit: extra space

+  //**************************************************************************//
+  // Initialization & Destruction
+
+  _init: function ContentPrefService__init() {
+    // Observe shutdown so we can shut down the database connection.
+    this._observerSvc.addObserver(this, "xpcom-shutdown", false);
+
+    // XXX What should we do if database initialization throws an exception?
+    // Currently, it causes the getService call to fail, but that just means
+    // the client will try again next time it needs a pref and entrain the whole
+    // database initialization process once again, which I don't think we want.
+    this._dbInit();
+  },

When will dbInit() throw?  is it something that will possibly succeed later?  if not, we should just set _initFailed to true and check that up front.  We also probably want to move the addObserver until after we successfully init the DB, no?

+  //**************************************************************************//
+  // nsIContentPrefService
+
+  getPref: function ContentPrefService_getPref(uri, name) {
+    if (uri) {
+      var group = this.grouper.group(uri);
+      return this._selectPref(group, name);
+    }
+    else
+      return this._selectGlobalPref(name);
+  },

hmm, do we want to reverse name and uri for these methods, since uri is basically optional?

+  hasPref: function ContentPrefService_hasPref(uri, name) {
+    // XXX Should we optimize this to query the database directly?
+    return (typeof this.getPref(uri, name) != "undefined");
+  },

if we're keeping it, probably not worth the optimization unless a lot more callers are going to use it

+  removePref: function ContentPrefService_removePref(uri, name) {
+    // If there's no old value, then there's nothing to remove.
+    if (!this.hasPref(uri, name))
+      return;

worth dumping to console if callers are screwing this up?


+    if (uri)
+      this._deleteGroupIfUnused(groupID);

why not check groupID here? not that it nets out differently, but it seems more correct

I didn't dig into the sqlite calls much at all, but they looked good overall.

r=me, really good work here, I don't have a lot to add.
Attachment #264346 - Flags: review?(mconnor) → review+
(Assignee)

Comment 16

10 years ago
Created attachment 265030 [details] [diff] [review]
patch v2.1: addresses review comments

Myk Melez said:

> 1. nsIVariant isn't thread-safe, so we either need to limit use of the 
> service to consumers on the main thread, make nsIVariant thread-safe, or stop
> using nsIVariant and instead implement type-specific methods for getting and
> setting prefs like nsIPrefBranch and nsIAnnotationService do.  Making 
> nsIVariant thread-safe would be ideal, but it may not be realistic (still 
> looking into this).

Per conversation in this bug, we're limiting use of the service to consumers on the main thread.


> 2. Seth and I have been talking about whether or not to use annotations to
> store site-specific prefs.  There are some advantages to doing so 
> (particularly consolidated schema management and cache preloading), but there
> are also some disadvantages, primarily around the practical limitations and
> conceptual mismatch of treating sites as URIs.  See bug 380080 for more
> discussion.

Note: in discussion with Seth, we decided that it doesn't make sense to store site-specific prefs in the annotations database because of the reasons listed in bug 380080.


> 3. Places uses mozIStorageConnection::preload to preload the database cache
> on startup to speed up read operations out of the gate.  If we stick with a
> separate database, we'll need to figure out if this would be a
> significant-enough win to justify doing it for the site-specific prefs
> database too.

I think that in the short run this probably isn't a win given that we will only need to access a small subset of the records in the database on startup (specifically, the records relating to the page loading in the current tab).

But we should keep an eye on this and take advantage of preloading if it turns out to be valuable.


> 4. There's some question about whether site-specific prefs are private data
> that we should allow to be cleared when users clear private data.  I did some
> investigating, and it looks like we don't clear existing site-specific
> settings when clearing private data, specifically the exceptions lists for
> the "block pop-up windows", "load images automatically", "accept cookies from
> sites", "remember passwords for sites", and "warn me when sites try to
> install add-ons" settings, so I'm inclined to treat site-specific prefs the
> same way initially.

I'm still inclined to be consistent with the way we treat other site-specific settings currently, although we should separately consider whether all these settings (whether they use this service, the permissions manager, or some other site-specific datastore) deserve an option in the "clear private data" window.

I have filed bug 380852 to consider that issue.


> 5. dveditz and seth suggested it might be useful to namespace settings via a
> separate "type" attribute.  Currently consumers namespace settings using the
> same approach as nsIPrefBranch: by prefixing the setting name with its
> namespace followed by a period.  Creating a separate "type" attribute might
> make extensions more likely to namespace their settings appropriately, but it
> also makes the interface more complex.

Given that both the prefs and the annotations APIs rely on consumers to namespace pref/annotation names in the name itself, and typing adds significant complexity, I think the current approach, which is consistent with what the prefs and annotations APIs do, is the optimal one, for this initial implementation anyway.


John P. Baker said:

> Did I imagine it? or is there, currently, a limit on the number of databases
> that can be used? For some reason I have the number six in my head; I make
> this, if it stays separate, the fifth.

I asked Vlad, and he says he knows of no such limit and it wouldn't be this low if there was one, so I think we're ok.


> I hate to ask, doesn't the javascript style guide suggest that argument names
> begin with 'a' - aName, aValue, aURI, ...?

Yes, it does.  I've gone through the patch and added this prefix to those names.


Mike Connor said:

> Index: toolkit/components/Makefile.in
> 
> +# XXX Firefox-only until other apps say they want it.
> 
> hmm, we should ask bsmedberg about this, especially since this becomes an 
> issue with Fx-on-xulrunner..

I asked him, and he said we should turn it on by default for other apps besides browser, so I have modified Makefile.in to do that.


> +   * Called when a content pref is removed.
> +   * 
> +   * @param    group      the group to which the pref belongs, or null
> +   *                      if it's a global pref (applies to all URIs)
> +   * @param    name       the name of the pref that was set
> 
> s/set/removed/ I'd think

Err, right, fixed.


> +  boolean hasPref(in nsIURI uri, in AString name);
> 
> is this necessary?  the impl seems like its straightforward enough for
> consumers to do themselves.  not sure if its worth abstracting away, but its 
> in line with the prefs API

It's consistent with both the prefs and the annotations APIs, and it also makes it much easier to optimize this functionality later if it turns out to be frequently used, since we'll only have to optimize this one method instead of having to traipse through consumers looking for instances of code (perhaps written in several different ways) that do this.


> Index: toolkit/components/contentprefs/src/nsContentPrefService.js
> 
> +      this.__consoleSvc = Cc["@mozilla.org/consoleservice;1"].
> +                           getService(Ci.nsIConsoleService);
> 
> nit: extra space

Oops, fixed.


> When will dbInit() throw?  is it something that will possibly succeed later? 
> if not, we should just set _initFailed to true and check that up front.

It's not entirely clear to me when dbInit() will throw.  I've only actually experienced it when there was a syntax error in my database initialization/migration code, but those'll get hammered out quickly in development and testing, so it doesn't help to catch those here.

Since we're creating and/or opening a file in the profile directory, the method could throw if we don't have permissions to create or read the file.  That could happen because the filesystem is out of space, in which case initialization could succeed later, and we should probably let consumers continue to try to instantiate it, so I'll leave it as it is.


> We also probably want to move the addObserver until after we successfully
> init the DB, no?

Yup, absolutely, good catch.  I switched them.


> +  getPref: function ContentPrefService_getPref(uri, name) {
...
> hmm, do we want to reverse name and uri for these methods, since uri is
> basically optional?

I don't think so, for four reasons:

1. It's consistent with the annotations API, which contains many methods in which a URI parameter precedes a name one.

2. It's consistent with both the prefs and annotations APIs, where there's a strong "name, value" (i.e. "first name, then value" or "value follows name") meme that this would break if we put uri after name but before value in  setPref.

Of course we could fix that by putting uri after value for setPref, but then it would appear after name in some cases (getPref, hasPref, removePref) and after value in another (setPref) instead of consistently appearing before name in all cases.

3. "uri" is not really optional.  It's more that "null" is a legitimate value that has a specific meaning, namely "the global group".

4. Because this is an XPCOM interface, callers can't leave off optional parameters for which they aren't providing values, so there's no call simplicity advantage to putting optional parameters at the end of the list.


> +  hasPref: function ContentPrefService_hasPref(uri, name) {
> +    // XXX Should we optimize this to query the database directly?
> +    return (typeof this.getPref(uri, name) != "undefined");
> +  },
> 
> if we're keeping it, probably not worth the optimization unless a lot more
> callers are going to use it

Yeah, I've left it unoptimized but with a note that we should optimize it if callers start to use it frequently.


> +  removePref: function ContentPrefService_removePref(uri, name) {
> +    // If there's no old value, then there's nothing to remove.
> +    if (!this.hasPref(uri, name))
> +      return;
> 
> worth dumping to console if callers are screwing this up?

Yes, if we consider this to be screwing up, but I was thinking that we should treat removePref on a non-existent pref as an acceptable behavior by callers, since in certain cases (namely, when "making sure a pref isn't set") this approach simplifies calling code, which doesn't have to check for the presence of a pref before removing it.


> +    if (uri)
> +      this._deleteGroupIfUnused(groupID);
> 
> why not check groupID here? not that it nets out differently, but it seems 
> more correct

It nets out the same, since groupID must be defined if uri is, but you're right that groupID seems more correct, and it'll probably make more sense to someone looking at this code in six months (they won't have to read back through the code to see that uri implies groupID to figure out what this is doing), so I've switched it to checking groupID.


mconnor says to carry forward his r+, so this is the version of the patch I'm going to check in.
Attachment #264346 - Attachment is obsolete: true

Updated

10 years ago
Flags: blocking-firefox3?
(Per PRD, not blocking on P2 items, but really-really want!)
Whiteboard: PREF-001a [wanted-firefox3]

Updated

10 years ago
Flags: blocking-firefox3? → blocking-firefox3-
(Assignee)

Comment 18

10 years ago
Checked in to the trunk:

Checking in toolkit/components/Makefile.in;
/cvsroot/mozilla/toolkit/components/Makefile.in,v  <--  Makefile.in
new revision: 1.66; previous revision: 1.65
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/Makefile.in,v
done
Checking in toolkit/components/contentprefs/Makefile.in;
/cvsroot/mozilla/toolkit/components/contentprefs/Makefile.in,v  <--  Makefile.in
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/public/Makefile.in,v
done
Checking in toolkit/components/contentprefs/public/Makefile.in;
/cvsroot/mozilla/toolkit/components/contentprefs/public/Makefile.in,v  <--  Makefile.in
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/public/nsIContentPrefService.idl,v
done
Checking in toolkit/components/contentprefs/public/nsIContentPrefService.idl;
/cvsroot/mozilla/toolkit/components/contentprefs/public/nsIContentPrefService.idl,v  <--  nsIContentPrefService.idl
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/public/nsIContentURIGrouper.idl,v
done
Checking in toolkit/components/contentprefs/public/nsIContentURIGrouper.idl;
/cvsroot/mozilla/toolkit/components/contentprefs/public/nsIContentURIGrouper.idl,v  <--  nsIContentURIGrouper.idl
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/src/Makefile.in,v
done
Checking in toolkit/components/contentprefs/src/Makefile.in;
/cvsroot/mozilla/toolkit/components/contentprefs/src/Makefile.in,v  <--  Makefile.in
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/src/nsContentPrefService.js,v
done
Checking in toolkit/components/contentprefs/src/nsContentPrefService.js;
/cvsroot/mozilla/toolkit/components/contentprefs/src/nsContentPrefService.js,v  <--  nsContentPrefService.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/tests/Makefile.in,v
done
Checking in toolkit/components/contentprefs/tests/Makefile.in;
/cvsroot/mozilla/toolkit/components/contentprefs/tests/Makefile.in,v  <--  Makefile.in
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/tests/unit/head_contentPrefs.js,v
done
Checking in toolkit/components/contentprefs/tests/unit/head_contentPrefs.js;
/cvsroot/mozilla/toolkit/components/contentprefs/tests/unit/head_contentPrefs.js,v  <--  head_contentPrefs.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/tests/unit/tail_contentPrefs.js,v
done
Checking in toolkit/components/contentprefs/tests/unit/tail_contentPrefs.js;
/cvsroot/mozilla/toolkit/components/contentprefs/tests/unit/tail_contentPrefs.js,v  <--  tail_contentPrefs.js
initial revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/components/contentprefs/tests/unit/test_contentPrefs.js,v
done
Checking in toolkit/components/contentprefs/tests/unit/test_contentPrefs.js;
/cvsroot/mozilla/toolkit/components/contentprefs/tests/unit/test_contentPrefs.js,v  <--  test_contentPrefs.js
initial revision: 1.1
done
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: Firefox 3 alpha5 → Firefox 3 alpha6
It looks like this caused Bug 385085

Comment 20

10 years ago
Basic functions (setPref, getPref, removePref, onContentPrefSet, onContentPrefRemoved) verified on Windows with HashColouredTabs+ 0.4.12c

Updated

10 years ago
Flags: in-testsuite+
Keywords: dev-doc-needed
how do i use this from web content?

Comment 22

10 years ago
(In reply to comment #21)
> how do i use this from web content?
> 

You don't (I hope), it's meant for privileged chrome scripts (browser + extensions). 
ok, i thought it was related to xul pref element. nevermind.
Anyone have any examples I can steal -- er, borrow -- for the docs?  So far I have raw reference content:

http://developer.mozilla.org/en/docs/nsIContentPrefObserver
http://developer.mozilla.org/en/docs/nsIContentPrefService
I had assumed that this would let you actually set browser settings on a per-site basis... from testing, that doesn't appear to be correct.  (I'd hoped to be able to use this to set different default font sizes for different sites).

Is that the expected behavior?
OK, also added a document with an example:

http://developer.mozilla.org/en/docs/Using_content_preferences
Keywords: dev-doc-needed → dev-doc-complete
(Assignee)

Comment 27

10 years ago
(In reply to comment #25)
> I had assumed that this would let you actually set browser settings on a
> per-site basis... from testing, that doesn't appear to be correct.  (I'd hoped
> to be able to use this to set different default font sizes for different
> sites).

It makes it possible to do so, but in order for any given setting to be applied per-site the code that uses that setting has to be updated to get and set that preference using the content pref service rather than the regular pref service.

Thanks for the docs!
How does one determine what settings are supported by the content pref service?
Flags: wanted-firefox3+
Whiteboard: PREF-001a [wanted-firefox3] → PREF-001a
(Assignee)

Comment 29

10 years ago
(In reply to comment #28)
> How does one determine what settings are supported by the content pref service?

Eric: sorry, somehow I missed this question.  At the moment, there isn't a good way to determine which settings the content pref service is being used to store.

For documentation purposes, you could browse the code for references to the service (at the moment we only store text zoom and page zoom there), but that won't help you find extensions that use it.

And power users could use an extension like SQLite Manager to browse the database for the settings being stored in the database.
OK, that's basically what I suspected.  Thanks.

Updated

5 years ago
Depends on: 752960
You need to log in before you can comment on or make changes to this bug.