[webkit-reviews] review canceled: [Bug 31331] step attribute and ValidityState.stepMismatch for type=number/range : [Attachment 43246] Proposed patch (rev.3)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Nov 17 02:22:21 PST 2009
TAMURA, Kent <tkent at chromium.org> has canceled TAMURA, Kent
<tkent at chromium.org>'s request for review:
Bug 31331: step attribute and ValidityState.stepMismatch for type=number/range
https://bugs.webkit.org/show_bug.cgi?id=31331
Attachment 43246: Proposed patch (rev.3)
https://bugs.webkit.org/attachment.cgi?id=43246&action=review
------- Additional Comments from TAMURA, Kent <tkent at chromium.org>
(In reply to comment #4)
> (In reply to comment #3)
> > > Is it really right to call fabs on doubleValue - min? Is that the right
thing
> > > to do with cases where doubleValue is < min?
> >
> > Yes. HTML5 draft doesn't define exceptional handling for such case and we
need
> > to check stepMismatch.
>
> Did you send mail to HTML5 or WHATWG mentioning this is undefined? I'd like
to
> see this case covered in the specification.
>
> I'm not sure absolute value is best for this. Perhaps we should treat
negative
> numbers as if they were zero or always just use 1 in that case? Having the
same
> behavior in edge cases can sometimes prevent accidental site
incompatibilities
> so I'd prefer to have this defined in the specification and covered by test
> cases even if it's not.
I renamed the variable `min' here to `stepBase.'
I'm asking about this to WHATWG now, and have no reply yet.
If the spec will be updated as "stepMismatch is false if rangeUnderflow or
rangeOverflow", I'll upadte the code.
I don't think this section in the current spec is ambiguous.
We need to handle cases without min attribute. stepMismatch for
<input type=number step=2 value=-2> should be false, and
<input type=number step=2 value=-1> should be true
because the `step base' is 0 in these cases. So we must handle a case of
value < stepBase anyway.
> > > Is there a way to do this same check without hard-coding the value 52?
> >
> > I found DBL_MANT_DIG, which is 53. I replaced 52 with DBL_MANT_DIG.
> > Note: double's precision is 53-bit, and the fractional part size is 52-bit
> > because the most significant bit is omitted.
>
> Is DBL_MANT_DIG available on all the platforms we target?
It's defined in ISO C standard. I confirmed on Mac OS, VS 2005, and Linux.
If other ports have no DBL_MANT_DIG, we may add it to wtf/MathExtras.h.
> I suggest we break out the code to compute the default step and scale factor
> into a separate function with two out arguments (I'd use references for
those,
> but others seem to often chose pointers for that purpose). Named something
like
> getStep, getStepConstraints, getStepDefaults, getStepParameters, or something
> along those lines. That function can use a switch with return statements, and
> then have code after the switch that does:
Ok, I introduced getStepParameters(double*,double*).
More information about the webkit-reviews
mailing list