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

[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 139687: Patch V3
https://bugs.webkit.org/attachment.cgi?id=139687&action=review

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


> LayoutTests/ChangeLog:11
> +	   * animations/first-letter-animation-expected.txt: Added.
> +	   * animations/first-letter-animation.html: Added.
> +	   * transitions/first-letter-color-transition-expected.txt: Added.
> +	   * transitions/first-letter-color-transition.html: Added.

I think you should also test:
* transition on both an element and its pseudoelement, of the same and
different properties
* animation on an element and its pseudoelement, to check that they can be
controlled independently.

> Source/WebCore/rendering/RenderBlock.cpp:5975
> +    RefPtr<RenderStyle> temporaryStyle = RenderStyle::create();
> +    temporaryStyle->inheritFrom(firstLetterBlock->style());
> +    firstLetter->setStyle(temporaryStyle);  
>      firstLetterContainer->addChild(firstLetter, currentChild);
> +    
> +    RefPtr<RenderStyle> pseudoElementStyle =
animation()->updateAnimations(firstLetter, pseudoStyle);
> +    firstLetter->setStyle(pseudoElementStyle);

It seems odd to call updateAnimaitons explicitly here. I think this logic
should be wrapped up in a RenderObject method that we can possibly use
elsewhere.