Closed Bug 879431 Opened 8 years ago Closed 7 years ago
Track Cue's Text Track property is not being set correctly
TextTrackCue needs a GetTrack() method. http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#texttrackcue
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → WONTFIX
The bug is already fixed (GetTrack method is already added to the TextTrackCue class) and can be marked as resolved.
Okay cool. I don't know why I thought it wasn't at the time :S heh. Can you please add some code to the test content/media/test/test_texttrackcue.html to verify that this is working. You'll probably want to check that GetTrack() works on Cues that were created by a TrackElement and also by Cues that were created and then added to a TextTrack. You'll also want to check that it returns null, or whatever the spec says, when a Cue is created, but not attached to a TextTrack. You can read up here on Mochitests, which is what this test uses, https://developer.mozilla.org/en/docs/Mochitest.
Attachment #819882 - Flags: review?(giles) → review-
I'm going to take over finish this patch up since Andriy doesn't have time to finish it off right now and we're going to need this implemented for when the pref gets turned on, which means landing it in time for FF31.
Assignee: justandy50 → rick.eyre
I've kept the author as Andriy. I've only rebased and fixed the issues that Ralph pointed out.
Comment on attachment 8400745 [details] [diff] [review] v2: Set VTTCue's TextTrack from TextTrack::AddCue r=rillian Review of attachment 8400745 [details] [diff] [review]: ----------------------------------------------------------------- r=me with nit. ::: content/media/TextTrackCue.cpp @@ +7,5 @@ > #include "mozilla/dom/TextTrackCue.h" > #include "mozilla/dom/TextTrackRegion.h" > #include "nsComponentManagerUtils.h" > #include "mozilla/ClearOnShutdown.h" > +#include "mozilla/dom/TextTrack.h" Why is this needed if it wasn't before?
Attachment #8400745 - Flags: review?(giles) → review+
(In reply to Ralph Giles (:rillian) from comment #7) > Why is this needed if it wasn't before? Woops, I missed taking that out when moving the SetTrack function from the .cpp file to the .h file. Carrying forward r=rillian.
Attachment #8400745 - Attachment is obsolete: true
Had to include TextTrack.h, but in the TextTrackCue header, not cpp, which Ralph questioned earlier. We need it in the TextTrack header because we're assigning into a TextTrack smart pointer. If we don't add it complains about access into an incomplete type. Carrying forward r=rillian.
Attachment #8400812 - Attachment is obsolete: true
Try looks good: https://tbpl.mozilla.org/?tree=Try&rev=b0ebc90da4d8
Status: REOPENED → RESOLVED
Closed: 8 years ago → 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.