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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Nov 18 16:10:21 PST 2009


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





--- Comment #6 from Darin Adler <darin at apple.com>  2009-11-18 16:10:21 PST ---
(From update of attachment 43350)
> +// Constant values for getAllowedValueStep().
> +static const double numberDefaultStep = 1.0;
> +static const double numberStepScaleFactor = 1.0;

These are actually for getStepParameters; the comment is wrong.

> +        doubleValue = fabs(doubleValue - stepBase);

I still remain skeptical about the absolute value here. But it's really the
tests that will guarantee we have this right.

> +        if (isinf(doubleValue))
> +            return false;
> +        // double's fractional part size is DBL_MAN_DIG-bit.  If the current
> +        // value is greater than step*2^DBL_MANT_DIG, the following fmod() makes
> +        // no sense.
> +        if (doubleValue / pow(2, DBL_MANT_DIG) > step)
> +            return false;

The pow-related check will take care of the infinity case naturally, so you
don't need the explicit isinf check.

> +        double remainder = fmod(doubleValue, step);
> +        // Accepts errors in lower 7-bit.
> +        double acceptableError = step / pow(2, DBL_MANT_DIG - 7);
> +        return acceptableError < remainder && remainder < (step - acceptableError);

Where does the constant 7 come from? The HTML5 specification? The comment
should make that clear.

> +    // Sets the "allowed value step" defined in the HTML spec to the specified double pointer.
> +    // Returns false if there is no "allowed value step."
> +    bool getAllowedValueStep(double*) const;

For non-optional out parameters, I really prefer references to pointers. But I
suppose that's just a personal preference.

>      PassRefPtr<HTMLFormElement> createTemporaryFormForIsIndex();
> -
> +    // Helper for getAllowedValueStep();
> +    bool getStepParameters(double* defaultStep, double* stepScaleFactor) const;
>  #if ENABLE(DATALIST)
>      HTMLDataListElement* dataList() const;
>  #endif

The new function should not be put in the same paragraph with the other
functions, especially the dataList function which is inside an ifdef. I think
we need a blank line.

r=me as is but it would be nice if you did another round considering my
comments above

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



More information about the webkit-unassigned mailing list