TextTrackList not sorted correctly

• Component: Media
• Status: RESOLVED FIXED

[Eric Carlson]

The spec mandates the sort order of the text tracks in a TextTrackList:

A media element can have a group of associated text tracks, known as the media element's list of text tracks. The text tracks are sorted as follows:

1. The text tracks corresponding to track element children of the media element, in tree order.
2. Any text tracks added using the addTextTrack() method, in the order they were added, oldest first.
3. Any media-resource-specific text tracks (text tracks corresponding to data in the media resource), in the order defined by the media resource's format specification.

[Radar WebKit Bug Importer]

<rdar://problem/10457840>

[Eric Carlson]

Created attachment 115451 [details]
Proposed patch

[WebKit Review Bot]

Comment on attachment 115451 [details]
Proposed patch

Attachment 115451 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10483897

[Darin Adler]

Comment on attachment 115451 [details]
Proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=115451&action=review

> Source/WebCore/html/LoadableTextTrack.cpp:128
> +    for (Node* node = m_trackElement->parentNode()->firstChild(); node; node = node->nextSibling()) {

What guarantees the track element has a non-zero parent?

> Source/WebCore/html/LoadableTextTrack.cpp:131
> +        if (static_cast<HTMLTrackElement*>(node) == m_trackElement)

This cast shouldn’t be needed because m_trackElement should automatically convert to a Node*.

[Eric Carlson]

(In reply to comment #4)
> (From update of attachment 115451 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=115451&action=review
> 
> > Source/WebCore/html/LoadableTextTrack.cpp:128
> > +    for (Node* node = m_trackElement->parentNode()->firstChild(); node; node = node->nextSibling()) {
> 
> What guarantees the track element has a non-zero parent?
> 
That should not happen because a track element are only added to the list when insertedIntoTree is called, and it is always removed from the list when willRemove is called, but I added an ASSERT.

> > Source/WebCore/html/LoadableTextTrack.cpp:131
> > +        if (static_cast<HTMLTrackElement*>(node) == m_trackElement)
> 
> This cast shouldn’t be needed because m_trackElement should automatically convert to a Node*.

Thanks.

[Eric Carlson]

http://trac.webkit.org/changeset/100616

[Eric Carlson]

Comment on attachment 115451 [details]
Proposed patch

Clearing flags, changes committed.