Open Bug 1615083 Opened 4 years ago Updated 4 years ago

PostRestyleForAnimation should also work on non-existing pseudo element

Categories

(Core :: DOM: Animation, defect, P3)

defect

Tracking

()

People

(Reporter: boris, Assigned: boris)

References

Details

Attachments

(1 file)

No description provided.

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).

As Emilio mentioned, we shouldn't have restyling involved in this case, so this may be a bug when calling getComputedStyle().

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

(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: nobody → boris.chiou

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.

(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.

And becaue there is no restyling involved for non-existing
pseudo-elements involved, so we have to update cascade result if needed.

Severity: normal → S4
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: