Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[css-animations] composite operation of implicit keyframes for CSS An…
…imations should be "replace"

https://bugs.webkit.org/show_bug.cgi?id=251499

Reviewed by Antti Koivisto.

The CSS Animations spec indicates in its "Keyframes" section [0] that the default composite is
"replace". As such, when generating an implicit keyframe for an effect tied to a CSS Animation,
we should set the composite value to "replace".

This issue was caught by a new version of css/css-animations/KeyframeEffect-getKeyframes.tentative.html
which was not yet in our repository, so we update it with the most recent changes.

We make an additional change in the final subtest of this WPT to correctly have "replace" as
the composite value for the implicit keyframes. I suspect this change was not made when Google
last changed that test [1] because Chrome fails to generate the right implicit keyframes in this case.

[0] https://drafts.csswg.org/css-animations-2/#keyframes
[1] web-platform-tests/wpt@1a5c61d

* LayoutTests/imported/w3c/web-platform-tests/css/css-animations/KeyframeEffect-getKeyframes.tentative.html:
* Source/WebCore/rendering/style/KeyframeList.cpp:
(WebCore::KeyframeList::fillImplicitKeyframes):

Canonical link: https://commits.webkit.org/259739@main
  • Loading branch information
graouts committed Feb 2, 2023
1 parent a83d5ab commit 5292316
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 12 deletions.
Expand Up @@ -394,10 +394,12 @@

const frames = getKeyframes(div);

// Final keyframe should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
color: "rgb(0, 0, 255)" },
{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
{ offset: 1, computedOffset: 1, easing: "ease", composite: "replace",
color: "rgb(255, 255, 255)" },
];
assert_frame_lists_equal(frames, expected);
Expand All @@ -411,8 +413,10 @@

const frames = getKeyframes(div);

// Initial keyframe should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
{ offset: 0, computedOffset: 0, easing: "ease", composite: "replace",
color: "rgb(255, 255, 255)" },
{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
color: "rgb(0, 0, 255)" },
Expand All @@ -428,12 +432,14 @@

const frames = getKeyframes(div);

// Initial and final keyframes should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
{ offset: 0, computedOffset: 0, easing: "ease", composite: "replace",
color: "rgb(255, 255, 255)" },
{ offset: 0.5, computedOffset: 0.5, easing: "ease", composite: "auto",
color: "rgb(0, 0, 255)" },
{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
{ offset: 1, computedOffset: 1, easing: "ease", composite: "replace",
color: "rgb(255, 255, 255)" },
];
assert_frame_lists_equal(frames, expected);
Expand Down Expand Up @@ -631,8 +637,10 @@

const frames = getKeyframes(div);

// Initial keyframe should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
{ offset: 0, computedOffset: 0, easing: "ease", composite: "replace",
filter: "none" },
{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
filter: "blur(5px) sepia(60%) saturate(30%)" },
Expand Down Expand Up @@ -670,8 +678,10 @@

const frames = getKeyframes(div);

// Initial keyframe should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
{ offset: 0, computedOffset: 0, easing: "ease", composite: "replace",
textShadow: "rgb(0, 0, 0) 1px 1px 2px,"
+ " rgb(0, 0, 255) 0px 0px 16px,"
+ " rgb(0, 0, 255) 0px 0px 3.2px" },
Expand All @@ -694,8 +704,10 @@

assert_equals(frames.length, 2, "number of frames");

// Initial keyframe should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
{ offset: 0, computedOffset: 0, easing: "ease", composite: "replace",
backgroundSize: "auto" },
{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
backgroundSize: "50% auto, 6px auto, contain" },
Expand Down Expand Up @@ -724,8 +736,10 @@

const frames = getKeyframes(div);

// Initial keyframe should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
{ offset: 0, computedOffset: 0, easing: "ease", composite: "replace",
transform: "none" },
{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto",
transform: "translate(100px)" },
Expand All @@ -740,8 +754,10 @@

const frames = getKeyframes(div);

// Initial keyframe should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto",
{ offset: 0, computedOffset: 0, easing: "ease", composite: "replace",
marginBottom: "0px",
marginLeft: "0px",
marginRight: "0px",
Expand All @@ -762,8 +778,10 @@

const frames = getKeyframes(div);

// Initial keyframe should be replace as per sections 7 and 8 of
// https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "steps(2, start)", composite: "auto",
{ offset: 0, computedOffset: 0, easing: "steps(2, start)", composite: "replace",
color: "rgb(0, 0, 0)" },
{ offset: 1, computedOffset: 1, easing: "steps(2, start)", composite: "auto",
color: "rgb(0, 255, 0)" },
Expand Down Expand Up @@ -842,12 +860,14 @@

const frames = getKeyframes(div);

// Implicit initial and final keyframes should be replace as per sections
// 7 and 8 of https://drafts.csswg.org/css-animations-2/#keyframes
const expected = [
{ offset: 0, computedOffset: 0, easing: "linear", composite: "auto" },
{ offset: 0, computedOffset: 0, easing: "ease", composite: "auto", left: "auto" },
{ offset: 0, computedOffset: 0, easing: "ease", composite: "replace", left: "auto" },
{ offset: 0.5, computedOffset: 0.5, easing: "ease", composite: "auto", left: "10px" },
{ offset: 1, computedOffset: 1, easing: "linear", composite: "auto" },
{ offset: 1, computedOffset: 1, easing: "ease", composite: "auto", left: "auto" }
{ offset: 1, computedOffset: 1, easing: "ease", composite: "replace", left: "auto" }
];
assert_frame_lists_equal(frames, expected);
}, 'KeyframeEffect.getKeyframes() returns expected values for ' +
Expand Down
4 changes: 4 additions & 0 deletions Source/WebCore/rendering/style/KeyframeList.cpp
Expand Up @@ -189,6 +189,10 @@ void KeyframeList::fillImplicitKeyframes(const KeyframeEffect& effect, const Ren
keyframeValue.setStyle(styleResolver.styleForKeyframe(element, underlyingStyle, { nullptr }, keyframeRule, keyframeValue));
for (auto property : implicitProperties)
keyframeValue.addProperty(property);
// Step 2 of https://drafts.csswg.org/css-animations-2/#keyframes defines the
// default composite property as "replace" for CSS Animations.
if (is<CSSAnimation>(effect.animation()))
keyframeValue.setCompositeOperation(CompositeOperation::Replace);
insert(WTFMove(keyframeValue));
};

Expand Down

0 comments on commit 5292316

Please sign in to comment.