Opened 13 months ago

Closed 9 months ago

Last modified 7 months ago

#26843 closed task (fixed)

TBA: Investigate how Mozilla Fennec provides localization

Reported by: sysrqb Owned by: tbb-team
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-mobile, TBA-a2, GeorgKoppen201812, TorBrowserTeam201812
Cc: igt0, gk, emmapeel, intrigeri Actual Points:
Parent ID: #26782 Points:
Reviewer: Sponsor:

Description

Fennec is shipped with multi-locale support. Under the Settings menu option, General -> Language -> Browser Language there are tens of languages and locales available. How are these included and shipped?

Child Tickets

Change History (30)

comment:1 Changed 13 months ago by gk

Parent ID: #5709#26782

comment:2 Changed 13 months ago by sysrqb

Okay. So, there's a doc for this. Specifically, the mozilla build system clones l10n-central and creates the localization package from there. I haven't look at the logic used for this, yet.

We'll want to clone l10n-central separately, and add the location into .mozconfig-android:

ac_add_options --with-l10n-base=/make/this/a/absolute/path

The specific locale is repackaged using:

      ./mach build installers-$AB_CD

The doc says:

You should find a re-packaged build at ``OBJDIR/dist/``, and a
runnable binary in ``OBJDIR/dist/l10n-stage/``.

Running the installers- build target (by default):

The ``installers`` target runs quite a few things for you, including getting
the repository for the requested locale from
https://hg.mozilla.org/l10n-central/. It will clone them into
``~/.mozbuild/l10n-central``.

Multilocale packages are created using:

      export MOZ_CHROME_MULTILOCALE="de it zh-TW"
      for AB_CD in $MOZ_CHROME_MULTILOCALE; do
         ./mach build chrome-$AB_CD
      done

      AB_CD=multi ./mach package

comment:3 Changed 12 months ago by gk

This sounds like the mechanism we eventually want to use for getting rid of our bundle-language-packs-approach. So, we want that for #26465 as well which makes me think we should keep desktop in mind once we file follow-up bugs to implement the things we learned from the investigation in this ticket.

comment:4 Changed 12 months ago by gk

FWIW: I plan to work on the repack idea for desktop in #27466 and might think about how to integrate the multi-locale support for Tor Browser on Android into our build system while I am at it.

comment:5 Changed 10 months ago by gk

Keywords: TorBrowserTeam201811 TBA-a2 GeorgKoppen201811 added

comment:6 Changed 10 months ago by gk

Adding some notes:

https://hg.mozilla.org/l10n-central is the repo to use (not something like https://hg.mozilla.org/releases/l10n/mozilla-release); changesets that are shipping can be found at URLs like

https://product-details.mozilla.org/1.0/l10n/Firefox-60.3.0esr-build1.json

(there are no tags in repos anymore).

comment:7 Changed 9 months ago by sysrqb

Adding some more notes.

  1. mkdir l10n-central
  2. pushd l10n-central
  3. clone target locale (such as es-ES, hg clone https://hg.mozilla.org/l10n-central/es-ES)
  4. popd
  5. Add --with-l10n-base= in mozconfig
  6. ./mach configure ...
  7. ./mach build && ./mach package
  8. ./mach build installers-es-ES

This results in a localized apk. There is an inconsistency when built with #28051 because Orbot doesn't learn about the new locale until later, but we can correct that. Overall, Fennec, torbutton, and Orbot use the correct localized strings.

comment:8 Changed 9 months ago by sysrqb

Okay. comment:7 is correct, but this becomes more complicated with Orbot. When we build a single-locale package, it replaces the localizable strings - meaning each English string that is localized is replaced with the localized string, so the English string is not available within the app. This is good - but it becomes more confusing.

Because all the English strings are substituted with localized strings, the user cannot choose between the localized string and the English strings. This usually wouldn't be a problem, except within Fennec's preferences menu "English (United States)" is always shown. It seems Mozilla thought it important to hard-code "en-US" as an option, even when it is not really available. sigh. But that is relatively easy to fix.

The difficulty comes when Orbot is added into the app. The single-locale re-package process only replaces the Fennec strings, it doesn't touch the Orbot strings, so Orbot uses the system default locale when it starts. This isn't a problem if the user downloaded Tor Browser for the same locale as their system, but if the user has (as an example) en-US as their system default locale but downloads the es-ES localized Tor Browser, then it becomes weird. In this situation, Orbot shows everything in English and Tor Browser shows everything in es-ES. Orbot allows the user switch their locale, but this is confusing.

I think we should add a mechanism within Orbot so it automatically chooses the correct locale when it starts. We can likely add this in the logic when Tor Browser runs Orbot the first time. I prefer having the user control their locale on the Tor Browser-side of the app, rather than the Orbot-side - but others can change my mind :)

comment:9 Changed 9 months ago by sysrqb

I built a multi-locale package, including the 28 locales provided by the desktop alpha builds. It is as simple as the documentation says. The trick is you must do it exactly as the doc says, and don't change the name of the environment variable it says :) (specifically, MOZ_CHROME_MULTILOCALE must be used and set correctly)

Follow the same steps as comment:7, except substitute the last step (./mach build installers-es-ES) with the following three steps:

$ export MOZ_CHROME_MULTILOCALE="ar ca cs da de el es-ES fa fr ga-IE he hu id is it ja ka ko nb-NO nl pl pt-BR ru sv-SE tr vi zh-CN zh-TW"
$ for AB_CD in $MOZ_CHROME_MULTILOCALE; do
    ./mach build chrome-$AB_CD;
done
$ AB_CD=multi ./mach package

The resulting apk is ~48MB, including Orbot and the addons. The filename contains en-US:

$ # before
$ du -sh ~/tor-browser/obj-x86-linux-android/dist/fennec-60.3.0.en-US.android-i686.apk
46M     /home/android/tor-browser/obj-x86-linux-android/dist/fennec-60.3.0.en-US.android-i686.apk
$ # after
$ du -sh ~/tor-browser/obj-x86-linux-android/dist/fennec-60.3.0.en-US.android-i686.apk
48M     /home/android/tor-browser/obj-x86-linux-android/dist/fennec-60.3.0.en-US.android-i686.apk

Comparing the mutli-locale package with the single-locale package, the multi-locale package provides a better user experience. Orbot and Fennec use the system default locale on first-run, so they should use the same locale when the user opens the app. When a different locale is chosen within Fennec, Orbot uses the new locale, too. It seems like this mitigates the major problems in the single-locale package.

comment:10 Changed 9 months ago by boklm

Keywords: TorBrowserTeam201811R added; TorBrowserTeam201811 removed
Status: newneeds_review

Branch bug_26843_v2 has a patch adding projects/firefox-locale-bundle:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_26843_v2&id=85086602ebe3c25c6a3d81d7efeb7d0f6f905605

With this we are getting a directory containing one tarball for each locale.

comment:11 Changed 9 months ago by boklm

Branch bug_26843_v3 has a new revision of the patch, creating one tarball with all locales instead of one for each locale:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_26843_v3&id=6b77a6908c798304e6539c66ed4889b5aae4c875

comment:12 in reply to:  11 ; Changed 9 months ago by gk

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201811R removed
Status: needs_reviewneeds_revision

Replying to boklm:

Branch bug_26843_v3 has a new revision of the patch, creating one tarball with all locales instead of one for each locale:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_26843_v3&id=6b77a6908c798304e6539c66ed4889b5aae4c875

Looks mostly good, thanks. One thing we should fix:

exite_error("Error reading $file", 2) unless defined $json_text;

There is no exite_error function.

comment:13 Changed 9 months ago by emmapeel

Cc: emmapeel added

comment:14 in reply to:  12 ; Changed 9 months ago by boklm

Keywords: TorBrowserTeam201811R added; TorBrowserTeam201811 removed
Status: needs_revisionneeds_review

Replying to gk:

Looks mostly good, thanks. One thing we should fix:

exite_error("Error reading $file", 2) unless defined $json_text;

There is no exite_error function.

This typo is fixed in bug_26843_v4:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_26843_v4&id=43c9452946313a5ab3dd064501daa05096db86eb

comment:15 in reply to:  14 Changed 9 months ago by gk

Keywords: TorBrowserTeam201811 added; TorBrowserTeam201811R removed
Status: needs_reviewnew

Replying to boklm:

Replying to gk:

Looks mostly good, thanks. One thing we should fix:

exite_error("Error reading $file", 2) unless defined $json_text;

There is no exite_error function.

This typo is fixed in bug_26843_v4:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_26843_v4&id=43c9452946313a5ab3dd064501daa05096db86eb

Looks good, thanks so much for this. I merged the patch to master (commit 43c9452946313a5ab3dd064501daa05096db86eb).

I created #28573 for the testbuild issue I had.

