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

Cherry-pick: Fix Windows TZ offset when Automatic DST setting is OFF #65

Merged
merged 1 commit into from
Jan 29, 2021

Conversation

jefgen
Copy link
Member

@jefgen jefgen commented Jan 28, 2021

This change cherry-picks the upstream fix for the issue of the Windows Time Zone offset being wrong when Automatic DST setting is OFF in the Control Panel.

Upstream ticket:
ICU-21465 Windows Time Zone offset is wrong when Automatic DST setting is OFF
https://unicode-org.atlassian.net/browse/ICU-21465

Upstream PR into master is here: unicode-org/icu#1539
Upstream PR into maint/maint-68 is here: unicode-org/icu#1543

Summary

PR Checklist

  • I have verified that my change is specific to this fork and cannot be made upstream.
  • I am making a maintenance related change.
  • I am making a change that is related to usage internal to Microsoft.
  • I am making a change that is related to the Windows OS build of ICU.
  • CLA signed. If not, please see here to sign the CLA.

Detailed Description

From the upstream PR description:

This fixes a bug in the changes for ICU-13845, for handling when the "Automatic DST" setting is OFF in the Control Panel.

It turns out that the offset value for the calculated time zone when the "Automatic DST" setting is turn off is wrong. The sign for the time zone offset is flipped. (It's positive when it should be negative, and vice-versa).

I think this was perhaps due to confusion about the value for the UTC offset, how POSIX style offsets are handled, and what is reported by the Win32 API GetDynamicTimeZoneInformation. It seems that this issue was actually present in the original PR for ICU-13845 and it was missed in the reviews on the other PR for the ticket.

I also missed this when I was manually testing out the changes, as I was expecting the wrong values for the offset to be reported. (For example, Etc/GMT-8 instead of Etc/GMT+8).

Note: The actual change here is only one character. The rest are comments that I added in order to help explain things for others in the future.

Testing:

I created a document that has different manual test cases that exercise various scenarios for most of the code paths for the Windows TZ mapping in ICU. (I also added notes on what the various test cases are exercising in the code.)
Link: https://docs.google.com/document/d/17Y_Ns4EBV8wnHioJNFc-dJCRRpJ3hpJZDYmTq6ekIsM
(The test case 7 in the document reproduces the issue that this PR fixes).

This change cherry-picks the upstream fix for the issue of the Windows
Time Zone offset being wrong when Automatic DST setting is OFF in the Control Panel.

ICU-21465 Windows Time Zone offset is wrong when Automatic DST setting is OFF
https://unicode-org.atlassian.net/browse/ICU-21465

Cherry-picked from upstream commit: c6934ff7476200fc22abdf4300c94e2a3460c793

The upstream PR into master is here:
unicode-org/icu#1465

The upstream PR into maint/maint-68 is here:
unicode-org/icu#1543
@jefgen jefgen requested review from axelandrejs, daniel-ju and a team January 28, 2021 02:36
@jefgen jefgen merged commit 2f7df17 into master Jan 29, 2021
@jefgen jefgen deleted the user/jefgen/cherry-pick-WinTZ-fix branch January 29, 2021 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants