[webkit-reviews] review denied: [Bug 85253] Apply animations and transitions for first-letter element : [Attachment 140139] Patch V4.

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Igor Trindade Oliveira
<igor.oliveira at webkit.org>'s request for review:
Bug 85253: Apply animations and transitions for first-letter element
https://bugs.webkit.org/show_bug.cgi?id=85253

Attachment 140139: Patch V4.
https://bugs.webkit.org/attachment.cgi?id=140139&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=140139&action=review


Tests still need to be improved.

> LayoutTests/animations/first-letter-animation.html:13
> +    .box {
> +	 position: relative;
> +	 left: 400px;
> +	 top: 100px;
> +	 width: 100px;
> +	 height: 100px;
> +	 background: blue;
> +	 -webkit-animation: boxAnim 1s linear;

A one second test is way too slow. Can this test be changed to use the
pauseAPI? (though that might need fixing for pseudos).

> LayoutTests/animations/first-letter-animation.html:29
> +    @-webkit-keyframes firstLetterAnim {
> +	 from { margin: 0px; }
> +	 to { margin: 100px; }
> +    }
> +
> +    @-webkit-keyframes boxAnim {
> +	 from { -webkit-transform: scale(1); }
> +	 to { -webkit-transform: scale(3); }

The interesting case is the same property being animated (in different ways) on
the element and the pseudo, but you're not testing that here.

You should also test that setting animation-play-state affects the correct
animation.

> LayoutTests/transitions/first-letter-color-transition.html:15
> +	 -webkit-transition: color 1s;

Ditto.

> LayoutTests/transitions/first-letter-transition.html:23
> +	 -webkit-transition: color 1s;
> +    }
> +
> +    #first {
> +	 -webkit-transition: left 1s;
> +    }
> +
> +    #second {
> +	 -webkit-transition: color 1s;

This test is also way too slow. Can it use the transition-test-helpers.js?

You also need to test a transition of the same property on the element and the
pseudo, preferably where the transitions begin at different times.