983207 - [webvtt] Tracks created from TrackElements should have their mode set to 'disabled' by default

Status: VERIFIED FIXED

(Core :: Audio/Video, defect)

Description

[Rick Eyre (:reyre)]

See http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#sourcing-out-of-band-text-tracks, specifically:

"The text track readiness state is initially not loaded, and the text track mode is initially disabled."

Comment 1

[Rick Eyre (:reyre)]

Attachment: v1: Tracks created through a TrackElement should have a default mode of 'disabled' r=cpearce

Comment 2

[Chris Pearce [:cpearce (Not reading bugmail)]]

Comment on attachment 8390634 [details] [diff] [review]
v1: Tracks created through a TrackElement should have a default mode of 'disabled' r=cpearce

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

Looks good.

::: content/media/TextTrack.cpp
@@ +36,4 @@
>    SetDefaultSettings();
>    SetIsDOMBinding();
>  }
>  

Does the default constructor need to initialize aMode, aLabel, and aLanguage? If so, please add this before landing. If not, please say so. :)

Comment 3

[Chris Pearce [:cpearce (Not reading bugmail)]]

(In reply to Chris Pearce (:cpearce) from comment #2)
> Comment on attachment 8390634 [details] [diff] [review]
> Does the default constructor need to initialize aMode, aLabel, and
> aLanguage? If so, please add this before landing. If not, please say so. :)

I guess not, given that you remove this in a later patch. ;)

Comment 4

[Rick Eyre (:reyre)]

Thanks for review Chris.

(In reply to Chris Pearce (:cpearce) from comment #3)
> I guess not, given that you remove this in a later patch. ;)
Yeah :-). We're also using the SetDefaultSettings() function in this patch to set the default mode, label, and language. I get rid of it later though, like you noticed.

Comment 5

[Rick Eyre (:reyre)]

Try looks green: https://tbpl.mozilla.org/?tree=Try&rev=13780e4ef5aa

Comment 6

[Rick Eyre (:reyre)]

Attachment: v2: Tracks created through a TrackElement should have a default mode of 'disabled' r=cpearce

Had to update the other TextTrack tests to have their TextTrack mode set to 'hidden' where applicable so that when they call textTrack.cues the cue lists are returned.

Carrying forward r=cpearce.

This is the patch that I pushed to the try server. Just didn't post them on the bug in order.

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c5a1be123b8

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1c5a1be123b8

Comment 9

[Ada [:adalucinet]]

On Windows 7 64-bit and Mac OS X 10.8.5 with a HMTL page which contains:

<video src="http://people.mozilla.org/~rgiles/2013/demo.webm" controls >
   <track src="sample.vtt" kind="subtitles" srclang="en" label="English" />
</video>

and the corresponding sample.vtt file saved in the same folder, the subtitles are not displayed. I'm inclined to say it is the expected result based on this fix. Any thoughts?

Comment 10

[Rick Eyre (:reyre)]

Correct. If a 'default' attribute is added to the track tag it will show by default if it is the first track to have it.

Comment 11

[Rick Eyre (:reyre)]

Without that default attribute though they do not display by default.

Comment 12

[Ada [:adalucinet]]

On latest Aurora (Build ID: 20140604004003), I've encountered the same result as in comment 9 - without the 'default' attribute, the tracks are not displayed.

Verified on:
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Firefox/31.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:31.0) Gecko/20100101 Firefox/31.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0