Skip to content
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

Clean up normalizeLocale for mono browser target #112575

Merged
merged 1 commit into from
Feb 17, 2025

Conversation

IDisposable
Copy link
Contributor

@IDisposable IDisposable commented Feb 14, 2025

In the removal of HybridLocalization in PR #110567 the following code was introduced into globalization-locale.ts

locale = locale.toLocaleLowerCase();
if (locale.includes("zh")) {
    // browser does not recognize "zh-chs" and "zh-cht" as equivalents of "zh-HANS" "zh-HANT", we are helping, otherwise
    // it would throw on getCanonicalLocales with "RangeError: Incorrect locale information provided"
    locale = locale.replace("chs", "HANS").replace("cht", "HANT");
}

Firstly, the Chinese language check should be a check of startsWith( the prefix "zh-", not include( the more ambiguous "zh". This could lead to problems if someone was using the substring zh in any of the other parts of the locale string. To ensure this works with locales formatted with underscores, we need to move up that locale.replace("_", "-") to the first line.

Secondly, per RFC 5646 the script code should be upper-case initial letter and lower-case the rest, so the replacement constants should be "Hans" and "Hant" respectively. Also, since those replacement patterns for those subparts are supposed to be targeting the script indicator, it would be much safer to include the '-' prefix.

Fixes #112573

Copy link
Member

@ilonatommy ilonatommy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for spotting this. The improvements look good.

@ilonatommy ilonatommy merged commit 8de6e01 into dotnet:main Feb 17, 2025
44 of 47 checks passed
grendello added a commit to grendello/runtime that referenced this pull request Feb 18, 2025
* main: (71 commits)
  Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20250212.3 (dotnet#112626)
  JIT: Unify struct arg morphing (dotnet#112612)
  Enable `SA1015`: Closing generic bracket should not be followed by a space (dotnet#112597)
  Clean up normalizeLocale for mono browser target (dotnet#112575)
  SPMI: Ensure proper zero extension for isObjectImmutable and friends (dotnet#112617)
  Quote --version-scripts path (dotnet#112603)
  Remove incompatible API from PKCS netstandard2.0 lib
  [main] Update dependencies from dotnet/emsdk (dotnet#112393)
  Avoid `Unsafe.As` in `RangeCharSearchValues` (dotnet#112606)
  Fixed the issue of incorrect return value of PalVirtualAlloc (dotnet#112579)
  Fix size used for vectorization check in BitArray (dotnet#111558) (dotnet#111564)
  Fix build of windows arm64 crossdac (dotnet#112553)
  Simplify `ShuffleTakeIterator.GetCount` (dotnet#112593)
  Fix VS div-by-0 in DacEnumerableHashTable code (dotnet#112542)
  R2RDump: normalize GC info totalInterruptibleLength (dotnet#112003)
  Fix alignment padding and add test for saving managed resources (dotnet#110915)
  Adds `ccmp` logic into emitter backend. (dotnet#112153)
  Disable AVX10.2 by default (dotnet#112572)
  Outbox AesGcm in to Microsoft.Bcl.Cryptography
  Make test `IUnknown` conforming (dotnet#112566)
  ...
@IDisposable IDisposable deleted the mono-browser-locale branch February 21, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-System.Globalization community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mono] Possible fragile code in /src/mono/browser/runtime/globalization-locale.ts
3 participants