[webkit-reviews] review granted: [Bug 174529] [WebIDL] Replace some custom bindings code in JSCSSStyleDeclarationCustom.cpp with named getters/setters : [Attachment 315566] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Sam Weinig <sam at webkit.org>'s
request for review:
Bug 174529: [WebIDL] Replace some custom bindings code in
JSCSSStyleDeclarationCustom.cpp with named getters/setters
https://bugs.webkit.org/show_bug.cgi?id=174529

Attachment 315566: Patch

https://bugs.webkit.org/attachment.cgi?id=315566&action=review




--- Comment #25 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 315566
  --> https://bugs.webkit.org/attachment.cgi?id=315566
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=315566&action=review

r=me with comments.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:906
> +    push(@$outputArray, $indent . "bool propertySupported = true;\n") if
$namedSetterOperation->extendedAttributes->{PutOnlyForSupportedProperties};

nit: Variable name should have "is" prefix.

> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:965
> +	   if
($namedSetterOperation->extendedAttributes->{PutOnlyForSupportedProperties}) {

Isn't this confusingly named? What the code seems to do is call JSObject::put()
if the property is NOT supported. The name seems to indicate the opposite.

> Source/WebCore/css/CSSStyleDeclaration.h:74
> +    // Bindings support

Missing period at the end.

> Source/WebCore/css/CSSStyleDeclaration.idl:49
> +    // recosider this decision.

Typo.