-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] Intl and TimezoneType are not interchangeable #22302
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
Comments
What's the action plan here? Using intl for datetime doesn't sound right since that's the not the database used to create datetimezone objects used in datetime objects. Is it issue with datetimezone and intl having different sources for timezone data or they represent 2 different things? |
I've invesigated the timezone info from Intl a bit, see also
this could be leveraged by a new TimezoneBundle in the Intl component, solving #26851 #22262, #25165 while at it. From what ive seen so far, I played around with it already (see https://github.com/ro0NL/symfony/commit/2ef271bf4096dc48031888da23f424c43673364a) but ran into some compile issues :( https://hub.docker.com/r/jakzal/php-intl/ is missing the git binary, so doesnt work well out-of-the-box, and locally im stuck on icu 55. Would be nice if @jakzal could fix it ;) |
For now you can just create your own image based on the above and add git into it (I added jakzal/docker-symfony-intl#6 for myself). |
@rvanlaak the issue here with the difference in timezones is related to BC. Intl is more up to date then PHP, which includes still some BC timezones. for
Others, e.g. |
Great work on investigating the root cause. 👏 I think solving this issue is a matter of choosing:
We use our custom form type that intersects both sets since we found out about this bug, so we would prefer 1. |
the relevant timezone ids can be used both with php datetimezone and intl, there's no difference here. we should only allow for timezone ids from intl that are supported by datetimezone, in a dedicated timezonebundle (symfony/intl) so the list is fixed and based on latest ICU. the unsupported ones are not really relevant anyway :) https://3v4l.org/1Xit4 |
alternatively the timezonebundle could provide both lists, for the form type: we've recently added the so we could decide which list to use based on |
@rvanlaak im not sure about excluding timezones not supported by So the topic is if we should exclude BC zones in the TimezoneType list.... it didnt raise any issues so far and it's basically is a BC break :) so im skeptical. Looking at the timezone widget from Google Calendar, even there BC zones seem to be included (e.g. I've also opened #28836 to propose switching to symfony/intl instead. That would still includes BC zones though, but we could check for that from there on (if needed). So im 👎 for the suggested change here as is, but after #28836 we have a much better base to check it and e.g. support For now, i think your best of to set the available zones manually. |
This PR was squashed before being merged into the 4.3-dev branch (closes #31195). Discussion ---------- [Form] Add intltimezone input to TimezoneType | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #22302 (ref #28836) | License | MIT | Doc PR | symfony/symfony-docs#11463 cc @rvanlaak Commits ------- e169dfb [Form] Add intltimezone input to TimezoneType
Uh oh!
There was an error while loading. Please reload this page.
We make use of the
intl
extension to nicely renderDateTime
objects according to the activelocale
andtimezone
. The problem with this combination is that it are two different kind of PHP modules, and their datasets do not completely match.When the diff gets calculate over the
intl
anddatetime
timezones, you expected the result to be empty, but it is not. The outcome looks like the following:This can be solved by using the intersect of both datasets instead:
Would it be wise to let the
TimezoneType
choice populater take this into account in case theintl
extension is available / activated? Twig does useintl
by default if I'm correct?The text was updated successfully, but these errors were encountered: