Skip to content

Conversation

jonsimantov
Copy link
Contributor

@jonsimantov jonsimantov commented Jun 29, 2023

Description

Provide details of the change, and generalize the change in the PR title above.

This fixes two issues: UTF-8 characters that can't fit into a single UTF-16 character, and obtaining the Windows time zone name.

Turns out we do need to use SetThreadUILanguage and GetTimeZoneInformation to get the time zone name in English, which can then be converted to IANA time zone format via the conversion library added earlier.


Testing

Describe how you've tested these changes. Link any manually triggered Integration tests or CPP binary SDK Packaging Github Action workflows, if applicable.

Integration test in this PR, plus unit test. Also tested by user reporting the bug in #1367.


Type of Change

Place an x the applicable box:

  • Bug fix. Add the issue # below if applicable.
  • New feature. A non-breaking change which adds functionality.
  • Other, such as a build process or documentation change.

Notes

  • Bug fixes and feature changes require an update to the Release Notes section of release_build_files/readme.md.
  • Read the contribution guidelines CONTRIBUTING.md.
  • Changes to the public API require an internal API review. If you'd like to help us make Firebase APIs better, please propose your change in a feature request so that we can discuss it together.

@github-actions github-actions bot added the tests: in-progress This PR's integration tests are in progress. label Jun 29, 2023
@github-actions
Copy link

github-actions bot commented Jun 29, 2023

Integration test with FLAKINESS (succeeded after retry)

Requested by @jonsimantov on commit 4fd4ce9
Last updated: Thu Jul 6 16:30 PDT 2023
View integration test log & download artifacts

Failures Configs
gma [TEST] [FLAKINESS] [Android] [1/3 os: windows] [android_target]
(1 failed tests)  CRASH/TIMEOUT

Add flaky tests to go/fpl-cpp-flake-tracker

@github-actions github-actions bot added the tests: succeeded This PR's integration tests succeeded. label Jun 29, 2023
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jun 29, 2023
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. and removed tests: succeeded This PR's integration tests succeeded. labels Jul 5, 2023
@jonsimantov jonsimantov requested a review from a-maurice July 5, 2023 23:15
@github-actions github-actions bot added the tests: succeeded This PR's integration tests succeeded. label Jul 5, 2023
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 5, 2023
@jonsimantov jonsimantov merged commit 4fd4ce9 into main Jul 6, 2023
@github-actions github-actions bot added tests: in-progress This PR's integration tests are in progress. tests: succeeded This PR's integration tests succeeded. and removed tests: succeeded This PR's integration tests succeeded. labels Jul 6, 2023
@firebase-workflow-trigger firebase-workflow-trigger bot removed the tests: in-progress This PR's integration tests are in progress. label Jul 6, 2023
tom-andersen pushed a commit that referenced this pull request Jul 17, 2023
* Fix for Unicode characters above U+FFFF.

* Add error handling to time zone conversion.

* Fix scoping error.

* Convert Windows TZ environment variable from CP-1252 to Unicode.

* Don't use a variable length array.

* Fix compiler error.

* Modify readme.

* Fix array lookup.

* Update readme.

* Also fix the fallback daylight saving time string encoding.

* Rephrase readme.

* Add unit test for conversions on Windows.

* Add test for CP1252 conversion and fix conversion function that we have a test for it.

* Remove duplicate code.

* Add missing platform header.

* Add additional characters to locale test.

* Let's go wild, even more characters. This should cover most of what we
need.

* Format code.

* Modified locale test to work with Windows wchar_t.

* Format code.

* Set thread language to English and use GetTimeZoneInformation.

* Format code.

* Add a comment about why we don't use daylight savings time in one spot.
@firebase firebase locked and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tests: succeeded This PR's integration tests succeeded.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants