Bug 49218 – WebIDL conversions from string to number and array to number should not generate TypeError

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Gregg Tavares]

If I you have a WebIDL function defined like this

    void stencilMask(long mask);

According the the WebIDL spec, values passed as the "mask" argument are converted to a number by the spec defined in ECMAScript ToNumber in section 9.

http://www.ecma-international.org/publications/standards/Ecma-262.htm

That spec appears to say that strings are converted to numbers by calling ToNumber on the string which returns a number or NaN and in the case of an integral type, NaN is converted to 0.

That means gl.stencilMask("123") should be the same as gl.stencilMask(123) and gl.stencilMask("bar") should be the same as gl.stencilMask(0);

For objects it says first toString is called on the object, then that string is passed toNumber. Since an array is a type of object that means means that gl.stencilMask([123]) is the same as gl.stencilMask(123);

All of these cases are currently returning TypeError in WebKit.

Please read the WebIDL spec and ECMAScript spec to verify
http://www.w3.org/TR/WebIDL/

Section 4.1.7 says long calls ToInt32
Section 4 says ToInt32 is defined in the EMCAScript spec.
ECMAScript spec
Section 9.5 says ToInt32 calls ToNumber
Seciton 9.3 says ToNumber calls ToString on strings and ToNumber(ToPrimitve()) on objects.
Section 9.1 says ToPrimitive uses the DefaultValue for objects
Section 8.12.8 says default value is derived by calling toString() on the object.

[Gregg Tavares]

I think I tracked this down to 

http://trac.webkit.org/changeset/70410
http://trac.webkit.org/changeset/70411
http://trac.webkit.org/changeset/70414

I downloaded r70429 which fails where as r70284 passes

You can see this issue by going to this page

https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/type-conversion-test.html

If you scroll down the results of the tests, on WebKit r70429 and greater (top of tree) more than half the tests fail. On r70284 or before almost all the tests pass.

I don't know what other WebIDL generated APIs were effected by this.

[Nikolas Zimmermann]

(In reply to comment #1)
> I think I tracked this down to 
> 
> http://trac.webkit.org/changeset/70410
> http://trac.webkit.org/changeset/70411
> http://trac.webkit.org/changeset/70414
> 
> I downloaded r70429 which fails where as r70284 passes
> 
> You can see this issue by going to this page
> 
> https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/tests/conformance/type-conversion-test.html
> 
> If you scroll down the results of the tests, on WebKit r70429 and greater (top of tree) more than half the tests fail. On r70284 or before almost all the tests pass.
> 
> I don't know what other WebIDL generated APIs were effected by this.

That's true I changed the behaviour, but I wasn't aware I was breaking something, as these webgl tests are not in our LayoutTests/ framework. I'll look into unbreaking.

[Kenneth Russell]

Because we need this fixed urgently for a Chrome milestone release I'm taking this bug. Nikolas, please let me know if you had made progress on it already.

[Kenneth Russell]

Created attachment 73958 [details]
Patch

[Dimitri Glazkov (Google)]

Comment on attachment 73958 [details]
Patch

looks good.

[Nikolas Zimmermann]

(In reply to comment #3)
> Because we need this fixed urgently for a Chrome milestone release I'm taking this bug. Nikolas, please let me know if you had made progress on it already.

Thanks a lot Kenneth, I'm just moving flats and thus a bit busy...
Thanks also for catching several bugs in the SVG primitive tests!

Great, that it's now fixed!

[Dirk Schulze]

IIRC we don't check for NaN values in the SVG*Element.cpp files. This could cause problems! I don't have examples right now. But I could imagine that there are cases, it needs at least checking. Niko?

[Nikolas Zimmermann]

(In reply to comment #7)
> IIRC we don't check for NaN values in the SVG*Element.cpp files. This could cause problems! I don't have examples right now. But I could imagine that there are cases, it needs at least checking. Niko?

Absolutely true. Though note, that Kenneth just restored the "old" behaviour, that existed prior to my StrictTypeChecking changes.

The NaN cases are now possible again, and as it's even correct according to WebIDL/ECMA, we need to add lots of more checks into the whole svg/ code regarding numeric value interpretation.

[Kenneth Russell]

Committed r72123: <http://trac.webkit.org/changeset/72123>