Bug 68194

Summary: Layout tests for <track> JS bindings
Product: WebKit Reporter: Anna Cavender <annacc>
Component: MediaAssignee: Anna Cavender <annacc>
Status: RESOLVED FIXED    
Severity: Normal CC: eric.carlson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 62887    
Attachments:
Description Flags
Patch
none
addressing eric's comments none

Anna Cavender
Reported 2011-09-15 14:36:04 PDT
Layout tests needed for testing the functionality of JS bindings related to <track>: e.g. TextTrack, TextTrackCue, TextTrackCueList, MutableTextTrack, and the textTracks attribute on HTMLMediaElement.
Attachments
Patch (14.79 KB, patch)
2011-09-15 15:06 PDT, Anna Cavender 		no flags
DetailsFormatted DiffDiff
addressing eric's comments (12.03 KB, patch)
2011-09-20 17:38 PDT, Anna Cavender 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Anna Cavender
Comment 1 2011-09-15 15:06:53 PDT
Created attachment 107554 [details] Patch
Eric Carlson
Comment 2 2011-09-16 07:03:05 PDT
Comment on attachment 107554 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=107554&action=review Does it make sense to commit this until the issues you mention in https://bugs.webkit.org/show_bug.cgi?id=62887 are resolved? > LayoutTests/ChangeLog:3 > + New layout tests for the JS bindings related to <track>: e.g. TextTrack, TextTrackCue, TextTrackCueList, MutableTextTrack, and the textTracks attribute on HTMLMediaElement. Not all editors wrap long lines, so it would be better to break this into multiple shorter lines > LayoutTests/media/track/track-mutable.html:1 > +<html> These are definitely HTML5 tests so they should have a DOCTYPE. > LayoutTests/media/track/track-mutable.html:27 > + testExpected("video.textTracks[0].cues.length", 0); > + captionsTrack.addCue("junk"); > + testExpected("video.textTracks[0].cues.length", 0); > + var cue1 = new TextTrackCue("1", 1.2, 3.4, "Cue #1") > + captionsTrack.addCue(cue1); > + testExpected("video.textTracks[0].cues.length", 1); I think it is helpful to be able to see both the condition being tested and the result of the test, so you might consider using "RUN(...)" so someone reading the output can understand what was tested without looking at the source. These are your tests though, so it is really your call.
Eric Carlson
Comment 3 2011-09-16 07:04:34 PDT
(In reply to comment #2) > > Does it make sense to commit this until the issues you mention in https://bugs.webkit.org/show_bug.cgi?id=62887 are resolved? > Does it make sense to commit this *before* the issues...
Anna Cavender
Comment 4 2011-09-20 17:37:40 PDT
Comment on attachment 107554 [details] Patch Thanks Eric! You're right about not committing the textTracks test until issues in https://bugs.webkit.org/show_bug.cgi?id=62887 are resolved. I've removed it for now. View in context: https://bugs.webkit.org/attachment.cgi?id=107554&action=review >> LayoutTests/ChangeLog:3 >> + New layout tests for the JS bindings related to <track>: e.g. TextTrack, TextTrackCue, TextTrackCueList, MutableTextTrack, and the textTracks attribute on HTMLMediaElement. > > Not all editors wrap long lines, so it would be better to break this into multiple shorter lines Done. >> LayoutTests/media/track/track-mutable.html:1 >> +<html> > > These are definitely HTML5 tests so they should have a DOCTYPE. Done. >> LayoutTests/media/track/track-mutable.html:27 >> + testExpected("video.textTracks[0].cues.length", 1); > > I think it is helpful to be able to see both the condition being tested and the result of the test, so you might consider using "RUN(...)" so someone reading the output can understand what was tested without looking at the source. These are your tests though, so it is really your call. Good idea. Done.
Anna Cavender
Comment 5 2011-09-20 17:38:04 PDT
Created attachment 108085 [details] addressing eric's comments
Eric Carlson
Comment 6 2011-09-21 07:45:37 PDT
Comment on attachment 108085 [details] addressing eric's comments Thanks!
WebKit Review Bot
Comment 7 2011-09-21 09:26:39 PDT
Comment on attachment 108085 [details] addressing eric's comments Clearing flags on attachment: 108085 Committed r95639: <http://trac.webkit.org/changeset/95639>
WebKit Review Bot
Comment 8 2011-09-21 09:26:43 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.