Skip to content
This repository

Validation issue with Zend\Form\Element\Time #4162

Closed
SGI495 opened this Issue · 11 comments

5 participants

Magnus Berglund Robert Basic Martin Keckeis George Cooksey Matthew Weier O'Phinney
Magnus Berglund

When creating a new form element of type Zend\Form\Element\Time, the Zend\Validator\Date validator, automatically added to the input filter, validates the input value against format "H:i:s" (i.e. HH:MM:SS).

According to the HTML5 specification, the seconds part of a time object is optional. This is also implemented in Google Chrome's and Opera's UIs for the time element.

The effect of this is that using the Zend\Form\Element\Time element with the Zend\Form\View\Helper\FormTime view helper, renders an element that only accepts a value on the format "HH:MM", but this value is then validated against the format "HH:MM:SS". So no input value will ever be considered valid and it is not possible to use these classes as it was intended.

AFAIK it is not possible to either set parts of the date format in Zend\Validator\Date as optional, or specify two acceptable formats. Hence I don't know how to fix this problem. Though to me it seems writing a new Validator for time strings could be a solution.

Workarounds includes using Zend\Form\Element\Text or overriding the validator, but that seems to defeat the purpose of the view helper.

Please advice. (I've been told @bakura10 might have some insight)

Robert Basic

Well, given that the DateTime element does not include the seconds part in it's format (https://github.com/zendframework/zf2/blob/master/library/Zend/Form/Element/DateTime.php#L23), maybe removing the seconds part from the Time element's format (https://github.com/zendframework/zf2/blob/master/library/Zend/Form/Element/Time.php#L32) would fix this.

Magnus Berglund

That is true, but it wouldn't cover the case when somebody would actually want to include the seconds part, which is also a valid timestring.
It is the fact that strings with and without seconds parts are both considered valid timestrings that is causing the problem.
The validator in it's current form can only check one of the possibilities at the time.

Robert Basic

Wait, why don't you just pass a new format to the time element when you are creating it? You can always specify a custom format with only hours and minutes and the validator will then use that. https://github.com/zendframework/zf2/blob/master/library/Zend/Form/Element/DateTime.php#L59

Magnus Berglund

Hmm, somehow I missed that option.
Yes, that is a good solution, but I still believe the default behavior of the Element and the Helper should match, even though the cause is actually external to the library. But I suppose that point is moot if another browser decides to use the seconds part in their implementation.
I think the default format for the time element should not include a seconds part and then you can use the format option to add it, instead of the other way around like now. Or I can just add a notice to the docs.

Robert Basic

Adding a comment or two about this to the docs would be a good idea. As for changing the current behaviour, I have no idea, probably won't happen as it might count as a BC change. I'll leave that to the others :)

Thanks!

Magnus Berglund

Come to think of it, that won't solve the actual problem, it will just work as long as all user agents behave the same way. Currently they do though, perhaps that's enough.

Magnus Berglund SGI495 referenced this issue in zendframework/zf2-documentation
Closed

Fix/zend.form.element.time and related time/date elements #817

Martin Keckeis

@SGI495 your pull request was merged -> can this get closed? Or is something still open?

Magnus Berglund
SGI495 commented
Martin Keckeis

@SGI495 i've just tried it this Zend\I18n\Validor\DateTime if it's possible there...with no success.

To achieve this behaviour you want, you would have to check both "versions" otherwise i think it's not possible.
So try it with HH:mm format and then with HH:mm:ss format....

Maybe an optional parameter or extra validator, to keep BC compability

$validator = new \Zend\I18n\Validator\DateTime();
$validator->setLocale('de_AT');
$validator->setDateType(\IntlDateFormatter::NONE);
$validator->setTimeType(\IntlDateFormatter::MEDIUM);
if($validator->isValid('00:00')){
    echo 'is valid';
} else{
    echo 'is not valid'; //will be printed
}

$validator->setTimeType(\IntlDateFormatter::MEDIUM);
if($validator->isValid('10:30:00')){
    echo 'is valid'; //will be printed
} else{
    echo 'is not valid';
}
Magnus Berglund
SGI495 commented

As it is now Zend\Form\Element\Time (and all related elements) extends Zend\Form\Element\DateTime, which accepts a single date format to be useful for all other elements. This date format is then passed to Zend\Validator\Date for validation of the value. Since the validators are added to the chain before the value is known, Zend\Validator\Date would have to be rewritten to accept multiple date formats.
The other option is to Rewrite Zend\Validator\Date to check if the error given by DateTime::getLastErrors() is equal to 'Data missing' and in that case check if the date format ends in :s and if so remove that part from the date format and try again to see if the error is gone. This is probably easier, but less strict, so I'm not sure that it's acceptable.

George Cooksey

@SGI495 I wouldn't add multiple formats to the Date validator. Instead, write a ValidatorChain with a $breakChainOnFailure = false validator for each format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.