Setting to new, while double-checking my patch for this bug.

comment:16 Changed 9 months ago by gk

Keywords: TorBrowserTeam201811R added; TorBrowserTeam201811 removed
Status: newneeds_review

bug_26843_v5 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_26843_v5&id=a87de530dc5f5bde37079107bfa55b1033fba358) has the changes for review. I get a multi-locale Tor Browser that way. I am not sure why the extensions strings are not shown in the selected locale, though, they are always only showing up in en-US. I opened #28575 and I guess we have #27656 for the Torbutton case.

Last edited 9 months ago by gk (previous) (diff)

comment:17 Changed 9 months ago by gk

Okay, I have an updated branch for review, bug_26843_v6 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_26843_v6&id=1313336490abb4773e20d40347db79e46c26727e).

Compared to bug_26843_v5 it is rebased on the latest master and contains an indication in the bundle name that the .apk is actually not a single-locale one, using "multi" as Mozilla does.

comment:18 Changed 9 months ago by gk

Keywords: TorBrowserTeam201812R added; TorBrowserTeam201811R removed

Move review tickets to Decemeber.

comment:19 in reply to:  17 ; Changed 9 months ago by boklm

Keywords: TorBrowserTeam201812 added; TorBrowserTeam201812R removed
Status: needs_reviewneeds_revision

Replying to gk:

Okay, I have an updated branch for review, bug_26843_v6 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_26843_v6&id=1313336490abb4773e20d40347db79e46c26727e).

Compared to bug_26843_v5 it is rebased on the latest master and contains an indication in the bundle name that the .apk is actually not a single-locale one, using "multi" as Mozilla does.

It seems that when we are running AB_CD=multi ./mach package, the variable MOZ_CHROME_MULTILOCALE contains all the locales we want. However, when we run ./mach build chrome-[% lang %], only the current and previous locales are included in MOZ_CHROME_MULTILOCALE. Is ./mach build chrome-[% lang %] ignoring the variable MOZ_CHROME_MULTILOCALE (and in this case it doesn't matter if it doesn't contain all the locales), or should we add all the locales before running the first ./mach build chrome-[% lang %]?

We could use this line to set all the locales in MOZ_CHROME_MULTILOCALE:

export MOZ_CHROME_MULTILOCALE='[% tmpl(c('var/locales').join(' ')) %]'

An other minor thing: I think it would be better to extract the locales in a sub-directory (such as /var/tmp/dist/locales) instead of directly in /var/tmp/dist.

comment:20 in reply to:  19 Changed 9 months ago by gk

Replying to boklm:

Replying to gk:

Okay, I have an updated branch for review, bug_26843_v6 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_26843_v6&id=1313336490abb4773e20d40347db79e46c26727e).

Compared to bug_26843_v5 it is rebased on the latest master and contains an indication in the bundle name that the .apk is actually not a single-locale one, using "multi" as Mozilla does.

It seems that when we are running AB_CD=multi ./mach package, the variable MOZ_CHROME_MULTILOCALE contains all the locales we want. However, when we run ./mach build chrome-[% lang %], only the current and previous locales are included in MOZ_CHROME_MULTILOCALE. Is ./mach build chrome-[% lang %] ignoring the variable MOZ_CHROME_MULTILOCALE (and in this case it doesn't matter if it doesn't contain all the locales), or should we add all the locales before running the first ./mach build chrome-[% lang %]?

Yeah, ./mach build is ignoring it. But ./mach package needs it. I used the FOREACH loop to populate the env variable while cycling through the locales anyway.

We could use this line to set all the locales in MOZ_CHROME_MULTILOCALE:

export MOZ_CHROME_MULTILOCALE='[% tmpl(c('var/locales').join(' ')) %]'

That's probably better, yes. I'll take that approach instead of my clumsy one.

An other minor thing: I think it would be better to extract the locales in a sub-directory (such as /var/tmp/dist/locales) instead of directly in /var/tmp/dist.

Okay, fine with me.

comment:21 Changed 9 months ago by gk

Keywords: GeorgKoppen201812 TorBrowserTeam201812R added; GeorgKoppen201811 TorBrowserTeam201812 removed
Status: needs_revisionneeds_review

comment:23 Changed 9 months ago by gk

Hm, so the new product details URL for 60.4.0esr should be https://product-details.mozilla.org/1.0/l10n/Firefox-60.4.0esr-build2.json but it is not available despite build2 is already done. We should not merge this patch before we can be sure the new product details are available once we start building.

