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

Parser drops identifier on the floor #310

Closed
zcorpan opened this issue Jul 1, 2016 · 2 comments
Closed

Parser drops identifier on the floor #310

zcorpan opened this issue Jul 1, 2016 · 2 comments

Comments

@zcorpan
Copy link
Member

zcorpan commented Jul 1, 2016

@alastor0325 noticed that the spec's parser doesn't set the cue identifier to anything but the empty string. Oops. This regressed in #246

Fix is to set the cue identifier to buffer on cue creation in the parser.

@alastor0325
Copy link
Collaborator

Before to set the cue identifier to buffer on cue creation in the parser, whether there is another mistake that can't handle --> correctly for cue identifier?

Could you help me to verify the following steps?

Take this file as example,

In step 13-1 in WebVTT parser algorithm, we would go to read the the line -->.

Collect a WebVTT block, and let block be the returned value.

Since line contains "-->", then go to the step 11-4 in collect a WebVTT block

If line contains the three-character substring "-->" (U+002D HYPHEN-MINUS, U+002D HYPHEN-MINUS, U+003E GREATER-THAN SIGN), then run these substeps:

In step 11-4-1-4, run Collect WebVTT cue timings and settings, but it would fails because the line doesn't have any timestamp.

Collect WebVTT cue timings and settings from line using regions for cue. If that fails, let cue be null. Otherwise, let buffer be the empty string.

After above steps, we parse identifier fail, so we would get next line to line. Since the line still contains "-->", go to the step 11-4 in collect a WebVTT block again.

However, seen arrow has already be set to true in previous steps, so we won't go to step 11-4-1

line count is 2 and seen arrow is false

We would go here, and break out the loop. At that time, the cue is null and the position is previous postion, that means it points back to the cue identifier place of the file. (string -->)

Otherwise, let position be previous position and break out of loop.

Therefore, go back to step 13-1 in WebVTT parser algorithm. Since we don't get the valid value for block, we go to next loop and the result would still be the same. (doesn't get the cue)

@zcorpan
Copy link
Member Author

zcorpan commented Jul 6, 2016

I think your analysis is correct except for this:

At that time, the cue is null and the position is previous postion, that means it points back to the cue identifier place of the file. (string -->)

In particular consider steps 11-3 and 11-4-1-2 of collect a WebVTT block in the first iteration. previous position will point to the start of the line 00:00:00.000 --> 00:00:01.000.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants