stylo: Animation restyles are completely bogus for pseudo-elements.

RESOLVED FIXED

Status

()

Core
CSS Parsing and Computation
P1
normal
RESOLVED FIXED
3 months ago
3 days ago

People

(Reporter: emilio, Assigned: emilio)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Assignee)

Description

3 months ago
So I was debugging the animation failures of bug 1355351, and I thought that we might not be getting a proper animation rule.

While looking into it, I found out that we were sending animation-only restyles to the pseudo-element, which is completely bogus because we don't handle those in the traversal (need to send it to the parent instead).

Also, I don't see what code would make us follow the post-traversal updating styles in the frames for animation-only restyles, which is the bug bug 1355351 uncovers.
(Assignee)

Updated

3 months ago
Blocks: 1358580
I am looking this.
Assignee: nobody → hikezoe
Status: NEW → ASSIGNED
Apparently we are passing wrong parent element; 
https://hg.mozilla.org/mozilla-central/file/8e969cc9aff4/servo/components/style/gecko/wrapper.rs#l686
(Assignee)

Comment 3

3 months ago
Hi hiro,

I'm fixing a bunch of this stuff in bug 1331047, current tip is at https://hg.mozilla.org/try/rev/2f69e6cced796e04c882608def36ce61b198dd49. It needs a bunch more work, but if fixing this is non-trivial, perhaps we can wait for that?
Sure.  I will open a new bug for the issue in comment 2.
Also, I might be wrong, CSS animations on pseudo element work fine (except for the issue in comment 2).  I've been seeing two test cases [2] failure locally, and I thought it's caused by wrong pseudo element handling, but I noticed it's caused by bug 1336863.

[1] https://hg.mozilla.org/mozilla-central/file/8e969cc9aff4/dom/animation/test/css-animations/file_effect-target.html#l19
Assignee: hikezoe → nobody
Status: ASSIGNED → NEW
(Assignee)

Comment 5

3 months ago
Cool, thanks!

Yeah, the problem with transitions modulo that is that we actually may miss to update the frame style sometimes (which is what bug 1355351 exposes). I hope to fix that too with bug 1331047, but let's see :)
Assignee: nobody → emilio+bugs
Priority: -- → P1
(Assignee)

Comment 6

3 days ago
This should also be fixed now.
Status: NEW → RESOLVED
Last Resolved: 3 days ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.