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

Supporting Cobalt #5804

Closed
kaidokert opened this issue Oct 24, 2023 · 4 comments · Fixed by #5806
Closed

Supporting Cobalt #5804

kaidokert opened this issue Oct 24, 2023 · 4 comments · Fixed by #5806
Labels
priority: P4 Nice to have / wishful thinking status: archived Archived and locked; will not be updated type: enhancement New feature or request
Milestone

Comments

@kaidokert
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Cobalt ( https://cobalt.dev ) currently doesn't work with Shaka out of the box, due to a few minor missing APIs that currently throw JavaScript exceptions.

Describe the solution you'd like

I have a small patch that stubs out the usage of video.textTracks and document.createNodeIterator that would disable textTracks and some XSS security guards in Shaka.

Describe alternatives you've considered

We could also try and add stub implementations of those web APIs in Cobalt quite easily, and we likely will. However, that wouldn't let all the deployed versions of Cobalt to run with Shaka.

Additional context

Preview of quick 15-minute changes here ( i would remove the lint suppressions, of course ):
kaidokert@f516703

@kaidokert kaidokert added the type: enhancement New feature or request label Oct 24, 2023
@shaka-bot shaka-bot added this to the Backlog milestone Oct 24, 2023
@avelad avelad added the priority: P4 Nice to have / wishful thinking label Oct 24, 2023
@avelad
Copy link
Member

avelad commented Oct 24, 2023

I have reviewed the link, I agree with including the changes in lib/util/xml_utils.js, but I think the changes in lib/text/simple_text_displayer.js should be to make a new text displayer that does nothing, what do you think?

@kaidokert
Copy link
Contributor Author

Yep, i think there was only one call site for the textDisplayer so that change would be cleaner and would throw less surprises.

@avelad
Copy link
Member

avelad commented Oct 24, 2023

I have created #5805 to solve the problem of lib/util/xml_utils.js

For the other change I am willing to review your PR if you are interested in sending it, I think the new class should be called StubTextDisplayer (lib/text/stub_text_displayer.js), then the change would only be to add a new if in https://github.com/shaka-project/shaka-player/blob/main/lib/player.js#L5441

@kaidokert
Copy link
Contributor Author

Thanks much for such a quick response - i'll put a PR together and send it.

kaidokert added a commit to kaidokert/shaka-player that referenced this issue Oct 24, 2023
kaidokert added a commit to kaidokert/shaka-player that referenced this issue Oct 24, 2023
kaidokert added a commit to kaidokert/shaka-player that referenced this issue Oct 24, 2023
kaidokert added a commit to kaidokert/shaka-player that referenced this issue Oct 24, 2023
kaidokert added a commit to kaidokert/shaka-player that referenced this issue Oct 25, 2023
kaidokert added a commit to kaidokert/shaka-player that referenced this issue Oct 25, 2023
@avelad avelad modified the milestones: Backlog, v4.6 Oct 25, 2023
avelad pushed a commit that referenced this issue Oct 26, 2023
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Dec 25, 2023
@shaka-project shaka-project locked as resolved and limited conversation to collaborators Dec 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P4 Nice to have / wishful thinking status: archived Archived and locked; will not be updated type: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants