ICU-13845 Windows timezone detection: When DST is off, map to an Etc/GMT+-hh zone

[jefgen]

@jungshik: This PR is a continuation of PR #1297.
I created a new PR for this in order to get the change into ICU 68 by the code freeze. :)

Description from the previous PR and the bug:
The Windows 'Date and Time' Control Panel has a setting for "Automatically adjust clock for DST".
When this setting is manually unchecked, the DST setting is considered OFF, and the clock does not adjust for DST.
However, ICU currently doesn't detect this setting and will continue to use the time zone ID which has DST enabled.

In order to handle this situation, this change detects when the DST setting is OFF and uses the non-DST offset to map to an Etc/GMT+-hh zone.

This change also refactors the code in uprv_detectWindowsTimeZone somewhat as well.

Note: Much of this change was originally authored by @jungshik -- so marking this commit as:
Co-authored-by: Jungshik Shin.

Checklist

• Issue filed: https://unicode-org.atlassian.net/browse/ICU-13845
• Updated PR title and link in previous line to include Issue number
• Issue accepted
• Tests included
• Documentation is changed or added

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/common/wintz.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[jefgen]

Here's the output from testing locally (using the icuinfo.exe program) and toggling the "Automatically adjust for DST" setting.

With the system set to Pacific Standard Time and the country/region (GeoID) set to CA:

With DST setting enabled:

 <icuSystemParams type="icu4c">
    <param name="copyright"> Copyright (C) 2016 and later: Unicode, Inc. and others. License & terms of use: http://www.unicode.org/copyright.html </param>
    <param name="product">icu4c</param>
    <param name="product.full">International Components for Unicode for C/C++</param>
    <param name="version">68.1</param>
    <param name="version.unicode">13.0</param>
    <param name="platform.number">1000</param>
    <param name="platform.type">Windows</param>
    <param name="locale.default">en_CA</param>
    <param name="locale.default.bcp47">en-CA</param>
    <param name="converter.default">ibm-5348_P100-1997</param>
    <param name="icudata.name">icudt68l</param>
    <param name="icudata.path"></param>
    <param name="cldr.version">38.0</param>
    <param name="tz.version">2020a</param>
    <param name="tz.default">America/Vancouver</param>
    <param name="cpu.bits">64</param>
    <param name="cpu.big_endian">0</param>
    <param name="os.wchar_width">2</param>
    <param name="os.charset_family">0</param>
    <param name="uconfig.internal_digitlist">1</param>
    <param name="uconfig.have_parseallinput">1</param>
 </icuSystemParams>

With DST setting disabled (no DST):

 <icuSystemParams type="icu4c">
    <param name="copyright"> Copyright (C) 2016 and later: Unicode, Inc. and others. License & terms of use: http://www.unicode.org/copyright.html </param>
    <param name="product">icu4c</param>
    <param name="product.full">International Components for Unicode for C/C++</param>
    <param name="version">68.1</param>
    <param name="version.unicode">13.0</param>
    <param name="platform.number">1000</param>
    <param name="platform.type">Windows</param>
    <param name="locale.default">en_CA</param>
    <param name="locale.default.bcp47">en-CA</param>
    <param name="converter.default">ibm-5348_P100-1997</param>
    <param name="icudata.name">icudt68l</param>
    <param name="icudata.path"></param>
    <param name="cldr.version">38.0</param>
    <param name="tz.version">2020a</param>
    <param name="tz.default">Etc/GMT-8</param>
    <param name="cpu.bits">64</param>
    <param name="cpu.big_endian">0</param>
    <param name="os.wchar_width">2</param>
    <param name="os.charset_family">0</param>
    <param name="uconfig.internal_digitlist">1</param>
    <param name="uconfig.have_parseallinput">1</param>
 </icuSystemParams>

The diff between the above outputs:

<param name="tz.default">America/Vancouver</param>
<param name="tz.default">Etc/GMT-8</param>

So when the DST setting is turned off (or unchecked) then we get Etc/GMT-8. :)

[jira-pull-request-webhook]

Notice: the branch changed across the force-push!

• icu4c/source/common/wintz.cpp is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

[jefgen]

Testing with the time zone set to one that has no offset/bias from GMT/UTC:
(This is the output from testing locally using the icuinfo.exe program).

 <icuSystemParams type="icu4c">
    <param name="copyright"> Copyright (C) 2016 and later: Unicode, Inc. and others. License & terms of use: http://www.unicode.org/copyright.html </param>
    <param name="product">icu4c</param>
    <param name="product.full">International Components for Unicode for C/C++</param>
    <param name="version">68.1</param>
    <param name="version.unicode">13.0</param>
    <param name="platform.number">1000</param>
    <param name="platform.type">Windows</param>
    <param name="locale.default">en_CA</param>
    <param name="locale.default.bcp47">en-CA</param>
    <param name="converter.default">ibm-5348_P100-1997</param>
    <param name="icudata.name">icudt68l</param>
    <param name="icudata.path"></param>
    <param name="cldr.version">38.0</param>
    <param name="tz.version">2020a</param>
    <param name="tz.default">Etc/UTC</param>
    <param name="cpu.bits">64</param>
    <param name="cpu.big_endian">0</param>
    <param name="os.wchar_width">2</param>
    <param name="os.charset_family">0</param>
    <param name="uconfig.internal_digitlist">1</param>
    <param name="uconfig.have_parseallinput">1</param>
 </icuSystemParams>

It now uses the canonical zone Etc/UTC. :)

[jefgen]

PTAL. (I'll need another review due to the force-push).

[FrankYFTang]

The landing cause this issue https://unicode-org.atlassian.net/browse/ICU-21336

break chrome

[jefgen]

The landing cause this issue https://unicode-org.atlassian.net/browse/ICU-21336

Sorry for causing this issue, and thanks for catching this. I missed this when reviewing the changes. (I wonder why the warnings-as-error build bots didn’t catch this though…?)

The variable utcOffsetMins is type LONG (typedef for long), so the calculation of -utcOffsetMins/60 is also a long as well.

I think the fix would be to either use %+ld in the format string, or to use a static_cast<int>(-utcOffsetMins/60) to make it an int. (The value should never require the full range of the LONG, so both should be fine.)

[FrankYFTang]

We got Window only timezone bug and I am suspect that is rooted from here. We need someway to unit test this change
see https://bugs.chromium.org/p/chromium/issues/detail?id=1147305

[jefgen]

We got Window only timezone bug and I am suspect that is rooted from here. We need someway to unit test this change
see https://bugs.chromium.org/p/chromium/issues/detail?id=1147305

Thanks Frank, I was able to reproduce the issue and I've created a PR #1465 with a fix.