Closed Bug 1301611 Opened 9 years ago Closed 8 years ago

Need test cases that changedAnimations is notified to mutation observers when keyframes are changed

Categories

(Core :: DOM: Animation, defect, P5)

defect

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)

We have no test cases when we call setKeyframes().
Mentor: boris.chiou
Keywords: good-first-bug
I'd like to work on this!
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
Attached patch 1301611.patch (obsolete) — Splinter Review
I hope that this test is sufficient! It builds and passes.
Attachment #8867796 - Flags: review?(boris.chiou)
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+
(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.
Attached patch 1301611_v2.patchSplinter Review
Attachment #8867796 - Attachment is obsolete: true
Attachment #8868354 - Flags: review?(boris.chiou)
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+
Keywords: checkin-needed
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
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1366441
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: