-
Notifications
You must be signed in to change notification settings - Fork 372
Fix paywall locale receiving UN M.49 standard localization #5309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense!
// This covers cases where dictionary keys contain the UN M.49 standard. For example: | ||
// If locale = "es_MX" and dictionary key = "es_419" (both have rc_languageAndScript = "es-Latn") | ||
if let onlyLanguageAndScriptIdentifier = locale.rc_languageAndScript, | ||
let match = self.valueForLanguageAndScript(onlyLanguageAndScriptIdentifier) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if it's possible/worth to add some tests for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. Good point that I totally forgot about! Thank you for the suggestion!
Actually, I was writing a test for this and the first one I wrote did not pass func test_es_MX_Matches_es_419_ByLanguageAndScript() {
let localizations = [
"es_419": Self.expectedTranslations,
"es": ["wrong": "this is es"]
]
let locale = Locale(identifier: "es_MX")
// This tests that "es_MX" matches "es_419" due to both sharing the same language and script ("es-Latn")
let foundLocalizations = localizations.findLocale(locale)
expect(foundLocalizations).to(equal(Self.expectedTranslations))
} The reason is that I originally added the new check as a last resort (it was the last check). But that makes "es_MX" match "es" before "es_419", which is wrong. Therefore, I think the new check should happen before checking only the language identifier (done 14d3c99) Please lmk if you think otherwise! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeei for tests! 🙌
Motivation
Found a bug where displaying a paywall with the only Spanish localization being

"es_MX"
would not show some localization strings (device region: Spain, preferred languages: es, en_US)Description
In this case, the server's
ui_config.localization
contains"es_419"
(UN M.49 standard for the Latin America and the Caribbean region) but no"es_MX"
. With the existing implementation, the SDK was not able to match"es_MX"
with"es_419"
.This PR adds an extra step when trying to find the matching locale from
UIConfig.localizations
. As a last resort, it tries to find a key inui_config.localization
with matching language and script.