Skip to content

E
edx-platform
Commit 1ce7d98b by Calen Pennington
Options

Merge pull request #82 from MITx/ps-fix-player 

Fix Youtube player glitches on iOS
parents 6b9130fa 5e9b4bd1
Showing with 297 additions and 134 deletions
static/coffee/spec/helper.coffee
......@@ -11,12 +11,8 @@ jasmine.stubbedMetadata =
duration: 300
jasmine.stubbedCaption =
start: [0, 10000, 20000, 30000, 40000, 50000, 60000, 70000, 80000, 90000,
100000, 110000, 120000]
text: ['Caption at 0', 'Caption at 10000', 'Caption at 20000',
'Caption at 30000', 'Caption at 40000', 'Caption at 50000', 'Caption at 60000',
'Caption at 70000', 'Caption at 80000', 'Caption at 90000', 'Caption at 100000',
'Caption at 110000', 'Caption at 120000']
start: [0, 10000, 20000, 30000]
text: ['Caption at 0', 'Caption at 10000', 'Caption at 20000', 'Caption at 30000']
jasmine.stubRequests = ->
spyOn($, 'ajax').andCallFake (settings) ->
......
static/coffee/spec/modules/video/video_caption_spec.coffee
......@@ -9,66 +9,82 @@ describe 'VideoCaption', ->
describe 'constructor', ->
beforeEach ->
spyOn($, 'getWithPrefix').andCallThrough()
@caption = new VideoCaption @player, 'def456'
it 'set the player', ->
expect(@caption.player).toEqual @player
it 'set the youtube id', ->
expect(@caption.youtubeId).toEqual 'def456'
it 'create the caption element', ->
expect($('.video')).toContain 'ol.subtitles'
it 'add caption control to video player', ->
expect($('.video')).toContain 'a.hide-subtitles'
it 'fetch the caption', ->
expect($.getWithPrefix).toHaveBeenCalledWith @caption.captionURL(), jasmine.any(Function)
it 'render the caption', ->
expect($('.subtitles').html()).toMatch new RegExp('''
<li data-index="0" data-start="0">Caption at 0</li>
<li data-index="1" data-start="10000">Caption at 10000</li>
<li data-index="2" data-start="20000">Caption at 20000</li>
<li data-index="3" data-start="30000">Caption at 30000</li>
<li data-index="4" data-start="40000">Caption at 40000</li>
<li data-index="5" data-start="50000">Caption at 50000</li>
<li data-index="6" data-start="60000">Caption at 60000</li>
<li data-index="7" data-start="70000">Caption at 70000</li>
<li data-index="8" data-start="80000">Caption at 80000</li>
<li data-index="9" data-start="90000">Caption at 90000</li>
<li data-index="10" data-start="100000">Caption at 100000</li>
<li data-index="11" data-start="110000">Caption at 110000</li>
<li data-index="12" data-start="120000">Caption at 120000</li>
'''.replace(/\n/g, ''))
it 'add a padding element to caption', ->
expect($('.subtitles li:first')).toBe '.spacing'
expect($('.subtitles li:last')).toBe '.spacing'
it 'bind all the caption link', ->
$('.subtitles li[data-index]').each (index, link) =>
expect($(link)).toHandleWith 'click', @caption.seekPlayer
it 'bind window resize event', ->
expect($(window)).toHandleWith 'resize', @caption.onWindowResize
it 'bind player resize event', ->
expect($(@player)).toHandleWith 'resize', @caption.onWindowResize
it 'bind player updatePlayTime event', ->
expect($(@player)).toHandleWith 'updatePlayTime', @caption.onUpdatePlayTime
it 'bind the hide caption button', ->
expect($('.hide-subtitles')).toHandleWith 'click', @caption.toggle
it 'bind the mouse movement', ->
expect($('.subtitles')).toHandleWith 'mouseenter', @caption.onMouseEnter
expect($('.subtitles')).toHandleWith 'mouseleave', @caption.onMouseLeave
expect($('.subtitles')).toHandleWith 'mousemove', @caption.onMovement
expect($('.subtitles')).toHandleWith 'mousewheel', @caption.onMovement
expect($('.subtitles')).toHandleWith 'DOMMouseScroll', @caption.onMovement
describe 'always', ->
beforeEach ->
@caption = new VideoCaption @player, 'def456'
it 'set the player', ->
expect(@caption.player).toEqual @player
it 'set the youtube id', ->
expect(@caption.youtubeId).toEqual 'def456'
it 'create the caption element', ->
expect($('.video')).toContain 'ol.subtitles'
it 'add caption control to video player', ->
expect($('.video')).toContain 'a.hide-subtitles'
it 'fetch the caption', ->
expect($.getWithPrefix).toHaveBeenCalledWith @caption.captionURL(), jasmine.any(Function)
it 'bind window resize event', ->
expect($(window)).toHandleWith 'resize', @caption.onWindowResize
it 'bind player resize event', ->
expect($(@player)).toHandleWith 'resize', @caption.onWindowResize
it 'bind player updatePlayTime event', ->
expect($(@player)).toHandleWith 'updatePlayTime', @caption.onUpdatePlayTime
it 'bind player play event', ->
expect($(@player)).toHandleWith 'play', @caption.onPlay
it 'bind the hide caption button', ->
expect($('.hide-subtitles')).toHandleWith 'click', @caption.toggle
it 'bind the mouse movement', ->
expect($('.subtitles')).toHandleWith 'mouseenter', @caption.onMouseEnter
expect($('.subtitles')).toHandleWith 'mouseleave', @caption.onMouseLeave
expect($('.subtitles')).toHandleWith 'mousemove', @caption.onMovement
expect($('.subtitles')).toHandleWith 'mousewheel', @caption.onMovement
expect($('.subtitles')).toHandleWith 'DOMMouseScroll', @caption.onMovement
describe 'when on a non touch-based device', ->
beforeEach ->
spyOn(window, 'onTouchBasedDevice').andReturn false
@caption = new VideoCaption @player, 'def456'
it 'render the caption', ->
expect($('.subtitles').html()).toMatch new RegExp('''
<li data-index="0" data-start="0">Caption at 0</li>
<li data-index="1" data-start="10000">Caption at 10000</li>
<li data-index="2" data-start="20000">Caption at 20000</li>
<li data-index="3" data-start="30000">Caption at 30000</li>
'''.replace(/\n/g, ''))
it 'add a padding element to caption', ->
expect($('.subtitles li:first')).toBe '.spacing'
expect($('.subtitles li:last')).toBe '.spacing'
it 'bind all the caption link', ->
$('.subtitles li[data-index]').each (index, link) =>
expect($(link)).toHandleWith 'click', @caption.seekPlayer
it 'set rendered to true', ->
expect(@caption.rendered).toBeTruthy()
describe 'when on a touch-based device', ->
beforeEach ->
spyOn(window, 'onTouchBasedDevice').andReturn true
@caption = new VideoCaption @player, 'def456'
it 'show explaination message', ->
expect($('.subtitles li')).toHaveHtml "Caption will be displayed when you start playing the video."
it 'does not set rendered to true', ->
expect(@caption.rendered).toBeFalsy()
describe 'mouse movement', ->
beforeEach ->
......@@ -145,8 +161,34 @@ describe 'VideoCaption', ->
expect(@caption.search(9999)).toEqual 0
expect(@caption.search(10000)).toEqual 1
expect(@caption.search(15000)).toEqual 1
expect(@caption.search(120000)).toEqual 12
expect(@caption.search(120001)).toEqual 12
expect(@caption.search(30000)).toEqual 3
expect(@caption.search(30001)).toEqual 3
describe 'onPlay', ->
describe 'when the caption was not rendered', ->
beforeEach ->
spyOn(window, 'onTouchBasedDevice').andReturn true
@caption = new VideoCaption @player, 'def456'
@caption.onPlay()
it 'render the caption', ->
expect($('.subtitles').html()).toMatch new RegExp('''
<li data-index="0" data-start="0">Caption at 0</li>
<li data-index="1" data-start="10000">Caption at 10000</li>
<li data-index="2" data-start="20000">Caption at 20000</li>
<li data-index="3" data-start="30000">Caption at 30000</li>
'''.replace(/\n/g, ''))
it 'add a padding element to caption', ->
expect($('.subtitles li:first')).toBe '.spacing'
expect($('.subtitles li:last')).toBe '.spacing'
it 'bind all the caption link', ->
$('.subtitles li[data-index]').each (index, link) =>
expect($(link)).toHandleWith 'click', @caption.seekPlayer
it 'set rendered to true', ->
expect(@caption.rendered).toBeTruthy()
describe 'onUpdatePlayTime', ->
beforeEach ->
......
static/coffee/spec/modules/video/video_control_spec.coffee
describe 'VideoControl', ->
beforeEach ->
@player = jasmine.stubVideoPlayer @
$('.video-controls').html ''
describe 'constructor', ->
beforeEach ->
@control = new VideoControl @player
it 'render the video controls', ->
new VideoControl @player
expect($('.video-controls').html()).toContain '''
<div class="slider"></div>
<div>
<ul class="vcr">
<li><a class="video_control play">Play</a></li>
<li><a class="video_control play" href="#">Play</a></li>
<li>
<div class="vidtime">0:00 / 0:00</div>
</li>
......@@ -23,12 +22,33 @@ describe 'VideoControl', ->
'''
it 'bind player events', ->
expect($(@player)).toHandleWith 'play', @control.onPlay
expect($(@player)).toHandleWith 'pause', @control.onPause
expect($(@player)).toHandleWith 'ended', @control.onPause
control = new VideoControl @player
expect($(@player)).toHandleWith 'play', control.onPlay
expect($(@player)).toHandleWith 'pause', control.onPause
expect($(@player)).toHandleWith 'ended', control.onPause
it 'bind the playback button', ->
expect($('.video_control')).toHandleWith 'click', @control.togglePlayback
control = new VideoControl @player
expect($('.video_control')).toHandleWith 'click', control.togglePlayback
describe 'when on a touch based device', ->
beforeEach ->
spyOn(window, 'onTouchBasedDevice').andReturn true
it 'does not add the play class to video control', ->
new VideoControl @player
expect($('.video_control')).not.toHaveClass 'play'
expect($('.video_control')).not.toHaveHtml 'Play'
describe 'when on a non-touch based device', ->
beforeEach ->
spyOn(window, 'onTouchBasedDevice').andReturn false
it 'add the play class to video control', ->
new VideoControl @player
expect($('.video_control')).toHaveClass 'play'
expect($('.video_control')).toHaveHtml 'Play'
describe 'onPlay', ->
beforeEach ->
......@@ -54,20 +74,47 @@ describe 'VideoControl', ->
beforeEach ->
@control = new VideoControl @player
describe 'when the video is playing', ->
describe 'when the control does not have play or pause class', ->
beforeEach ->
spyOn(@player, 'isPlaying').andReturn true
spyOnEvent @player, 'pause'
@control.togglePlayback jQuery.Event('click')
$('.video_control').removeClass('play').removeClass('pause')
it 'trigger the pause event', ->
expect('pause').toHaveBeenTriggeredOn @player
describe 'when the video is playing', ->
beforeEach ->
spyOn(@player, 'isPlaying').andReturn true
spyOnEvent @player, 'pause'
@control.togglePlayback jQuery.Event('click')
describe 'when the video is paused', ->
beforeEach ->
spyOn(@player, 'isPlaying').andReturn false
spyOnEvent @player, 'play'
@control.togglePlayback jQuery.Event('click')
it 'does not trigger the pause event', ->
expect('pause').not.toHaveBeenTriggeredOn @player
describe 'when the video is paused', ->
beforeEach ->
spyOn(@player, 'isPlaying').andReturn false
spyOnEvent @player, 'play'
@control.togglePlayback jQuery.Event('click')
it 'does not trigger the play event', ->
expect('play').not.toHaveBeenTriggeredOn @player
for className in ['play', 'pause']
describe "when the control has #{className} class", ->
beforeEach ->
$('.video_control').addClass className
describe 'when the video is playing', ->
beforeEach ->
spyOn(@player, 'isPlaying').andReturn true
spyOnEvent @player, 'pause'
@control.togglePlayback jQuery.Event('click')
it 'trigger the pause event', ->
expect('pause').toHaveBeenTriggeredOn @player
describe 'when the video is paused', ->
beforeEach ->
spyOn(@player, 'isPlaying').andReturn false
spyOnEvent @player, 'play'
@control.togglePlayback jQuery.Event('click')
it 'trigger the play event', ->
expect('play').toHaveBeenTriggeredOn @player
it 'trigger the play event', ->
expect('play').toHaveBeenTriggeredOn @player
static/coffee/spec/modules/video/video_player_spec.coffee
......@@ -90,7 +90,7 @@ describe 'VideoPlayer', ->
describe 'when not on a touch based device', ->
beforeEach ->
window.onTouchBasedDevice = -> false
spyOn(window, 'onTouchBasedDevice').andReturn false
spyOn @player, 'play'
@player.onReady()
......@@ -99,7 +99,7 @@ describe 'VideoPlayer', ->
describe 'when on a touch based device', ->
beforeEach ->
window.onTouchBasedDevice = -> true
spyOn(window, 'onTouchBasedDevice').andReturn true
spyOn @player, 'play'
@player.onReady()
......
static/coffee/spec/modules/video/video_progress_slider_spec.coffee
......@@ -3,34 +3,51 @@ describe 'VideoProgressSlider', ->
@player = jasmine.stubVideoPlayer @
describe 'constructor', ->
beforeEach ->
spyOn($.fn, 'slider').andCallThrough()
@slider = new VideoProgressSlider @player
describe 'on a non-touch based device', ->
beforeEach ->
spyOn($.fn, 'slider').andCallThrough()
spyOn(window, 'onTouchBasedDevice').andReturn false
@slider = new VideoProgressSlider @player
it 'build the slider', ->
expect(@slider.slider).toBe '.slider'
expect($.fn.slider).toHaveBeenCalledWith
range: 'min'
change: @slider.onChange
slide: @slider.onSlide
stop: @slider.onStop
it 'build the seek handle', ->
expect(@slider.handle).toBe '.ui-slider-handle'
expect($.fn.qtip).toHaveBeenCalledWith
content: "0:00"
position:
my: 'bottom center'
at: 'top center'
container: @slider.handle
hide:
delay: 700
style:
classes: 'ui-tooltip-slider'
widget: true
it 'bind player events', ->
expect($(@player)).toHandleWith 'updatePlayTime', @slider.onUpdatePlayTime
expect($(@player)).toHandleWith 'ready', @slider.onReady
expect($(@player)).toHandleWith 'play', @slider.onPlay
describe 'on a touch-based device', ->
beforeEach ->
spyOn($.fn, 'slider').andCallThrough()
spyOn(window, 'onTouchBasedDevice').andReturn true
@slider = new VideoProgressSlider @player
it 'does not build the slider', ->
expect(@slider.slider).toBeUndefined
expect($.fn.slider).not.toHaveBeenCalled()
it 'build the slider', ->
expect(@slider.slider).toBe '.slider'
expect($.fn.slider).toHaveBeenCalledWith
range: 'min'
change: @slider.onChange
slide: @slider.onSlide
stop: @slider.onStop
it 'build the seek handle', ->
expect(@slider.handle).toBe '.ui-slider-handle'
expect($.fn.qtip).toHaveBeenCalledWith
content: "0:00"
position:
my: 'bottom center'
at: 'top center'
container: @slider.handle
hide:
delay: 700
style:
classes: 'ui-tooltip-slider'
widget: true
it 'bind player events', ->
expect($(@player)).toHandleWith 'updatePlayTime', @slider.onUpdatePlayTime
it 'bind player events', ->
expect($(@player)).toHandleWith 'updatePlayTime', @slider.onUpdatePlayTime
describe 'onReady', ->
beforeEach ->
......@@ -41,6 +58,45 @@ describe 'VideoProgressSlider', ->
it 'set the max value to the length of video', ->
expect(@slider.slider.slider('option', 'max')).toEqual 120
describe 'onPlay', ->
beforeEach ->
@slider = new VideoProgressSlider @player
spyOn($.fn, 'slider').andCallThrough()
describe 'when the slider was already built', ->
beforeEach ->
@slider.onPlay()
it 'does not build the slider', ->
expect($.fn.slider).not.toHaveBeenCalled
describe 'when the slider was not already built', ->
beforeEach ->
@slider.slider = null
@slider.onPlay()
it 'build the slider', ->
expect(@slider.slider).toBe '.slider'
expect($.fn.slider).toHaveBeenCalledWith
range: 'min'
change: @slider.onChange
slide: @slider.onSlide
stop: @slider.onStop
it 'build the seek handle', ->
expect(@slider.handle).toBe '.ui-slider-handle'
expect($.fn.qtip).toHaveBeenCalledWith
content: "0:00"
position:
my: 'bottom center'
at: 'top center'
container: @slider.handle
hide:
delay: 700
style:
classes: 'ui-tooltip-slider'
widget: true
describe 'onUpdatePlayTime', ->
beforeEach ->
@slider = new VideoProgressSlider @player
......
static/coffee/src/modules/video/video_caption.coffee
......@@ -10,6 +10,7 @@ class @VideoCaption
$(window).bind('resize', @onWindowResize)
$(@player).bind('resize', @onWindowResize)
$(@player).bind('updatePlayTime', @onUpdatePlayTime)
$(@player).bind('play', @onPlay)
@$('.hide-subtitles').click @toggle
@$('.subtitles').mouseenter(@onMouseEnter).mouseleave(@onMouseLeave)
.mousemove(@onMovement).bind('mousewheel', @onMovement)
......@@ -32,7 +33,11 @@ class @VideoCaption
$.getWithPrefix @captionURL(), (captions) =>
@captions = captions.text
@start = captions.start
@renderCaption()
if onTouchBasedDevice()
$('.subtitles li').html "Caption will be displayed when you start playing the video."
else
@renderCaption()
renderCaption: ->
container = $('<ol>')
......@@ -49,6 +54,8 @@ class @VideoCaption
@$('.subtitles').prepend($('<li class="spacing">').height(@topSpacingHeight()))
.append($('<li class="spacing">').height(@bottomSpacingHeight()))
@rendered = true
search: (time) ->
min = 0
max = @start.length - 1
......@@ -62,6 +69,9 @@ class @VideoCaption
return min
onPlay: =>
@renderCaption() unless @rendered
onUpdatePlayTime: (event, time) =>
# This 250ms offset is required to match the video speed
time = Math.round(Time.convert(time, @player.currentSpeed(), '1.0') * 1000 + 250)
......
static/coffee/src/modules/video/video_control.coffee
......@@ -17,7 +17,7 @@ class @VideoControl
<div class="slider"></div>
<div>
<ul class="vcr">
<li><a class="video_control play">Play</a></li>
<li><a class="video_control" href="#"></a></li>
<li>
<div class="vidtime">0:00 / 0:00</div>
</li>
......@@ -26,7 +26,10 @@ class @VideoControl
<a href="#" class="add-fullscreen" title="Fill browser">Fill Browser</a>
</div>
</div>
"""
"""
unless onTouchBasedDevice()
@$('.video_control').addClass('play').html('Play')
onPlay: =>
@$('.video_control').removeClass('play').addClass('pause').html('Pause')
......@@ -36,7 +39,8 @@ class @VideoControl
togglePlayback: (event) =>
event.preventDefault()
if @player.isPlaying()
$(@player).trigger('pause')
else
$(@player).trigger('play')
if $('.video_control').hasClass('play') || $('.video_control').hasClass('pause')
if @player.isPlaying()
$(@player).trigger('pause')
else
$(@player).trigger('play')
static/coffee/src/modules/video/video_progress_slider.coffee
class @VideoProgressSlider
constructor: (@player) ->
@buildSlider()
@buildHandle()
@buildSlider() unless onTouchBasedDevice()
$(@player).bind('updatePlayTime', @onUpdatePlayTime)
$(@player).bind('ready', @onReady)
$(@player).bind('play', @onPlay)
$: (selector) ->
@player.$(selector)
......@@ -14,6 +14,7 @@ class @VideoProgressSlider
change: @onChange
slide: @onSlide
stop: @onStop
@buildHandle()
buildHandle: ->
@handle = @$('.ui-slider-handle')
......@@ -30,10 +31,13 @@ class @VideoProgressSlider
widget: true
onReady: =>
@slider.slider('option', 'max', @player.duration())
@slider.slider('option', 'max', @player.duration()) if @slider
onPlay: =>
@buildSlider() unless @slider
onUpdatePlayTime: (event, currentTime) =>
if !@frozen
if @slider && !@frozen
@slider.slider('option', 'max', @player.duration())
@slider.slider('value', currentTime)
......
static/sass/courseware/_video.scss
......@@ -137,6 +137,7 @@ section.course-content {
float: left;
margin-bottom: 0;
a {
border-bottom: none;
border-right: 1px solid #000;
......@@ -146,11 +147,17 @@ section.course-content {
line-height: 46px;
padding: 0 lh(.75);
text-indent: -9999px;
@include transition();
@include transition(background-color, opacity);
width: 14px;
background: url('../images/vcr.png') 15px 15px no-repeat;
&:empty {
height: 46px;
background: url('../images/vcr.png') 15px 15px no-repeat;
}
&.play {
background: url('../images/play-icon.png') center center no-repeat;
background-position: 17px -114px;
&:hover {
background-color: #444;
......@@ -158,7 +165,7 @@ section.course-content {
}
&.pause {
background: url('../images/pause-icon.png') center center no-repeat;
background-position: 16px -50px;
&:hover {
background-color: #444;
......@@ -361,7 +368,6 @@ section.course-content {
cursor: pointer;
margin-bottom: 8px;
padding: 0;
@include transition(all, .5s, ease-in);
&.current {
color: #333;
......@@ -386,7 +392,6 @@ section.course-content {
}
ol.subtitles {
height: 0;
width: 0px;
}
}
......@@ -408,7 +413,6 @@ section.course-content {
&.closed {
ol.subtitles {
height: auto;
right: -(flex-grid(4));
width: auto;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment