This bug will cover the work which is necessary to port the handling of preferences to the new module system.

That patch brings everything we need for the handling of preferences. Not sure yet how to document the constants at the top. A metatag somehow doesn't work.

Comment on attachment 521079 [details] [diff] [review] Patch v1 Code looks fine, r+ on that front. But now that I'm looking at it, I have second thoughts on the pref strings. I suspect the following is all the reasons you were second-guessing it as well, but just getting it on the table: I guess I didn't really connect that these are the ones you can simply view at about:config. So, they're already easily visible to the test writer. That makes renaming problematic: prefs.COOKIE_BEHAVIOR = "network.cookie.cookieBehavior" ...I already know from about:config it's network.cookie.cookieBehavior, and now I have to find a different name for it. That's actually counter-intuitive. So the remedy for that is to always name after the string the user can find: prefs.NETWORK_COOKIE_COOKIEBEHAVIOR = "network.cookie.cookieBehavior"; ...but that's just kind of lame, as constructs go. We haven't abstracted it at all, just wrapped a literal value with a near-literal identifier name. We'd almost be as well off using the straight string. The constants do have two advantages: 1) The interpreter will tell us if we misspell a constant; it won't tell us if we misspell a magic preference name string. That does make things more debug-friendly. 2) We can document the prefs as to what type they are and more importantly, what they actually do. So question is, are these reasons worth enough to define and maintain these constants? I'm on the fence, as you were earlier. 1) is definitely worth something if we're willing to pay the cost. 2) has less value since I realized you can just do about:config to get the list. If we can't arrive at a solid answer, I'd err on the side of keeping them (as I did earlier) just under the principle that magic strings are universally bad. However, I do think it deserves another look and more discussion. I do think if we keep the prefs constants, we should probably match the naming exactly as I suggest above, though, so they're easy to match to the real pref. Preferably, that'd be an r=me item, but if you're out of time on this you can land and I can do naming as a followup patch.

One other thought--these pref names aren't L10Ned, are they?

Keep in mind that constants exist to prevent developers to change a huge amount of code if its value changes. Same does apply to preferences. If a name changes we only have to change one single instance and all tests will work again. Using a naming convention which applies to the preference name will break that possibility. Instead the constant should have a well-chosen name everyone can understand. Keep in mind that some preference names are kinda hard to understand in the first place and having constants will make it much easier for us to support test writers. As also talked on IRC yesterday, preferences are bound to specific modules. If a test want to make use of some networking code we could have a network module which also lists preferences according to this module. In such a case the constants will not exist in a single module but are placed closer to the underlying API the test is using. Some examples: var network = require('network'); network.prefs.PROXY_TYPE network.prefs.SECURITY_SSL3_ENABLED var tabs = require('tabs'); tabs.prefs.ANIMATE That's the way how Mochitests are working, whereby the modules are isolated against each other and we span our tests over all the modules. (In reply to comment #3) > One other thought--these pref names aren't L10Ned, are they? No, preference names aren't localized because that put a massive overhead on-top of the preference system.

I recognize what you're saying, re: the value of abstraction in protecting against future preference string changes. But I also think creating new names will make them considerably harder to find. So then the question comes down to this: historically speaking, how often do preferences change name? If it's often, then you're right, picking generic names is the way to go. If it's not often I don't think it's valuable enough to offset the possible confusion, and we should either go with names that mirror the dot paths, or not define these constants at all. Re: spreading constants out across modules, I'm firmly against it for the reasons I said yesterday. It'll make them much harder to find because people will have to guess which module we placed them in. At least with a single module they can go to the docs for that module and cmd-f find the string in browser. It's an even more extensive version of my issue with the renames: we're making people guess how we think in order to find things. We shouldn't do that. These strings are most closely tied to the functions that manipulate the preferences, not the functional areas to which they're related. That's why I want them in the prefs module, if we're going to have them. It's the most obvious place to go look for them. Also, there's no guarantee we'll even -have- modules established for some areas of the preferences. As a simple example, there's no startup.js to contain the homepage preference. Since we can't guarantee a clean 1:1 correspondence between preference area and module--and I have no interest in starting a bunch of functional modules just to contain preference constants--we should group them together in prefs.

Prefs are getting changed mostly never. Only during the development phase, or when they get removed. Lets find a solution later in a call. That would work best.

As agreed on yesterday, I have removed all the constants from the preferences module. Also a full set of tests have been implemented.

Comment on attachment 521470 [details] [diff] [review] Patch v2 Looks awesome, r+. Thanks a bunch!

Landed as: https://github.com/geoelectric/mozmill-api-refactor/commit/be1f91d449bd0985ed39980ca3b1d3406910c924 Clint and Heather, I know that Mozmill is using the old getPref/setPref functions I have implemented a long time ago. If you want to also port over this module, feel free to do so.