1851618 - Cannot remove packaged locales from the locale alternatives list (multi-locale build)

Status: ASSIGNED

(Firefox :: Settings UI, defect, P3)

Description

[Henry Wilkes (they/them) [:henry-x]]

In tor browser we use a multi-locale build with all locales packaged with the browser at build time.

We have the following problem which would also effect a multi-locale build of firefox.

Steps to reproduce

1. Go to "Language" in "about:preferences".
2. Click "Set Alternatives...".
3. Add a language to the list (from the packaged locales).

Result

No way to remove the added language from the alternatives list because the "Remove" button is disabled. The only option is to move it up and down.

Expect

Be able to remove locales from the list.

Origin

Packaged locales are explicitly blocked from being removed. My guess is that the original idea was to prevent removing en-US, or because the packaged locale cannot be removed entirely as a language. But this doesn't work with multi-locale builds.

Comment 1

[Henry Wilkes (they/them) [:henry-x]]

@mstriemer you wrote the original code for this area, so I was hoping you might be able to help.

Packaged locales are explicitly blocked from being removed. My guess is that the original idea was to prevent removing en-US, or because the packaged locale cannot be removed entirely as a language. But this doesn't work with multi-locale builds.

I've looked through the code and I haven't found anything definite about the motivation for this line. Do you remember if there was any particular reason I should be aware about?

I've tested with canRemove being always true using a multi-locale packaged build, and removing the packaged locales doesn't seem to cause any problems. Technically this allows the user to remove the defaultLocale (en-US) from the requestedLocales (it will still appear at the end of appLocalesAsBCP47 though).

So, do you think we could simply change the condition to

canRemove: code != Services.locale.defaultLocale

Comment 2

[Mark Striemer [:mstriemer]]

I unfortunately don't recall why we use that check specifically, but I suspect we were only expecting 1/2 packaged locales.

Eemeli, do you know if we can relax this check at all?

Comment 3

[Eemeli Aro [:eemeli]]

I'm not sure.

The list of packaged locales is loaded from a text file and gets used by the L10n registry, and I'm not sure if we've tests that even consider what happens if they don't match.

Comment 4

[Henry Wilkes (they/them) [:henry-x]]

@eemeli and @mstriemer

I'm pretty sure the canRemove part is just whether it can be removed from Services.locale.requestedLocales:

• https://searchfox.org/mozilla-central/rev/b94e479d0b79b157029379832d05229df646e134/browser/components/preferences/dialogs/browserLanguages.js#133-148
• https://searchfox.org/mozilla-central/rev/b94e479d0b79b157029379832d05229df646e134/browser/components/preferences/dialogs/browserLanguages.js#482
• https://searchfox.org/mozilla-central/rev/b94e479d0b79b157029379832d05229df646e134/browser/components/preferences/dialogs/browserLanguages.js#707-716
• https://searchfox.org/mozilla-central/rev/b94e479d0b79b157029379832d05229df646e134/browser/components/preferences/dialogs/browserLanguages.js#469
• https://searchfox.org/mozilla-central/rev/b94e479d0b79b157029379832d05229df646e134/browser/components/preferences/main.js#1845-1875

I'm not aware of this being able to actually remove the underlying packaged locale. Testing myself I didn't notice anything like that.

In Tor Browser we have adopted this patch: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/5be723a9be40487b91991cb1aaeb0d3538c67ad1.

Comment 5

[Henry Wilkes (they/them) [:henry-x]]

Attachment: Bug 1851618 - Allow removing packaged locales from requestedLocales. r=#settings-reviewers

We open up the UI to allow the user to remove locales from their
requestedLocales list, except for the default locale.

Comment 6

[Eemeli Aro [:eemeli]]

Reviewed the patch and the history here; looks good. In particular this comment was likely why we ended up with our current code.