Closed
Bug 1267547
Opened 9 years ago
Closed 9 years ago
Need an automation test which checks behavior of script animations on display:none element
Categories
(Core :: DOM: Animation, defect)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: hiro, Unassigned)
Details
Attachments
(1 file)
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.
Reporter | ||
Comment 1•9 years ago
|
||
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.
Reporter | ||
Comment 2•9 years ago
|
||
(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?
Reporter | ||
Comment 3•9 years ago
|
||
We need to clear isRunningOnCompositor flag in nsFrame::DestroyFrom.
Reporter | ||
Comment 4•9 years ago
|
||
Reporter | ||
Comment 5•9 years ago
|
||
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/
Attachment #8745739 -
Flags: review?(bbirtles)
Comment 6•9 years ago
|
||
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.
Attachment #8745739 -
Flags: review?(bbirtles) → review+
Reporter | ||
Comment 7•9 years ago
|
||
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/
Reporter | ||
Comment 8•9 years ago
|
||
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. ;-)
Reporter | ||
Comment 10•9 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)
> We need to clear isRunningOnCompositor flag in nsFrame::DestroyFrom.
Filed bug 1268385.
Comment 11•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•