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][HybridGlobalization] Fix incorrect value in FirstDayOfWeek test #93595

Merged
merged 3 commits into from
Oct 18, 2023

Conversation

ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Oct 17, 2023

Fixes #93354.
0 decodes Sunday, 1 decodes Monday. We did not have exception wrapping when returning the value from JS function, so it was pointing to a random value in memory. If we were unlucky that this value was equal 0 then we were assuming the JS code returned exception and returning the default value for DayOfWeek which is Sunday.

@ilonatommy ilonatommy added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 17, 2023
@ilonatommy ilonatommy self-assigned this Oct 17, 2023
@ghost
Copy link

ghost commented Oct 17, 2023

Tagging subscribers to this area: @dotnet/area-system-globalization
See info in area-owners.md if you want to be subscribed.

Issue Details

This draft PR checks if the change fixes failures. It is not a fix itself.

Author: ilonatommy
Assignees: ilonatommy
Labels:

NO-MERGE, area-System.Globalization

Milestone: -

@ilonatommy ilonatommy added the arch-wasm WebAssembly architecture label Oct 17, 2023
@ghost
Copy link

ghost commented Oct 17, 2023

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

Issue Details

This draft PR checks if the change fixes failures from #93354. It is not a fix itself.

Author: ilonatommy
Assignees: ilonatommy
Labels:

NO-MERGE, arch-wasm, area-System.Globalization

Milestone: -

@ilonatommy ilonatommy removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 17, 2023
@ilonatommy ilonatommy marked this pull request as ready for review October 17, 2023 14:02
@ilonatommy ilonatommy merged commit 93fc05c into dotnet:main Oct 18, 2023
183 of 186 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Nov 17, 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.

Incorrect value in FirstDayOfWeek test
2 participants