Remove explicit media.webvtt.enabled pref setting from tests

RESOLVED FIXED in mozilla34

Status

()

Core
Audio/Video
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: RyanVM, Assigned: RyanVM)

Tracking

Trunk
mozilla34
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

4 years ago
Bug 887978 set media.webvtt.enabled to true by default without conditions, so we shouldn't need all the various test-specific setting of the pref now, i.e. SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]} or  test-pref(media.webvtt.enabled,true).
(Assignee)

Comment 1

4 years ago
Created attachment 8468191 [details] [diff] [review]
remove explicit setting of media.webvtt.enabled from tests

Lots of indentation changes are needed now, but I figured it was easier for readability to split those into another patch.
Assignee: nobody → ryanvm
Status: NEW → ASSIGNED
Attachment #8468191 - Flags: review?(giles)
(Assignee)

Comment 2

4 years ago
Created attachment 8468197 [details] [diff] [review]
whitespace fixes (to be folded in before landing)
Comment on attachment 8468191 [details] [diff] [review]
remove explicit setting of media.webvtt.enabled from tests

Waiting for updated patch per irc.
Attachment #8468191 - Flags: review?(giles)
(Assignee)

Comment 5

4 years ago
Created attachment 8474150 [details] [diff] [review]
remove explicit setting of media.webvtt.enabled from tests

Substantially greener than the last attempt :)

https://tbpl.mozilla.org/?tree=Try&rev=5f8021af2396
Attachment #8468191 - Attachment is obsolete: true
Attachment #8474150 - Flags: review?(giles)
(Assignee)

Comment 6

4 years ago
Created attachment 8474151 [details] [diff] [review]
whitespace fixes (to be folded in before landing)
Attachment #8468197 - Attachment is obsolete: true
Attachment #8474151 - Flags: review?(giles)
Comment on attachment 8474150 [details] [diff] [review]
remove explicit setting of media.webvtt.enabled from tests

Review of attachment 8474150 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! r=me

::: content/html/content/test/test_track.html
@@ -16,5 @@
>  <div id="content" style="display: none">
>  </div>
>  <pre id="test">
>  <script class="testbody" type="text/javascript">
>  SimpleTest.waitForExplicitFinish();

I don't see anything asynchronous here besides the SpecialPowers push. Can you remove this the corresponding SimpleTest.finish() as well? One less thing to timeout, assuming it runs to completion before 'load' fires.
Attachment #8474150 - Flags: review?(giles) → review+
Comment on attachment 8474151 [details] [diff] [review]
whitespace fixes (to be folded in before landing)

Review of attachment 8474151 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the separate reformatting patch. Much appreciated.

::: content/html/content/test/test_track.html
@@ +22,5 @@
> +reflectLimitedEnumerated({
> +  element: document.createElement("track"),
> +  attribute: "kind",
> +  validValues: ["subtitles", "captions", "descriptions", "chapters", "metadata"],
> +  invalidValues: ["foo", "bar", "\u0000", "null", "", "subtitle", "caption", "description", "chapter", "meta"],

Would be nice if you wrapped this to 80 columns while you're here.

::: content/html/content/test/test_track_disabled.html
@@ +20,5 @@
>  SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", false]]},
>    function() {
>      var track = document.createElement("track");
>      is(track.constructor, HTMLUnknownElement, "Track should actually be an HTMLUnknownElement.");
> +

The review interface shows trailing whitespace here, and just about everywhere you've added a blank line, but it doesn't seem to be in the actual patch. Please double-check.

I don't think you need a newline here either, but don't especially mind.
Attachment #8474151 - Flags: review?(giles) → review+
(Assignee)

Comment 10

4 years ago
https://hg.mozilla.org/mozilla-central/rev/6ae6e5032735
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.