[webkit-reviews] review granted: [Bug 100801] Remove the V8 custom code for WebSockets constructor : [Attachment 173234] Patch (merge changes only)
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Fri Nov 9 05:44:04 PST 2012
Kentaro Hara <haraken at chromium.org> has granted Tommy Widenflycht
<tommyw at google.com>'s request for review:
Bug 100801: Remove the V8 custom code for WebSockets constructor
https://bugs.webkit.org/show_bug.cgi?id=100801
Attachment 173234: Patch (merge changes only)
https://bugs.webkit.org/attachment.cgi?id=173234&action=review
------- Additional Comments from Kentaro Hara <haraken at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=173234&action=review
Looks OK with some nits. Thanks for a great patch!
> Source/WebCore/Modules/websockets/WebSocket.cpp:176
> + ec = SYNTAX_ERR;
This change is non-trivial but looks OK for the following reasons:
(1) This changes the current behavior. The current implementation throws
SyntaxError (one of JavaScript errors) instead of SYNTAX_ERR (one of DOM
exceptions).
(2) But the spec says it should be SYNTAX_ERR.
(http://www.w3.org/TR/2009/WD-websockets-20091222/) So the new implementation
is conformed to the spec.
(3) As no developers would care about the difference, this change won't break
compatibility.
> Source/WebCore/Modules/websockets/WebSocket.idl:43
> + ConstructorParameters=1,
Nit: Redundant ','
> Source/WebCore/bindings/scripts/IDLParser.pm:1326
> +sub copyAttributes
copyAttributes => copyExtendedAttributes ?
More information about the webkit-reviews
mailing list