1267547 - Need an automation test which checks behavior of script animations on display:none element

Status: RESOLVED FIXED

(Core :: DOM: Animation, defect)

Description

[Hiroyuki Ikezoe (:hiro)]

Script animations keep running even when the target element has display:none, but should not cause any restyles.  We need a test to check this behavior.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

A test case revealed that compositor animations keep running on the compositor even when the target element has display:none.  Thanks for the test!  It does not really matter for now, but we need to fix it at some point.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
> A test case revealed that compositor animations keep running on the
> compositor even when the target element has display:none.  Thanks for the
> test!  It does not really matter for now, but we need to fix it at some
> point.

I might be wrong.  isRunningOnCompositor flag might be not cleared in that case?

Comment 3

[Hiroyuki Ikezoe (:hiro)]

We need to clear isRunningOnCompositor flag in nsFrame::DestroyFrom.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8aeba1686d0e

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: MozReview Request: Bug 1267547 - Test to check script animations keep running on display:none element but does not cause restyles. r?birtles

Currently isRunningOnCompositor flag is not cleared when 'display:none'
is set to the target element.
We need to fix it at some point.

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

Comment 6

[Brian Birtles (:birtles)]

Comment on attachment 8745739 [details]
MozReview Request: Bug 1267547 - Test to check script animations keep running on display:none element but does not cause restyles. r?birtles

https://reviewboard.mozilla.org/r/49095/#review45941

r=me

Please retrigger a few more times on other platforms on try to make sure we're not introducing a new intermittent here.

::: dom/animation/test/chrome/test_restyles.html:381
(Diff revision 1)
> +
> +    yield animation.ready;
> +
> +    div.style.display = 'none';
> +
> +    // We need a frame to apply display:none style.

// We need to wait a frame to apply display:none

?

(Otherwise some people might think 'need a frame' means 'need an nsFrame'?)

::: dom/animation/test/chrome/test_restyles.html:398
(Diff revision 1)
> +    is(markers.length, 0,
> +       'Script animations on "display: none" element should not update styles');
> +
> +    div.style.display = '';
> +
> +    // We need a frame to unapply display:none style.

s/need a frame/need to wait a frame/

Likewise two places in the next test.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Comment on attachment 8745739 [details]
MozReview Request: Bug 1267547 - Test to check script animations keep running on display:none element but does not cause restyles. r?birtles

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

Comment 8

[Hiroyuki Ikezoe (:hiro)]

https://reviewboard.mozilla.org/r/49095/#review45941

> // We need to wait a frame to apply display:none
> 
> ?
> 
> (Otherwise some people might think 'need a frame' means 'need an nsFrame'?)

Thanks!  Actually I was sometimes confused it last year. ;-)

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b3087c375b1f

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)
> We need to clear isRunningOnCompositor flag in nsFrame::DestroyFrom.

Filed bug 1268385.

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b3087c375b1f