comment:24 in reply to:  22 ; Changed 9 months ago by boklm

Replying to gk:

bug_26843_v8 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_26843_v8&id=524a9364ead98593507f8f881cc1aa9764945324) has the indentation fixed up (too).

This patch looks good to me. I did not merge it yet as the json file for 60.4.0 is not available yet.

comment:25 Changed 9 months ago by gk

Keywords: TorBrowserTeam201812 added; TorBrowserTeam201812R removed
Status: needs_reviewneeds_revision

Alright I got some answers back. The location where we currently get the JSON file from is only updated for builds that actually ship. But we can do better here. There seems to be in-tree JSON files we can (and should) use instead, e.g. https://hg.mozilla.org/releases/mozilla-esr60/raw-file/tip/browser/locales/l10n-changesets.json or https://hg.mozilla.org/releases/mozilla-esr60/raw-file/tip/mobile/locales/l10n-changesets.json. Or, if we want to have a particular tag we could use something like https://hg.mozilla.org/releases/mozilla-esr60/raw-file/FIREFOX_60_4_0esr_BUILD2/browser/locales/l10n-changesets.json instead.

I think we don't use the specific tag but just what the branch we build from currently has.

comment:26 Changed 9 months ago by boklm

Keywords: TorBrowserTeam201812R added; TorBrowserTeam201812 removed
Status: needs_revisionneeds_review

In bug_26843_v6, I added a commit that will get the json file from our tor-browser.git clone:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_26843_v6&id=1cffd6ce018e4114fa0e814602d5793646e6b10b

To get the json file we use the exec() function, which will checkout the git_hash commit and run the command from there.

comment:27 in reply to:  26 ; Changed 9 months ago by gk

Keywords: TorBrowserTeam201812 added; TorBrowserTeam201812R removed
Status: needs_reviewneeds_revision

Replying to boklm:

In bug_26843_v6, I added a commit that will get the json file from our tor-browser.git clone:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_26843_v6&id=1cffd6ce018e4114fa0e814602d5793646e6b10b

To get the json file we use the exec() function, which will checkout the git_hash commit and run the command from there.

You used cat browser/locales/l10n-changesets.json but we should take the .json file for now from the mobile directory as the l10n repos and revisions are used for Android only at the moment. Thus, cat mobile/locales/l10n-changesets.json. Otherwise this looks reasonable. I still need to test it, though.

comment:28 in reply to:  24 Changed 9 months ago by gk

Replying to boklm:

Replying to gk:

bug_26843_v8 (https://gitweb.torproject.org/user/gk/tor-browser-build.git/commit/?h=bug_26843_v8&id=524a9364ead98593507f8f881cc1aa9764945324) has the indentation fixed up (too).

This patch looks good to me. I did not merge it yet as the json file for 60.4.0 is not available yet.

We have a clear way forward (see previous comments). Thus, taking this patch now (commit 524a9364ead98593507f8f881cc1aa9764945324 on master) as it does not break anything until we bump the esr60 tor-browser branch.

comment:29 in reply to:  27 Changed 9 months ago by gk

Resolution: fixed
Status: needs_revisionclosed

Replying to gk:

Replying to boklm:

In bug_26843_v6, I added a commit that will get the json file from our tor-browser.git clone:
https://gitweb.torproject.org/user/boklm/tor-browser-build.git/commit/?h=bug_26843_v6&id=1cffd6ce018e4114fa0e814602d5793646e6b10b

To get the json file we use the exec() function, which will checkout the git_hash commit and run the command from there.

You used cat browser/locales/l10n-changesets.json but we should take the .json file for now from the mobile directory as the l10n repos and revisions are used for Android only at the moment. Thus, cat mobile/locales/l10n-changesets.json. Otherwise this looks reasonable. I still need to test it, though.

Alright, that seems to work. I compared the mobile/locales/ and the browser/locales/ l10n-changesets.json and it seems they differ a bit. Looking at https://product-details.mozilla.org/1.0/l10n/Firefox-60.3.0esr-build1.json it seems that one mirrors the file in browser/locales. Thus, as we ship for mobile I think it is good to use the one on mobile/locales instead.

To save a roundtrip I've made that small change in the patch myself. The result is commit bad46e532fa614ffdd3ed0e21f4b8fae3ba7793d on master.

comment:30 Changed 7 months ago by intrigeri

Cc: intrigeri added
Note: See TracTickets for help on using tickets.