[Web Animations] Always expose "composite" in output of getKeyframes()
Created attachment 333365 [details] Patch
Attachment 333365 [details] did not pass style-queue: ERROR: Source/WebCore/animation/KeyframeEffect.cpp:454: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 333365 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333365&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2191 > + return "std::nullopt" if $defaultValue eq "null"; and $type->isNullable ? If the enum type is not nullable, then null should not be a valid default value. Maybe we could "die" if the default value is null and the enum type is not nullable. Bindings change LGTM otherwise, although it'd be nice to have bindings test coverage. It is hard to review a bindings change without such coverage as I do not see the code it generates.
(In reply to Chris Dumez from comment #3) > Comment on attachment 333365 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333365&action=review > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2191 > > + return "std::nullopt" if $defaultValue eq "null"; > > and $type->isNullable ? > > If the enum type is not nullable, then null should not be a valid default > value. Maybe we could "die" if the default value is null and the enum type > is not nullable. I'll add this. > Bindings change LGTM otherwise, although it'd be nice to have bindings test > coverage. It is hard to review a bindings change without such coverage as I > do not see the code it generates. Can you point me to a recent commit that adds a bindings test for "inspiration"?
B(In reply to Antoine Quint from comment #4) > (In reply to Chris Dumez from comment #3) > > Comment on attachment 333365 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=333365&action=review > > > > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2191 > > > + return "std::nullopt" if $defaultValue eq "null"; > > > > and $type->isNullable ? > > > > If the enum type is not nullable, then null should not be a valid default > > value. Maybe we could "die" if the default value is null and the enum type > > is not nullable. > > I'll add this. > > > Bindings change LGTM otherwise, although it'd be nice to have bindings test > > coverage. It is hard to review a bindings change without such coverage as I > > do not see the code it generates. > > Can you point me to a recent commit that adds a bindings test for > "inspiration"? Basically just add new IDL to Source/WebCore/bindings/scripts/test/TestObj.idl , then run: Tools/Scripts/run-bindings-tests --reset-results
Created attachment 333393 [details] Patch
Attachment 333393 [details] did not pass style-queue: ERROR: Source/WebCore/animation/KeyframeEffect.cpp:454: Should have only a single space after a punctuation in a comment. [whitespace/comments] [5] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 333393 [details] Patch bindings LGTM
Committed r228312: <https://trac.webkit.org/changeset/228312>
<rdar://problem/37381726>