Android 13 per app language preferences #18363
Conversation
with localeConfig tag
the build system only includes language resource in the APK for these specified languages, preventing translated strings from being included from other libraries that might support languages that our app does not support
Update in app picker related when Language changed in System settings
Updates app locale from in app picker to System Settings
Let AndroidX handle the locale storage so that the user's preference persists.
|
 |
App Name |  WordPress |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr18363-4e144ce | |
| Commit | 4e144ce | |
| Direct Download | wordpress-prototype-build-pr18363-4e144ce.apk |
|
 |
App Name |  Jetpack |
| Flavor | Jalapeno | |
| Build Type | Debug | |
| Version | pr18363-4e144ce | |
| Commit | 4e144ce | |
| Direct Download | jetpack-prototype-build-pr18363-4e144ce.apk |
There was a problem hiding this comment.
I don't feel knowledgeable enough in Android internals to review that part of the PR, so I'll let that to @ParaskP7 and other Android devs.
As for the tooling aspect:
1️⃣ Locale codes format discrepency
Running the bundle exec fastlane download_translations lane after this change will result in an error, because our check_declared_locales_consistency method will find discrepencies between the locales declared in resourceConfigurations and the ones declared in the fastlane/lanes/translations.rb's JP_APP_LOCALES constant used to tell what locales to download from GlotPress.
- One of the reason for this discrepancy is that in the
resourceConfigurationsyou added, you use thept_BRsyntax for locale codes, while in the Fastfile constant we use thept-rBRsyntax (because that value is used to know in whichres/values-{code}/strings.xmlfile to write the downloaded translations into)- To be honest I'm not sure if your syntax (
pt_BR) is valid in the context ofresourceConfigurations, or if you should have usedpt-rBRsyntax for it to work as expected, or if both are supported and it doesn't matter to gradle. - If the right syntax is
pt_BR(the one you used) and values likept-rBRwould not work in the context ofresourceConfigurationsfield, then that means we've been doing it wrong for thejetpackflavor all that time (see line 213) 😱 and we'll need to fixjetpack'sresourceConfigurations(and adjust the logic of thecheck_declared_locales_consistencycode) - But if
pt-rBRsyntax works in theresourceConfigurationscontext, then I'd suggest to use that syntax instead so that we're consistent with thevalues-*folder names and so thatcheck_declared_locales_consistencywould not need fixing
- To be honest I'm not sure if your syntax (
- Another reason for why
check_declared_locales_consistencyreports an issue is that there are indeed some locales in one place and not the other- In your
resourceConfigurations, you includeden_US, while in theFastfilethat's not a locale we download from GlotPress (in fact, that locale is not configured in GlotPress' WordPress project, because US English is considered to be the originals for the strings, not needing translations). Also, we don't have avalues-en-rUSfolder insrc/main/resanyway — instead, US English is considered our base language living invalues. Given all that, I think it's not necessary to includeen-rUS(oren_US, whatever the syntax) locale in yourresourceConfigurationsforwordpress - There are inconsistencies around
he/idused in theFastfilevsiw/inused inresourceConfigurations. Those are legacy vs more modern locale codes for Hebrew and Indonesian, and this issue has already been discussed and addressed [in a recent separate PR]. To match how we dealt with this for Jetpack, we need to usehe/idinresourceConfigurations(as those are the modern ones), and addin/iwseparately (to highlight that they are legacy ones), like how it's done on lines 214-216 forjetpack - There's also an inconsistency between
kmrused in Fastfile's constant (since the folder isvalues-kmr) vskmused inresourceConfigurations - Finally, the
Fastfilealso downloads thelvanduzlocales from GlotPress (and we have the correspondingvalues-lvandvalues-uzfolders insrc/main/res) but those were not added to yourresourceConfigurationslist
- In your
locales_config.xml, are we sure Android expects us to use the pt-BR syntax in that context vs pt-rBR or pt_BR? (open question; haven't checked the docs). Maybe it's ok, because it seems to be what's used in the available_languages.xml file too… but available_languages.xml is not an official file expected by Android (but instead just a regular string-array resource read/used by the java code of our custom language picker), so worth double-checking the case for locales_config.xml (boy those different formats for Android locales codes depending on contexts is so annoying!)
💡 Tip: How I figured out the differences
To help me figure out the differences in the two lists, I made the following changes in the `check_declared_locales_consistency` method… which could actually be a good idea to commit while we're at it, to help for future debugging too: UI.user_error! <<~ERROR
The list of `resourceConfigurations` declared in your `build.gradle` for the `#{app_flavor}` flavor
does not match the list of locales we hardcoded in the `fastlane/lanes/localization.rb` for this app.
+ - In `build.gradle`'s `resourceConfigurations`:
+ #{resource_configs.inspect}`
+ - In `Fastfile`:
+ #{expected_locales.inspect}
If you recently updated the hardcoded list of locales to include for this app, be sure to apply those
changes in both places, to keep the Fastlane scripts consistent with the gradle configuration of your app.
ERRORThen, for testing purposes, I commented out the android_download_translations calls in the download_translations lane (so that this lane would only do the calls to check_declared_locales_consistency) and ran that download_translations lane to check if the consistency checks were happy.
2️⃣ Generate the locales_config.xml by code in Fastfile
This is to make sure it's always kept consistent with the list of locales we download from GlotPress and we include as resourceConfigurations, to avoid future discrepancies. I know that you mentioned in your PR description:
Potential improvements to handle separately
- Auto generate locales_config.xml using release-toolkit when we download the list of locales from GlotPress. This can also be auto generated apparently with upcoming Android Studio Giraffe
And I agree that implementing it in release-toolkit can be kept for later. But I'd still advocate to quickly jot some simple code in the Fastfile to generate that XML (see 3️⃣ below for my take at such an implementation).
We can then keep for later the idea of migrating that code into a proper fastlane action in release-toolkit when we have the time, but in the meantime, but in the meantime having that simple code directly in the Fastfile is simple enough and will ensure we don't break the app for some locales later… without realizing it until after a long time.
3️⃣ Issue with the locales_config being common with WP and JP flavors
Finally, shouldn't we also add a locales_config.xml file for Jetpack in src/jetpack/res/xml, that would restrict the list to the smaller subset of locales supported by Jetpack?
src/jetpack/res/xml/locales_config.xml file would be used as an override of the src/main/res/xml/locales_config.xml for Jetpack, instead of having it merged with the locales declared in src/main/res/xml when compiling the jetpack flavor (which would result in Jetpack exposing more locales in the language switch that it actually supports).
So maybe that means also moving src/main/res/xml/locales_config.xml to src/wordpress/res/xml/locales_config.xml instead?
I've taken a stab at generating those with some ruby code from the Fastfile, and this could look like this:
def generate_locales_config_file(app_flavor:, locales_list: )
locale_codes = locales_list.map { |l| l[:android].gsub('-r', '_') }
# Support for legacy locale codes
expected_locales << 'in' if expected_locales.include?('id')
expected_locales << 'iw' if expected_locales.include?('he')
doc = Nokogiri::XML::Builder.new(encoding: 'UTF-8') do |xml|
xml.comment('Warning: Auto-generated file, do not edit.')
xml.send(:'locale-config', 'xmlns:tools': 'http://schemas.android.com/tools') do
locale_codes.each { |code| xml.locale('android:name': code.gsub('-r', '-')) }
end
end
File.write(File.join(PROJECT_ROOT_FOLDER, 'WordPress', 'src', app_flavor.to_s, 'res', 'xml', 'locales_config.xml'), doc.to_xml)
endThen we can call this generate_locales_config_file(app_flavor: …, locales_list: …) inside the download_translations lane each for WP and JP case, just after and with the same parameters as when we call check_declared_locales_consistency(…) there.
|
👋 @ravishanker ! I am not sure what's the latest status on this, but I just wanted to let you know now that we've merged AGP 8.1.0 into For more info see: Automatic per-app language support |
|
👋 @ravishanker ! FYI: I'll unassign myself from this PR, just so that it gets removed from my Review requests list. Feel free to assign me again on it when you progress it a bit further. |
|
Thank you all for your input. I'll attempt to do this afresh later given some more developments in this space like Android Studio Giraffe |
This PR adds Per-app language preferences introduced in Android 13 (API 33) level. Some of this work was done as part of HACK week, and was put on hold pending Android 13 upgrade. Since Android 13 upgrade is now complete, it is time to roll this out.
TODO:
RELEASE-NOTES.txtbefore mergedevice-2022-09-16-114901.mp4
device-2022-09-16-114935.mp4
To test:
Test 1:
Test 2:
Test 3:
Test 4:
Test 5:
To test:
Regression Notes
Potential unintended areas of impact
Language and Locale in some places
What I did to test those areas of impact (or what existing automated tests I relied on)
Tested various features and in both ways from in-app picker to system settings and vice versa
What automated tests I added (or what prevented me from doing so)
Existing unit tests
PR submission checklist:
RELEASE-NOTES.txtif necessary.