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: `firefox.js`, `mobile.js`, `geckoview-prefs.js`, and `mobile-l10n.js`. We also have some channel-specific prefs files that aren't under consideration: `channel-prefs.js` and `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 25 devtools/shared/preferences/devtools-shared.js 4 devtools/startup/preferences/devtools-startup.js 1 mobile/android/installer/mobile-l10n.js 2523 modules/libpref/init/all.js 3 remote/pref/remote.js 3 services/common/services-common.js 46 services/sync/services-sync.js 7 testing/marionette/prefs/marionette.js 8 toolkit/components/telemetry/datareporting-prefs.js 2 toolkit/components/telemetry/healthreport-prefs.js ``` 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 `.prettierignore`, 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 `security/manager/ssl/security-prefs` to `all.js`. - 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 `security-prefs.js`. - 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 `true` in `all.js`, and once as `false` in `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.
Bug 1572331 Comment 3 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
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: `firefox.js`, `mobile.js`, `geckoview-prefs.js`, and `mobile-l10n.js`. We also have some channel-specific prefs files that aren't under consideration: `channel-prefs.js` and `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 [*] 25 devtools/shared/preferences/devtools-shared.js 4 devtools/startup/preferences/devtools-startup.js 2523 modules/libpref/init/all.js 3 remote/pref/remote.js 3 services/common/services-common.js 46 services/sync/services-sync.js 7 testing/marionette/prefs/marionette.js 8 toolkit/components/telemetry/datareporting-prefs.js 2 toolkit/components/telemetry/healthreport-prefs.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 `.prettierignore`, 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 `security/manager/ssl/security-prefs` to `all.js`. - 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 `security-prefs.js`. - 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 `true` in `all.js`, and once as `false` in `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.