Bug 72215 - Enable WebVTT parsing layout tests
Summary: Enable WebVTT parsing layout tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Anna Cavender
URL:
Keywords:
Depends on: 72369 72398 72399
Blocks: 43668
  Show dependency treegraph
 
Reported: 2011-11-12 08:42 PST by Anna Cavender
Modified: 2011-11-16 16:56 PST (History)
4 users (show)

See Also:


Attachments
Patch (119.64 KB, patch)
2011-11-12 08:55 PST, Anna Cavender
no flags Details | Formatted Diff | Diff
Patch for landing (120.12 KB, patch)
2011-11-12 15:15 PST, Anna Cavender
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Anna Cavender 2011-11-12 08:42:35 PST
Now that more <track> pieces are in place, we can enable the WebVTT parsing tests!

A few little tweeks are required: (1) create a TextTrackCueList when TextTrack's cues() is called and (2) pay attention to chronological timestamp order requirement of WebVTT files.  I'll include both small changes in with the patch.
Comment 1 Anna Cavender 2011-11-12 08:55:06 PST
Created attachment 114835 [details]
Patch
Comment 2 Eric Carlson 2011-11-12 13:44:39 PST
Comment on attachment 114835 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114835&action=review

> LayoutTests/media/track/track-webvtt-tc000-empty.html:2
> +<html>
> +    <head>

These are tests of an HTML5 feature, so they might as well have "<!DOCTYPE html>"
Comment 3 Eric Carlson 2011-11-12 13:59:29 PST
Comment on attachment 114835 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114835&action=review

> Source/WebCore/html/TextTrackCueList.cpp:86
>  void TextTrackCueList::add(PassRefPtr<TextTrackCue> cue)
>  {
> +    // WebVTT cue timings
> +    // 1. The time represented by this WebVTT timestamp must be greater than or equal
> +    // to the start time offsets of all previous cues in the file.
> +    // http://www.whatwg.org/specs/web-apps/current-work/#webvtt-cue-timings
> +    // Note: because this requirement is specific to WebVTT, we may want to check first
> +    // whether the cues in this list came from a WebVTT file.

This needs a layout test if we don't already have one.
Comment 4 Anna Cavender 2011-11-12 15:05:29 PST
Comment on attachment 114835 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=114835&action=review

>> Source/WebCore/html/TextTrackCueList.cpp:86
>> +    // whether the cues in this list came from a WebVTT file.
> 
> This needs a layout test if we don't already have one.

Yes, this is the one: track-webvtt-tc012-out-of-order.html

>> LayoutTests/media/track/track-webvtt-tc000-empty.html:2
>> +    <head>
> 
> These are tests of an HTML5 feature, so they might as well have "<!DOCTYPE html>"

Will do, thanks.
Comment 5 Anna Cavender 2011-11-12 15:15:36 PST
Created attachment 114844 [details]
Patch for landing
Comment 6 WebKit Review Bot 2011-11-12 16:48:15 PST
Comment on attachment 114844 [details]
Patch for landing

Clearing flags on attachment: 114844

Committed r100085: <http://trac.webkit.org/changeset/100085>
Comment 7 WebKit Review Bot 2011-11-12 16:48:21 PST
All reviewed patches have been landed.  Closing bug.