1760013 - Migrate intl.properties prefs from .properties

Status: RESOLVED FIXED

(Core :: Internationalization, task)

Description

[Eemeli Aro [:eemeli]]

Strings from the platform-specific files should be gathered together, using PLATFORM() to switch between the platforms if it turns out that the intl.ellipsis string should indeed have a platform dependency.

Comment 1

[Francesco Lodolo [:flod]]

This is a weird file, and more often than not, I think it shouldn't be a localizable file at all (it should be configurations centrally stored).
https://searchfox.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/intl.properties

Is migration going to cope well with empty values?

Comment 2

[Eemeli Aro [:eemeli]]

(In reply to Francesco Lodolo [:flod] from comment #1)

This is a weird file, and more often than not, I think it shouldn't be a localizable file at all (it should be configurations centrally stored).
https://searchfox.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/intl.properties

Is there an existing alternative for storing locale-specific data/configuration other than, well, localisation?

Is migration going to cope well with empty values?

Yes, an empty string maps to an empty string.

Comment 3

[Francesco Lodolo [:flod]]

(In reply to Eemeli Aro [:eemeli] from comment #2)

(In reply to Francesco Lodolo [:flod] from comment #1)

This is a weird file, and more often than not, I think it shouldn't be a localizable file at all (it should be configurations centrally stored).
https://searchfox.org/mozilla-central/source/toolkit/locales/en-US/chrome/global/intl.properties

Is there an existing alternative for storing locale-specific data/configuration other than, well, localisation?

Search plugins and protocol handlers (bug 1733497) used to live in l10n repositories, they now live in mozilla-central. Some other data is implicitly centralized via CLDR (e.g. plural rules)..

intl.properties comes from a period where CLDR wasn't a thing, and most localizers were very technical, working directly on files via text editor and VCS. That's really not true anymore.

On top of that, a lot of the options in this file were created to cater for one local (Japanese). So, variations are mostly exceptions (example), and would result in a very simple configuration, instead of 100+ strings across 100+ repositories.

Last but not least: having these stored in l10n repositories makes tracking hard, and regressions way too easy (that's why I ended up creating scripts…). Let's say a locale starts displaying access keys all the time: first you need to track if the key changed and when, then when that specific revision started shipping in products (including cross-channel in the picture). If it was stored in mozilla-central, it would be bound to a specific version of Firefox and, more importantly, it would only be changed with good reasons in a bug.

Comment 4

[Eemeli Aro [:eemeli]]

Okay, that makes sense. Renamed to not imply that these need to go to Fluent.

One of the "strings" in the file is pluralRule, which is used by PluralForm (both intl/locale/PluralForm.jsm and devtools/shared/plural-form.js) and compare-locales. We really ought to be able to get rid of it as well, but that may well turn out to be easier to do once all plural messages are migrated from .properties to Fluent.

Comment 5

[Eemeli Aro [:eemeli]]

Having looked through the actual data, the value of intl.ellipsis is currently platform-independent, and consistently \u2026 in all locales except for ja and ja-JP-mac, which use ..., i.e. three period-characters, for all platforms.

I think adding a similar JSON config as was done to replace region.properties is the right move here.

Comment 6

[Eemeli Aro [:eemeli]]

Attachment: localizations.json

Finally starting to look into this more properly. The current values of the prefs have no platform dependency, and seem to all have some common value that is used by a majority of locales, with some exceptions. For intl.accept_languages, the psedocode-y default ("$lc, en-US, en") is locale-dependent, but this is not the case for other prefs.

One potential solution here would be to move the prefs to firefox-l10n.js files, but see bug 1975815 for discussion of its viability. Otherwise, it would probably make sense to include a data file like the attached JSON (in some format, possibly even inline in code?) and to have it be consulted when Services.prefs.getComplexValue("intl...", nsIPrefLocalizedString) is called.

Some of the current values are clearly invalid; I've prefixed their locale identifiers with _ in the data.

Comment 7

[:Gijs (he/him)]

(In reply to Eemeli Aro [:eemeli] from comment #6)

Thanks for picking this up.

One potential solution here would be to move the prefs to firefox-l10n.js files, but see bug 1975815 for discussion of its viability. Otherwise, it would probably make sense to include a data file like the attached JSON (in some format, possibly even inline in code?) and to have it be consulted when Services.prefs.getComplexValue("intl...", nsIPrefLocalizedString) is called.

It's possible you meant this but just to make it explicit: in my perfect world we would remove these calls altogether and replace them with direct consultation of (strawman proposal:) Services.intl.acceptLanguage or its equivalents for the other prefs, and have the C++ property getter do the requisite work (probably just doing a lookup once when the service is initialized, and replacing values on the occasion of any future language changes, and caching the value in a threadsafe/atomic manner otherwise).

This has the advantage we can fix the type of each pref/attribute, whereas right now everything is stringly typed and all the consumers have to do work to parse it, it's more natural for the consumers in terms of the API access, and it avoids having to wedge the reading/parsing/whatever-ing of all of this data into libpref (either directly or as a dependency). It also (eventually) gets rid of the weird nsIPrefLocalizedString thing, and we could switch consumers incrementally, in case any of them are difficult - though I suppose that may not be an advantage?

I'm curious if instead you think it's significantly easier to do this directly in libpref and leave the callsites looking the same? Do we have any nsIPrefLocalizedString consumers we actually want to keep using stringbundles?

Comment 8

[:Gijs (he/him)]

... oh. I guess one thing I hadn't considered is people changing these so the pref reads produce the user-branch value instead of the (locale-specific) default. We could change the prefs to have no default value, make the relevant new APIs check for a user-branch value before falling back to the locale-specific default that we "bake in" somehow... but that does make it a bit more messy, and might make keeping the nsIPrefLocalizedString route more attractive, I suppose?

Comment 9

[Eemeli Aro [:eemeli]]

(In reply to :Gijs (he/him) from comment #7)

[...] in my perfect world we would remove these calls altogether and replace them with direct consultation of (strawman proposal:) Services.intl.acceptLanguage or its equivalents for the other prefs, and have the C++ property getter do the requisite work (probably just doing a lookup once when the service is initialized, and replacing values on the occasion of any future language changes, and caching the value in a threadsafe/atomic manner otherwise).

Oh, that sounds like it could be a fine solution here. My thinking was to first make sure whether the existing firefox-l10n.js-y solution would work as an alternative, rather than building a new one. Hence the question of bug 1975815; the alternative I drafted above was even more of a strawman than your proposal.

We might also want consider having more than one solution to these prefs, as they fall into multiple buckets:

• intl.ellipsis, intl.menuitems.alwaysappendaccesskeys, and intl.menuitems.insertseparatorbeforeaccesskeys only have two values each, and are only different for a few locales.
• font.language.group varies primarily based on the preferred locale's script
• browser.fixup.alternate.suffix is really obscure and depends primarily on the preferred locale's region (customized in firefox-l10n.js files)
• browser.menu.showCharacterEncoding is probably much less relevant these days than earlier, and is vaguely dependent on the preferred locale's script (from browser.properties)
• intl.accept_languages is much more variable than any of the above, and gets much more user customization than any of the above

So while a single solution for all of the above could work, we could also choose to consider intl.accept_languages as sufficiently different from the others to do something different with it. But I'd prefer to first ensure that our existing solutions really are worse options than a brand new one.

I'm curious if instead you think it's significantly easier to do this directly in libpref and leave the callsites looking the same? Do we have any nsIPrefLocalizedString consumers we actually want to keep using stringbundles?

Probably not, esp. given how often their values are broken? I need to ensure that the prefs list above is exhaustive. It would be nice to be able to get rid of Ci.nsIPrefLocalizedString completely if it's replaced by a better system. It doesn't look like there are too many callsites for it, so migrating all of the accesses to a new place shouldn't be too arduous.

Comment 10

[Eemeli Aro [:eemeli]]

Attachment: Bug 1760013 - Add intl/locale/rust/locale_service_glue/. r?hsivonen!,bolsson!

Comment 11

[Eemeli Aro [:eemeli]]

Attachment: Bug 1760013 - Use Services.locale.ellipsis. r?glandium!

Comment 12

[Eemeli Aro [:eemeli]]

Attachment: WIP: Bug 1760013 - Remove chrome/global-platform/*/intl.properties

Comment 13

[Eemeli Aro [:eemeli]]

I'm getting close to an overall solution here, by adding getters (ultimately implemented in Rust) on Services.locale for each of the following:

• browser/locales/en-US/chrome/browser/browser.properties
  • browser.menu.showCharacterEncoding
• toolkit/locales/en-US/chrome/global/intl.properties
  • intl.accept_languages
  • font.language.group
  • intl.menuitems.alwaysappendaccesskeys
  • intl.menuitems.insertseparatorbeforeaccesskeys
• toolkit/locales/en-US/chrome/global-platform/{mac,unix,win}/intl.properties
  • intl.ellipsis

A couple of devtools-only localizable values are also obsoleted by inling their functionality:

• toolkit/locales/en-US/chrome/global/intl.properties
  • pluralRule
• devtools/shared/locales/en-US/shared.properties
  • ellipsis

As this effectively removes all of the active users of complex/localizable preferences in Firefox, we should probably consider removing that capability entirely. I'll file a separate bug for that work.

Comment 14

[Eemeli Aro [:eemeli]]

Attachment: Bug 1760013 - Use Services.locale.{alwaysAppendAccesskeys,insertSeparatorBeforeAccesskeys}. r?glandium!

Comment 15

[Eemeli Aro [:eemeli]]

Attachment: Bug 1760013 - Use Services.locale.fontLanguageGroup. r?glandium!

Comment 16

[Eemeli Aro [:eemeli]]

Attachment: Bug 1760013 - Use Services.locale.acceptLanguages. r?glandium!

Comment 17

[Eemeli Aro [:eemeli]]

Attachment: Bug 1760013 - Replace custom localized ellipsis with Services.locale.ellipsis in devtools. r?#devtools!

Comment 18

[Eemeli Aro [:eemeli]]

Attachment: Bug 1760013 - Inline plural rule selection into devtools/shared/plural-form.js. r?#devtools!

The messages depending on this should really be migrated to a better format, but this change at least reduces the burden on translators a little bit.

Comment 19

[Eemeli Aro [:eemeli]]

Attachment: Bug 1760013 - Remove toolkit/locales/en-US/chrome/global/intl.properties. r?glandium!

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/cb2cd703778b
https://hg.mozilla.org/integration/autoland/rev/73e45fcd546b
Add intl/locale/rust/locale_service_glue/. r=hsivonen,bolsson
https://github.com/mozilla-firefox/firefox/commit/b8cbddc6aef7
https://hg.mozilla.org/integration/autoland/rev/5afd2f177442
Use Services.locale.ellipsis. r=glandium,extension-reviewers,credential-management-reviewers,places-reviewers,dimi
https://github.com/mozilla-firefox/firefox/commit/020575f64604
https://hg.mozilla.org/integration/autoland/rev/afe8742d9082
Use Services.locale.{alwaysAppendAccesskeys,insertSeparatorBeforeAccesskeys}. r=glandium,layout-reviewers,hjones,emilio
https://github.com/mozilla-firefox/firefox/commit/b86d2953fa5f
https://hg.mozilla.org/integration/autoland/rev/30c7d47a17c7
Use Services.locale.fontLanguageGroup. r=glandium,mstriemer
https://github.com/mozilla-firefox/firefox/commit/786d8a9520d1
https://hg.mozilla.org/integration/autoland/rev/e774e2687037
Use Services.locale.acceptLanguages. r=glandium,necko-reviewers,search-reviewers,dom-storage-reviewers,translations-reviewers,toolkit-telemetry-reviewers,firefox-svg-reviewers,valentin,asuth,jdescottes,Gijs,mstriemer,jesup
https://github.com/mozilla-firefox/firefox/commit/158e32b20064
https://hg.mozilla.org/integration/autoland/rev/333a08c11530
Replace custom localized ellipsis with Services.locale.ellipsis in devtools. r=devtools-reviewers,nchevobbe
https://github.com/mozilla-firefox/firefox/commit/49a892dd01e5
https://hg.mozilla.org/integration/autoland/rev/1944458149a6
Inline plural rule selection into devtools/shared/plural-form.js. r=devtools-reviewers,nchevobbe
https://github.com/mozilla-firefox/firefox/commit/3a61285f7cc3
https://hg.mozilla.org/integration/autoland/rev/7d94e6fb6b9c
Remove toolkit/locales/en-US/chrome/global/intl.properties. r=glandium,geckoview-reviewers,nalexander

Comment 21

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/73e45fcd546b
https://hg.mozilla.org/mozilla-central/rev/5afd2f177442
https://hg.mozilla.org/mozilla-central/rev/afe8742d9082
https://hg.mozilla.org/mozilla-central/rev/30c7d47a17c7
https://hg.mozilla.org/mozilla-central/rev/e774e2687037
https://hg.mozilla.org/mozilla-central/rev/333a08c11530
https://hg.mozilla.org/mozilla-central/rev/1944458149a6
https://hg.mozilla.org/mozilla-central/rev/7d94e6fb6b9c