Bug 48220 – constraint validation: stepMismatch (rounding error)

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Dai Mikurube]

Steps to reproduce:
1. use following input <input type="number" step="0.005" min="4" value="5.005" />
2. check the validity-property (valid and stepMismatch

Expected:
validity.valid should be true.

Actually happens:
validity.valid is false and validity.stepMismatch is true.

[Dai Mikurube]

Created attachment 71719 [details]
Patch

[Kent Tamura]

Comment on attachment 71719 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=71719&action=review

You need to add a test case to LayoutTests/fast/forms/script-tests/ValidityState-stepMismatch.js

> WebCore/html/NumberInputType.cpp:120
> +    // Accepts errors in the equvalent precision to IEEE 754 single-precision numbers and additional 7-bits.

This comment doesn't match to the code?

[Dai Mikurube]

Created attachment 71863 [details]
Patch

[Dai Mikurube]

(In reply to comment #2)
Thank you for the comments. Added tests and modified the comment.

[Kent Tamura]

Comment on attachment 71863 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=71863&action=review

> WebCore/html/NumberInputType.cpp:119
>      // is greater than step*2^DBL_MANT_DIG, the following fmod() makes no sense.

Needs to update the comment.  fmod() won't be used.

> WebCore/html/NumberInputType.cpp:122
> +    double remainder = fabs(doubleValue - step * round(doubleValue / step));

Would you explain why we need to use fabs() instead of fmod() in a code comment or ChangeLog please?

> WebCore/html/NumberInputType.cpp:123
> +    // Accepts errors in the equvalent precision to IEEE 754 single-precision

This comment is confusing.  "Accepts erros in lower fractional part which IEEE 754 single-precision can't represent." ?

[Dai Mikurube]

Created attachment 71979 [details]
Patch

[Dai Mikurube]

Comment on attachment 71863 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=71863&action=review

>> WebCore/html/NumberInputType.cpp:122
>> +    double remainder = fabs(doubleValue - step * round(doubleValue / step));
> 
> Would you explain why we need to use fabs() instead of fmod() in a code comment or ChangeLog please?

Ok, added a detailed description in ChangeLog.

>> WebCore/html/NumberInputType.cpp:123
>> +    // Accepts errors in the equvalent precision to IEEE 754 single-precision
> 
> This comment is confusing.  "Accepts erros in lower fractional part which IEEE 754 single-precision can't represent." ?

Thank you. Replaced with your expression.

[Kent Tamura]

Comment on attachment 71979 [details]
Patch

Looks good.  Thanks!

[WebKit Commit Bot]

Comment on attachment 71979 [details]
Patch

Clearing flags on attachment: 71979

Committed r70615: <http://trac.webkit.org/changeset/70615>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.