Summary: | [Web Animations] Always expose "composite" in output of getKeyframes() | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Antoine Quint <graouts> | ||||||
Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, dino, esprehn+autocc, ews-watchlist, kondapallykalyan, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Antoine Quint
2018-02-08 03:13:50 PST
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> |