Closed
Bug 1159743
Opened 9 years ago
Closed 9 years ago
Stop forcing dom.animations-api.core.enabled to true in the test harness
Categories
(Core :: Layout, defect)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: bzbarsky, Assigned: bzbarsky)
References
Details
Attachments
(1 file, 1 obsolete file)
206.87 KB,
patch
|
birtles
:
review+
|
Details | Diff | Splinter Review |
This makes it impossible to properly test that we don't ship support when we don't mean to, for example. The tests that rely on this preference should set it as needed.
Comment 1•9 years ago
|
||
Almost all the tests that rely on this preference being set are tests intended web-platform-tests where we can't tweak preferences. I suppose we could just wrap these tests and drop the wrapper when we upload.
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8599665 -
Flags: review?(bbirtles)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Comment 3•9 years ago
|
||
As discussed on IRC, this patch needs to apply the same treatment to the tests in dom/animation/test/css-transitions too. The tests in dom/animation/test/mozilla/ and dom/animation/test/chrome/ could just have the pref-setting code included directly. If we do that then we could merge dom/animation/test/css-animation/testcommon.js into dom/animation/test/testcommon.js.
Assignee | ||
Comment 4•9 years ago
|
||
The test in dom/animation/test/mozilla/ that uses animation API stuff needs the same treatment, because it needs to set the prefs before any Element instances are created in the test file. The tests in dom/animation/test/chrome/ don't need any changes, because we always enable animations API in chrome, looks like.
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8599689 -
Flags: review?(bbirtles)
Assignee | ||
Updated•9 years ago
|
Attachment #8599665 -
Attachment is obsolete: true
Attachment #8599665 -
Flags: review?(bbirtles)
Comment 6•9 years ago
|
||
Comment on attachment 8599689 [details] [diff] [review] Stop forcing the dom.animations-api.core.enabled preference on in the test harness >diff --git a/dom/animation/test/css-animations/test_animation-cancel.html b/dom/animation/test/css-animations/file_animation-cancel.html >copy from dom/animation/test/css-animations/test_animation-cancel.html >copy to dom/animation/test/css-animations/file_animation-cancel.html >--- a/dom/animation/test/css-animations/test_animation-cancel.html >+++ b/dom/animation/test/css-animations/file_animation-cancel.html >@@ -1,26 +1,25 @@ > <!doctype html> > <meta charset=utf-8> >-<script src="/resources/testharness.js"></script> >-<script src="/resources/testharnessreport.js"></script> > <script src="../testcommon.js"></script> >-<div id="log"></div> >+<script src="testcommon.js"></script> I don't think we need the additional reference to testcommon.js anymore right? If not, we should drop it from all the files in css-animations/file_* >diff --git a/dom/tests/mochitest/general/test_interfaces.html b/dom/tests/mochitest/general/test_interfaces.html >--- a/dom/tests/mochitest/general/test_interfaces.html >+++ b/dom/tests/mochitest/general/test_interfaces.html > // IMPORTANT: Do not change this list without review from a DOM peer! >- {name: "AnimationEffectReadonly", pref: "dom.animations-api.core.enabled"}, >+ {name: "AnimationEffectReadonly", release: false}, (Just as a heads up, I just renamed this to AnimationEffectReadOnly.) > // IMPORTANT: Do not change this list without review from a DOM peer! >- {name: "KeyframeEffectReadonly", pref: "dom.animations-api.core.enabled"}, >+ {name: "KeyframeEffectReadonly", release: false}, (Also, now KeyframeEffectReadOnly.)
Attachment #8599689 -
Flags: review?(bbirtles) → review+
Assignee | ||
Comment 7•9 years ago
|
||
> I don't think we need the additional reference to testcommon.js anymore right?
Er, yes. I thought I'd removed those, but clearly not. Will fix before pushing.
Various android and b2g animation tests are failing after this landed. https://hg.mozilla.org/integration/mozilla-inbound/rev/3ce2f7f110af
Assignee | ||
Comment 10•9 years ago
|
||
In particular: 1) dom/animation/test/css-transitions/test_effect-name.html on Android: https://treeherder.mozilla.org/logviewer.html#?job_id=9465221&repo=mozilla-inbound (test timeout) and 2) dom/animation/test/mozilla/test_deferred_start.html on b2g ICS emulator opt: 218 INFO TEST-UNEXPECTED-FAIL | dom/animation/test/mozilla/test_deferred_start.html | Starting an animation with a delay starts from the correct point - Starting an animation with a delay starts from the correct point: Error getting pref
Assignee | ||
Comment 11•9 years ago
|
||
Oh, Android == Android 4.3 API11+ opt
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 12•9 years ago
|
||
So the b2g emulator error is, in particular: Error getting pref 'layers.offmainthreadcomposition.async-animations' Except that pref is always set in all.js. What gives?
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 14•9 years ago
|
||
Looks like for some reason I have to use opener.SpecialPowers for that getPref call...
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 15•9 years ago
|
||
In particular, for this code: var omtaEnabled = SpecialPowers.DOMWindowUtils.layerManagerRemote && SpecialPowers.getBoolPref(OMTAPrefKey); 1) If I use opener.SpecialPowers for neither one, I get the failure. 2) If I use opener.SpecialPowers for just the DOMWindowUtils bit, I get the failure. 3) If I use opener.SpecialPowers for both there is no failure. 4) If I use opener.SpecialPowers for just the getBoolPref, no failure. The Android thing doesn't seem to be reproducing on try at all, fwiw.
Comment 18•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1103c98d1a93 https://hg.mozilla.org/mozilla-central/rev/20cbfb10ef7a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•