1346408 - stylo: CSS animations on pseudo elements

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[Hiroyuki Ikezoe (:hiro)]

I did totally misunderstand about the pseudo styles on servo.
We had to pass pseudo_style to process_animations.

Comment 1

[Bobby Holley (:bholley)]

Note that I'm reworking pseudo-element handling in bug 1331047. Hopefully that will land next week, but probably best not to work on this until that lands.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Bobby, do you change how pseudo-styles are stored on elements? Otherwise this is probably not really dependent on it.

This is about the pseudo-element styles in bug 1341985 comment 47.

Comment 3

[Bobby Holley (:bholley)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> Bobby, do you change how pseudo-styles are stored on elements? Otherwise
> this is probably not really dependent on it.
> 
> This is about the pseudo-element styles in bug 1341985 comment 47.

I'm not sure I understand exactly what this bug is about after reading that comment, so I can't tell.

My changes mostly revolve around resolving the correct style for NAC-backed pseudo-elements like ::before and ::after, particularly in cases where the pseudos are nested, and particular during restyle. That may be unrelated to this bug, I just didn't want Hiro to spend a bunch of time debugging weird pseudo-related edge cases when that architecture will soon be overhauled.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #3)
> (In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> > Bobby, do you change how pseudo-styles are stored on elements? Otherwise
> > this is probably not really dependent on it.
> > 
> > This is about the pseudo-element styles in bug 1341985 comment 47.
> 
> I'm not sure I understand exactly what this bug is about after reading that
> comment, so I can't tell.
> 
> My changes mostly revolve around resolving the correct style for NAC-backed
> pseudo-elements like ::before and ::after, particularly in cases where the
> pseudos are nested, and particular during restyle. That may be unrelated to
> this bug, I just didn't want Hiro to spend a bunch of time debugging weird
> pseudo-related edge cases when that architecture will soon be overhauled.

Yeah, this is unrelated then, this is about choosing the wrong style for inheritance, but fixing this doesn't change how it's computed or anything. So this bug can be worked on then.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #4)
> (In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #3)
> > (In reply to Emilio Cobos Álvarez [:emilio] from comment #2)
> > > Bobby, do you change how pseudo-styles are stored on elements? Otherwise
> > > this is probably not really dependent on it.
> > > 
> > > This is about the pseudo-element styles in bug 1341985 comment 47.
> > 
> > I'm not sure I understand exactly what this bug is about after reading that
> > comment, so I can't tell.
> > 
> > My changes mostly revolve around resolving the correct style for NAC-backed
> > pseudo-elements like ::before and ::after, particularly in cases where the
> > pseudos are nested, and particular during restyle. That may be unrelated to
> > this bug, I just didn't want Hiro to spend a bunch of time debugging weird
> > pseudo-related edge cases when that architecture will soon be overhauled.
> 
> Yeah, this is unrelated then, this is about choosing the wrong style for
> inheritance

Ok, I assume you mean inheritance when computing eagerly cascaded pseudos on the originating element? If so, that should be ok. Bug 1331047 is all about picking the correct style for the elements that implement those eagerly-cascaded pseudos.

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Thank you guys! I will post a fix here soon.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1346408 - Pass pseudo computed style and its parent style of the pseudo element.

Review commit: https://reviewboard.mozilla.org/r/119842/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119842/

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1346408 - Update mochitest expectation.

Review commit: https://reviewboard.mozilla.org/r/119844/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/119844/

Comment 9

[Cameron McCormack (:heycam)]

Comment on attachment 8846844 [details]
Bug 1346408 - Pass pseudo computed style and its parent style of the pseudo element.

https://reviewboard.mozilla.org/r/119842/#review121848

::: commit-message-72741:1
(Diff revision 1)
> +Bug 1346408 - Pass pseudo computed style and its parent style of the pseudo element. r?heycam

Pass to what?  Let's be a bit clearer in the commit message, maybe something like:

"Use computed styles of the pseudo and its parent when calling Gecko_UpdateAnimations to update CSS Animations on pseudo-elements."

Comment 10

[Cameron McCormack (:heycam)]

Comment on attachment 8846845 [details]
Bug 1346408 - Update mochitest expectation.

https://reviewboard.mozilla.org/r/119844/#review121852

Comment 11

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8846845 [details]
Bug 1346408 - Update mochitest expectation.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/119844/diff/1-2/

Comment 12

[Hiroyuki Ikezoe (:hiro)]

https://github.com/servo/servo/pull/15943

Comment 13

[Hiroyuki Ikezoe (:hiro)]

Thank you Cameron!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/126f0f9a82d2
Update mochitest expectation. r=heycam

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/126f0f9a82d2