[webkit-reviews] review denied: [Bug 27451] Support HTML5 step attribute for <input type=number/range> : [Attachment 42009] Patch part 3: stepUp() and stepDown()

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 27451: Support HTML5 step attribute for <input type=number/range>
https://bugs.webkit.org/show_bug.cgi?id=27451

Attachment 42009: Patch part 3: stepUp() and stepDown()
https://bugs.webkit.org/attachment.cgi?id=42009&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +JSValue JSHTMLInputElement::stepUp(ExecState* exec, const ArgList& args)
> +{
> +    HTMLInputElement* input = static_cast<HTMLInputElement*>(impl());
> +    ExceptionCode ec = 0;
> +    int step;
> +    if (args.size() < 1)
> +	   input->stepUp(1, ec);
> +    else {
> +	   bool ok;
> +	   step = args.at(0).toInt32(exec, ok);
> +	   if (exec->hadException())
> +	       return jsUndefined();
> +	   if (!ok)
> +	       ec = TYPE_MISMATCH_ERR;
> +	   else
> +	       input->stepUp(step, ec);
> +    }
> +    setDOMException(exec, ec);
> +    return jsUndefined();
> +}

This sort of custom binding may be needed for this function for V8, but I do
not think it’s needed for WebKit’s own JavaScript engine. DOM bindings already
treat all arguments as optional, and missing arguments turn into the number 0
if the argument is of numeric type.

The only unusual bit is the idea that an argument of the type needs to lead to
TYPE_MISMATCH_ERR. Other DOM functions that take integral parameters do not do
this kind of type checking; they simply convert to a number. I don't think is a
good idea to write custom bindings for these functions. If we do need to move
to more type checking for functions that take integral parameters in the DOM,
I'd like to do it within the automatic generation mechanism.

Again, the issues may be different for V8. I have not looked at the V8 DOM
bindings at all.