Force video player in flash mode. - #3195
Conversation
There was a problem hiding this comment.
By using current helpers you shouldn't remember possible values for the player mode.
|
Am I right, that there is no way to use this code to force flash mode in manual tests? |
|
Just set the cookie. |
|
I will try then) |
|
Works fine manually. |
|
@valera-rozuvan please review. |
There was a problem hiding this comment.
@polesye Please write a comment what is being hidden here, and why. Is it the entire transcripts panel?
There was a problem hiding this comment.
Nothing is being hidden. Just a class that provides us some information about video player.
There was a problem hiding this comment.
@polesye I didn't know this. When I look at the code in two month, I will now know why we are doing this. Please write a comment what information is provided and where it is used. For example do we use this in tests?
There was a problem hiding this comment.
I think the name speaks for itself. Yes, it is.
There was a problem hiding this comment.
@polesye Do you mind if I add a comment for this line of code, and commit?
|
@polesye I have tried running this branch locally in Chrome, and I get the following JavaScript error: |
|
I think it's because latest rebase with conflicts. I'll fix it. |
|
Try it now. |
|
@polesye The error is not present any longer. Cool. |
|
@polesye I have added a comment. Please see 266ce6ebefb655a7f8c7afff6dbb0cb8d0bdf844 commit. |
|
@polesye I have checked locally. Video player works as expected in Firefox (Flash, and HTML5 modes), and in Chrome. Code looks good. 👍 Good to merge! |
|
@polesye Removed my commit 266ce6ebefb655a7f8c7afff6dbb0cb8d0bdf844 from this PR. Sorry for the troubles! = ) |
|
@polesye Please squash commits before merging. |
Force video player in flash mode.
3 participants
Ticket: BLD-958
Solution: add cookie
edX_video_player_modewithflashvalue to force video player in flash mode.@auraz , @valera-rozuvan please review.