Bug 158199

Summary: Multiple selectors break keyframes animation
Product: WebKit Reporter: Matthew Gertner <matthew>
Component: CSSAssignee: Dean Jackson <dino>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: iOS 9.3   
Attachments:
Description Flags
Testcase
none
Patch none

Description Matthew Gertner 2016-05-30 03:04:22 PDT
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.
Comment 1 Radar WebKit Bug Importer 2016-06-06 10:43:17 PDT
<rdar://problem/26652591>
Comment 2 Simon Fraser (smfr) 2016-06-06 15:33:03 PDT
Created attachment 280638 [details]
Testcase
Comment 3 Simon Fraser (smfr) 2016-06-06 15:59:38 PDT
KeyframeList::insert() is just replacing earlier keyframes with later ones having the same key, rather than merging.
Comment 4 Dean Jackson 2016-06-08 11:05:15 PDT
Created attachment 280815 [details]
Patch
Comment 5 Simon Fraser (smfr) 2016-06-08 11:09:53 PDT
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.
Comment 6 Dean Jackson 2016-06-08 11:45:25 PDT
Committed r201818: <http://trac.webkit.org/changeset/201818>