Bug 123825

Created attachment 216070 [details] Patch This adds "id" to the API. I figured I'd do this patch separately from implementing the ID in GStreamer.

Oh, and the reason TextTrack didn't have an "id" before was that it wasn't in the spec, but it is now: http://www.w3.org/TR/html5/embedded-content-0.html#dom-texttrack-id

Comment on attachment 216070 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=216070&action=review > Source/WebCore/html/track/InbandTextTrack.cpp:62 > InbandTextTrack::InbandTextTrack(ScriptExecutionContext* context, TextTrackClient* client, PassRefPtr<InbandTextTrackPrivate> tracksPrivate) > - : TextTrack(context, client, emptyString(), tracksPrivate->label(), tracksPrivate->language(), InBand) > + : TextTrack(context, client, emptyString(), tracksPrivate->id(), tracksPrivate->label(), tracksPrivate->language(), InBand) > , m_private(tracksPrivate) Nit: while you are here, can you please change "tracksPrivate" to "trackPrivate" since there is only one private track? > Source/WebCore/html/track/LoadableTextTrack.cpp:40 > LoadableTextTrack::LoadableTextTrack(HTMLTrackElement* track, const String& kind, const String& label, const String& language) > - : TextTrack(&track->document(), track, kind, label, language, TrackElement) > + : TextTrack(&track->document(), track, kind, emptyString(), label, language, TrackElement) Doesn't LoadableTextTrack need to override "id()" so it can return the HTMLTrackElement id attribute? From the spec: For tracks that correspond to track elements, the track's identifier is the value of the element's id attribute, if any > Source/WebCore/html/track/TrackBase.h:50 > + AtomicString id() const { return m_id; } This should be virtual, see above.

(In reply to comment #3) > For tracks that correspond to track elements, the track's identifier is the value of the element's id attribute, if any > > > Source/WebCore/html/track/TrackBase.h:50 > > + AtomicString id() const { return m_id; } > > This should be virtual, see above. Oh, somehow I missed that. I'll add it.

Created attachment 216083 [details] Patch I fixed tracksPrivate, made LoadableTextTrack use m_trackElement->id, and added a test using out-of-band tracks.

Comment on attachment 216083 [details] Patch Attachment 216083 [details] did not pass efl-wk2-ews (efl-wk2): Output: http://webkit-queues.appspot.com/results/21238019

Internal compiler error eh? :\

Should I resubmit this, or just ignore the weird EFL compiler crash?

Created attachment 216179 [details] Patch I rebased this after another patch was committed.

Comment on attachment 216179 [details] Patch Attachment 216179 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/21948006 New failing tests: media/track/track-id.html

Created attachment 216182 [details] Archive of layout-test-results from webkit-ews-09 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-09 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5

Created attachment 216183 [details] Patch Apparently I need to add an extra blank line to media tests after r158743. I'll fix the in-band tests in a different patch.

Comment on attachment 216183 [details] Patch Clearing flags on attachment: 216183 Committed r158760: <http://trac.webkit.org/changeset/158760>

All reviewed patches have been landed. Closing bug.