Bugzilla

(In reply to Mike Kaply [:mkaply] from comment #28)
> I said nsiPrefLocalizedString, but what I specifically meant was complex preferences.

I'm confused - AFAICT the only use for `nsIPrefLocalizedString` is for complex preferences whose internal values are .properties file URIs, which then get loaded and in which the same pref name is then looked up as a string identifier. Complex prefs, on the other hand, have other "complex" value possibilities, like nsIFile (with a system-specific file path, like [here](https://searchfox.org/mozilla-central/rev/080e18fa4748456003164f58b0d925b8c3826a67/browser/base/content/pageinfo/pageInfo.js#682)). There's definitely still complex preferences left, even if we did remove all the `nsIPrefLocalizedString` instances that use `.properties` files.

> We no longer have any complex preferences in about:config which is what was being tested. There are no .properties references in firefox.js or all.js.

I don't think that last bit is true, at least: https://hg.mozilla.org/integration/autoland/file/tip/browser/app/profile/firefox.js#l1394 and https://searchfox.org/mozilla-central/search?q=.properties&path=all.js&case=false&regexp=false are all properties references, AFAICT?

And even if you did remove all of those, for some values we still *read* the prefs using these constructs, and therefore user profiles may still have such values stored (e.g. for the homepage pref, x-ref https://bugzilla.mozilla.org/show_bug.cgi?id=1490339 - we changed the value in firefox.js/all.js ages ago, but we're still trying to read the value as a .properties file for backwards compat. We should really fix that, but it's not directly related to this bug). As a result, the condition for which those tests appear to be testing (AFAICT, a complex value pointing to a .properties file that doesn't have the relevant identifier) can still occur.

Basically, I think the test removal was too hasty and we should (a) put that back and (b) file a follow-up bug to consider removing `nsIPrefLocalizedString as a concept - but that would entail removing all the other consumers, removing the last few instances from `all.js` and `firefox.js`, giving Thunderbird a courtesy heads-up, adding a data migration bit for user-branch values, removing remaining `.properties` files from the locale files we ship, and finally removing the interface.

Am I missing something else?

(In reply to Mike Kaply [:mkaply] from comment #28)
> I said nsiPrefLocalizedString, but what I specifically meant was complex preferences.

I'm confused - AFAICT the only use for `nsIPrefLocalizedString` is for complex preferences whose internal values are .properties file URIs, which then get loaded and in which the same pref name is then looked up as a string identifier. Complex prefs, on the other hand, have other "complex" value possibilities, like nsIFile (with a system-specific file path, like [here](https://searchfox.org/mozilla-central/rev/080e18fa4748456003164f58b0d925b8c3826a67/browser/base/content/pageinfo/pageInfo.js#682)). There's definitely still complex preferences left, even if we did remove all the `nsIPrefLocalizedString` instances that use `.properties` files.

> We no longer have any complex preferences in about:config which is what was being tested. There are no .properties references in firefox.js or all.js.

I don't think that last bit is true, at least: https://hg.mozilla.org/integration/autoland/file/tip/browser/app/profile/firefox.js#l1394 and https://searchfox.org/mozilla-central/search?q=.properties&path=all.js&case=false&regexp=false are all properties references, AFAICT?

And even if you did remove all of those, for some values we still *read* the prefs using these constructs, and therefore user profiles may still have such values stored (e.g. for the homepage pref, x-ref https://bugzilla.mozilla.org/show_bug.cgi?id=1490339 - we changed the value in firefox.js/all.js ages ago, but we're still trying to read the value as a .properties file for backwards compat. We should really fix that, but it's not directly related to this bug). As a result, the condition for which those tests appear to be testing (AFAICT, a complex value pointing to a .properties file that doesn't have the relevant identifier) can still occur.

Basically, I think the test removal was too hasty and we should (a) put that back and (b) file a follow-up bug to consider removing `nsIPrefLocalizedString` as a concept - but that would entail removing all the other consumers, removing the last few instances from `all.js` and `firefox.js`, giving Thunderbird a courtesy heads-up, adding a data migration bit for user-branch values, removing remaining `.properties` files from the locale files we ship, and finally removing the interface.

Am I missing something else?