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

NEW
Unassigned

Status

()

defect
P3
normal
2 months ago
21 days ago

People

(Reporter: robwu, Unassigned)

Tracking

(Regression, {regression})

56 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

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);
    }
See Also: → 1497581
See Also: → 1554744
See Also: → 1537704

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.

  1. 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")).

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 ?

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.

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

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.

P3 since this isn't normal configuration.

Priority: -- → P3

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

Flags: needinfo?(l10n)

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

Flags: needinfo?(l10n)

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.

You need to log in before you can comment on or make changes to this bug.