[webkit-reviews] review denied: [Bug 70816] Get rid of optional parameters in the middle in IDLs. : [Attachment 112332] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied	review:
Bug 70816: Get rid of optional parameters in the middle in IDLs.
https://bugs.webkit.org/show_bug.cgi?id=70816

Attachment 112332: Patch
https://bugs.webkit.org/attachment.cgi?id=112332&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=112332&action=review


Specific consequences of this change look like they are both for the worse.

I don't feel very strong about this, and postMessage specifically seems to be
in need of change, but this needs to be more clearly an improvement.
Specifically, postMessage documentation changes should go together actual
implementation changes (in a dedicated patch).

>> Source/WebCore/page/DOMWindow.idl:218
>> +		raises(DOMException);
> 
> This is being discussed in the working group, but I see that you're not
changing behavior.

But is this change an improvement? [Optional] is a very useful bit of
documentation here. Documentation is why there are arguments specified for
[Custom] functions - they are ignored otherwise.

What this change does is making documentation not match reality.

>> LayoutTests/fast/canvas/canvas-putImageData-expected.txt:147
>> +PASS context.putImageData({}, 0, 0) threw exception TypeError: Type error.
> 
> Interesting.	I didn't know these were different.  This looks like an OK
behavior change.

Is it specified what should happen? This looks like a change for worse -
incorrect arguments for a built-in function should raise a DOM exception, not a
JS one.