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

Set initial video quality to large instead of default to avoid automatic switch to HD when iframe resizes. [BLD 981] #3447

Merged
merged 1 commit into from
Apr 24, 2014

Conversation

jmclaus
Copy link

@jmclaus jmclaus commented Apr 23, 2014

@polesye @valera-rozuvan Please review.

@polesye
Copy link
Contributor

polesye commented Apr 23, 2014

Do we need some test for this change?

@jmclaus
Copy link
Author

jmclaus commented Apr 23, 2014

@polesye It's a minor change. But I will add a test to check that player.setPlaybackQuality gets called.

@polesye
Copy link
Contributor

polesye commented Apr 23, 2014

Just check that initial value is 'large'.

@jmclaus
Copy link
Author

jmclaus commented Apr 23, 2014

@polesye Added a test.

@polesye
Copy link
Contributor

polesye commented Apr 24, 2014

It works well in all browsers except Safari. But there is an issue of youtube API.

@polesye
Copy link
Contributor

polesye commented Apr 24, 2014

👍

@polesye
Copy link
Contributor

polesye commented Apr 24, 2014

hmm..looks like this test doesn't make sense and you was right with checking that player.setPlaybackQuality gets called.

@jmclaus
Copy link
Author

jmclaus commented Apr 24, 2014

@polesye I just tested it again with Safari 6.1.3 on OSX 10.7.5, it looks like it works, I cannot reproduce the bug when I follow the steps outlined in:

https://edx-wiki.atlassian.net/browse/BLD-981

@jmclaus
Copy link
Author

jmclaus commented Apr 24, 2014

@polesye I will modify the test.

@polesye
Copy link
Contributor

polesye commented Apr 24, 2014

@jmclaus Click play button -> adjust the speed of any other except 1st -> change quality. And you'll see that video continue playing from the start on 1st speed.

@jmclaus
Copy link
Author

jmclaus commented Apr 24, 2014

@polesye Yes, I could reproduce it. But this should be a different ticket. Should I file one on Jira?

@polesye
Copy link
Contributor

polesye commented Apr 24, 2014

There was one BLD-977. I think, we should wait for the fix from Youtube team. What do you think?

…tic switch to HD when iframe resizes. [BLD 981]
@jmclaus
Copy link
Author

jmclaus commented Apr 24, 2014

@polesye Yes, we should wait for the fix instead of hacking around the issue.

I just added the modified test, rebased and squashed commits. Good to merge?

@valera-rozuvan Please review this when you have time.

@valera-rozuvan
Copy link
Contributor

@jmclaus Looks good! Good to merge! 👍

@polesye
Copy link
Contributor

polesye commented Apr 24, 2014

👍

jmclaus pushed a commit that referenced this pull request Apr 24, 2014
…by_itself

Set initial video quality to large instead of default to avoid automatic switch to HD when iframe resizes. [BLD 981]
@jmclaus jmclaus merged commit e7e9fb3 into master Apr 24, 2014
@jmclaus jmclaus deleted the jmclaus/bugfix_hd_button_activates_by_itself branch April 24, 2014 14:32
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.

3 participants