Here; https://hg.mozilla.org/mozilla-central/file/b1628ac71fcc/dom/animation/KeyframeEffectReadOnly.cpp#l727 nsIFrame* f = mTarget->mElement->GetPrimaryFrame(); while (f) { f->MarkNeedsDisplayItemRebuild(); f = f->GetNextContinuation(); } In case of pseudos, mTarget->mElement is the parent element of the pseudos, so it gets a wrong nsIFrame. Though this is actually independent from bug 1320608, but I do want to use KeyframeEffectReadyOnly::GetPrimaryFrame() which will be introduced in bug 1320608 here.

Hi Matt, I'd like to write a test case for this (a reftest I guess?), would you mind pointing reference test cases for retained display list in our tree?

Comment on attachment 8973585 [details] Bug 1459533 - Use GetPrimaryFrame() to get correct nsIFrame for the animation on pseudo elements. https://reviewboard.mozilla.org/r/241954/#review247800 ::: dom/animation/KeyframeEffectReadOnly.cpp:727 (Diff revision 1) > EffectSet* effectSet = > EffectSet::GetOrCreateEffectSet(mTarget->mElement, mTarget->mPseudoType); > effectSet->AddEffect(*this); > mInEffectSet = true; > UpdateEffectSet(effectSet); > - nsIFrame* f = mTarget->mElement->GetPrimaryFrame(); > + nsIFrame* f = GetPrimaryFrame(); I belive we have to use the primary frame instead of the style frame since it's for diplay list stuff. Apart from this patch, I will add another patch for the test case.

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1) > Hi Matt, I'd like to write a test case for this (a reftest I guess?), would > you mind pointing reference test cases for retained display list in our tree? MarkNeedsDisplayItemRebuild() rebuilds all display items for the given frame, and all descendants, so I believe the current code isn't wrong, just excessive. There are existing tests for this sort of thing in layout/reftests/display-list. If you create a page where there is another element that is a descendant of the pseudo's parent (and doesn't intersect the pseudo in screen space), and annotate it with class="reftest-no-display-list" then you should see it fail the current code and pass with the patch (if you trigger this invalidation code from MozReftestInvalidate).

(In reply to Matt Woodrow (:mattwoodrow) from comment #4) > (In reply to Hiroyuki Ikezoe (:hiro) from comment #1) > > Hi Matt, I'd like to write a test case for this (a reftest I guess?), would > > you mind pointing reference test cases for retained display list in our tree? > > MarkNeedsDisplayItemRebuild() rebuilds all display items for the given > frame, and all descendants, so I believe the current code isn't wrong, just > excessive. Oh I see. I didn't look into MarkNeedsDisplayItemRebuild. Thanks for the correction. > There are existing tests for this sort of thing in > layout/reftests/display-list. > > If you create a page where there is another element that is a descendant of > the pseudo's parent (and doesn't intersect the pseudo in screen space), and > annotate it with class="reftest-no-display-list" then you should see it fail > the current code and pass with the patch (if you trigger this invalidation > code from MozReftestInvalidate). Thanks for the detailed instruction! I could write the test case easily.

Comment on attachment 8973858 [details] Bug 1459533 - A reftest that animation on a pseudo elemnt. https://reviewboard.mozilla.org/r/242224/#review248072

Comment on attachment 8973585 [details] Bug 1459533 - Use GetPrimaryFrame() to get correct nsIFrame for the animation on pseudo elements. https://reviewboard.mozilla.org/r/241954/#review248074

Pushed by hikezoe@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d18a439c983f Use GetPrimaryFrame() to get correct nsIFrame for the animation on pseudo elements. r=mattwoodrow https://hg.mozilla.org/integration/autoland/rev/ccbde630d5c4 A reftest that animation on a pseudo elemnt. r=mattwoodrow

https://hg.mozilla.org/mozilla-central/rev/d18a439c983f https://hg.mozilla.org/mozilla-central/rev/ccbde630d5c4