Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing: should drop line:100,invalid setting #336

Closed
zcorpan opened this issue Mar 1, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@zcorpan
Copy link
Member

commented Mar 1, 2017

http://w3c.github.io/webvtt/#cue-timings-and-settings-parsing

Let cue’s WebVTT cue line be number.

If the last character in linepos is a U+0025 PERCENT SIGN character (%), then let cue’s WebVTT cue snap-to-lines flag be false. Otherwise, let it be true.

I think we should validate linealign before setting the WebVTT cue line. Equivalent for position setting. It is bad for future extensibility to use the line/position with unrecognized alignment.

(Noticed this when looking at @BenjaminSchaaf's revamp of parsing tests.)

@silviapfeiffer

This comment has been minimized.

Copy link
Member

commented Mar 1, 2017

They are just numbers at this stage, not interpreted into actual positions. Why would it matter?

@zcorpan

This comment has been minimized.

Copy link
Member Author

commented Mar 2, 2017

The normal parsing of settings in WebVTT is to drop it on the floor if there is a syntax error. This case does not do that.

It matters because it gives unexpected result and makes it difficult to start using new alignments in the transition period when some implementations support the new value and some do not.

Let's say browsers implement what the spec requires now. Then we add a new alignment "foo". Then the author uses it like this, providing a fallback for legacy UAs:

00:00:00.000 --> 00:00:10.000 line:50% line:25%,foo
Test

The expectation is that legacy UAs use the first setting and ignore line:25%,foo, and new UAs apply the second setting because they support foo. With the current spec legacy UAs would apply line:25% and ignore ,foo. This is bad.

@silviapfeiffer

This comment has been minimized.

Copy link
Member

commented Mar 2, 2017

Ah yeah that makes sense. Good find!

zcorpan added a commit that referenced this issue Mar 3, 2017

zcorpan added a commit that referenced this issue Mar 27, 2017

zcorpan added a commit that referenced this issue Mar 27, 2017

@zcorpan zcorpan closed this in #337 Mar 27, 2017

zcorpan added a commit that referenced this issue Mar 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.