Skip to content

netteForms.js handles empty inputs differently from server side #146

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

Closed
janmottl opened this issue Mar 9, 2017 · 8 comments
Closed

netteForms.js handles empty inputs differently from server side #146

janmottl opened this issue Mar 9, 2017 · 8 comments

Comments

@janmottl
Copy link

janmottl commented Mar 9, 2017

  • my oppinion is that it is a bug
  • Nette.version = '2.4'

netteForms.js handles empty inputs differently from server side:

  • RANGE Validation of empty input is always true on server side. I suppose that validation is never executed in php.
  • RANGE Validation of empty input is always false in browser. Empty string is of course never in RANGE in js.

Example:
Following RANGE validation of empty input is always successful on server side (php) and always unsuccessful in browser (js).

       $form->addText('obrad_doba_min', '')
            ->setAttribute('class', 'obrad')
            ->addRule($form::RANGE, 'Doba tvání obřadu musí být v rozsahu %d do %d minut', [0, 59]);
dg added a commit to nette/utils that referenced this issue Mar 29, 2017
@dg
Copy link
Member

dg commented Mar 29, 2017

In this case you should add setRequired(TRUE or FALSE)

dg added a commit that referenced this issue Mar 29, 2017
@janmottl
Copy link
Author

janmottl commented Mar 29, 2017

That's misunderstanding. Sorry for unclear specification.
I expected a netteForms.js fix. I expect that '' will be ignored in netteForms.js the same way like in php.
'' is alway validate as invalid in netteForms.js

@dg
Copy link
Member

dg commented Mar 29, 2017

netteForms.js validation was correct (empty string is not in interval) and PHP validation is now fixed (returns FALSE too). In your case you should add setRequired(FALSE) to prevent validation when field is empty.

dg added a commit that referenced this issue Mar 29, 2017
dg added a commit to nette/utils that referenced this issue Mar 29, 2017
dg added a commit to nette/utils that referenced this issue Mar 29, 2017
dg added a commit to nette/utils that referenced this issue Mar 29, 2017
dg added a commit to nette/utils that referenced this issue Mar 29, 2017
@janmottl
Copy link
Author

janmottl commented Mar 30, 2017

Thanks for help.

I'm sorry the problem was on my side. I use input masking by maskedInput.js which sets its placeholder to empty input. Nette.validateControl was called on blur and placeholder was not cleared by maskedInput.js yet. So validation was correct but it was done on wrong event.

Anyway my feeling is that calling of setRequired(FALSE) in this case is something strange because "not required" is a default state. addCondition(Form::FILLED) would make more sense.

From my point of view should validation work with empty values in similar way like database works with empty values. "SELECT null BETWEEN 1 AND 59" returns NULL.

So particular validation function should return NULL on empty value. NULL should be interpreted as correct value and no validation exception should be fired.

@dada-amater
Copy link

This breaks some nette components like DatePicker, which use Validators::isInRange() for comparing dates.

@dg
Copy link
Member

dg commented Apr 25, 2017

Can you write exact parameter values?

@dada-amater
Copy link

$form->addDatePicker('date_of_birth', 'Date of birth')
     ->setRequired(FALSE)
     ->addRule(Form::RANGE, 'The entered date is not within the allowed range.', [new DateTime('-120 years'), new DateTime('-18 years')]);

DatePicker returns DateTime object (not string, not int), so Validators::isInRange() always returns FALSE. This new implementation is unexpected. If you compare numbers and the control is number ($control->setType('number')), than the $control->getValue()should return int (maybe it does) and the hack like$value * 1` is not necessary.

This is big BC break in minor version.

@dg
Copy link
Member

dg commented Apr 26, 2017

Fixed

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

No branches or pull requests

3 participants