983293 - [webvtt] Refactor TextTrack initialization

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Rick Eyre (:reyre)]

Currently we're initializing TextTracks and setting ReadyState and Mode values in a number of disjointed places. We're also initializing them with unnecessary default values.

Making it clear what part of the code triggers a TextTrack's ready state or mode value to be set will help us prevent future problems and improve code readability.

Comment 1

[Rick Eyre (:reyre)]

Attachment: Part 1 v1: Remove unused TextTrack constructor r=cpearce.

Comment 2

[Rick Eyre (:reyre)]

Attachment: Part 2 v1: Move the READY_STATE enum to Track instead of HTMLTrackElement r=cpearce.

Comment 3

[Rick Eyre (:reyre)]

Attachment: Part 3 v1: Rename the READY_STATE enum to TextTrackReadyState r=cpearce..patch

Comment 4

[Rick Eyre (:reyre)]

Attachment: Part 4 v1: Pass ReadyState variables to AddTextTrack methods and TextTrack ctors r=cpearce.

Comment 5

[Rick Eyre (:reyre)]

Attachment: Part 5 v1: Remove unecessary default settings in TextTrack ctors r=cpearce.

Comment 6

[Rick Eyre (:reyre)]

Attachment: Part 3 v2: Rename the READY_STATE enum to TextTrackReadyState r=cpearce..patch

I had to rename two of the values in the TextTrackReadyState enum because we were having problems on Linux caused by there being another name called 'None'. I've changed the names to be a bit more closer to spec as well.

Comment 7

[Rick Eyre (:reyre)]

Attachment: Part 4 v2: Pass ReadyState variables to AddTextTrack methods and TextTrack ctors r=cpearce.

Updated to have the new enum names.

Comment 8

[Rick Eyre (:reyre)]

Attachment: Part 5 v2: Remove unecessary default settings in TextTrack ctors r=cpearce.

Updated to have the new names.

Comment 9

[Rick Eyre (:reyre)]

See http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#text-track-readiness-state for where the new TextTrackReady state names are from.

Comment 10

[Rick Eyre (:reyre)]

Thanks for review Chris.

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/94a86e25eb1b
https://hg.mozilla.org/integration/mozilla-inbound/rev/44517fc04307
https://hg.mozilla.org/integration/mozilla-inbound/rev/5fc6b482da8e
https://hg.mozilla.org/integration/mozilla-inbound/rev/f653d96cd70b
https://hg.mozilla.org/integration/mozilla-inbound/rev/4523d1eeae4a

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/94a86e25eb1b
https://hg.mozilla.org/mozilla-central/rev/44517fc04307
https://hg.mozilla.org/mozilla-central/rev/5fc6b482da8e
https://hg.mozilla.org/mozilla-central/rev/f653d96cd70b
https://hg.mozilla.org/mozilla-central/rev/4523d1eeae4a