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

feat(WebVTT): Handle badly formed VTT #6147

Merged

Conversation

dave-nicholas
Copy link
Contributor

Handle remove chevrons that appear as part of the inner text of the element to avoid parse failure.

@avelad avelad added type: enhancement New feature or request component: WebVTT The issue involves WebVTT subtitles specifically priority: P3 Useful but not urgent labels Jan 24, 2024
@avelad avelad added this to the v5.0 milestone Jan 24, 2024
@avelad avelad changed the title feat: Handle badly formed VTT feat(WebVTT): Handle badly formed VTT Jan 24, 2024
@avelad avelad requested a review from gkatsev January 25, 2024 13:29
@avelad
Copy link
Collaborator

avelad commented Jan 25, 2024

@gkatsev can your review it also? Thanks!

@gkatsev
Copy link
Contributor

gkatsev commented Jan 25, 2024

Yes, taking a look. Decided it was worth taking a look at the spec, and it seems like < isn't allowed in cue text body, but > is allowed: https://www.w3.org/TR/webvtt/#webvtt-cue-text-span

I've also confirmed the behavior in firefox, chrome, and safari, where if there's < things after it get swallowed up and not rendered.

For example:

t.addCue(new VTTCue(0, 10, '<c>foobar < > > fdjkfdj</c>'))

will only render foobar > fdjkfdj

and

t.addCue(new VTTCue(0, 10, '<c>foobar > foo >foo <test> > < fdjkfdj</c>')) 

will only render foobar > foo >foo >

and

t.addCue(new VTTCue(0, 10, '<c>foobar --> foobaz</c>'))

will render as foobar --> foobaz

Given this, we can probably even simplify the escape a bit.

@dave-nicholas
Copy link
Contributor Author

dave-nicholas commented Jan 25, 2024

I will remove the function I added to string utils and create a method in VTT parser just for this.

@avelad avelad merged commit 335eab0 into shaka-project:main Jan 26, 2024
11 of 15 checks passed
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Mar 26, 2024
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Mar 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
component: WebVTT The issue involves WebVTT subtitles specifically priority: P3 Useful but not urgent status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants