1554742 - LocaleService::GetDefaultLocale returns "und" when Firefox is built with --disable-updater (MOZ_UPDATER=)

Status: NEW

(Core :: Internationalization, defect, P3)

Description

[Rob Wu [:robwu]]

STR:

1. Build Firefox with --disable-updater, e.g. use one of the (Linux) distributions that disabled the updater, or build one yourself using:

$ cat .mozconfig
# This is the content of .mozconfig
ac_add_options --enable-artifact-builds
ac_add_options --disable-updater

$ ./mach build
$ DESTDIR=/tmp/fxpackage ./mach install
$ mkdir /tmp/profdir
$ /tmp/fxpackage/usr/local/lib/firefox/firefox --no-remote -profile /tmp/profdir

Visit `about:preferences, open the console and run:

Services.locale.defaultLocale

Expected:

• "en-US"

Actual:

• "und"

LocaleService::GetDefaultLocale has the following logic. The intentions are clear, but the logic does not work because the value for invalid tags is not empty but "und".

    // Hard-coded fallback to allow us to survive even if update.locale was
    // missing/broken in some way.
    if (mDefaultLocale.IsEmpty()) {
      GetLastFallbackLocale(mDefaultLocale);
    }

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

yeah, that's a shortcoming of not using a real Locale here (where empty and "und" would be the same).

Ok, I think there are two ways to fix it:

1. Correct one - we should stop conflating update mechanism with the default locale. They're not the same, and your case shows that - there's nothing about updating, but we still have the default locale of the build. So replace update.locale file with something generated at build/package time that contains information of what locale we're using. Preferably, I'd love to see that bit to be written to a file that is already on main thread startup I/O, so that we don't need to open a new file just to read a single locale code.

Then, maybe update mechanism will use that locale bit, or maybe it'll use its own, but we won't rely on update mechanism (which is opt-out) to know what the default locale.

2. Shortcut one - recognize the difference between messed up content of update.locale (which gets sanitized to und) vs missing file. If the file is missing, take the last fallback.
   This can arguably also be done by just extending the if to be if (mDefaultLocale.IsEmpty() || mDefaultLocale.EqualsLiteral("und")).

Comment 2

[Axel Hecht [:Pike]]

Looking at https://searchfox.org/mozilla-central/rev/ddb81c7a43ffada1f6cb4200c4f625e50e44dcf3/intl/locale/LocaleService.cpp#520-569, the problem is that res/multilocale.txt is snafu, and then the fallback to update.locale isn't any better.

How can we get the right results into multilocale.txt ?

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

Looking at https://searchfox.org/mozilla-central/rev/ddb81c7a43ffada1f6cb4200c4f625e50e44dcf3/intl/locale/LocaleService.cpp#520-569, the problem is that res/multilocale.txt is snafu, and then the fallback to update.locale isn't any better.

I thought it'll be two problems.

1. multilocale.txt doesn't return the right value
2. getDefaultLocale doesn't retrieve the right value, because it tries non-existing update.locale and then sanitizes an empty result into und.

Comment 4

[Axel Hecht [:Pike]]

Rob, can you give us the results of both resource://gre/update.locale and resource://gre/res/multilocale.txt in your build?

Comment 5

[Rob Wu [:robwu]]

My build is from the ArchLinux repositories - https://www.archlinux.org/packages/extra/x86_64/firefox/ (PKGBUILD)

resource://gre/update.locale does not exist (it is not in omni.ja).

resource://gre/res/multilocale.txt is en-US.

I'm not sure why comment 2 mentions multilocale.txt. The issue is that LocaleService::GetDefaultLocale returns "und", and that is independent of what LocaleService::InitPackagedLocales does.

Comment 6

[Makoto Kato [:m_kato]]

P3 since this isn't normal configuration.

Comment 7

[Rob Wu [:robwu]]

(answered your question from comment 4 in in comment 5; please follow up if needed.)

Comment 8

[Axel Hecht [:Pike]]

So, yeah, this is a bug in LocaleService::GetDefaultLocale, and it's even documented that way, kinda, in https://searchfox.org/mozilla-central/rev/06bd14ced96f25ff1dbd5352cb985fc0fa12a64e/intl/locale/LocaleService.cpp#69-70.

We should only call into SanitizeForBCP47 is locale is non-empty, and we should probably drop the MOZ_ASSERT, given it's totally fine to build w/out the updater.

That's around https://searchfox.org/mozilla-central/rev/06bd14ced96f25ff1dbd5352cb985fc0fa12a64e/intl/locale/LocaleService.cpp#554-558.

Or, and that might be even more useful, compare mDefaultLocale not just to empty, but also to und, and if so, set it to GetLastFallbackLocale

Comment 9

[Zibi Braniecki [:zbraniecki][:gandalf]]

We should only call into SanitizeForBCP47 is locale is non-empty, and we should probably drop the MOZ_ASSERT, given it's totally fine to build w/out the updater.

That's (2) in comment 1.

Or, and that might be even more useful, compare mDefaultLocale not just to empty, but also to und, and if so, set it to GetLastFallbackLocale

That sounds good to me.