From f8467f794741cc9b98a4ca521ebeccb5d58a5d7d Mon Sep 17 00:00:00 2001 From: Rick Eyre Date: Fri, 28 Jun 2013 13:31:43 -0400 Subject: [PATCH] Bug 867823 - Implement TextTrack Add/RemoveCue. r=rillian - Updated RemoveCue to throw NotFoundError if the cue being removed is not in the cue list. - Updated AddCue to not add cues that are already in the list. This is done by reference comparing. --- content/media/TextTrack.cpp | 6 ++---- content/media/TextTrack.h | 2 +- content/media/TextTrackCueList.cpp | 9 ++++++++- content/media/TextTrackCueList.h | 3 ++- content/media/test/test_texttrackcue.html | 21 +++++---------------- dom/webidl/TextTrack.webidl | 1 + 6 files changed, 19 insertions(+), 23 deletions(-) diff --git a/content/media/TextTrack.cpp b/content/media/TextTrack.cpp index a4ca21b18b2..4e9ef27f3f2 100644 --- a/content/media/TextTrack.cpp +++ b/content/media/TextTrack.cpp @@ -73,15 +73,13 @@ TextTrack::SetMode(TextTrackMode aValue) void TextTrack::AddCue(TextTrackCue& aCue) { - //XXX: If cue exists, remove. Bug 867823. mCueList->AddCue(aCue); } void -TextTrack::RemoveCue(TextTrackCue& aCue) +TextTrack::RemoveCue(TextTrackCue& aCue, ErrorResult& aRv) { - //XXX: If cue does not exists throw NotFoundError. Bug 867823. - mCueList->RemoveCue(aCue); + mCueList->RemoveCue(aCue, aRv); } void diff --git a/content/media/TextTrack.h b/content/media/TextTrack.h index ee81c6889a5..3f75571aa00 100644 --- a/content/media/TextTrack.h +++ b/content/media/TextTrack.h @@ -99,7 +99,7 @@ public: void Update(double aTime); void AddCue(TextTrackCue& aCue); - void RemoveCue(TextTrackCue& aCue); + void RemoveCue(TextTrackCue& aCue, ErrorResult& aRv); void CueChanged(TextTrackCue& aCue); IMPL_EVENT_HANDLER(cuechange) diff --git a/content/media/TextTrackCueList.cpp b/content/media/TextTrackCueList.cpp index 4cf04ff1a3f..66aa32072f2 100644 --- a/content/media/TextTrackCueList.cpp +++ b/content/media/TextTrackCueList.cpp @@ -66,12 +66,19 @@ TextTrackCueList::GetCueById(const nsAString& aId) void TextTrackCueList::AddCue(TextTrackCue& cue) { + if (mList.Contains(&cue)) { + return; + } mList.AppendElement(&cue); } void -TextTrackCueList::RemoveCue(TextTrackCue& cue) +TextTrackCueList::RemoveCue(TextTrackCue& cue, ErrorResult& aRv) { + if (!mList.Contains(&cue)) { + aRv.Throw(NS_ERROR_DOM_NOT_FOUND_ERR); + return; + } mList.RemoveElement(&cue); } diff --git a/content/media/TextTrackCueList.h b/content/media/TextTrackCueList.h index 81aac89821e..10123eeacbb 100644 --- a/content/media/TextTrackCueList.h +++ b/content/media/TextTrackCueList.h @@ -11,6 +11,7 @@ #include "nsCOMPtr.h" #include "nsCycleCollectionParticipant.h" #include "nsWrapperCache.h" +#include "mozilla/ErrorResult.h" namespace mozilla { namespace dom { @@ -47,7 +48,7 @@ public: TextTrackCue* GetCueById(const nsAString& aId); void AddCue(TextTrackCue& cue); - void RemoveCue(TextTrackCue& cue); + void RemoveCue(TextTrackCue& cue, ErrorResult& aRv); private: nsCOMPtr mParent; diff --git a/content/media/test/test_texttrackcue.html b/content/media/test/test_texttrackcue.html index 5a1616fcd4b..f19cc76a1f7 100644 --- a/content/media/test/test_texttrackcue.html +++ b/content/media/test/test_texttrackcue.html @@ -84,40 +84,29 @@ SpecialPowers.pushPrefEnv({"set": [["media.webvtt.enabled", true]]}, is(cue.text, "foo", "Cue's text should be foo."); // Adding the same cue again should not increase the cue count. - // TODO: https://bugzilla.mozilla.org/show_bug.cgi?id=867823 - // http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#dom-texttrack-addcue trackElement.track.addCue(vttCue); - todo_is(cueList.length, 5, "Cue list length should be 5."); + is(cueList.length, 5, "Cue list length should be 5."); // Check that we are able to remove cues. trackElement.track.removeCue(cue); - // TODO: Marked as todo as incorrect addition up top increases cue count - // to 4 -- https://bugzilla.mozilla.org/show_bug.cgi?id=867823 - todo_is(cueList.length, 4, "Cue list length should be 4."); + is(cueList.length, 4, "Cue list length should be 4."); var exceptionHappened = false; try { // We should not be able to remove a cue that is not in the list. cue = new VTTCue(1, 2, "foo"); - trackElement.removeCue(cue); + trackElement.track.removeCue(cue); } catch (e) { // "NotFoundError" should be thrown when trying to remove a cue that is // not in the list. - // TODO: https://bugzilla.mozilla.org/show_bug.cgi?id=867823 - // http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#dom-texttrack-removecue - todo_is(e.name, "NotFoundError", "We should have caught an error."); + is(e.name, "NotFoundError", "Should have thrown NotFoundError."); exceptionHappened = true; } // If this is false then we did not throw an error and probably removed a cue // when we shouln't have. ok(exceptionHappened, "Exception should have happened."); - // We should not have removed a cue so the cue list length should not - // have changed. - // TODO: Marked as todo as incorrect addition of cue up top increases - // count erroneously. - // https://bugzilla.mozilla.org/show_bug.cgi?id=867823 - todo_is(cueList.length, 4, "Cue list length should be 4."); + is(cueList.length, 4, "Cue list length should be 4."); SimpleTest.finish(); }); diff --git a/dom/webidl/TextTrack.webidl b/dom/webidl/TextTrack.webidl index aaacd8273ce..391bf411916 100644 --- a/dom/webidl/TextTrack.webidl +++ b/dom/webidl/TextTrack.webidl @@ -37,6 +37,7 @@ interface TextTrack : EventTarget { readonly attribute TextTrackRegionList? regions; void addCue(VTTCue cue); + [Throws] void removeCue(VTTCue cue); attribute EventHandler oncuechange;