[Webkit-unassigned] [Bug 31331] step attribute and ValidityState.stepMismatch for type=number/range

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31331





--- Comment #4 from Darin Adler <darin at apple.com>  2009-11-15 14:52:17 PST ---
(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.

> > > +        // double's fractional part size is 52-bit.  If the current value is
> > > +        // greater than step*2^52, the following fmod() makes no sense.
> > > +        static const double fractionalSize = 52;
> > > +        if (doubleValue / pow(2, fractionalSize) > step)
> > > +            return false;
> > 
> > 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?

> We'll need to use different values for
> date/datetime/datetime-local/month/time/week.
> So the ideal form would be:
>     double defaultStep;
>     double stepScaleFactor;
>     switch (inputType()) {
>     case NUMBER:
>     case RANGE:
>         defaultStep = numberDefaultStep;
>         stepScaleFactor = numberStepScaleFactor;
>         break;
>     case DATE:
>         defaultStep = ...;
>         ...
> 
> But MSVC warns "uninitialized variable" for defaltStep and stepScaleFactor.
> I changed the code like the following:
>     double defaultStep = 0;
>     double stepScaleFactor = 0;
>     switch (inputType()) {
>     case NUMBER:
>     case RANGE:
>         defaultStep = numberDefaultStep;
>         stepScaleFactor = numberStepScaleFactor;
>         break;
>     case DATE:
>         // FIXME

The warning is due to the case where inputType() is not one of the values in
the switch statement.

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:

    ASSERT_NOT_REACHED();
    defaultStep = 1;
    stepScaleFactor = 1;

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.