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

fix(tests): Skip a test on old Safari #8356

Merged
merged 1 commit into from
Jul 19, 2023

Conversation

mister-ben
Copy link
Contributor

@mister-ben mister-ben commented Jul 12, 2023

Description

Safari versions less than 14 have a broken document.styleSheets - not all style elements are included, which breaks a recent test. We're currently running tests on Brpwserstack on Safari 12 on main.

Specific Changes proposed

Since we use the copyStyleSheets function only in browsers that support documentPictureInPicture, this simply skips the test on Safari < 14. Should Safari implement documentPictureInPicture in a future update this isn't a concern as document.styleSheets behaves properly on 14+.

Requirements Checklist

  • Feature implemented / Bug fixed
  • Change has been verified in an actual browser (Chrome, Firefox, IE)
  • Unit Tests updated or fixed
  • Reviewed by Two Core Contributors

Copy link
Contributor

@amtins amtins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mister-ben would there be a way to run all the tests when a PR has reached its review quota? This would make it possible to see that some tests don't pass before reaching main.

@mister-ben mister-ben merged commit 452a918 into videojs:main Jul 19, 2023
@mister-ben
Copy link
Contributor Author

@amtins agree we need to have PR tests running on browserstack

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

Successfully merging this pull request may close these issues.

2 participants