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)
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•9 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•8 years ago
|
Mentor: boris.chiou
Keywords: good-first-bug
Assignee | ||
Comment 2•8 years ago
|
||
I'd like to work on this!
Comment 3•8 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•8 years ago
|
||
I hope that this test is sufficient!
It builds and passes.
Attachment #8867796 -
Flags: review?(boris.chiou)
Comment 5•8 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•8 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•8 years ago
|
||
Attachment #8867796 -
Attachment is obsolete: true
Attachment #8868354 -
Flags: review?(boris.chiou)
Comment 8•8 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•8 years ago
|
||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 10•8 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•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 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
•