-
Couldn't load subscription status.
- Fork 5.2k
[browser] Revert to full NativeName by interop with JS
#99956
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
and DisplayName for browser.
NativeName by interop with JS
src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Icu.cs
Show resolved
Hide resolved
src/libraries/System.Runtime/tests/System.Globalization.Tests/CultureInfo/CultureInfoNames.cs
Show resolved
Hide resolved
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.
These test cases also need to be updated for browser (there can be even more)
Lines 115 to 127 in 8fbfb32
| if (PlatformDetection.IsNotUsingLimitedCultures || PlatformDetection.IsAndroid || PlatformDetection.IsHybridGlobalizationOnApplePlatform) | |
| { | |
| yield return new object[] { "GB", "United Kingdom" }; | |
| yield return new object[] { "SE", "Sverige" }; | |
| yield return new object[] { "FR", "France" }; | |
| } | |
| else | |
| { | |
| // Browser's ICU doesn't contain RegionInfo.NativeName | |
| yield return new object[] { "GB", "GB" }; | |
| yield return new object[] { "SE", "SE" }; | |
| yield return new object[] { "FR", "FR" }; | |
| } |
Lines 16 to 27 in 8fbfb32
| if (PlatformDetection.IsNotUsingLimitedCultures || PlatformDetection.IsAndroid || PlatformDetection.IsHybridGlobalizationOnApplePlatform) | |
| { | |
| yield return new object[] { "en-US", "English (United States)" }; | |
| yield return new object[] { "en-CA", "English (Canada)" }; | |
| yield return new object[] { "en-GB", "English (United Kingdom)" }; | |
| } | |
| else | |
| { | |
| // Mobile / Browser ICU doesn't contain CultureInfo.NativeName | |
| yield return new object[] { "en-US", "en (US)" }; | |
| yield return new object[] { "en-CA", "en (CA)" }; | |
| } |
src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Outdated
Show resolved
Hide resolved
…CultureData.Browser.cs Co-authored-by: Meri Khamoyan <[email protected]>
Awesome, thank you. I think we can simplify even the condition. Currently tests are checking for
|
Yes, that sounds great. I guess we can modify |
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.
LGTM!
…on CultureInfo constructio
…T, change to `FEATURE_WASM_MANAGED_THREADS`.
Fixes #44739, #45951.
Reason for this PR:
For size savings we removed
lang_treeandregion_treefrom ICU WASM filters. We were returning a short form ofNativeNameandDisplayNamefor ~200 supported locales in the browser, e.g.en (US). After experimenting withHybridGlobalizationwe proved that we can get the missing data from JS to patch the regression caused by the size reduction. Now,NativeNameandDisplayNamewill be fetched from WebAPIIntl.DisplayNames. Other Globalization APIs (otherLocaleStringDatatypes) continue to be fetched from ICU.Logic change
Without this PR:
We ask ICU about
NativeDisplayNamebut it does not have this info and returns empty data. Inruntime/src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.cs
Line 1086 in 8fbfb32
Similar mechanism is used for
LocalizedDisplayName.new CultureInfo("zh-Hans").EnglishName-> "zh-Hans"With this PR:
For all
LocaleStringDatatypes butNativeDisplayName,EnglishDisplayNameandLocalizedDisplayNamewe continue fetching the data from ICU. In these three cases we call to JS.Fixed APIs:
CultureInfo.NativeName,CultureInfo.EnglishName,CultureInfo.DisplayName,RegionInfo.NativeName,RegionInfo.EnglishName,RegionInfo.DisplayName.They depend on ECMAScript's
IntlAPI, so some responses might differ from ICU responses, e.g.new CultureInfo("zh-Hans").EnglishName-> "Simplified Chinese", not "Chinese (Simplified)".Result:
Performance impact
The
CultureInfoconstructor call interops with JS / ICU API, the next calls toEnglish/Nativeproperties rely on cached value. Measuring performance forNativeNameandDisplayName:Without this PR:
With this PR:
Multithreaded runtime still uses the truncated data version, fetching data from JS fails there see: #98483