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

fix: Do not patch canPlayType on Android Chrome #4885

Merged
merged 1 commit into from Jan 30, 2018

Conversation

Projects
None yet
2 participants
@mister-ben
Copy link
Contributor

mister-ben commented Jan 17, 2018

Description

Patching canPlayType on Android shouldn't be necessary on Android Chrome since it reports its ability to player HLS correctly.

Specific Changes proposed

Skip patching if UA is Android Chrome.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors
@gkatsev
Copy link
Member

gkatsev left a comment

Seems reasonable. What are your thoughts on removing the IS_OLD_ANDROID piece?
Google doesn't even track 2.2 on their dashboard anymore.

@mister-ben

This comment has been minimized.

Copy link
Contributor

mister-ben commented Jan 23, 2018

I think there's a vanishingly small chance that anyone's still watching video on Andorid 2.2 but as it would be a breaking change ought it stay for now?

@gkatsev

This comment has been minimized.

Copy link
Member

gkatsev commented Jan 23, 2018

I guess we should leave it in.

@gkatsev

This comment has been minimized.

Copy link
Member

gkatsev commented Jan 23, 2018

@mister-ben also, should we do this as a fix or would it be better to release as a minor?

@gkatsev gkatsev added the confirmed label Jan 25, 2018

@gkatsev

This comment has been minimized.

Copy link
Member

gkatsev commented Jan 25, 2018

Actually, since I'm going to merge this and the other features, it'll be released as a minor.

@gkatsev gkatsev added the patch label Jan 25, 2018

@gkatsev gkatsev merged commit f03ac5e into videojs:master Jan 30, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment