Add support for header fields. #6
Conversation
We don't actually parse VTT header fields, but we ignore them for now. Fixes w3c#5
|
If they are not actually being validated, is this useful? |
|
Yes, because now the parser won't fail VTT files with extra lines at the top. This makes it spec compliant. |
|
But this is also a validator. And since some of those lines have normative effect as @foolip pointed out, don't we want something more? |
|
Until we all agree to support Regions, there's nothing normative to parse. |
|
I can't quite figure out how many lines this patch skips, but it doesn't seem to match http://dev.w3.org/html5/webvtt/#webvtt-file-structure since it doesn't check for ":" anywhere. I guess there is invalid markup which is now silently skipped over? |
|
I was only taking a first step in making it conformant to http://dev.w3.org/cvsweb/html5/webvtt/Overview.html?rev=1.62;content-type=text%2Fhtml#syntax , which is Ian's old spec. The new spec indeed requires header lines to have the "name : value" structure. I'm happy to extend the patch with that. |
|
That spec you linked to says:
That doesn't allow for any extra lines at all. |
|
In a browser, there's no reason to bother with these lines unless one supports regions. If this were my validator, I'd either want full regions support or none at all. But it's not. |
|
@foolip I misread the old spec, sorry. I seemed to remember it skipping lines. I'll add the parsing as you suggest. (Just not tonight). |
|
What you remember is probably the parser, which does skip lines. |
|
So it seems not everyone is interested in shipping regions. I'm not sure I would accept a patch for the validator until that is resolved somehow. |
|
@annevk right, not everyone wants regions, but everyone skips those extra lines, which is what my current patch does. |
|
It's not clear to me that consensus has shifted to make that actually conforming. There's a lot of things that are ignored or processed in a certain way that are still not acceptable from a conformance perspective. |
|
Firefox, Chrome, Safari, Opera ignore them. Only IE doesn't and that's because it's not been updated in a while. |
|
Okay, but should those lines be flagged as being in error since they are not comments? That's what I was getting at mainly. |
|
OK, I have added error handling for metadata headers. Anything that doesn't have a ":" will be flagged as an error. Also, lines that have a time spec in them raise an error that cues need to be separated from metadata headers. This should match the spec: http://dev.w3.org/html5/webvtt/#webvtt-file-structure |
|
Thanks @silviapfeiffer. I'm still a bit concerned as I haven't heard from implementers that they want to pursue this direction of the format. E.g. are implementers planning on exposing this metadata through an API? @foolip per #6 (comment) seems against it and he's a co-editor of the specification... |
|
As far as I know no browser has shipped regions yet. I currently have no plan to work on regions in Blink, but can't predict what happens if someone else does the work and sends an Intent to Ship. Anyway, if you want to support regions in the validator, I'd recommend warning about any metadata header except "Region". Otherwise you won't notice the typo "Regoin" etc. |
|
It was behind a flag in Chrome - has it been ripped out? Safari has released it - I can test in it in version 7.1 though it does seem buggy. Apple have also released them in QuickTime players and AVFoundation. I think they're doing this to be able to claim full compliance with US law. |
|
It's still behind a flag in Chromium. You're right that Safari does expose VTTRegion, which is news to me. That makes the case for supporting it in the validator a lot stronger. |
|
Allowing Also it would make sense to fully support regions, including the |
|
I learned the other day that STYLE blocks are being implemented in webkit. |
update code
updated your fork with latest commits
|
now that there are no conflicts, can this be merged? |
|
What's the state of implementations? |
|
I believe regions are implemented in webkit, chromium & gecko, and style is implemented in webkit, chromium has an "intent to implement", I haven't seen anything for style in gecko. |
|
It looks like this PR is using the old region syntax. The new syntax uses blocks instead of "headers". |
|
This PR definitely needs an update to the latest spec. |
|
Forgot to send this: https://www.w3.org/wiki/TimedText/WebVTT_Implementation_Report best review of what's implemented and what's not |
We don't actually parse VTT header fields, but we ignore them for now.
Fixes #5
Now, a file without any blank lines is perfectly valid, so we will never get an error for missing a blank line after the headers. Instead, just look-ahead if after the next blank line is a cue line to stop parsing blank lines.