I haven't explained my motivation for these changes as well as I could have. Let me try again.
We have some app-specific prefs files that are included conditionally into the relevant app, and aren't under consideration for this bug:
mobile-l10n.js. We also have some channel-specific prefs files that aren't under consideration:
firefox-branding.js. Finally, we have various testing prefs files, which must remain separate.
That leaves the following prefs files, containing "unconditional prefs" that are included into all products and channels unconditionally. I have marked each one with the number of prefs it contains.
71 devtools/client/preferences/debugger.js [*]
189 devtools/client/preferences/devtools-client.js [*]
9 devtools/client/webide/preferences/webide.js [*]
[*] (I subsequently learned that these ones are Firefox only, and so would go into
firefox.js instead of
all.js, but the arguments below still hold.)
We have something close to a monolithic system -- 87% of the unconditional prefs are in
all.js. I want to move the remaining 13% of prefs into
all.js, for the following reasons.
- It is simpler. Fewer files, and fewer mentions of files (in
moz.build files, in
.eslintignore, in package manifests, etc.)
- It is more consistent, and less surprising. Fewer exceptional cases.
- It's already underway. Bug 1571934 moved 72 prefs from
- There is no mechanism to enforce any kind of pref namespacing. So even if you want to keep all of the prefs in a particular group separate, it's hard to do so. E.g. there are some
devtools.* prefs in
all.js. And before bug 1571934 landed, some
security.* prefs were defined in
all.js and some were in
- It's less error-prone.
- In bug 1542244, one developer didn't realize that
testing/marionette/prefs/marionette.js defined prefs that shipped in Firefox. He instead thought it defined prefs only used in tests. (Understandable, given that the file only contains 7 prefs.) He added some prefs to disable two critical browser features, resulting in those features being temporarily disabled in Nightly builds.
- Prior to bug 1571934, the
security.tls.enable_0rtt_data pref was defined twice, once as
all.js, and once as
security-prefs.js. Fortunately, the definition from
all.js ended up being read second so the feature was enabled as expected, but this more by luck than design.
Possible arguments against consolidation, with my pre-emptive rebuttals.
- I like having the prefs for my component separate.
- I believe that. But only a handful of components currently have that luxury, and I believe the consistency argument is stronger. Also, we've seen in practice that keeping prefs separate is difficult and error-prone.
- We could add a mechanism to enforce separation.
- Yes, but that would add complexity. libpref is already far more complex than I would like, and many of my efforts since I took over as module owner two years ago have been toward making it simpler. Consolidating these files is another simplification step.
- Wouldn't a modular system be better, with one pref file per component?
- Perhaps. But we are a long way from that situation now. It would be a large amount of work to convert to a fully modular system, which would likely have 100s of prefs files, and I think the benefits would be small relative to the amount of effort. So a fully modular system isn't realistic. Given the realistic choice between a fully monolithic system and a mostly-monolithic system, a fully monolithic system is better for the reasons I gave above.
all.js is a cryptic name.
- That's a fair point. We could rename it, perhaps as
base-prefs.js. It's mentioned in quite a few places, including in comm-central, so care would be needed, but it should be possible.