Bug 48220

Summary: constraint validation: stepMismatch (rounding error)
Product: WebKit Reporter: Dai Mikurube <dmikurube>
Component: FormsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, tkent
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Bug Depends on: 48221    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Dai Mikurube 2010-10-24 23:56:30 PDT
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.
Comment 1 Dai Mikurube 2010-10-25 00:42:12 PDT
Created attachment 71719 [details]
Patch
Comment 2 Kent Tamura 2010-10-25 01:51:16 PDT
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?
Comment 3 Dai Mikurube 2010-10-26 04:10:39 PDT
Created attachment 71863 [details]
Patch
Comment 4 Dai Mikurube 2010-10-26 04:12:02 PDT
(In reply to comment #2)
Thank you for the comments. Added tests and modified the comment.
Comment 5 Kent Tamura 2010-10-26 07:01:29 PDT
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." ?
Comment 6 Dai Mikurube 2010-10-26 21:35:22 PDT
Created attachment 71979 [details]
Patch
Comment 7 Dai Mikurube 2010-10-26 21:46:27 PDT
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.
Comment 8 Kent Tamura 2010-10-26 22:25:16 PDT
Comment on attachment 71979 [details]
Patch

Looks good.  Thanks!
Comment 9 WebKit Commit Bot 2010-10-26 22:42:46 PDT
Comment on attachment 71979 [details]
Patch

Clearing flags on attachment: 71979

Committed r70615: <http://trac.webkit.org/changeset/70615>
Comment 10 WebKit Commit Bot 2010-10-26 22:42:51 PDT
All reviewed patches have been landed.  Closing bug.