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

[browser][non-icu] HybridGlobalization normalization unblocking APIs that became supported. #86694

Closed
wants to merge 4 commits into from

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented May 24, 2023

In non-hybrid globalization we are not supporting normalization forms: KD and KC.
In the hybrid mode, both forms are supported and we need them for implemenation of hybrid IDN/ACE mapping.
This PR is changing the public API behavior by enabling both forms on Browser platform. When the HybridGlobalziation (that is an opt-in feature) is switched off and the user is not in InvariantGlobalization mode, they will get PNSE exception when they try to use KD/KC. It is asured by validation in

Contributes to #79989.

@lewing this PR changes the behavior: from failing in the build to failing during runtime when KD/KC forms are used.

@ghost
Copy link

ghost commented May 24, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

In non-hybrid globalization we are not supporting nromalization forms: KD and KC.
In the hybrid mode, both forms are supported and we need them for implemenation of hybrid IDN/ACE mapping.
This PR is changing the public API behavior by enabling both forms on Browser platform. When the HybridGlobalziation (that is an opt-in feature) is switched off and the user is not in InvariantGlobalization mode, they will get PNSE exception when they try to use KD/KC. It is asured by validation in

Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-System.Globalization

Milestone: -

@tarekgh
Copy link
Member

tarekgh commented May 24, 2023

Looks you need a small tweak to this fix?

/__w/1/s/src/libraries/System.Private.CoreLib/src/System/Globalization/Normalization.Icu.cs(104,36): error CS0117: 'GlobalizationMode' does not contain a definition for 'Hybrid' [/__w/1/s/src/coreclr/System.Private.CoreLib/System.Private.CoreLib.csproj]
##[error]src/libraries/System.Private.CoreLib/src/System/Globalization/Normalization.Icu.cs(104,36): error CS0117: (NETCORE_ENGINEERING_TELEMETRY=Build) 'GlobalizationMode' does not contain a definition for 'Hybrid'

@ilonatommy
Copy link
Member Author

Drafting till we:

  • will be able to provide unified behavior for non-hybrid and hybrid mode e.g. by adding more data to ICU, we could leverage custom ICU build [browser][icu] Automate custom icu creation #82908
    OR
  • introduce code analysis that will detect HybridGlobalization mode, so we could preserve failing the build for non-hybrid mode.

@ilonatommy ilonatommy marked this pull request as draft May 25, 2023 14:36
@ilonatommy
Copy link
Member Author

Closing - normalization is getting reverted with #87007

@ilonatommy ilonatommy closed this Jun 2, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jul 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants