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] Date\TimeType marked as invalid on request with single_text and zero seconds #30253

Closed
RevisionTen opened this issue Feb 15, 2019 · 13 comments

Comments

@RevisionTen
Copy link

RevisionTen commented Feb 15, 2019

Symfony version(s) affected: 3.4 up to 4.2

Description
Submitting a TimeType without seconds and with input option set to "string" causes an TransformationFailedException.

Possible Solution
Change 'H:i:s' to $format in src/Symfony/Component/Form/Extension/Core/Type/TimeType.php at line 167

Additional context

Data missing

  at vendor/symfony/form/Extension/Core/DataTransformer/DateTimeToStringTransformer.php:125
  at Symfony\Component\Form\Extension\Core\DataTransformer\DateTimeToStringTransformer->reverseTransform('13:00')
     (vendor/symfony/form/ReversedTransformer.php:36)
  at Symfony\Component\Form\ReversedTransformer->transform('13:00')
     (vendor/symfony/form/Form.php:1064)
  at Symfony\Component\Form\Form->modelToNorm('13:00')
     (vendor/symfony/form/Form.php:351)
  at Symfony\Component\Form\Form->setData('13:00')
     (vendor/symfony/form/Extension/Core/DataMapper/PropertyPathMapper.php:49)
@xabbuh
Copy link
Member

xabbuh commented Feb 15, 2019

This is not a bug. When using a string as the input data it currently is expected to be in the format hour:minute:seconds. We could think about adding support for a new input_format option tough (similar to #29887).

@RevisionTen
Copy link
Author

RevisionTen commented Feb 15, 2019

Or always add seconds if they are missing, similar to this:

// handle seconds ignored by user's browser when with_seconds enabled
// https://codereview.chromium.org/450533009/
if ($options['with_seconds']) {
    $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $e) {
        $data = $e->getData();
        if ($data && preg_match('/^\d{2}:\d{2}$/', $data)) {
            $e->setData($data.':00');
        }
    });
}

Fact is:
When using html5 time input without seconds the returned value does not match the format.
Solution:
Correct the format

@RevisionTen
Copy link
Author

RevisionTen commented Feb 15, 2019

I prefer your solution though, If I want to save the time as a string I want control of how the string looks.
But that doesn't fix the exception.
So maybe do both?

@xabbuh
Copy link
Member

xabbuh commented Feb 15, 2019

Can you show how you currently build your form?

@RevisionTen
Copy link
Author

Like this:

$builder->add('opens', TimeType::class, [
    'label' => 'Opens',
    'required' => false,
    'widget' => 'single_text',
    'html5' => true,
    'input' => 'string',
    'with_seconds' => false,
]);

@xabbuh
Copy link
Member

xabbuh commented Feb 15, 2019

Adding the above mentioned event listener when with_seconds is enabled or the input format is a string would solve the issue, right?

@RevisionTen
Copy link
Author

In theory yes, but for some reason the event listener is not called.

Tried it with this:

if ($options['with_seconds'] || 'string' === $options['input']) {
    // executes...
    $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $e) {
        // does not execute...
        $data = $e->getData();
        if ($data && preg_match('/^\d{2}:\d{2}$/', $data)) {
            $e->setData($data.':00');
        }
    });
}

@xabbuh
Copy link
Member

xabbuh commented Feb 15, 2019

Rereading the initial description now: Are you sure this happens on submission of the form? This rather looks like it is related to passing the wrong format when the initial data is set.

@RevisionTen
Copy link
Author

Yes, this happens when the form is submitted in the browser.

The value that is produced by the <input type="time"> element is apparently supposed to be in the h:i format, which means that the missing seconds definitely need to be appended if the time types underlying data is always supposed to be in the h:i:s format.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input/time#Time_value_format

The value of the time input is always in 24-hour format: hh:mm, regardless of the input format, which is likely to be selected based on the user's locale (or by the user agent). If the time includes seconds (see Using the step attribute), the format is always hh:mm:ss.

@xabbuh
Copy link
Member

xabbuh commented Feb 18, 2019

I am sorry, but I cannot reproduce this issue. Can you create a small example application that allows to reproduce?

@RevisionTen
Copy link
Author

You were correct, this happened because the initial data was not in the expected format. Sorry for wasting everyones time.

@xabbuh
Copy link
Member

xabbuh commented Feb 18, 2019

Thank you for the feedback. 👍

@xabbuh
Copy link
Member

xabbuh commented Feb 23, 2019

For the records: with #30358 we will also be able to customise the input format for the TimeType.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants