Fix incorrect time patterns for some cultures on browser in HybridGlobalization - #103804

Merged
ilonatommy merged 6 commits into
dotnet:mainfrom
ilonatommy:fix-103745
Jun 24, 2024
Merged

Fix incorrect time patterns for some cultures on browser in HybridGlobalization#103804
ilonatommy merged 6 commits into
dotnet:mainfrom
ilonatommy:fix-103745

Conversation

@ilonatommy

@ilonatommyilonatommy commented Jun 21, 2024

Copy link
Copy Markdown
Member

Fixes#103745.

This PR makes improvements to values returned in date/time format APIs. It does not make all the locales' values match ICU's values due to Web API responses differences, but corrects majority of them.
Originally the ICU's expected behavior was saved in a form of a comment next to the specific test case. If this PR's fix made HybridGlobalization response match the comment , the comment got removed. If there are still discrepancies (e.g. node behaves different than chrome) the ICU value is left in the comment.

@ilonatommyilonatommy self-assigned this Jun 21, 2024
@dotnet-policy-service

dotnet-policy-serviceBot commented Jun 21, 2024

Copy link
Copy Markdown
Contributor

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

@ilonatommy

ilonatommy commented Jun 21, 2024•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

cc @PopSlime, HybridGlobalization relies on JS methods and there's no method in Intl that would return a "format" of date. We are working it around by taking a specific date, formatting it using among others toLocaleString and then we parse the result to determine the generic format of date / time for the given locale. I added a function that tries to find the parts of the already formatted date pattern that are "fixed" and these that are "variant" and put the fixed ones in quotes. If you have any suggestions how to improve this process, I would be glad to talk about it.

@ilonatommy

ilonatommy commented Jun 21, 2024

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Jun 21, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara
pavelsavara reviewed Jun 21, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts Outdated
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts Outdated
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts Outdated
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts Outdated
@pavelsavara

pavelsavara commented Jun 21, 2024

Copy link
Copy Markdown
Member

are we going to backport it to Net8 ?

@ilonatommy

ilonatommy commented Jun 21, 2024

Copy link
Copy Markdown
MemberAuthor

are we going to backport it to Net8 ?

I would like to.

@ilonatommy

ilonatommy commented Jun 21, 2024

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Jun 21, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024
Closed
Open
Closed
Closed
Closed
@ilonatommy

ilonatommy commented Jun 21, 2024

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Jun 21, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024
Closed
Open
Closed
mkhamoyan
mkhamoyan reviewed Jun 24, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
@ilonatommy

ilonatommy commented Jun 24, 2024

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Jun 24, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@mkhamoyan

mkhamoyan commented Jun 24, 2024

Copy link
Copy Markdown
Contributor

Can we also enable test cases from #103681 ?

@ilonatommy

ilonatommy commented Jun 24, 2024

Copy link
Copy Markdown
MemberAuthor

/azp run runtime-wasm

@azure-pipelines

azure-pipelinesBot commented Jun 24, 2024

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 24, 2024
Closed
Closed
Closed
Open
Open
@ilonatommy
ilonatommy merged commit 8169bde into dotnet:mainJun 24, 2024
@ilonatommy

ilonatommy commented Jun 25, 2024

Copy link
Copy Markdown
MemberAuthor

/backport to release/8.0-staging

@github-actions

github-actionsBot commented Jun 25, 2024

Copy link
Copy Markdown
Contributor

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9662243985

@github-actions

github-actionsBot commented Jun 25, 2024

Copy link
Copy Markdown
Contributor

@ilonatommy backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix.
Using index info to reconstruct a base tree...
M	src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
A	src/mono/browser/runtime/hybrid-globalization/calendar.ts
A	src/mono/browser/runtime/hybrid-globalization/culture-info.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
Auto-merging src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (modify/delete): src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs deleted in HEAD and modified in Fix.. Version Fix. of src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs left in tree.
Auto-merging src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
CONFLICT (content): Merge conflict in src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix.
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@github-actions

