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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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: nobody → bzbarsky
Status: NEW → ASSIGNED
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.
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.
Attachment #8599665 - Attachment is obsolete: true
Attachment #8599665 - Flags: review?(bbirtles)
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+
> 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.
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
Oh, Android == Android 4.3 API11+ opt
Flags: needinfo?(bzbarsky)
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)
And is _also_ set in b2g.js!!!
Flags: needinfo?(bzbarsky)
Looks like for some reason I have to use opener.SpecialPowers for that getPref call...
Flags: needinfo?(bzbarsky)
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.
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
Depends on: 1161034
You need to log in before you can comment on or make changes to this bug.