Closed
Bug 1301611
Opened 7 years ago
Closed 6 years ago
Need test cases that changedAnimations is notified to mutation observers when keyframes are changed
Categories
(Core :: DOM: Animation, defect, P5)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: hiro, Assigned: gabrielle.singhcadieux, Mentored)
References
Details
(Keywords: good-first-bug)
Attachments
(1 file, 1 obsolete file)
1.77 KB,
patch
|
boris
:
review+
|
Details | Diff | Splinter Review |
We have no test cases when we call setKeyframes().
Reporter | ||
Comment 1•7 years ago
|
||
Set spacing test case in test_animation_observers.html will be a reference. http://hg.mozilla.org/mozilla-central/file/82d0a583a9a3/dom/animation/test/chrome/test_animation_observers.html#l1970
Reporter | ||
Updated•6 years ago
|
Mentor: boris.chiou
Keywords: good-first-bug
Assignee | ||
Comment 2•6 years ago
|
||
I'd like to work on this!
Comment 3•6 years ago
|
||
Thanks Gabrielle! Since you fixed bug 1293495, I think you know what to do but please let me (or Boris or Hiro) know if you need help!
Assignee: nobody → gabrielle.singhcadieux
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•6 years ago
|
||
I hope that this test is sufficient! It builds and passes.
Attachment #8867796 -
Flags: review?(boris.chiou)
Comment 5•6 years ago
|
||
Comment on attachment 8867796 [details] [diff] [review] 1301611.patch Review of attachment 8867796 [details] [diff] [review]: ----------------------------------------------------------------- LGTM. ::: dom/animation/test/chrome/test_animation_observers_sync.html @@ +870,5 @@ > + assert_equals_records(observer.takeRecords(), > + [{ added: [], changed: [anim], removed: [] }], > + "records after keyframes are set to empty"); > + > + }, "setKeyframes"); It seems that we use snake_case in this test file, so it's better to write "set_keyframes".
Attachment #8867796 -
Flags: review?(boris.chiou) → review+
Assignee | ||
Comment 6•6 years ago
|
||
(In reply to Boris Chiou [:boris] from comment #5) > Comment on attachment 8867796 [details] [diff] [review] > 1301611.patch > > Review of attachment 8867796 [details] [diff] [review]: > ----------------------------------------------------------------- > > LGTM. > > ::: dom/animation/test/chrome/test_animation_observers_sync.html > @@ +870,5 @@ > > + assert_equals_records(observer.takeRecords(), > > + [{ added: [], changed: [anim], removed: [] }], > > + "records after keyframes are set to empty"); > > + > > + }, "setKeyframes"); > > It seems that we use snake_case in this test file, so it's better to write > "set_keyframes". No problem, I will change that! I was trying to make reference to the use of the `setKeyframes()` function, specifically.
Assignee | ||
Comment 7•6 years ago
|
||
Attachment #8867796 -
Attachment is obsolete: true
Attachment #8868354 -
Flags: review?(boris.chiou)
Comment 8•6 years ago
|
||
Comment on attachment 8868354 [details] [diff] [review] 1301611_v2.patch Review of attachment 8868354 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for updating. LGTM
Attachment #8868354 -
Flags: review?(boris.chiou) → review+
Comment 9•6 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9f0ea2dfc0d68631865bd14b712a7570aca9b20
Updated•6 years ago
|
Keywords: checkin-needed
Comment 10•6 years ago
|
||
Pushed by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/2a0949cc856f Test that mutation observer is notified when keyframes are changed using setKeyframes(). r=boris
Keywords: checkin-needed
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2a0949cc856f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in
before you can comment on or make changes to this bug.
Description
•