github-actionsBot commented Jun 25, 2024

Copy link
Copy Markdown
Contributor

@ilonatommy an error occurred while backporting to release/8.0-staging, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

@ilonatommyilonatommy mentioned this pull request Jun 25, 2024
Merged
ilonatommy added a commit that referenced this pull request Jun 26, 2024
@ilonatommy
Backport: Fix incorrect time patterns for some cultures on browser in…
5753765
… `HybridGlobalization` (#103964)
Backport of #103804.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Jul 26, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@matouskozakmatouskozakAwaiting requested review from matouskozak
@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
1 more reviewer
@mkhamoyanmkhamoyanmkhamoyan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ilonatommyilonatommy

Labels

arch-wasmWebAssembly architecturearea-System.Globalization

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Incorrect time patterns for some cultures on browser in HybridGlobalization

3 participants

@ilonatommy@pavelsavara@mkhamoyan
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Fix incorrect time patterns for some cultures on browser in HybridGlobalization - #103804
Merged
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Jun 24, 2024
Merged
Fix incorrect time patterns for some cultures on browser in HybridGlobalization#103804
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Conversation
@ilonatommy
@ilonatommyilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
Fixes#103745.
This PR makes improvements to values returned in date/time format APIs. It does not make all the locales' values match ICU's values due to Web API responses differences, but corrects majority of them.

Originally the ICU's expected behavior was saved in a form of a comment next to the specific test case. If this PR's fix made HybridGlobalization response match the comment , the comment got removed. If there are still discrepancies (e.g. node behaves different than chrome) the ICU value is left in the comment.
@ilonatommyilonatommy
self-assigned this
Jun 21, 2024
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jun 21, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @dotnet/area-system-globalization

See info in area-owners.md if you want to be subscribed.
@ilonatommy
ilonatommy
commented
Jun 21, 2024•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
cc @PopSlime, HybridGlobalization relies on JS methods and there's no method in Intl that would return a "format" of date. We are working it around by taking a specific date, formatting it using among others toLocaleString and then we parse the result to determine the generic format of date / time for the given locale. I added a function that tries to find the parts of the already formatted date pattern that are "fixed" and these that are "variant" and put the fixed ones in quotes. If you have any suggestions how to improve this process, I would be glad to talk about it.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
pavelsavara
pavelsavara
reviewed
Jun 21, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

@pavelsavara
pavelsavara
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
are we going to backport it to Net8 ?
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
are we going to backport it to Net8 ?
I would like to.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed


Closed


Closed

@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed

mkhamoyan
mkhamoyan
reviewed
Jun 24, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@mkhamoyan
mkhamoyan
commented
Jun 24, 2024

Copy link


Copy Markdown

Contributor
Can we also enable test cases from #103681 ?
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 24, 2024

Closed


Closed


Closed


Open


Open

@ilonatommy
ilonatommy
merged commit 8169bde
into
dotnet:mainJun 24, 2024
@ilonatommy
ilonatommy
commented
Jun 25, 2024

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0-staging
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9662243985
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:
$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix.
Using index info to reconstruct a base tree...
M	src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
A	src/mono/browser/runtime/hybrid-globalization/calendar.ts
A	src/mono/browser/runtime/hybrid-globalization/culture-info.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
Auto-merging src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (modify/delete): src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs deleted in HEAD and modified in Fix.. Version Fix. of src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs left in tree.
Auto-merging src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
CONFLICT (content): Merge conflict in src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix.
Error: The process '/usr/bin/git' failed with exit code 128
Please backport manually!
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy an error occurred while backporting to release/8.0-staging, please check the run log for details!
Error: git am failed, most likely due to a merge conflict.
@ilonatommyilonatommy
mentioned this pull request
Jun 25, 2024
 Merged

ilonatommy added a commit
that referenced
this pull request
Jun 26, 2024
@ilonatommy
Backport: Fix incorrect time patterns for some cultures on browser in…
5753765
… `HybridGlobalization` (#103964)
Backport of #103804.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 26, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@matouskozakmatouskozakAwaiting requested review from matouskozak
@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner

1 more reviewer

@mkhamoyanmkhamoyanmkhamoyan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ilonatommyilonatommy

Labels

arch-wasmWebAssembly architecturearea-System.Globalization

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Incorrect time patterns for some cultures on browser in HybridGlobalization

3 participants

@ilonatommy@pavelsavara@mkhamoyan

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix incorrect time patterns for some cultures on browser in HybridGlobalization - #103804
Merged
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Jun 24, 2024
Merged
Fix incorrect time patterns for some cultures on browser in HybridGlobalization#103804
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Conversation
@ilonatommy
@ilonatommyilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
Fixes#103745.
This PR makes improvements to values returned in date/time format APIs. It does not make all the locales' values match ICU's values due to Web API responses differences, but corrects majority of them.

Originally the ICU's expected behavior was saved in a form of a comment next to the specific test case. If this PR's fix made HybridGlobalization response match the comment , the comment got removed. If there are still discrepancies (e.g. node behaves different than chrome) the ICU value is left in the comment.
@ilonatommyilonatommy
self-assigned this
Jun 21, 2024
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jun 21, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @dotnet/area-system-globalization

See info in area-owners.md if you want to be subscribed.
@ilonatommy
ilonatommy
commented
Jun 21, 2024•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
cc @PopSlime, HybridGlobalization relies on JS methods and there's no method in Intl that would return a "format" of date. We are working it around by taking a specific date, formatting it using among others toLocaleString and then we parse the result to determine the generic format of date / time for the given locale. I added a function that tries to find the parts of the already formatted date pattern that are "fixed" and these that are "variant" and put the fixed ones in quotes. If you have any suggestions how to improve this process, I would be glad to talk about it.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
pavelsavara
pavelsavara
reviewed
Jun 21, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

@pavelsavara
pavelsavara
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
are we going to backport it to Net8 ?
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
are we going to backport it to Net8 ?
I would like to.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed


Closed


Closed

@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed

mkhamoyan
mkhamoyan
reviewed
Jun 24, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@mkhamoyan
mkhamoyan
commented
Jun 24, 2024

Copy link


Copy Markdown

Contributor
Can we also enable test cases from #103681 ?
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 24, 2024

Closed


Closed


Closed


Open


Open

@ilonatommy
ilonatommy
merged commit 8169bde
into
dotnet:mainJun 24, 2024
@ilonatommy
ilonatommy
commented
Jun 25, 2024

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0-staging
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9662243985
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:
$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix.
Using index info to reconstruct a base tree...
M	src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
A	src/mono/browser/runtime/hybrid-globalization/calendar.ts
A	src/mono/browser/runtime/hybrid-globalization/culture-info.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
Auto-merging src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (modify/delete): src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs deleted in HEAD and modified in Fix.. Version Fix. of src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs left in tree.
Auto-merging src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
CONFLICT (content): Merge conflict in src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix.
Error: The process '/usr/bin/git' failed with exit code 128
Please backport manually!
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy an error occurred while backporting to release/8.0-staging, please check the run log for details!
Error: git am failed, most likely due to a merge conflict.
@ilonatommyilonatommy
mentioned this pull request
Jun 25, 2024
 Merged

ilonatommy added a commit
that referenced
this pull request
Jun 26, 2024
@ilonatommy
Backport: Fix incorrect time patterns for some cultures on browser in…
5753765
… `HybridGlobalization` (#103964)
Backport of #103804.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 26, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@matouskozakmatouskozakAwaiting requested review from matouskozak
@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner

1 more reviewer

@mkhamoyanmkhamoyanmkhamoyan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ilonatommyilonatommy

Labels

arch-wasmWebAssembly architecturearea-System.Globalization

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Incorrect time patterns for some cultures on browser in HybridGlobalization

3 participants

@ilonatommy@pavelsavara@mkhamoyan

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix incorrect time patterns for some cultures on browser in HybridGlobalization - #103804
Merged
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Jun 24, 2024
Merged
Fix incorrect time patterns for some cultures on browser in HybridGlobalization#103804
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Conversation
@ilonatommy
@ilonatommyilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
Fixes#103745.
This PR makes improvements to values returned in date/time format APIs. It does not make all the locales' values match ICU's values due to Web API responses differences, but corrects majority of them.

Originally the ICU's expected behavior was saved in a form of a comment next to the specific test case. If this PR's fix made HybridGlobalization response match the comment , the comment got removed. If there are still discrepancies (e.g. node behaves different than chrome) the ICU value is left in the comment.
@ilonatommyilonatommy
self-assigned this
Jun 21, 2024
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jun 21, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @dotnet/area-system-globalization

See info in area-owners.md if you want to be subscribed.
@ilonatommy
ilonatommy
commented
Jun 21, 2024•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
cc @PopSlime, HybridGlobalization relies on JS methods and there's no method in Intl that would return a "format" of date. We are working it around by taking a specific date, formatting it using among others toLocaleString and then we parse the result to determine the generic format of date / time for the given locale. I added a function that tries to find the parts of the already formatted date pattern that are "fixed" and these that are "variant" and put the fixed ones in quotes. If you have any suggestions how to improve this process, I would be glad to talk about it.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
pavelsavara
pavelsavara
reviewed
Jun 21, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

@pavelsavara
pavelsavara
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
are we going to backport it to Net8 ?
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
are we going to backport it to Net8 ?
I would like to.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed


Closed


Closed

@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed

mkhamoyan
mkhamoyan
reviewed
Jun 24, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@mkhamoyan
mkhamoyan
commented
Jun 24, 2024

Copy link


Copy Markdown

Contributor
Can we also enable test cases from #103681 ?
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 24, 2024

Closed


Closed


Closed


Open


Open

@ilonatommy
ilonatommy
merged commit 8169bde
into
dotnet:mainJun 24, 2024
@ilonatommy
ilonatommy
commented
Jun 25, 2024

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0-staging
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9662243985
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:
$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix.
Using index info to reconstruct a base tree...
M	src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
A	src/mono/browser/runtime/hybrid-globalization/calendar.ts
A	src/mono/browser/runtime/hybrid-globalization/culture-info.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
Auto-merging src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (modify/delete): src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs deleted in HEAD and modified in Fix.. Version Fix. of src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs left in tree.
Auto-merging src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
CONFLICT (content): Merge conflict in src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix.
Error: The process '/usr/bin/git' failed with exit code 128
Please backport manually!
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy an error occurred while backporting to release/8.0-staging, please check the run log for details!
Error: git am failed, most likely due to a merge conflict.
@ilonatommyilonatommy
mentioned this pull request
Jun 25, 2024
 Merged

ilonatommy added a commit
that referenced
this pull request
Jun 26, 2024
@ilonatommy
Backport: Fix incorrect time patterns for some cultures on browser in…
5753765
… `HybridGlobalization` (#103964)
Backport of #103804.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 26, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@matouskozakmatouskozakAwaiting requested review from matouskozak
@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner

1 more reviewer

@mkhamoyanmkhamoyanmkhamoyan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ilonatommyilonatommy

Labels

arch-wasmWebAssembly architecturearea-System.Globalization

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Incorrect time patterns for some cultures on browser in HybridGlobalization

3 participants

@ilonatommy@pavelsavara@mkhamoyan

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Fix incorrect time patterns for some cultures on browser in HybridGlobalization - #103804
Merged
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Jun 24, 2024
Merged
Fix incorrect time patterns for some cultures on browser in HybridGlobalization#103804
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Conversation
@ilonatommy
@ilonatommyilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
Fixes#103745.
This PR makes improvements to values returned in date/time format APIs. It does not make all the locales' values match ICU's values due to Web API responses differences, but corrects majority of them.

Originally the ICU's expected behavior was saved in a form of a comment next to the specific test case. If this PR's fix made HybridGlobalization response match the comment , the comment got removed. If there are still discrepancies (e.g. node behaves different than chrome) the ICU value is left in the comment.
@ilonatommyilonatommy
self-assigned this
Jun 21, 2024
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jun 21, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @dotnet/area-system-globalization

See info in area-owners.md if you want to be subscribed.
@ilonatommy
ilonatommy
commented
Jun 21, 2024•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
cc @PopSlime, HybridGlobalization relies on JS methods and there's no method in Intl that would return a "format" of date. We are working it around by taking a specific date, formatting it using among others toLocaleString and then we parse the result to determine the generic format of date / time for the given locale. I added a function that tries to find the parts of the already formatted date pattern that are "fixed" and these that are "variant" and put the fixed ones in quotes. If you have any suggestions how to improve this process, I would be glad to talk about it.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
pavelsavara
pavelsavara
reviewed
Jun 21, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

@pavelsavara
pavelsavara
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
are we going to backport it to Net8 ?
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
are we going to backport it to Net8 ?
I would like to.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed


Closed


Closed

@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed

mkhamoyan
mkhamoyan
reviewed
Jun 24, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@mkhamoyan
mkhamoyan
commented
Jun 24, 2024

Copy link


Copy Markdown

Contributor
Can we also enable test cases from #103681 ?
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 24, 2024

Closed


Closed


Closed


Open


Open

@ilonatommy
ilonatommy
merged commit 8169bde
into
dotnet:mainJun 24, 2024
@ilonatommy
ilonatommy
commented
Jun 25, 2024

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0-staging
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9662243985
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:
$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix.
Using index info to reconstruct a base tree...
M	src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
A	src/mono/browser/runtime/hybrid-globalization/calendar.ts
A	src/mono/browser/runtime/hybrid-globalization/culture-info.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
Auto-merging src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (modify/delete): src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs deleted in HEAD and modified in Fix.. Version Fix. of src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs left in tree.
Auto-merging src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
CONFLICT (content): Merge conflict in src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix.
Error: The process '/usr/bin/git' failed with exit code 128
Please backport manually!
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy an error occurred while backporting to release/8.0-staging, please check the run log for details!
Error: git am failed, most likely due to a merge conflict.
@ilonatommyilonatommy
mentioned this pull request
Jun 25, 2024
 Merged

ilonatommy added a commit
that referenced
this pull request
Jun 26, 2024
@ilonatommy
Backport: Fix incorrect time patterns for some cultures on browser in…
5753765
… `HybridGlobalization` (#103964)
Backport of #103804.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 26, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@matouskozakmatouskozakAwaiting requested review from matouskozak
@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner

1 more reviewer

@mkhamoyanmkhamoyanmkhamoyan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ilonatommyilonatommy

Labels

arch-wasmWebAssembly architecturearea-System.Globalization

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Incorrect time patterns for some cultures on browser in HybridGlobalization

3 participants

@ilonatommy@pavelsavara@mkhamoyan

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix incorrect time patterns for some cultures on browser in HybridGlobalization - #103804
Merged
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Jun 24, 2024
Merged
Fix incorrect time patterns for some cultures on browser in HybridGlobalization#103804
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Conversation
@ilonatommy
@ilonatommyilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
Fixes#103745.
This PR makes improvements to values returned in date/time format APIs. It does not make all the locales' values match ICU's values due to Web API responses differences, but corrects majority of them.

Originally the ICU's expected behavior was saved in a form of a comment next to the specific test case. If this PR's fix made HybridGlobalization response match the comment , the comment got removed. If there are still discrepancies (e.g. node behaves different than chrome) the ICU value is left in the comment.
@ilonatommyilonatommy
self-assigned this
Jun 21, 2024
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jun 21, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @dotnet/area-system-globalization

See info in area-owners.md if you want to be subscribed.
@ilonatommy
ilonatommy
commented
Jun 21, 2024•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
cc @PopSlime, HybridGlobalization relies on JS methods and there's no method in Intl that would return a "format" of date. We are working it around by taking a specific date, formatting it using among others toLocaleString and then we parse the result to determine the generic format of date / time for the given locale. I added a function that tries to find the parts of the already formatted date pattern that are "fixed" and these that are "variant" and put the fixed ones in quotes. If you have any suggestions how to improve this process, I would be glad to talk about it.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
pavelsavara
pavelsavara
reviewed
Jun 21, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

@pavelsavara
pavelsavara
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
are we going to backport it to Net8 ?
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
are we going to backport it to Net8 ?
I would like to.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed


Closed


Closed

@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed

mkhamoyan
mkhamoyan
reviewed
Jun 24, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@mkhamoyan
mkhamoyan
commented
Jun 24, 2024

Copy link


Copy Markdown

Contributor
Can we also enable test cases from #103681 ?
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 24, 2024

Closed


Closed


Closed


Open


Open

@ilonatommy
ilonatommy
merged commit 8169bde
into
dotnet:mainJun 24, 2024
@ilonatommy
ilonatommy
commented
Jun 25, 2024

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0-staging
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9662243985
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:
$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix.
Using index info to reconstruct a base tree...
M	src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
A	src/mono/browser/runtime/hybrid-globalization/calendar.ts
A	src/mono/browser/runtime/hybrid-globalization/culture-info.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
Auto-merging src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (modify/delete): src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs deleted in HEAD and modified in Fix.. Version Fix. of src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs left in tree.
Auto-merging src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
CONFLICT (content): Merge conflict in src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix.
Error: The process '/usr/bin/git' failed with exit code 128
Please backport manually!
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy an error occurred while backporting to release/8.0-staging, please check the run log for details!
Error: git am failed, most likely due to a merge conflict.
@ilonatommyilonatommy
mentioned this pull request
Jun 25, 2024
 Merged

ilonatommy added a commit
that referenced
this pull request
Jun 26, 2024
@ilonatommy
Backport: Fix incorrect time patterns for some cultures on browser in…
5753765
… `HybridGlobalization` (#103964)
Backport of #103804.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 26, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@matouskozakmatouskozakAwaiting requested review from matouskozak
@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner

1 more reviewer

@mkhamoyanmkhamoyanmkhamoyan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ilonatommyilonatommy

Labels

arch-wasmWebAssembly architecturearea-System.Globalization

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Incorrect time patterns for some cultures on browser in HybridGlobalization

3 participants

@ilonatommy@pavelsavara@mkhamoyan

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Fix incorrect time patterns for some cultures on browser in HybridGlobalization - #103804
Merged
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Jun 24, 2024
Merged
Fix incorrect time patterns for some cultures on browser in HybridGlobalization#103804
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Conversation
@ilonatommy
@ilonatommyilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
Fixes#103745.
This PR makes improvements to values returned in date/time format APIs. It does not make all the locales' values match ICU's values due to Web API responses differences, but corrects majority of them.

Originally the ICU's expected behavior was saved in a form of a comment next to the specific test case. If this PR's fix made HybridGlobalization response match the comment , the comment got removed. If there are still discrepancies (e.g. node behaves different than chrome) the ICU value is left in the comment.
@ilonatommyilonatommy
self-assigned this
Jun 21, 2024
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jun 21, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @dotnet/area-system-globalization

See info in area-owners.md if you want to be subscribed.
@ilonatommy
ilonatommy
commented
Jun 21, 2024•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
cc @PopSlime, HybridGlobalization relies on JS methods and there's no method in Intl that would return a "format" of date. We are working it around by taking a specific date, formatting it using among others toLocaleString and then we parse the result to determine the generic format of date / time for the given locale. I added a function that tries to find the parts of the already formatted date pattern that are "fixed" and these that are "variant" and put the fixed ones in quotes. If you have any suggestions how to improve this process, I would be glad to talk about it.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
pavelsavara
pavelsavara
reviewed
Jun 21, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

@pavelsavara
pavelsavara
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
are we going to backport it to Net8 ?
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
are we going to backport it to Net8 ?
I would like to.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed


Closed


Closed

@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed

mkhamoyan
mkhamoyan
reviewed
Jun 24, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@mkhamoyan
mkhamoyan
commented
Jun 24, 2024

Copy link


Copy Markdown

Contributor
Can we also enable test cases from #103681 ?
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 24, 2024

Closed


Closed


Closed


Open


Open

@ilonatommy
ilonatommy
merged commit 8169bde
into
dotnet:mainJun 24, 2024
@ilonatommy
ilonatommy
commented
Jun 25, 2024

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0-staging
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9662243985
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:
$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix.
Using index info to reconstruct a base tree...
M	src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
A	src/mono/browser/runtime/hybrid-globalization/calendar.ts
A	src/mono/browser/runtime/hybrid-globalization/culture-info.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
Auto-merging src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (modify/delete): src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs deleted in HEAD and modified in Fix.. Version Fix. of src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs left in tree.
Auto-merging src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
CONFLICT (content): Merge conflict in src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix.
Error: The process '/usr/bin/git' failed with exit code 128
Please backport manually!
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy an error occurred while backporting to release/8.0-staging, please check the run log for details!
Error: git am failed, most likely due to a merge conflict.
@ilonatommyilonatommy
mentioned this pull request
Jun 25, 2024
 Merged

ilonatommy added a commit
that referenced
this pull request
Jun 26, 2024
@ilonatommy
Backport: Fix incorrect time patterns for some cultures on browser in…
5753765
… `HybridGlobalization` (#103964)
Backport of #103804.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 26, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@matouskozakmatouskozakAwaiting requested review from matouskozak
@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner

1 more reviewer

@mkhamoyanmkhamoyanmkhamoyan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ilonatommyilonatommy

Labels

arch-wasmWebAssembly architecturearea-System.Globalization

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Incorrect time patterns for some cultures on browser in HybridGlobalization

3 participants

@ilonatommy@pavelsavara@mkhamoyan

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Fix incorrect time patterns for some cultures on browser in HybridGlobalization - #103804
Merged
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Jun 24, 2024
Merged
Fix incorrect time patterns for some cultures on browser in HybridGlobalization#103804
ilonatommy merged 6 commits into
dotnet:mainfrom 
ilonatommy:fix-103745
Conversation
@ilonatommy
@ilonatommyilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
Fixes#103745.
This PR makes improvements to values returned in date/time format APIs. It does not make all the locales' values match ICU's values due to Web API responses differences, but corrects majority of them.

Originally the ICU's expected behavior was saved in a form of a comment next to the specific test case. If this PR's fix made HybridGlobalization response match the comment , the comment got removed. If there are still discrepancies (e.g. node behaves different than chrome) the ICU value is left in the comment.
@ilonatommyilonatommy
self-assigned this
Jun 21, 2024
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jun 21, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @dotnet/area-system-globalization

See info in area-owners.md if you want to be subscribed.
@ilonatommy
ilonatommy
commented
Jun 21, 2024•

edited

Loading

Copy link


Copy Markdown

MemberAuthor
cc @PopSlime, HybridGlobalization relies on JS methods and there's no method in Intl that would return a "format" of date. We are working it around by taking a specific date, formatting it using among others toLocaleString and then we parse the result to determine the generic format of date / time for the given locale. I added a function that tries to find the parts of the already formatted date pattern that are "fixed" and these that are "variant" and put the fixed ones in quotes. If you have any suggestions how to improve this process, I would be glad to talk about it.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
pavelsavara
pavelsavara
reviewed
Jun 21, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/calendar.ts
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
Outdated

@pavelsavara
pavelsavara
commented
Jun 21, 2024

Copy link


Copy Markdown

Member
are we going to backport it to Net8 ?
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
are we going to backport it to Net8 ?
I would like to.
@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed


Closed


Closed

@ilonatommy
ilonatommy
commented
Jun 21, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 21, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 21, 2024

Closed


Open


Closed

mkhamoyan
mkhamoyan
reviewed
Jun 24, 2024
View reviewed changes
Comment threadsrc/mono/browser/runtime/hybrid-globalization/culture-info.ts
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).
@mkhamoyan
mkhamoyan
commented
Jun 24, 2024

Copy link


Copy Markdown

Contributor
Can we also enable test cases from #103681 ?
@ilonatommy
ilonatommy
commented
Jun 24, 2024

Copy link


Copy Markdown

MemberAuthor
/azp run runtime-wasm
@azure-pipelines
azure-pipelinesBot
commented
Jun 24, 2024

Copy link


Copy Markdown


Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jun 24, 2024

Closed


Closed


Closed


Open


Open

@ilonatommy
ilonatommy
merged commit 8169bde
into
dotnet:mainJun 24, 2024
@ilonatommy
ilonatommy
commented
Jun 25, 2024

Copy link


Copy Markdown

MemberAuthor
/backport to release/8.0-staging
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/9662243985
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy backporting to release/8.0-staging failed, the patch most likely resulted in conflicts:
$ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix.
Using index info to reconstruct a base tree...
M	src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
A	src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
A	src/mono/browser/runtime/hybrid-globalization/calendar.ts
A	src/mono/browser/runtime/hybrid-globalization/culture-info.ts
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/culture-info.ts
Auto-merging src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (content): Merge conflict in src/mono/wasm/runtime/hybrid-globalization/calendar.ts
CONFLICT (modify/delete): src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs deleted in HEAD and modified in Fix.. Version Fix. of src/libraries/System.Runtime/tests/System.Globalization.Tests/DateTimeFormatInfo/DateTimeFormatInfoLongDatePattern.cs left in tree.
Auto-merging src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
CONFLICT (content): Merge conflict in src/libraries/System.Private.CoreLib/src/System/Globalization/CultureData.Browser.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoShortTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoLongTimePattern.cs
Auto-merging src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
CONFLICT (content): Merge conflict in src/libraries/System.Globalization/tests/DateTimeFormatInfo/DateTimeFormatInfoFullDateTimePattern.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config advice.mergeConflict false"
Patch failed at 0001 Fix.
Error: The process '/usr/bin/git' failed with exit code 128
Please backport manually!
@github-actions
github-actionsBot
commented
Jun 25, 2024

Copy link


Copy Markdown

Contributor
@ilonatommy an error occurred while backporting to release/8.0-staging, please check the run log for details!
Error: git am failed, most likely due to a merge conflict.
@ilonatommyilonatommy
mentioned this pull request
Jun 25, 2024
 Merged

ilonatommy added a commit
that referenced
this pull request
Jun 26, 2024
@ilonatommy
Backport: Fix incorrect time patterns for some cultures on browser in…
5753765
… `HybridGlobalization` (#103964)
Backport of #103804.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Jul 26, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@matouskozakmatouskozakAwaiting requested review from matouskozak
@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner

1 more reviewer

@mkhamoyanmkhamoyanmkhamoyan approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@ilonatommyilonatommy

Labels

arch-wasmWebAssembly architecturearea-System.Globalization

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

Incorrect time patterns for some cultures on browser in HybridGlobalization

3 participants

@ilonatommy@pavelsavara@mkhamoyan