Add simplified Chinese as a supported language #5697
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rmol
requested review from
conorsch,
emkll,
kushaldas and
redshiftzero
as code owners
contributors: updated from: repo: https://github.com/freedomofpress/securedrop-i18n commit: e921bb1
The way we were naming locales in the selection dropdown, using only part of the language name, wasn't sufficient to distinguish simplified and traditional Chinese. It was also off for Norwegian, in that we were just calling it "Norsk", which isn't the complete language name, indicating which written standard we use (Bokmål). So I've changed it to use the complete language name, and include further distinguishing information as necessary when we have more than one translation for a language. We were also constructing the mapping of locale identifiers to display names every request, but always using the locale endonyms for the display names, instead of showing display names in the visitor's language, so it didn't need to be done per request. I moved the creation of the locale name map to the app setup. Our negotiation of locales using the request's accepted languages was also not working for Chinese, because we store the Chinese translations by language and script, not region, so if a browser indicated acceptable Chinese locales containing regions, they wouldn't be matched and the visitor's first contact would be in English. While cleaning up the locale selector presentation, I consolidated some of the things we were tacking onto the Flask g object into a container class, i18n.RequestLocaleInfo. The source metadata API endpoint could duplicate the default locale in its list of supported languages. That's been fixed.
kushaldas
previously requested changes
Jan 12, 2021
There was a problem hiding this comment.
Took a lot of time to figure out that I should delete my old config.py 👍🏽
Testing
Simplified Chinese is a functional option in the locale selector
- 中文 (简体) is an option in the locale selector, and choosing it results in pages rendered in simplified Chinese. Confirm that the content is different from traditional Chinese.
Browser locale negotiation
- Clearing the session cookie and visiting the site with the browser's preferred language set to simplified Chinese (Firefox calls it "Chinese (China)") results in the site being rendered in simplified Chinese by default, not English.
- Clearing the session cookie and visiting the site with the browser's preferred language set to traditional Chinese (Firefox calls it "Chinese (Taiwan)") results in the site being rendered in traditional Chinese by default, not English.
- Clearing the session cookie and visiting the site with the browser's preferred language set to something other than English or Chinese also results in pages rendered in that language.
Other locale functionality
- Ensure that manually selecting any locale changes the UI persistently -- if you navigate around the source and journalist interfaces, every page should be in your chosen locale.
- [❌] As a source, upload a file larger than a megabyte. When viewing the source's collection in the JI, the file size thousands separator is adjusted correctly (commas for Arabic, Chinese, English, and Hindi; periods for others). -- This came as . for me in Arabic or Hindi
- Reply to the source, then check the reply in the SI. The timestamp should be localized properly -- be sure to hover over it to check the full timestamp.
- After changing the locale, the
<html>element of any page has bothdirandlangattribute set correctly. For Arabic,diris set tortl. Thelangattribute should be a hyphenated locale identifier, per RFC5646/BCP47, where our locale includes a region or script (e.g.de_DE,zh_Hant).
Source metadata API
- Check http://localhost:8080/metadata and ensure that
supported_languagesmatchesconfig.SUPPORTED_LOCALES, with each identifier only appearing once (English used to be repeated). Ensure thatzh_Hans(simplified Chinese) is present.
"Impressive. Every word in that sentence was wrong." Sorry for the goose chase @kushaldas. There shouldn't ever be a thousands separator in the file size. There, the decimal separator should be adjusted correctly, and it should be a comma except for Arabic, Chinese, English and Hindi. I'll correct the test plan. |
conorsch
approved these changes
Jan 12, 2021
There was a problem hiding this comment.
Looks good. I noticed one deviation from the test plan, but it appears related to me being a stickler on interpreting a test plan, and not actually surprising behavior that needs to be changed in the app.
Simplified Chinese is a functional option in the locale selector
- 中文 (简体) is an option in the locale selector, and choosing it results in pages rendered in simplified Chinese. Confirm that the content is different from traditional Chinese.
Browser locale negotiation
- Clearing the session cookie and visiting the site with the browser's preferred language set to simplified Chinese (Firefox calls it "Chinese (China)") results in the site being rendered in simplified Chinese by default, not English.
- Clearing the session cookie and visiting the site with the browser's preferred language set to traditional Chinese (Firefox calls it "Chinese (Taiwan)") results in the site being rendered in traditional Chinese by default, not English.
- Clearing the session cookie and visiting the site with the browser's preferred language set to something other than English or Chinese also results in pages rendered in that language.
Other locale functionality
- 🟠 Ensure that manually selecting any locale changes the UI persistently -- if you navigate around the source and journalist interfaces, every page should be in your chosen locale.
- See screenshot below. I set the browser locale to Chinese, but used the in-app language selector to set German. On the Source Submit screen, the browser widget buttons were still Chinese, but the rest of the app test was in German. That's unsurprising to me and I believe acceptable.
- As a source, upload a file larger than a megabyte. When viewing the source's collection in the JI, the file size decimal separator should be a period for Arabic, Chinese, English, and Hindi, and a comma for others.
- Reply to the source, then check the reply in the SI. The timestamp should be localized properly -- be sure to hover over it to check the full timestamp.
- After changing the locale, the
<html>element of any page has bothdirandlangattribute set correctly. For Arabic,diris set tortl. Thelangattribute should be a hyphenated locale identifier, per RFC5646/BCP47, where our locale includes a region or script (e.g.de_DE,zh_Hant).
Source metadata API
- Check http://localhost:8080/metadata and ensure that
supported_languagesmatchesconfig.SUPPORTED_LOCALES, with each identifier only appearing once (English used to be repeated). Ensure thatzh_Hans(simplified Chinese) is present.
Screenshot as promised:
conorsch
dismissed
kushaldas’s stale review
Test plan has been updated based on feedback
|
I've dismissed @kushaldas's review since the test plan has been updated. @rmol please see screenshot above, otherwise fine to merge from my end! |
|
Yeah, we don't have any control over the native widget text, as far as I know; they're rendered like the rest of the browser UI. There are JS/CSS hacks around, but the standard behavior seems the most intuitive and least surprising. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status
Ready for review
Description of Changes
Fixes #5069.
This ended up requiring quite a bit more than just updating the supported languages in
securedrop/i18n_tool.pyand updating the translations from Weblate.The way we were naming locales in the selection dropdown, using only part of the language name, wasn't sufficient to distinguish simplified and traditional Chinese. It was also off for Norwegian, in that we were just calling it "Norsk", which isn't the complete language name, indicating which written standard we use (Bokmål). So I've changed it to use the complete language name, and include further distinguishing information as necessary when we have more than one translation for a language.
We were also constructing the mapping of locale identifiers to display names every request, but always using the locale endonyms for the display names, instead of showing display names in the visitor's language, so it didn't need to be done per request. I moved the creation of the locale name map to the app setup.
Our negotiation of locales using the request's accepted languages was also not working for Chinese, because we store the Chinese translations by language and script, not region, so if a browser indicated acceptable Chinese locales containing regions, they wouldn't be matched and the visitor's first contact would be in English.
While cleaning up the locale selector presentation, I consolidated some of the things we were tacking onto the Flask g object into a container class,
i18n.RequestLocaleInfo.The source metadata API endpoint could duplicate the default locale in its list of supported languages. That's been fixed.
Testing
If testing with the dev server, move or remove any existing
securedrop/config.py.Simplified Chinese is a functional option in the locale selector
Browser locale negotiation
Other locale functionality
<html>element of any page has bothdirandlangattribute set correctly. For Arabic,diris set tortl. Thelangattribute should be a hyphenated locale identifier, per RFC5646/BCP47, where our locale includes a region or script (e.g.de_DE,zh_Hant).Source metadata API
supported_languagesmatchesconfig.SUPPORTED_LOCALES, with each identifier only appearing once (English used to be repeated). Ensure thatzh_Hans(simplified Chinese) is present.Deployment
These changes:
securedrop/sdconfig.pyto produce a cleanerSUPPORTED_LOCALESsetting.Checklist
If you made changes to the server application code:
make lint) and tests (make test) pass in the development containerIf you made non-trivial code changes:
Choose one of the following: