Bug 158199 – Multiple selectors break keyframes animation

• Component: CSS
• Status: RESOLVED FIXED

[Matthew Gertner]

I am using CSS code like the following:

@keyframes foo {
  0% {
    transform: scale(0.3);
    opacity: 1;
  }
  50% {
    transform: scale(1);
    opacity: 0;
  }
  100% {
    transform: scale(2);
    opacity: 0;
  }
}

This works fine, but when run through css-nano, some selectors are combined as so:

@keyframes foo {
  0% {
    transform: scale(0.3);
    opacity: 1;
  }
  50% {
    transform: scale(1);
  }
  50%,to {
    opacity: 0;
  }
  to {
    transform: scale(2);
  }
}

Looks like in this case the opacity rule is applied to 50% but not to 100%. This appears to work fine in other browsers I tested.

[Radar WebKit Bug Importer]

<rdar://problem/26652591>

[Simon Fraser (smfr)]

Created attachment 280638 [details]
Testcase

[Simon Fraser (smfr)]

KeyframeList::insert() is just replacing earlier keyframes with later ones having the same key, rather than merging.

[Dean Jackson]

Created attachment 280815 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 280815 [details]
Patch

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

> Source/WebCore/css/StyleResolver.cpp:525
> +    const Vector<Ref<StyleKeyframe>>* keyframes = &keyframesRule->keyframes();

auto?

> Source/WebCore/css/StyleResolver.cpp:550
> +                    Ref<StyleKeyframe> styleKeyframe = StyleKeyframe::create(MutableStyleProperties::create());

auto?

> Source/WebCore/css/StyleResolver.cpp:552
> +                    styleKeyframe.ptr()->mutableProperties().mergeAndOverrideOnConflict(originalKeyframe->properties());

Weird to call mergeAndOverrideOnConflict() when mutableProperties() is empty. Could we not have just passed the properties to MutableStyleProperties::create() ?

> Source/WebCore/css/StyleResolver.cpp:566
> +

No blank line.

> LayoutTests/animations/duplicate-keys-expected.html:33
> +    testRunner.waitUntilDone();
> +    window.addEventListener("load", function () {
> +        var box = document.querySelector(".box");
> +        internals.pauseAnimationAtTimeOnElement("foo", 0.1, box);
> +        testRunner.notifyDone();

You don't need to use waitUntilDone/notifyDone if the test finishes inside the load event.

[Dean Jackson]

Committed r201818: <http://trac.webkit.org/changeset/201818>