PostRestyleForAnimation should also work on non-existing pseudo element
Categories
(Core :: DOM: Animation, defect, P3)
Tracking
()
People
(Reporter: boris, Assigned: boris)
References
Details
Attachments
(1 file)
Assignee | ||
Comment 1•4 years ago
•
|
||
While working on Bug 1610981, I notice an issue from wpt. For example:
pseudoa::before {
margin-left: 10px;
}
const d = createDiv(t);
d.classList.add('pseudoa');
const effect = new KeyframeEffect(null, { 'marginLeft': ['0px', '100px'] }, 100 * MS_PER_SEC);
const anim = new Animation(effect, document.timeline);
anim.play();
anim.currentTime = 50 * MS_PER_SEC;
effect.target = d;
effect.pseudoElement = '::before';
assert_equals(getComputedStyle(d, '::before').marginLeft, '50px',
'Value at 50% progress after setting new target'); // failed
We hit the assertion because we don't posts restyles for animtion when setting target/pseudoElement (i.e. we don't have the generated element for d::before
in this case).
Assignee | ||
Comment 2•4 years ago
•
|
||
As Emilio mentioned, we shouldn't have restyling involved in this case, so this may be a bug when calling getComputedStyle().
Comment 3•4 years ago
|
||
Yeah, in particular:
ah, so here may be the issue: https://searchfox.org/mozilla-central/rev/d5b34cc8872177d3ee071e06f787c2a14268595b/servo/components/style/style_resolver.rs#515
there we don't account for the animation rules for the pseudo-element. That's the "pseudo which doesn't exist yet" code-path
Assignee | ||
Comment 4•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)
Yeah, in particular:
ah, so here may be the issue: https://searchfox.org/mozilla-central/rev/d5b34cc8872177d3ee071e06f787c2a14268595b/servo/components/style/style_resolver.rs#515
there we don't account for the animation rules for the pseudo-element. That's the "pseudo which doesn't exist yet" code-path
Great. I just hacked this function to let GetAnimationRules() accept a PseudoStyleType. And it looks good to me.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 5•4 years ago
•
|
||
OK. There is one extra issue in this bug. It seems the update of cascade results after setting the new non-existing target has issues. So when we call nsComputedDOMStyle::GetComputedStyleNoFlush()
(by getComputedStyle(div)
or KeyframeEffect::GetTargetComputedStyle
), which gets animation rules for pseudos and tries to compose its style, EffectCompositor will passes a list of properties for skip into each KeyframeEffects, and I think this list is still out-of-date (because it includes margin-left
in my case), so KeyframeEffect::ComposeStyle()
skips this property. Therefore, EffectCompositor::GetServoAnimationRule()
returns an empty list of animation rules. Then the tests failed.
Need to figure out how to make sure EffectSet::PropertiesForAnimationsLevel()
is up-to-dated when setting a non-existing pseudo-element as the new target.
Comment 6•4 years ago
|
||
Assignee | ||
Comment 7•4 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)
that is https://searchfox.org/mozilla-central/rev/c79c0d65a183d9d38676855f455a5c6a7f7dadd3/dom/animation/EffectCompositor.cpp#409, right?
Yes, I think this is the case.
Assignee | ||
Comment 8•4 years ago
|
||
And becaue there is no restyling involved for non-existing
pseudo-elements involved, so we have to update cascade result if needed.
Updated•4 years ago
|
Description
•