Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

min/max validation broken for types range/number #648

Closed
ekonijn opened this Issue Feb 23, 2013 · 2 comments

Comments

Projects
None yet
3 participants
Contributor

ekonijn commented Feb 23, 2013

In 1.11.0, min/max validation is broken for types 'number' and 'range'.

For an example, see http://jsbin.com/awokex/1

The cause is that min/max are interpreted as strings; this is necessary for dates, but for numbers, "50" < "150" < "1000" does not hold.

See also #639, which seems to expect "50" < "150" < "1000" to hold even for type 'text',
and 69831a0 where this behaviour is introduced.

ekonijn added a commit to ekonijn/jquery-validation that referenced this issue Feb 23, 2013

fix #648 min/max validation for type number/range
In 1.11.0, min/max validation is broken for types 'number' and 'range'.

For an example, see http://jsbin.com/awokex/1

The cause is that min/max are interpreted as strings; this is necessary
for dates, but for numbers, "50" < "150" < "1000" does not hold.

See also #639, which seems to expect "50" < "150" < "1000" to hold even
for type 'text', and 69831a0 where this behaviour is introduced.
This commit does not change behaviour for type 'text': the html5 spec
says min/max do not apply to text (4.10.7.1.2).

Tested with grunt, added qunit test, verified in firefox and chromium.

ekonijn added a commit to ekonijn/jquery-validation that referenced this issue Feb 27, 2013

min/max validation for text interpreted as number
Issue #639 notes that min/max validation for input type text
is no longer interpreted numerically in 1.11.0.  This differs
from 1.10.0, where it was interpreted numerically, even though
html5 spec says the input type text cannot have min/max attributes.

This issue was caused when adding support for dates, where
interpreting min/max as number in 1.10.0 caused the error
that "2012-11-22" converted to number is NaN.

This commit once more interprets min/max for text numerically;
input type=date remains non-numerical.

It builds on a proposed fix for #648, that interprets
input with type number/range min/max numerically.

The effect is:

 * input type=text has numeric min/max, as in 1.10.0
 * input type=date has working min/max for type date;
   on mobile browsers you also get a date picker,
   plus the browser may reject invalid dates before
   javascript gets a chance to complain.
 * input type=number or range get numeric min/max,
   plus numeric keypad or slider on mobile browsers,
   plus browser may reject invalid input before javascript
   gets a chance to complain

Allowing use of min/max with type=number/range/date is important
for mobile browsers, where the numeric keypad or date picker
make the input much easier to use than a generic text input field.
In this situation jquery-validate remains necessary to support
older browsers that do not do input validation based on type
and min/max.

For situations where numeric input should be validated by jquery
without giving the browser a chance to validate the input format,
input type=text in combination with min/max can be used, as in 1.10.0.

See also #648.

Validation of this commit is problematic.  It passes validation
in the Chromium browser, and looks OK in a simple sample page
such as http://jsbin.com/awokex/3, but fails in grunt with
the message "Please enter a valid date."

I suspect this is caused by a bug in phantomjs:
http://code.google.com/p/phantomjs/issues/detail?id=187#c6

A fix for this bug exists:
ariya/phantomjs@35c1971
but that is released only in phantomjs 1.8.0, and the version
that I see grunt use is only 1.7.0.  Sigh,

ekonijn added a commit to ekonijn/jquery-validation that referenced this issue Feb 28, 2013

Fix #639/#648 min/max validation
In 1.10.0, min/max validation was supported for input type="text",
where min/max were interpreted as numbers.  This means min/max
for date would not work: min="2012-02-13" was interpreted as min="Not a Number".

In 1.11.0, min/max were no longer converted to numbers.  This means
min/max for dates worked, but min/max for numbers failed:
"50" < "150" < "1000" does not hold.

For an example, see http://jsbin.com/awokex/3

This commit makes the behaviour of min/max dependent on input type:

 * input type=text has numeric min/max, as in 1.10.0
 * input type=date has working min/max for type date;
   on mobile browsers you also get a date picker,
   plus the browser may reject invalid dates before
   javascript gets a chance to complain.
 * input type=number or range get numeric min/max,
   plus numeric keypad or slider on mobile browsers,
   plus browser may reject invalid input before javascript
   gets a chance to complain

Allowing use of min/max with type=number/range/date is important
for mobile browsers, where the numeric keypad or date picker
make the input much easier to use than a generic text input field.
In this situation jquery-validate remains necessary to support
older browsers that do not do input validation based on type
and min/max.

For situations where numeric input should be validated by jquery
without giving the browser a chance to validate the input format,
input type=text in combination with min/max can be used, as in 1.10.0.

Note that some older browsers do not correctly support Date conversion,
which can cause the validation suite to fail with message "Please enter a valid date."

In particular, this bug hit phantomjs 1.7.0 as used in validating 1.11.0;
the issue is solved in phantomjs 1.8.0 used now.

http://code.google.com/p/phantomjs/issues/detail?id=187#c6
ariya/phantomjs@35c1971

Tested with chromium, firefox, grunt.

ruado1987 added a commit to ruado1987/jquery-validation that referenced this issue Mar 22, 2013

Fix min/max validation. Closes gh-666. Fixes #648
In 1.10.0, min/max validation was supported for input type="text",
where min/max were interpreted as numbers.  This means min/max
for date would not work: min="2012-02-13" was interpreted as min="Not a Number".

In 1.11.0, min/max were no longer converted to numbers.  This means
min/max for dates worked, but min/max for numbers failed:
"50" < "150" < "1000" does not hold.

For an example, see http://jsbin.com/awokex/3

This commit makes the behaviour of min/max dependent on input type:

 * input type=text (or not type attribute) has numeric min/max, as in 1.10.0
 * input type=date has working min/max for type date;
   on mobile browsers you also get a date picker,
   plus the browser may reject invalid dates before
   javascript gets a chance to complain.
 * input type=number or range get numeric min/max,
   plus numeric keypad or slider on mobile browsers,
   plus browser may reject invalid input before javascript
   gets a chance to complain

Allowing use of min/max with type=number/range/date is important
for mobile browsers, where the numeric keypad or date picker
make the input much easier to use than a generic text input field.
In this situation jquery-validate remains necessary to support
older browsers that do not do input validation based on type
and min/max.

For situations where numeric input should be validated by jquery
without giving the browser a chance to validate the input format,
input type=text in combination with min/max can be used, as in 1.10.0.

This exemption should extend to type="tel" as well, as many rely on that for a numeric keyboard on iOS.

Collaborator

jzaefferer commented Jun 24, 2013

@nariman-haghighi thanks. I've created a new issue for that, #792

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