Bug 223124

Summary: Fix interpolation of orphans and widows CSS properties
Product: WebKit Reporter: Antoine Quint <graouts>
Component: AnimationsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: clopez, darin, dino, ews-watchlist, graouts, koivisto, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=223148
Attachments:
Description Flags
Patch darin: review+

Description Antoine Quint 2021-03-12 08:56:10 PST
Fix interpolation of orphans and widows CSS properties
Comment 1 Antoine Quint 2021-03-12 08:58:30 PST
Created attachment 423054 [details]
Patch
Comment 2 EWS Watchlist 2021-03-12 08:59:45 PST
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 3 Darin Adler 2021-03-12 15:25:04 PST
Comment on attachment 423054 [details]
Patch

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

> Source/WebCore/animation/CSSPropertyAnimation.cpp:642
> +class PositivePropertyWrapper : public PropertyWrapper<T> {

final

> Source/WebCore/animation/CSSPropertyAnimation.cpp:645
> +    PositivePropertyWrapper(CSSPropertyID prop, T (RenderStyle::*getter)() const, void (RenderStyle::*setter)(T))

Please use words, not abbreviations. "property" instead of "prop"

> Source/WebCore/animation/CSSPropertyAnimation.cpp:650
> +    void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* from, const RenderStyle* to, double progress) const override

"animator" or "animation" or "client", instead of "anim"

"destination" instead of "dst"

final instead of override

I suggest making this private.
Comment 4 Antoine Quint 2021-03-13 02:46:24 PST
(In reply to Darin Adler from comment #3)
> Comment on attachment 423054 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=423054&action=review
> 
> > Source/WebCore/animation/CSSPropertyAnimation.cpp:642
> > +class PositivePropertyWrapper : public PropertyWrapper<T> {
> 
> final
> 
> > Source/WebCore/animation/CSSPropertyAnimation.cpp:645
> > +    PositivePropertyWrapper(CSSPropertyID prop, T (RenderStyle::*getter)() const, void (RenderStyle::*setter)(T))
> 
> Please use words, not abbreviations. "property" instead of "prop"
> 
> > Source/WebCore/animation/CSSPropertyAnimation.cpp:650
> > +    void blend(const CSSPropertyBlendingClient* anim, RenderStyle* dst, const RenderStyle* from, const RenderStyle* to, double progress) const override
> 
> "animator" or "animation" or "client", instead of "anim"
> 
> "destination" instead of "dst"
> 
> final instead of override
> 
> I suggest making this private.

This is what happens when you lazily copy/paste code. I'll address those comments and follow up with another patch for bug 223148 which will fix similar issues throughout CSSPropertyAnimation.cpp.
Comment 5 Antoine Quint 2021-03-13 02:57:34 PST
Committed r274383 (235250@main): <https://commits.webkit.org/235250@main>
Comment 6 Radar WebKit Bug Importer 2021-03-13 02:58:14 PST
<rdar://problem/75392353>