Closed
Bug 887978
Opened 11 years ago
Closed 11 years ago
Turn WebVTT on by default on trunk.
Categories
(Core :: Audio/Video, defect)
Tracking
()
VERIFIED
FIXED
mozilla28
People
(Reporter: davidb, Assigned: rillian)
References
(Blocks 1 open bug)
Details
(Keywords: dev-doc-complete, relnote)
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
Filing this bug so we can track required dependencies for WebVTT on by default.
Depends on: 905320
Comment 2•11 years ago
|
||
Thanks Ralph :-). What needs to be completed for this to happen?
I'm guessing https://bugzilla.mozilla.org/show_bug.cgi?id=webvtt should be resolved and enough fuzz testing that we believe it's safe?
Assignee | ||
Comment 4•11 years ago
|
||
It looks like we've resolved all the issues blocking pref on. I'd like to get wider testing and decide if the implementation we have now is sufficiently useful to enable for releases. Anything else we need before doing this?
This patch enables webvtt by default on Nightly and Aurora only as a first step.
Attachment #8335770 -
Flags: review?(cpearce)
Attachment #8335770 -
Flags: feedback?(ehsan)
Updated•11 years ago
|
Attachment #8335770 -
Flags: feedback?(ehsan) → feedback+
Updated•11 years ago
|
Attachment #8335770 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Comment 6•11 years ago
|
||
Hey Ralph, sorry had backed this out because of the suspicion that this caused bustages on mochitest 2/3 like
https://tbpl.mozilla.org/php/getParsedLog.php?id=30886569&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=30887641&tree=Mozilla-Inbound
Assignee | ||
Comment 7•11 years ago
|
||
Definitely looks like our failure. Sorry, for the hassle.
Assignee | ||
Comment 8•11 years ago
|
||
Add new interfaces to the global list in test_interfaces.html. Carrying forward r=cpearce for media, passing to bz for DOM peer review.
What happens with this test in a release build?
Assignee: rick.eyre → giles
Attachment #8335770 -
Attachment is obsolete: true
Attachment #8336352 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 9•11 years ago
|
||
Handle the release case in the interface test. Thanks to smaug for help on that.
Carrying forward r=cpearce for media, passing to bz for DOM peer review.
Attachment #8336352 -
Attachment is obsolete: true
Attachment #8336352 -
Flags: review?(bzbarsky)
Attachment #8336372 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 10•11 years ago
|
||
bz: I moved the comment change to bug 941890.
Comment 11•11 years ago
|
||
Comment on attachment 8336372 [details] [diff] [review]
Enable for non-release builds v3
r=me
Attachment #8336372 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 12•11 years ago
|
||
Comment 13•11 years ago
|
||
(In reply to Ralph Giles (:rillian) from comment #12)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/1da0b2521da2
Hey Ralph,
had to backout this again :( seems it caused testfailures like https://tbpl.mozilla.org/php/getParsedLog.php?id=30947119&tree=Mozilla-Inbound on all platforms - android failure is this https://tbpl.mozilla.org/php/getParsedLog.php?id=30946777&tree=Mozilla-Inbound
Comment 14•11 years ago
|
||
Looks like the "problem" was (consistently) an unexpected-pass:
14651 ERROR TEST-UNEXPECTED-PASS | /tests/dom/imptests/html/microdata/microdata-dom-api/test_001.html | itemValue must reflect the src attribute on track elements
Maybe that's now expected?
Comment 15•11 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #14)
> 14651 ERROR TEST-UNEXPECTED-PASS |
> /tests/dom/imptests/html/microdata/microdata-dom-api/test_001.html |
> itemValue must reflect the src attribute on track elements
>
> Maybe that's now expected?
Yeah, looks like that.
Ralph seems to be on PTO now, so can somebody please fix the test so this can land in Fx 28?
Comment 16•11 years ago
|
||
(In reply to Florian Bender from comment #15)
> (In reply to Jonathan Kew (:jfkthame) from comment #14)
> > 14651 ERROR TEST-UNEXPECTED-PASS |
> > /tests/dom/imptests/html/microdata/microdata-dom-api/test_001.html |
> > itemValue must reflect the src attribute on track elements
> >
> > Maybe that's now expected?
>
> Yeah, looks like that.
>
> Ralph seems to be on PTO now, so can somebody please fix the test so this
> can land in Fx 28?
I'll see if I can fix it.
Assignee | ||
Comment 17•11 years ago
|
||
I didn't find existing conditional code for the imptests, beyond a 'debug' option we don't seem to be using. So we need to add a 'nonrelease' conditional, or a blanket pref for the imptests like the mochitests have.
Or enable it unconditionally, I suppose.
Flags: needinfo?(Ms2ger)
Assignee | ||
Comment 18•11 years ago
|
||
Since it's work to conditionalize the imptests, just enable it unconditionally. We have 8 weeks to uplift is reversion if we change our minds before release.
Pushed to try as https://tbpl.mozilla.org/?tree=Try&rev=d835ea8fb9fb
Attachment #8340150 -
Flags: review?(cpearce)
Attachment #8340150 -
Flags: review?(bzbarsky)
Updated•11 years ago
|
Attachment #8340150 -
Flags: review?(cpearce) → review+
Assignee | ||
Comment 19•11 years ago
|
||
Remove trailing comma in failures.json. Carrying forward r=cpearce.
Green on try https://tbpl.mozilla.org/?tree=Try&rev=264b069325d5.
Attachment #8336372 -
Attachment is obsolete: true
Attachment #8340150 -
Attachment is obsolete: true
Attachment #8340150 -
Flags: review?(bzbarsky)
Attachment #8340575 -
Flags: review?(bzbarsky)
Flags: needinfo?(Ms2ger)
Updated•11 years ago
|
Keywords: dev-doc-needed
Comment 20•11 years ago
|
||
Comment on attachment 8340575 [details] [diff] [review]
Enable unconditionally
r=me
Attachment #8340575 -
Flags: review?(bzbarsky) → review+
Comment 21•11 years ago
|
||
Oh, and I assume that the plan is in fact to enable this for the 28 release? If not, we should have something tracking re-disabling it on Aurora/Beta?
Assignee | ||
Comment 22•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/76e799f77069
Thanks for the review. Yes, the plan is to enable it for the 28 release.
Comment 23•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•11 years ago
|
relnote-firefox:
--- → ?
Updated•11 years ago
|
Assignee | ||
Comment 24•11 years ago
|
||
Thanks, missed the relnote-firefox? flag. I'd just noticed I'd forgotten to nominate WebVTT support for the release notes. I think it's important enough to mention, and there's already been some developer confusion about what versions support it.
Comment 25•11 years ago
|
||
We don't put the setting to version+ until it's in the notes DB and live on a channel's notes. We can leave this nominated and pick it up when we edit the current 28 Beta notes for 28 Release.
Updated•11 years ago
|
status-firefox28:
--- → affected
status-firefox29:
--- → affected
status-firefox30:
--- → fixed
tracking-firefox28:
--- → +
tracking-firefox29:
--- → +
Updated•11 years ago
|
Comment 26•11 years ago
|
||
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:28.0) Gecko/20100101 Firefox/28.0
Verified as fixed with Firefox 28 beta 8 (Build ID: 20140303165517): media.webvtt.enabled pref is set to true by default.
Status: RESOLVED → VERIFIED
Assignee | ||
Comment 28•11 years ago
|
||
Thanks!
Comment 29•11 years ago
|
||
I've backed this out of beta 28; see bug 981280.
Comment 30•8 years ago
|
||
While the Firefox 31 for devs page was updated to note this finally stuck in 31, I've updated other content to match:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/track
https://developer.mozilla.org/en-US/docs/Web/API/HTMLTrackElement
https://developer.mozilla.org/en-US/docs/Web/API/Web_Video_Text_Tracks_Format
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•