[Form] Fix DateTimeType html5 input format. #27254
Conversation
|
I don't see how this test could affect the failing travis php 5.4 test. Any hints? |
| /** | ||
| * Transforms a formatted string following RFC 3339 into a normalized date. | ||
| * | ||
| * @param string $rfc3339 Formatted string |
There was a problem hiding this comment.
The reference to RFC3339 here and anywhere else in this class is now not true anymore, is it?
| return ''; | ||
| } | ||
|
|
||
| if (!$dateTime instanceof \DateTime && !$dateTime instanceof \DateTimeInterface) { |
There was a problem hiding this comment.
the first is needed on PHP 5.3, isn't it?
There was a problem hiding this comment.
@ostrolucky So the idea was to not use DateTimeToRfc3339Transformer at all, so we can deprecate it in 4.2 (see #27233)
| * @author Franz Wilding <franz.wilding@me.com> | ||
| * @author Bernhard Schussek <bschussek@gmail.com> | ||
| */ | ||
| class DateTimeToHTML5DateTimeLocalTransformer extends BaseDateTimeTransformer |
There was a problem hiding this comment.
should be "Html" instead of "HTML"
There was a problem hiding this comment.
Thx. Going to fix this.
|
After working on the pull request I was wondering, do we still need this fix, described in https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Extension/Core/Type/DateTimeType.php#L49 Is there still a problem with supported PHP versions? If not, the view transformer could be removed at all and we could just use the default html5 format. This would be much cleaner. |
|
Hey! I wanted to ask if we should finalize this PR? Ich think we have two options here, first would be to do the refactoring, I suggested. Second would be to get the information, if this is still a problem in some PHP versions, or if we can remove the hack at all. I could not find any infos about a problem in a PHP version, does anyone know more about it? |
| $dateTime = $dateTime->setTimezone(new \DateTimeZone($this->outputTimezone)); | ||
| } | ||
|
|
||
| return preg_replace('/\+00:00$/', '', $dateTime->format('c')); |
There was a problem hiding this comment.
Would it not make more sense to do ->format(DateTimeType::HTML5_FORMAT) rather than preg_replaceing format(c)?
There was a problem hiding this comment.
Of course not, because wrong time of format, but maybe having a different const here with "Y-m-d\TH:i:s"
There was a problem hiding this comment.
This was taken from
. I just removed the 'Z' so it becomes a valid html5 datetime-local format. The idea was to remove DateTimeToRfc3339Transformer in a second refactoring stepThere was a problem hiding this comment.
I see that, but seems like it would be more efficient, and obvious what is happening, to pass to format the format you actually want, rather than the wrong format, and editing it with regex.
|
This bug is stopping forms being submitted, would be great to see if it merged... |
05659f9 to
cd93b98
Compare
cd93b98 to
07b98f7
Compare
|
Closing in favor of #28372, thank you @franzwilding (you're still the commit author there so your contribution should end up in git history.) |
…mcfedr) This PR was merged into the 2.8 branch. Discussion ---------- [Form] Fix DateTimeType html5 input format | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27233, #27254 | License | MIT | Doc PR | N/A Fix DateTimeType' HTML input format according to HTML specs. Currently `DateTimeType` produces html with format `yyyy-MM-dd'T'HH:mm:ssZ` but the HTML5 spec expects `yyyy-MM-dd'T'HH:mm:ss` (i.e. no `Z`). Chrome presents an empty date picker meaning edits or having a default date are broken. Also the reverseTransform was expect to have a timezone attached, which it does not - and incorrectly marks it as being a UTC time in this case, instead of using the Transformers output TZ. This is same as @franzwilding #27254 but with change to just straight use of `DateTime::format` and handling TZ in reverseTransform Commits ------- e21a1a4 Added relevent links for parsing to the phpdoc 4f06f15 Add stricter checking for valid date time string 253d0a6 [Form] Fix DateTimeType html5 input format
…mcfedr) This PR was merged into the 2.8 branch. Discussion ---------- [Form] Fix DateTimeType html5 input format | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #27233, #27254 | License | MIT | Doc PR | N/A Fix DateTimeType' HTML input format according to HTML specs. Currently `DateTimeType` produces html with format `yyyy-MM-dd'T'HH:mm:ssZ` but the HTML5 spec expects `yyyy-MM-dd'T'HH:mm:ss` (i.e. no `Z`). Chrome presents an empty date picker meaning edits or having a default date are broken. Also the reverseTransform was expect to have a timezone attached, which it does not - and incorrectly marks it as being a UTC time in this case, instead of using the Transformers output TZ. This is same as @franzwilding symfony/symfony#27254 but with change to just straight use of `DateTime::format` and handling TZ in reverseTransform Commits ------- e21a1a4df1 Added relevent links for parsing to the phpdoc 4f06f1524d Add stricter checking for valid date time string 253d0a683b [Form] Fix DateTimeType html5 input format
Fix DateTimeType' HTML input format according to HTML specs