Closed
Bug 1371388
Opened 7 years ago
Closed 7 years ago
Stylo: animation removal mutation not fired on class change
Categories
(Core :: CSS Parsing and Computation, defect, P2)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: jryans, Assigned: heycam)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
text/html
|
Details |
DevTools expects to receive an animation mutation (via MutationObserver) when an animation is removed because of a class change, but this appears to not happen with Stylo.
This leads to timeouts in tests like:
* devtools/server/tests/browser/browser_animation_emitMutations.js[1]
[1]: https://treeherder.mozilla.org/logviewer.html#?job_id=105301401&repo=try&lineNumber=3470
Comment 1•7 years ago
|
||
Interesting. Initially I thought we don't cancel the animation when the corresponding class name is removed, but as far as I can see, the animation is cancelled.
Comment 2•7 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
> Interesting. Initially I thought we don't cancel the animation when the
> corresponding class name is removed, but as far as I can see, the animation
> is cancelled.
There are definitely some cases for transitions where we don't cancel them, but for animations we should.
Comment 3•7 years ago
|
||
This may be the cause of the failures in dom/animation/test/chrome/test_animation_observers_a?sync.html too (a test I plan to look into later today).
Reporter | ||
Comment 4•7 years ago
|
||
The test page used by this DevTools test is:
http://searchfox.org/mozilla-central/rev/a798ee4fc323f9387b7576dbed177859d29d09b7/devtools/server/tests/browser/animation.html
Specifically, it appears to change the node with the class "not-animated" to "multiple-animations" (which seems to work) and then later back to "not-animated" (which is the step where removed animations are missed).
The test page seems to contain a mix of both animations and transitions together.
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → cam
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•7 years ago
|
||
This test passes for me, locally. (Despite a few assertions, which are bug 1189015.) Ryan, can you confirm that this test is passing on try in your next try run?
Flags: needinfo?(jryans)
Assignee | ||
Comment 6•7 years ago
|
||
(In reply to Brian Birtles (:birtles) from comment #3)
> This may be the cause of the failures in
> dom/animation/test/chrome/test_animation_observers_a?sync.html too (a test I
> plan to look into later today).
The sync version of that test is passing for me too. The async version is failing, and I think that's covered by bug 1376248.
Reporter | ||
Comment 7•7 years ago
|
||
(In reply to Cameron McCormack (:heycam) from comment #5)
> This test passes for me, locally. (Despite a few assertions, which are bug
> 1189015.) Ryan, can you confirm that this test is passing on try in your
> next try run?
Looks like it is now passing on try and locally for me as well! :)
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: needinfo?(jryans)
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•