Bug 72545 - TextTrackList not sorted correctly
Summary: TextTrackList not sorted correctly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks: 43668
  Show dependency treegraph
 
Reported: 2011-11-16 14:07 PST by Eric Carlson
Modified: 2011-11-17 06:56 PST (History)
4 users (show)

See Also:


Attachments
Proposed patch (14.49 KB, patch)
2011-11-16 14:33 PST, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2011-11-16 14:07:01 PST
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.
Comment 1 Radar WebKit Bug Importer 2011-11-16 14:07:32 PST
<rdar://problem/10457840>
Comment 2 Eric Carlson 2011-11-16 14:33:11 PST
Created attachment 115451 [details]
Proposed patch
Comment 3 WebKit Review Bot 2011-11-16 16:10:56 PST
Comment on attachment 115451 [details]
Proposed patch

Attachment 115451 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/10483897
Comment 4 Darin Adler 2011-11-16 16:48:50 PST
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*.
Comment 5 Eric Carlson 2011-11-17 06:55:06 PST
(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.
Comment 6 Eric Carlson 2011-11-17 06:55:35 PST
http://trac.webkit.org/changeset/100616
Comment 7 Eric Carlson 2011-11-17 06:56:11 PST
Comment on attachment 115451 [details]
Proposed patch

Clearing flags, changes committed.