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

[Form] Fix wrong DateTime on outdated ICU library #31865

Merged
merged 1 commit into from Jun 6, 2019

Conversation

Projects
None yet
6 participants
@aweelex
Copy link
Contributor

commented Jun 5, 2019

Q A
Branch? 3.4
Bug fix? Yes
New feature? No
BC breaks? No
Deprecations? No
Tests pass? Yes
Fixed tickets ---
License MIT

There is a problem, when server uses outdated version of ICU (php-intl).

It throws no exeption or debug message on unexisting Timezone. So sometimes you can get wrong DateTime in Forms, because Intl uses 'Etc/Unknown' (UTC+0) instead correct Timezone. And it happens very unobvious.

I added \IntlExeption for that cases.

@nicolas-grekas

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

/cc @ro0NL

@xabbuh

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

@aweelex Does it work if we pass the timezone as a DateTimeZone() instance to the IntlDateFormatter constructor instead?

@xabbuh xabbuh added Bug Intl labels Jun 6, 2019

@aweelex

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2019

@xabbuh oh, yes. And it seems to be the best way to solve this problem. Because we already have exeption on wrong DateTimeZone in DataTransformerInterface.

@aweelex aweelex changed the title [Form] Throw exception on 'Etc/Unknown' in DateTimeToLocalizedString [Form] Wrong DateTime on outdated Intl Timezone Jun 6, 2019

@xabbuh

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

Thank you for checking. Can you rebase your changes on the 3.4 branch? It's affected too, right?

@xabbuh xabbuh added this to the 3.4 milestone Jun 6, 2019

@aweelex aweelex changed the base branch from master to 3.4 Jun 6, 2019

@aweelex aweelex force-pushed the aweelex:master branch from db372de to a6025ab Jun 6, 2019

@aweelex

This comment has been minimized.

Copy link
Contributor Author

commented Jun 6, 2019

Ok, it seems that changes should go to 3.4 and 4.4. I rebased commits to 3.4. Is everything alright here? @nicolas-grekas?

@xabbuh

xabbuh approved these changes Jun 6, 2019

@aweelex aweelex changed the title [Form] Wrong DateTime on outdated Intl Timezone [Form] Fix Wrong DateTime on outdated ICU library Jun 6, 2019

@aweelex aweelex changed the title [Form] Fix Wrong DateTime on outdated ICU library [Form] Fix wrong DateTime on outdated ICU library Jun 6, 2019

@ro0NL

ro0NL approved these changes Jun 6, 2019

@fabpot

fabpot approved these changes Jun 6, 2019

@fabpot

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

Thank you @aweelex.

@fabpot fabpot merged commit a6025ab into symfony:3.4 Jun 6, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Jun 6, 2019

bug #31865 [Form] Fix wrong DateTime on outdated ICU library (aweelex)
This PR was merged into the 3.4 branch.

Discussion
----------

[Form] Fix wrong DateTime on outdated ICU library

| Q | A |
| --- | --- |
| Branch? | 3.4 |
| Bug fix? | Yes |
| New feature? | No |
| BC breaks? | No |
| Deprecations? | No |
| Tests pass? | Yes |
| Fixed tickets | --- |
| License | MIT |

There is a problem, when server uses outdated version of ICU (php-intl).

It throws no exeption or debug message on unexisting Timezone. So sometimes you can get wrong DateTime in Forms, because Intl uses 'Etc/Unknown' (UTC+0) instead correct Timezone. And it happens very unobvious.

I added `\IntlExeption` for that cases.

Commits
-------

a6025ab Change IntlTimeZone to DateTimeZone

@fabpot fabpot referenced this pull request Jun 6, 2019

Merged

Release v4.3.1 #31901

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.