1428099 - Remove mobile/android/locales/en-US/mobile-l10n.js

Status: NEW

(Firefox Build System :: Android Studio and Gradle Integration, enhancement, P5)

Description

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

Based on bug 1426943 on comment 9 - the file is empty and can be removed.

Comment 1

[Nick Alexander :nalexander [he/him]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #0)
> Based on bug 1426943 on comment 9 - the file is empty and can be removed.

This isn't quite correct.  This seems unnecessarily complicated, but it's empty for some builds -- at least for en-US builds -- but not empty for (at least) multi-locale builds.  That is, the single pref is localized, triggering the behaviour difference between 1) and 2) in

https://searchfox.org/mozilla-central/rev/8fa0b32c84f924c6809c690117dbd59591f79607/intl/locale/LocaleService.cpp#105

Therefore, I think this cannot be removed; but instead the whole crufty apparatus should be simplified.

Comment 2

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

Hmm, I'm not sure if I understand. There are two:

1) https://searchfox.org/mozilla-central/source/mobile/android/installer/mobile-l10n.js
2) https://searchfox.org/mozilla-central/source/mobile/android/locales/en-US/mobile-l10n.js

(1) is used for multilocale, (2) is empty. Do we need both?

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #2)
> Hmm, I'm not sure if I understand. There are two:
> 
> 1)
> https://searchfox.org/mozilla-central/source/mobile/android/installer/mobile-
> l10n.js
> 2)
> https://searchfox.org/mozilla-central/source/mobile/android/locales/en-US/
> mobile-l10n.js
> 
> (1) is used for multilocale, (2) is empty. Do we need both?

This is actually even cruftier than I thought!

We need the pref to be:

1) not set for en-US
2) set to the per-locale setting for single-locale repacks
3) set to "" for multilocale

The empty for en-US file is the way that 2) works.  What's crufty is the horrific hacks that make that 3) work for multilocale.  What happens is that https://searchfox.org/mozilla-central/source/mobile/android/installer/Makefile.in#9 is expecting to run _at package time_, since that's the only time the build system knows its producing a multi-locale build, and that to make that work we have to do https://searchfox.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/l10n/multi_locale_build.py#162.

That's terrible and needs to go away for other reasons, but it does explain that the two copies of mobile-l10n.js are required (right now).

Comment 4

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

> 2) set to the per-locale setting for single-locale repacks

If the pref is not set, we follow default locale.

That's scenario (1) in https://searchfox.org/mozilla-central/source/intl/locale/LocaleService.cpp#103

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #4)
> > 2) set to the per-locale setting for single-locale repacks
> 
> If the pref is not set, we follow default locale.
> 
> That's scenario (1) in
> https://searchfox.org/mozilla-central/source/intl/locale/LocaleService.
> cpp#103

Right.  In the build system, the empty file is rewritten to use the single-locale file, using the "regular" l10n MERGE_FILES magic.  Without the empty file shenanigans, that won't happen, and the fallback to "default locale" won't find anything for single-locale repacks.  I think.

Comment 6

[Firefox Bug Husbandry Bot]

Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.

Comment 7

[Eemeli Aro [:eemeli]]

Nick, could you confirm my understanding that we don't do single-locale Android builds any more?

Presuming that's the case, could the mobile/android/locales/en-US/mobile-l10n.js be removed, along with its localized versions? The only customizations of it that I see (ja, ja-JP-mac, and ko) are all obsolete, and as far as I can tell, ineffectual.

Comment 8

[Nick Alexander :nalexander [he/him]]

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

Nick, could you confirm my understanding that we don't do single-locale Android builds any more?

We do not.

Presuming that's the case, could the mobile/android/locales/en-US/mobile-l10n.js be removed, along with its localized versions? The only customizations of it that I see (ja, ja-JP-mac, and ko) are all obsolete, and as far as I can tell, ineffectual.

Don't we need something for plain old developer en-US versions? (But it might not need to be locale aware.). Or am I missing something?

Comment 9

[Eemeli Aro [:eemeli]]

(In reply to Nick Alexander :nalexander [he/him] from comment #8)

Don't we need something for plain old developer en-US versions? (But it might not need to be locale aware.). Or am I missing something?

Isn't that effectively mobile/android/installer/mobile-l10n.js? Or could it too be dropped? I see its only pref intl.locale.requested also being set here in mobile/android/app/geckoview-prefs.js.

To be clear, how/whether these pref files are even being included in the build is pretty mysterious to me.

Comment 10

[Nick Alexander :nalexander [he/him]]

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

(In reply to Nick Alexander :nalexander [he/him] from comment #8)

Don't we need something for plain old developer en-US versions? (But it might not need to be locale aware.). Or am I missing something?

Isn't that effectively mobile/android/installer/mobile-l10n.js?

Yes, I think you're correct.

Or could it too be dropped? I see its only pref intl.locale.requested also being set here in mobile/android/app/geckoview-prefs.js.

Oh, that's fun! So now that we only ship a multi-locale GV, it always makes sense to follow the device setting (intl.locale.requested=""), and we don't need a bunch of these shenanigans. I think we can probably get rid of both of these mobile-l10n.js files and verify that en-US builds still work (automated tests) and that CI builds of GV still work (testing a Nightly Fenix should do it).

To be clear, how/whether these pref files are even being included in the build is pretty mysterious to me.

Yeah, it's madness -- in some WIP patch in my tree, I have a comment explaining how this works. Get rid of it entirely and we don't need to worry about getting it correct at build-time, we can just do the locale sniffing at runtime.