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

Player crash in Apache Cordova Applications when using cordova-plugin-network-information #4542

Closed
cristian-atehortua opened this issue Oct 3, 2022 · 3 comments · Fixed by #5157
Assignees
Labels
priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@cristian-atehortua
Copy link
Contributor

Have you read the FAQ and checked for duplicate open issues?
Yes

What version of Shaka Player are you using?
3.3.9

Can you reproduce the issue with our latest release version?
Yes

Can you reproduce the issue with the latest code from main?
Yes

Are you using the demo app or your own custom app?
Custom App

If custom app, can you reproduce the issue using our demo app?
Yes

What browser and OS are you using?
Android, iOS - Apache Cordova application

For embedded devices (smart TVs, etc.), what model and firmware version are you using?
Any

What are the manifest and license server URIs?

What configuration are you using? What is the output of player.getConfiguration()?

Default

What did you do?

  1. Create an app using Apache Cordova https://cordova.apache.org/docs/en/11.x/guide/cli/index.html
  2. Add the Network information plugin https://cordova.apache.org/docs/en/11.x/reference/cordova-plugin-network-information/index.html
  3. Run the app on a device and playback any content.

What did you expect to happen?
The playback starts

What actually happened?
The player crashes while loading the source. There is no much detail in error but it says addEventListener is not a function. This happens because navigator.connection object provided by network information plugin of Apache Cordova doesn't include an addEventListener method. This can be solved by adding validation on simple_abr_manager.js to check if the method exists.

@cristian-atehortua cristian-atehortua added the type: bug Something isn't working correctly label Oct 3, 2022
@github-actions github-actions bot added this to the v4.3 milestone Oct 3, 2022
@koenoe
Copy link
Contributor

koenoe commented Oct 12, 2022

I think you could solve this by creating a custom ABR Manager based on the information available in cordova-plugin-network and pass that class to abrFactory in your player configuration? This way you skip using the default ABR Manager. As I think simple_abr_manager.js should be allowed to rely on default web apis and shouldn't handle edge cases like this in Cordova.

@cristian-atehortua
Copy link
Contributor Author

Hi, @koenoe I agree this can be bypassed by adding a custom abr manager.

However, for ease of use of Shaka Player, I think simple_abr_manager can avoid breaking by adding validation to check if that function exists before calling it.

We need to have into account that this Cordova plugin intends to implement a polyfill for standard apis but it doesn't include addEventListener function, I suppose, because there is no way to do it. I also know that the best solution would be this plugin to implement that function, but again, I think Shaka should not break.

@joeyparrish
Copy link
Member

It's the plugin's fault for exposing a half-complete version of the API, but a single "if" in Shaka isn't too bad. Please send a PR. Thanks!

Also, please keep in mind that Cordova isn't part of our automated testing. So while we can't guarantee future compatibility ourselves, we are happy to have the community support the platforms it cares about.

@joeyparrish joeyparrish added the priority: P2 Smaller impact or easy workaround label Oct 14, 2022
@avelad avelad modified the milestones: v4.3, v4.4 Nov 11, 2022
cristian-atehortua added a commit to cristian-atehortua/shaka-player that referenced this issue Apr 18, 2023
…removeEventListener in navigator.connection before using them

fixes shaka-project#4542
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jun 17, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
4 participants