[WebIDL] Replace some custom bindings code in JSCSSStyleDeclarationCustom.cpp with named getters/setters
Created attachment 315488 [details] Patch
Created attachment 315490 [details] Patch
Created attachment 315499 [details] Patch
Created attachment 315505 [details] Patch
Comment on attachment 315505 [details] Patch Attachment 315505 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4122497 New failing tests: fast/text/international/block-flow-parser-test.html imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/put-forwards.html fast/dom/StyleSheet/gc-rule-children-wrappers.html fast/css/inherit-initial-shorthand-values.html fast/dom/gc-9.html fast/dom/CSSStyleDeclaration/cssstyledeclaration-properties-descriptor.html
Created attachment 315513 [details] Archive of layout-test-results from ews114 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 315505 [details] Patch Attachment 315505 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4122600 New failing tests: fast/text/international/block-flow-parser-test.html imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/put-forwards.html fast/dom/StyleSheet/gc-rule-children-wrappers.html fast/css/inherit-initial-shorthand-values.html fast/dom/gc-9.html fast/dom/CSSStyleDeclaration/cssstyledeclaration-properties-descriptor.html
Created attachment 315516 [details] Archive of layout-test-results from ews106 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 315505 [details] Patch Attachment 315505 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4122637 New failing tests: fast/text/international/block-flow-parser-test.html imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/put-forwards.html fast/dom/StyleSheet/gc-rule-children-wrappers.html fast/css/inherit-initial-shorthand-values.html fast/dom/gc-9.html fast/dom/CSSStyleDeclaration/cssstyledeclaration-properties-descriptor.html
Created attachment 315518 [details] Archive of layout-test-results from ews100 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 315505 [details] Patch Attachment 315505 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4122722 New failing tests: fast/text/international/block-flow-parser-test.html imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/put-forwards.html fast/dom/StyleSheet/gc-rule-children-wrappers.html fast/css/inherit-initial-shorthand-values.html fast/dom/gc-9.html fast/dom/CSSStyleDeclaration/cssstyledeclaration-properties-descriptor.html
Created attachment 315521 [details] Archive of layout-test-results from ews121 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews121 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Created attachment 315554 [details] Patch
Created attachment 315555 [details] Patch
Comment on attachment 315555 [details] Patch Attachment 315555 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4126488 New failing tests: imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/put-forwards.html
Created attachment 315558 [details] Archive of layout-test-results from ews105 for mac-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 315555 [details] Patch Attachment 315555 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4126498 New failing tests: imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/put-forwards.html
Created attachment 315559 [details] Archive of layout-test-results from ews122 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews122 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Comment on attachment 315555 [details] Patch Attachment 315555 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4126559 New failing tests: imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/put-forwards.html
Created attachment 315561 [details] Archive of layout-test-results from ews115 for mac-elcapitan The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews115 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 315566 [details] Patch
Comment on attachment 315566 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=315566&action=review > Source/WebCore/css/CSSStyleDeclaration.idl:47 > + // these should be normal attributes, but there are too many combinations with Where is this is the spec? I could not find those normal attributes at: https://drafts.csswg.org/cssom/#the-cssstyledeclaration-interface Is our behavior really standard? Do other browsers really behave the same way as us?
(In reply to Chris Dumez from comment #22) > Comment on attachment 315566 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=315566&action=review > > > Source/WebCore/css/CSSStyleDeclaration.idl:47 > > + // these should be normal attributes, but there are too many combinations with > > Where is this is the spec? I could not find those normal attributes at: > https://drafts.csswg.org/cssom/#the-cssstyledeclaration-interface > > Is our behavior really standard? Do other browsers really behave the same > way as us? That spec has crazy formatting for me :(. The property bits are defined in: https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-webkit-cased-attribute https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-dashed-attribute Note the text like: "For each CSS property property that is a supported CSS property, the following partial interface applies where camel-cased attribute is obtained by running the CSS property to IDL attribute algorithm for property."
(In reply to Sam Weinig from comment #23) > (In reply to Chris Dumez from comment #22) > > Is our behavior really standard? Do other browsers really behave the same > > way as us? Not sure what the compat story is here. I tried to minimize behavior change as much as possible here. Matching other browsers and the spec can come next.
Comment on attachment 315566 [details] 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.
(In reply to Chris Dumez from comment #25) > Comment on attachment 315566 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=315566&action=review > > > 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. Yeah, I guess. I was thinking of Put as meaning the custom setter, but that is not at all clear from the name. Renamed it to [CallNamedSetterOnlyForSupportedProperties].
Committed r219622: <http://trac.webkit.org/changeset/219622>