882665 - [webvtt] Add List of Pending Text Tracks to Media Element

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Rick Eyre (:reyre)]

This comes into play to determine when the text tracks are ready and the Media Element can dispatch the loadedmetadata event.

Comment 1

[Andrew Quartey [:drexler]]

Attachment: Part A: Add readyState attribute to Text Track.

Changed how the Track element handles the readyState attribute per spec: http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#dom-track-readystate.

Comment 2

[Andrew Quartey [:drexler]]

Attachment: Part B: Add pending list of text tracks to media element

For this part of the spec: "When a media element is created by an HTML parser or XML parser, the user agent must set the element's blocked-on-parser flag to true. When a media element is popped off the stack of open elements of an HTML parser or XML parser, the user agent must honor user preferences for automatic text track selection, populate the list of pending text tracks, and set the element's blocked-on-parser flag to false", I'll merge it with the patch from bug 882668, pending your review comments.

Comment 3

[Ralph Giles (:rillian)]

Comment on attachment 817528 [details] [diff] [review]
Part A: Add readyState attribute to Text Track.

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

Looks good to me. Can you cover the dom peer aspect, khuey?

Comment 4

[Ralph Giles (:rillian)]

Comment on attachment 817531 [details] [diff] [review]
Part B: Add pending list of text tracks to media element

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

r=me with nits.

Did you implement "Whenever a track element's parent node changes, the user agent must remove the corresponding text track from any list of pending text tracks that it is in."? There should be something in UnbindFromTree, no?

::: content/html/content/public/HTMLMediaElement.h
@@ +1157,5 @@
>  
>    // List of our attached text track objects.
>    nsRefPtr<TextTrackList> mTextTracks;
> +
> +  //List of text track objects awating loading.

space between // and the comment text, please.

Comment 5

[Andrew Quartey [:drexler]]

(In reply to Ralph Giles (:rillian) from comment #4)
> Comment on attachment 817531 [details] [diff] [review]
> Part B: Add pending list of text tracks to media element
> 
> Review of attachment 817531 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with nits.
> 
> Did you implement "Whenever a track element's parent node changes, the user
> agent must remove the corresponding text track from any list of pending text
> tracks that it is in."? There should be something in UnbindFromTree, no?

Yes. It's taken care of already by: http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/HTMLTrackElement.cpp#298. Also, just realized that i needed to update the HTMLMediaElement constructor and cycle collection bits as well with respect to |mPendingTextTracks|.

Comment 6

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

(In reply to Andrew Quartey [:drexler] from comment #5)
> Yes. It's taken care of already by:
> http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/
> HTMLTrackElement.cpp#298. Also, just realized that i needed to update the
> HTMLMediaElement constructor and cycle collection bits as well with respect
> to |mPendingTextTracks|.

Should I be waiting for you to do this to review this patch?

Comment 7

[Andrew Quartey [:drexler]]

Attachment: Part B: Add pending list of text tracks to media element (v2)

No need. I updated the patch.

Comment 8

[Andrew Quartey [:drexler]]

Rebased and moved over relevants bits to the new TextTrackManager. Try looks green: https://tbpl.mozilla.org/?tree=Try&rev=ee45450fc5e9

Comment 9

[Andrew Quartey [:drexler]]

remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/2f3d1761b059
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/53dd3aa9be7e

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/2f3d1761b059
https://hg.mozilla.org/mozilla-central/rev/53dd3aa9be7e