RESOLVED FIXED 234812
Refactor code creating css values and lists for animation and transition properties 
https://bugs.webkit.org/show_bug.cgi?id=234812
Summary Refactor code creating css values and lists for animation and transition prop...
Antoine Quint
Reported 2022-01-03 04:13:29 PST
Refactor code creating css values and lists for animation and transition properties
Attachments
Patch (19.95 KB, patch)
2022-01-03 04:21 PST, Antoine Quint 		koivisto: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Antoine Quint
Comment 1 2022-01-03 04:14:29 PST
This is work that started from a comment made by Darin in bug 234792.
Antoine Quint
Comment 2 2022-01-03 04:21:18 PST
Created attachment 448225 [details] Patch
Antti Koivisto
Comment 3 2022-01-03 04:29:21 PST
Comment on attachment 448225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448225&action=review > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1450 > +void ComputedStyleExtractor::addCSSValueForAnimationPropertyToList(CSSPropertyID property, const Animation* animation, CSSValueList& list) I'd make the target list the first argument.
Antti Koivisto
Comment 4 2022-01-03 04:31:31 PST
Comment on attachment 448225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448225&action=review >> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1450 >> +void ComputedStyleExtractor::addCSSValueForAnimationPropertyToList(CSSPropertyID property, const Animation* animation, CSSValueList& list) > > I'd make the target list the first argument. Also "CSS" in the name is bit unnecessary.
Antoine Quint
Comment 5 2022-01-03 05:51:20 PST
Committed r287537 (245672@trunk): <https://commits.webkit.org/245672@trunk>
Radar WebKit Bug Importer
Comment 6 2022-01-03 05:52:18 PST
<rdar://problem/87065662>
Darin Adler
Comment 7 2022-01-03 14:08:04 PST
Comment on attachment 448225 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=448225&action=review >>> Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1450 >>> +void ComputedStyleExtractor::addCSSValueForAnimationPropertyToList(CSSPropertyID property, const Animation* animation, CSSValueList& list) >> >> I'd make the target list the first argument. > > Also "CSS" in the name is bit unnecessary. I’d also leave out the “to list” part of the name, and say “append” instead of “add” appendValueForAnimationProperty > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1493 > + auto shorthand = shorthandForProperty(CSSPropertyAnimation); No need for a local variable here. Reads nicely inside the for statement.
Antoine Quint
Comment 8 2022-01-05 01:26:30 PST
(In reply to Darin Adler from comment #7) > Comment on attachment 448225 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=448225&action=review > > > Source/WebCore/css/CSSComputedStyleDeclaration.cpp:1493 > > + auto shorthand = shorthandForProperty(CSSPropertyAnimation); > > No need for a local variable here. Reads nicely inside the for statement. I snuck that into some more refactoring in bug 234872.
Note You need to log in before you can comment on or make changes to this bug.