Bug 223124 - Fix interpolation of orphans and widows CSS properties
Summary: Fix interpolation of orphans and widows CSS properties
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Animations (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-12 08:56 PST by Antoine Quint
Modified: 2021-03-13 02:58 PST (History)
8 users (show)

See Also:

Attachments
Patch (38.09 KB, patch)
2021-03-12 08:58 PST, Antoine Quint 		darin: review+
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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>