Need test cases that changedAnimations is notified to mutation observers when keyframes are changed
RESOLVED
FIXED
in Firefox 55
Status
()
People
(Reporter: hiro, Assigned: gabrielle.singhcadieux, Mentored)
Tracking
({good-first-bug})
Firefox Tracking Flags
(firefox55 fixed)
Details
Attachments
(1 attachment, 1 obsolete attachment)
|
1.77 KB,
patch
|
boris
:
review+
|
Details | Diff | Splinter Review |
We have no test cases when we call setKeyframes().
|
(Reporter) | |
Comment 1•3 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•2 years ago
|
Mentor: boris.chiou
Keywords: good-first-bug
|
(Assignee) | |
Comment 2•2 years ago
|
||
I'd like to work on this!
|
||
Comment 3•2 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•2 years ago
|
||
Created attachment 8867796 [details] [diff] [review] 1301611.patch I hope that this test is sufficient! It builds and passes.
Attachment #8867796 -
Flags: review?(boris.chiou)
|
||
Comment 5•2 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•2 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•2 years ago
|
||
Created attachment 8868354 [details] [diff] [review] 1301611_v2.patch
Attachment #8867796 -
Attachment is obsolete: true
Attachment #8868354 -
Flags: review?(boris.chiou)
|
|
||
Comment 8•2 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•2 years ago
|
||
try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b9f0ea2dfc0d68631865bd14b712a7570aca9b20
|
|
||
Updated•2 years ago
|
Keywords: checkin-needed
|
||
Comment 10•2 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•2 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/2a0949cc856f
Status: ASSIGNED → RESOLVED
Last Resolved: 2 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
•