Closed
Bug 1264101
Opened 9 years ago
Closed 9 years ago
Various animation inspector tests are going to permafail when Gecko 48 merges to Beta (ReferenceError: KeyframeEffect is not defined)
Categories
(DevTools :: Inspector: Animations, defect, P1)
DevTools
Inspector: Animations
Tracking
(firefox46 unaffected, firefox47+ fixed, firefox48+ fixed)
RESOLVED
FIXED
Firefox 48
Tracking | Status | |
---|---|---|
firefox46 | --- | unaffected |
firefox47 | + | fixed |
firefox48 | + | fixed |
People
(Reporter: RyanVM, Assigned: jdescottes)
References
Details
Attachments
(1 file)
+++ This bug was initially created as a clone of Bug #1261651 +++
[Tracking Requested - why for this release]: Devtools permafail when Gecko 48 merges to Aurora.
Because the fun never stops around here :-)
https://treeherder.mozilla.org/logviewer.html#?job_id=19232205&repo=try
00:03:15 INFO - 14 INFO TEST-START | devtools/client/animationinspector/test/browser_animation_click_selects_animation.js
00:03:15 INFO - TEST-INFO | started process screentopng
00:03:17 INFO - TEST-INFO | screentopng: exit 0
00:03:17 INFO - 15 INFO checking window state
00:03:17 INFO - 16 INFO Entering test bound
00:03:17 INFO - 17 INFO Adding a new tab with URL: http://example.com/browser/devtools/client/animationinspector/test/doc_simple_animation.html
00:03:17 INFO - 18 INFO Waiting for event: 'load' on [object XULElement].
00:03:17 INFO - 19 INFO TEST-UNEXPECTED-FAIL | devtools/client/animationinspector/test/browser_animation_click_selects_animation.js | uncaught exception - ReferenceError: KeyframeEffect is not defined at @http://example.com/browser/devtools/client/animationinspector/test/doc_simple_animation.html:121:9
00:03:17 INFO - Stack trace:
00:03:17 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:simpletestOnerror:1563
00:03:17 INFO - JavaScript error: http://example.com/browser/devtools/client/animationinspector/test/doc_simple_animation.html, line 121: ReferenceError: KeyframeEffect is not defined
00:03:17 INFO - 20 INFO Console message: [JavaScript Error: "ReferenceError: KeyframeEffect is not defined" {file: "http://example.com/browser/devtools/client/animationinspector/test/doc_simple_animation.html" line: 121}]
00:03:17 INFO - @http://example.com/browser/devtools/client/animationinspector/test/doc_simple_animation.html:121:9
etc.
Flags: needinfo?(pbrosset)
Reporter | ||
Comment 1•9 years ago
|
||
Whoops, s/Aurora/Beta. Not quite as urgent as originally hyped to be :)
Severity: critical → major
Summary: Various animation inspector tests are going to permafail when Gecko 48 merges to Aurora (ReferenceError: KeyframeEffect is not defined) → Various animation inspector tests are going to permafail when Gecko 48 merges to Beta (ReferenceError: KeyframeEffect is not defined)
Comment 2•9 years ago
|
||
Similar to bug 1261651, the WebAnimations API isn't enabled fully in various releases. Pinging Brian so he can explain what's the way forward here.
Flags: needinfo?(pbrosset) → needinfo?(bbirtles)
Comment 3•9 years ago
|
||
For Chrome tests you don't need to do anything. For content tests you need to make sure the dom.animations-api.core.enabled pref is set.
If you're using a test framework that doesn't let you set prefs before running the test, you might need to set the pref in an outer doc, then load the test in an iframe to make sure the necessary globals are available. e.g. https://dxr.mozilla.org/mozilla-central/source/layout/style/test/test_animations_pausing.html
Flags: needinfo?(bbirtles)
Comment 4•9 years ago
|
||
Thanks Brian. I was under the impression that the pref was only required to use el.animate, and I specifically used the various animation constructors to avoid using el.animate in those tests.
Anyway, I'll fix this.
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Comment 5•9 years ago
|
||
(In reply to Patrick Brosset [:pbro] from comment #4)
> Thanks Brian. I was under the impression that the pref was only required to
> use el.animate, and I specifically used the various animation constructors
> to avoid using el.animate in those tests.
> Anyway, I'll fix this.
No, it's just that we explicitly disabled el.animate on Fx 47 due to compat issues that are fixed in Fx 48. That said, we are going to introduce a separate pref for el.animate (dom.animations-api.element-animate.enabled) in bug 1245000 but DevTools shouldn't be affected by that. As long as you set dom.animations-api.core.enabled for all content tests you get the whole API.
Assignee | ||
Comment 7•9 years ago
|
||
WebAnimations API is not enabled by default in all release channels yet.
It is behing the pref dom.animations-api.core.enabled.
Turn the preference on before any test of the animationinspector, and
clear it after the test.
Review commit: https://reviewboard.mozilla.org/r/46711/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/46711/
Attachment #8741730 -
Flags: review?(pbrosset)
Updated•9 years ago
|
Attachment #8741730 -
Flags: review?(pbrosset) → review+
Comment 8•9 years ago
|
||
Comment on attachment 8741730 [details]
MozReview Request: Bug 1264101 - force dom.animations-api.core.enabled pref to true before animation inspector tests;r=pbro
https://reviewboard.mozilla.org/r/46711/#review43315
Thanks a lot!
Reporter | ||
Comment 9•9 years ago
|
||
Is there any reason you used setBoolPref instead of pushPrefEnv? My understanding is that the latter is desirable these days.
Flags: needinfo?(jdescottes)
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8741730 [details]
MozReview Request: Bug 1264101 - force dom.animations-api.core.enabled pref to true before animation inspector tests;r=pbro
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/46711/diff/1-2/
Comment 11•9 years ago
|
||
I actually didn't know about pushPrefEnv! I guess I must have missed an email thread about it.
Julian, I'm going on PTO later today and as a result I disabled reviews on my bugzilla account. If you wanted to ping me for review for this one, consider this comment a R+.
Assignee | ||
Comment 12•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> Is there any reason you used setBoolPref instead of pushPrefEnv? My
> understanding is that the latter is desirable these days.
No particular reason! I've seen both and didn't know pushPrefEnv was the preferred option.
So thanks for the info.
I modified the implementation based on this. Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=359430035ddc
Flags: needinfo?(jdescottes)
Reporter | ||
Comment 13•9 years ago
|
||
pushPrefEnv is less-prone to racy pref setting and cleans up after itself at the end of the test, so it's a bit simpler to use and more reliable :)
Comment 15•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
tracking-firefox47:
--- → +
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•