[Webkit-unassigned] [Bug 85253] Apply animations and transitions for first-letter element

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Igor Trindade Oliveira <igor.oliveira at webkit.org>  2012-06-11 15:24:59 PST ---
(In reply to comment #17)
> (From update of attachment 145416 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=145416&action=review
> 
> I think this needs one more round, but it's close. If you rename generatingNode(), you should do it as a first step in a separate patch.
> 

Ok i will open a bug.

> > LayoutTests/animations/first-letter-play-state.html:11
> > +      -webkit-animation: "move1" 0.2s linear;
> 
> Keyframe names are IDENTS and should not be quoted.
> 
> > LayoutTests/animations/first-letter-play-state.html:17
> > +    @-webkit-keyframes "move1" {
> 
> Ditto.
> 

Ok.

> > LayoutTests/animations/first-letter-play-state.html:21
> > +    @-webkit-keyframes "firstLetterAnimation" {
> 
> Ditto.

Ok.

> 
> > LayoutTests/animations/first-letter-play-state.html:43
> > +    function stop()
> > +    {
> > +        document.getElementById("box1").style.webkitAnimationPlayState = "paused";
> > +    }
> > +
> > +    function start()
> > +    {
> > +        document.getElementById("box1").style.webkitAnimationPlayState = "running";
> > +    }
> 
> This isn't testing play-state on the pseudo animation, which I think is the interesting thing.
> 

You were right. I found a bug testing play-state on the pseudo element.

> > LayoutTests/animations/first-letter-play-state.html:61
> > +        }
> > +            
> > +    }
> 
> Extra line here.

Ok.

> 
> > LayoutTests/transitions/first-letter-color-transition.html:42
> > +    window.addEventListener( 'webkitTransitionEnd', transitionEnded, false );
> 
> Extra spaces inside the parens.

Ok

> 
> > Source/WebCore/rendering/RenderObject.h:585
> > +    Node* styledGeneratingNode() const;
> 
> The comment for generatingNode() seems totally wrong, so maybe that one should be renamed. I'm not a big fan of styledGeneratingNode().
> 

I also am not a big fan of styledGeneratingNode(), however i am not have a better name. I will open a bug to unify generatingNode and styledGeneratingNode so we can remove styledGeneratingNode soon.

> > Source/WebCore/rendering/RenderObject.h:668
> > +    void setAnimatableStyle(PassRefPtr<RenderStyle>, bool isGeneratedContent = false);
> 
> I don't like the boolean parameter here. It's hard to read at the call site. Maybe two methods would be clearer.

Yeah, in fact we do not need anymore this method. I am not animating the RenderTextFragment directly.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.