diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 1b1e57d9b0f70c7edd6b70a7d7c61585e101f640..302fb93b104534ff92bf67f1f6c74320cdd56ddc 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,13 @@ +2011-11-17 Eric Carlson + + TextTrackList not sorted correctly + https://bugs.webkit.org/show_bug.cgi?id=72545 + + Reviewed by Darin Adler. + + * media/track/track-texttracks-expected.txt: Added. + * media/track/track-texttracks.html: Added. + 2011-11-17 Steve Block [Chromium] Layout test failures under Snow Leopard diff --git a/LayoutTests/media/track/track-texttracks-expected.txt b/LayoutTests/media/track/track-texttracks-expected.txt new file mode 100644 index 0000000000000000000000000000000000000000..12f2de0c7a9cd7690f3a81e5c64234c14cfe5186 --- /dev/null +++ b/LayoutTests/media/track/track-texttracks-expected.txt @@ -0,0 +1,28 @@ +Tests that TextTracks in a TextTrackList are kept in the correct order. + +** Add a track with video.addTrack(). +RUN(video.addTrack('descriptions', 'Descriptions Track', 'en')) + +** Add a element with DOM API. +RUN(trackElement = document.createElement('track')) +RUN(trackElement.setAttribute('kind', 'chapters')) +RUN(video.appendChild(trackElement)) + +** Verify track order. +EXPECTED (video.textTracks.length == '3') OK +EXPECTED (video.textTracks[0].kind == 'captions') OK +EXPECTED (video.textTracks[1].kind == 'chapters') OK +EXPECTED (video.textTracks[2].kind == 'descriptions') OK + +** Add another element, is should insert before the addTrack() track. +RUN(trackElement = document.createElement('track')) +RUN(trackElement.setAttribute('kind', 'metadata')) +RUN(video.appendChild(trackElement)) +EXPECTED (video.textTracks.length == '4') OK +EXPECTED (video.textTracks[0].kind == 'captions') OK +EXPECTED (video.textTracks[1].kind == 'chapters') OK +EXPECTED (video.textTracks[2].kind == 'metadata') OK +EXPECTED (video.textTracks[3].kind == 'descriptions') OK + +END OF TEST + diff --git a/LayoutTests/media/track/track-texttracks.html b/LayoutTests/media/track/track-texttracks.html new file mode 100644 index 0000000000000000000000000000000000000000..7b096da62b1f049a6dac4d75537e8ab8ffc6fd61 --- /dev/null +++ b/LayoutTests/media/track/track-texttracks.html @@ -0,0 +1,53 @@ + + + + + + + + + + +

Tests that TextTracks in a TextTrackList are kept in the correct order.

+ + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 87a72b566cf91640b8174fde5961fbfe21ad41eb..1666f45531c6017313016370322810f32b91782f 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,34 @@ +2011-11-17 Eric Carlson + + TextTrackList not sorted correctly + https://bugs.webkit.org/show_bug.cgi?id=72545 + + Reviewed by Darin Adler. + + Test: media/track/track-texttracks.html + + * WebCore.xcodeproj/project.pbxproj: Add TextTrack.h to WebCore private headers because + it is included by HTMLMediaElement.h. + * html/LoadableTextTrack.cpp: + (WebCore::LoadableTextTrack::LoadableTextTrack): Pass track type to base class constructor. + (WebCore::LoadableTextTrack::trackElementIndex): New, return the element's tree order + for sorting. + * html/LoadableTextTrack.h: + + * html/TextTrack.cpp: + (WebCore::TextTrack::TextTrack): Set track type. + * html/TextTrack.h: + (WebCore::TextTrack::create): Ditto. + (WebCore::TextTrack::trackType): Ditto. + + * html/track/TextTrackList.cpp: + (TextTrackList::length): Update to deal with two TextTrack vectors. + (TextTrackList::item): Ditto. + (TextTrackList::append): Ditto. + (TextTrackList::remove): Ditto + * html/track/TextTrackList.h: Store the two types of TextTracks in separate Vectors to make + it simpler to keep them in the correct order. + 2011-11-17 Simon Hausmann [Qt] Layer violation: WebCore::dnsPrefetch uses QWebSettings::globalSettings() diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj index 373ce00d1699543c8a15538b73bc15f9e149df09..edad51fb5516c60b006c4f9135514721eb8b926c 100644 --- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj +++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj @@ -4181,7 +4181,7 @@ B1AD4E6613A12A0B00846B27 /* TextTrack.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B1AD4E5513A12A0B00846B27 /* TextTrack.cpp */; }; B1AD4E6713A12A0B00846B27 /* TextTrack.h in Headers */ = {isa = PBXBuildFile; fileRef = B1AD4E5613A12A0B00846B27 /* TextTrack.h */; settings = {ATTRIBUTES = (Private, ); }; }; B1AD4E6813A12A0B00846B27 /* TextTrackCue.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B1AD4E5713A12A0B00846B27 /* TextTrackCue.cpp */; }; - B1AD4E6913A12A0B00846B27 /* TextTrackCue.h in Headers */ = {isa = PBXBuildFile; fileRef = B1AD4E5813A12A0B00846B27 /* TextTrackCue.h */; }; + B1AD4E6913A12A0B00846B27 /* TextTrackCue.h in Headers */ = {isa = PBXBuildFile; fileRef = B1AD4E5813A12A0B00846B27 /* TextTrackCue.h */; settings = {ATTRIBUTES = (Private, ); }; }; B1AD4E6A13A12A0B00846B27 /* TextTrackCueList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B1AD4E5913A12A0B00846B27 /* TextTrackCueList.cpp */; }; B1AD4E6B13A12A0B00846B27 /* TextTrackCueList.h in Headers */ = {isa = PBXBuildFile; fileRef = B1AD4E5A13A12A0B00846B27 /* TextTrackCueList.h */; }; B1AD4E7313A12A4600846B27 /* TextTrackLoader.cpp in Sources */ = {isa = PBXBuildFile; fileRef = B1AD4E7113A12A4600846B27 /* TextTrackLoader.cpp */; }; diff --git a/Source/WebCore/html/LoadableTextTrack.cpp b/Source/WebCore/html/LoadableTextTrack.cpp index 415e8341b04f8ced49d18e502e7bf341f637d883..87ec56bc3063ff43ce907ef8a304bdd2b4e47b64 100644 --- a/Source/WebCore/html/LoadableTextTrack.cpp +++ b/Source/WebCore/html/LoadableTextTrack.cpp @@ -38,7 +38,7 @@ namespace WebCore { LoadableTextTrack::LoadableTextTrack(HTMLTrackElement* track, const String& kind, const String& label, const String& language, bool isDefault) - : TextTrack(track->document(), track, kind, label, language) + : TextTrack(track->document(), track, kind, label, language, TrackElement) , m_trackElement(track) , m_loadTimer(this, &LoadableTextTrack::loadTimerFired) , m_isDefault(isDefault) @@ -120,6 +120,24 @@ void LoadableTextTrack::fireCueChangeEvent() m_trackElement->dispatchEvent(Event::create(eventNames().cuechangeEvent, false, false), ec); } +size_t LoadableTextTrack::trackElementIndex() +{ + ASSERT(m_trackElement); + ASSERT(m_trackElement->parentNode()); + + size_t index = 0; + for (Node* node = m_trackElement->parentNode()->firstChild(); node; node = node->nextSibling()) { + if (!node->hasTagName(trackTag)) + continue; + if (node == m_trackElement) + return index; + ++index; + } + ASSERT_NOT_REACHED(); + + return 0; +} + } // namespace WebCore #endif diff --git a/Source/WebCore/html/LoadableTextTrack.h b/Source/WebCore/html/LoadableTextTrack.h index 42b9eac1275f22acf3ef096541f2ca42173c1676..add3618c10c9d30ec9f99d8ff36b47d044047a59 100644 --- a/Source/WebCore/html/LoadableTextTrack.h +++ b/Source/WebCore/html/LoadableTextTrack.h @@ -57,6 +57,8 @@ public: void scheduleLoad(const KURL&); virtual void clearClient(); + + size_t trackElementIndex(); private: // TextTrackLoaderClient diff --git a/Source/WebCore/html/TextTrack.cpp b/Source/WebCore/html/TextTrack.cpp index 1db33ba03d7e99d5c4a3eff13aa4398edac4414d..8d10bd783bccb262234fa1ca14ccee2ef00f8cd1 100644 --- a/Source/WebCore/html/TextTrack.cpp +++ b/Source/WebCore/html/TextTrack.cpp @@ -71,13 +71,14 @@ const AtomicString& TextTrack::metadataKeyword() return metadata; } -TextTrack::TextTrack(ScriptExecutionContext* context, TextTrackClient* client, const String& kind, const String& label, const String& language) +TextTrack::TextTrack(ScriptExecutionContext* context, TextTrackClient* client, const String& kind, const String& label, const String& language, TextTrackType type) : TrackBase(context, TrackBase::TextTrack) , m_label(label) , m_language(language) , m_readyState(TextTrack::NONE) , m_mode(TextTrack::HIDDEN) , m_client(client) + , m_trackType(type) { setKind(kind); } diff --git a/Source/WebCore/html/TextTrack.h b/Source/WebCore/html/TextTrack.h index 15d96fd3d454c42ffbe783f691e6c9dd633c1c61..733660e62218f57abaf02f6f5ecc0b853d8d9a54 100644 --- a/Source/WebCore/html/TextTrack.h +++ b/Source/WebCore/html/TextTrack.h @@ -56,7 +56,7 @@ class TextTrack : public TrackBase { public: static PassRefPtr create(ScriptExecutionContext* context, TextTrackClient* client, const String& kind, const String& label, const String& language) { - return adoptRef(new TextTrack(context, client, kind, label, language)); + return adoptRef(new TextTrack(context, client, kind, label, language, AddTrack)); } virtual ~TextTrack(); @@ -101,8 +101,11 @@ public: virtual void fireCueChangeEvent(); DEFINE_ATTRIBUTE_EVENT_LISTENER(cuechange); + enum TextTrackType { TrackElement, AddTrack, InBand }; + TextTrackType trackType() const { return m_trackType; } + protected: - TextTrack(ScriptExecutionContext*, TextTrackClient*, const String& kind, const String& label, const String& language); + TextTrack(ScriptExecutionContext*, TextTrackClient*, const String& kind, const String& label, const String& language, TextTrackType); void setReadyState(ReadyState); @@ -115,6 +118,7 @@ private: TextTrack::ReadyState m_readyState; TextTrack::Mode m_mode; TextTrackClient* m_client; + TextTrackType m_trackType; }; } // namespace WebCore diff --git a/Source/WebCore/html/track/TextTrackList.cpp b/Source/WebCore/html/track/TextTrackList.cpp index 9fb5e3551f97fc8bffb83c6eb162b73a51632b5a..0ecc42f9f781ec0759eef84bb53f4d33e5cbdbd6 100644 --- a/Source/WebCore/html/track/TextTrackList.cpp +++ b/Source/WebCore/html/track/TextTrackList.cpp @@ -30,6 +30,7 @@ #include "TextTrackList.h" #include "EventNames.h" +#include "LoadableTextTrack.h" #include "ScriptExecutionContext.h" #include "TextTrack.h" #include "TrackEvent.h" @@ -51,29 +52,60 @@ TextTrackList::~TextTrackList() unsigned TextTrackList::length() const { - return m_tracks.size(); + return m_addTrackTracks.size() + m_elementTracks.size(); } TextTrack* TextTrackList::item(unsigned index) { - if (index < m_tracks.size()) - return m_tracks[index].get(); + // 4.8.10.12.1 Text track model + // 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. + + if (index < m_elementTracks.size()) + return m_elementTracks[index].get(); + + index -= m_elementTracks.size(); + if (index < m_addTrackTracks.size()) + return m_addTrackTracks[index].get(); + return 0; } void TextTrackList::append(PassRefPtr track) { RefPtr trackRef = track; - m_tracks.append(trackRef); + + if (trackRef->trackType() == TextTrack::AddTrack) + m_addTrackTracks.append(trackRef); + else if (trackRef->trackType() == TextTrack::TrackElement) { + // Insert tracks added for element in tree order. + size_t index = static_cast(trackRef.get())->trackElementIndex(); + m_elementTracks.insert(index, trackRef); + } else + ASSERT_NOT_REACHED(); + scheduleAddTrackEvent(trackRef); } void TextTrackList::remove(PassRefPtr track) { - size_t index = m_tracks.find(track); + Vector >* tracks = 0; + + if (track->trackType() == TextTrack::TrackElement) + tracks = &m_elementTracks; + else if (track->trackType() == TextTrack::AddTrack) + tracks = &m_addTrackTracks; + else + ASSERT_NOT_REACHED(); + + size_t index = tracks->find(track); if (index == notFound) return; - m_tracks.remove(index); + tracks->remove(index); + } const AtomicString& TextTrackList::interfaceName() const diff --git a/Source/WebCore/html/track/TextTrackList.h b/Source/WebCore/html/track/TextTrackList.h index 1a8704b903b261a076f3aaefc0d0719971608257..3165697a20e365d18efde8a3889534e9b29a51c9 100644 --- a/Source/WebCore/html/track/TextTrackList.h +++ b/Source/WebCore/html/track/TextTrackList.h @@ -87,7 +87,8 @@ private: Timer m_pendingEventTimer; EventTargetData m_eventTargetData; - Vector > m_tracks; + Vector > m_addTrackTracks; + Vector > m_elementTracks; int m_dispatchingEvents; };