Closed Bug 1265808 Opened 4 years ago Closed 3 years ago
58 bytes, text/x-review-board-request
Replace uses of Services.prefs for devtools de-chrome-ification project.
We discussed this on irc and came to the following conclusions: * When running in chrome we want to continue using Services.prefs * When running in content, a wrapper around Web Storage should be ok * There's no need to try to unify the two stores, or to try to extract any current settings from prefs when running in content * We need some way to extract defaults from devtools.js * While some prefs don't make sense in content, we're leaving that up to the various bits of UI, and not this bug
I see a few uses of Services.prefs.getComplexValue to get "intl.ellipsis",. I think this can be part of bug 1266075. There are some other uses in webide, but they aren't part of this project yet.
Review commit: https://reviewboard.mozilla.org/r/52017/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/52017/
I filed an issue on ff-devtools-libs to try and get some consensus on how we can test this out with the debugger.html project (which is currently using the 'sham' prefs module that was made for the initial devtools.html project). Here's the issue: https://github.com/jlongster/ff-devtools-libs/issues/4#issuecomment-218580566
I'm going to be delayed on this review as I'm going to be unable to get to it next week - sorry! I do hope we can have some discussion on the github issue about how to sync this over to ff-devtools-libs in the meantime.
(In reply to Brian Grinstead [:bgrins] from comment #6) > > +// Exported for testing purposes. > > +exports._pref = pref; > > Says this is exported for testing but I don't see it used anywhere. Should > it be? Or should this be removed? The function is needed for a later bug, when we load devtools.js to install the default prefs. I'm not sure whether we'll actually need to export this at that time, but in the meanwhile the export prevents an eslint complaint. I've updated the comment to reflect this; however, I could just as easily remove this function and add it back later on. > > + // Add some starter prefs. > > Could these defaults be set down next to the call to > SpecialPowers.pushPrefEnv so it's all in one place? They have to be there before Services.js is loaded.
Comment on attachment 8751431 [details] MozReview Request: Bug 1265808 - add Services.prefs shim; r?bgrins Review request updated; see interdiff: https://reviewboard.mozilla.org/r/52017/diff/1-2/
Comment on attachment 8751431 [details] MozReview Request: Bug 1265808 - add Services.prefs shim; r?bgrins https://reviewboard.mozilla.org/r/52017/#review52320
Attachment #8751431 - Flags: review?(bgrinstead) → review+
You need to log in before you can comment on or make changes to this bug.