From 102d1cb25369b567b977dc841c9c250e534c1415 Mon Sep 17 00:00:00 2001 From: polesye <s2pak.anton@gmail.com> Date: Thu, 16 Jan 2014 15:42:02 +0200 Subject: [PATCH] Persist student progress in video. Student logins to edx, plays video, selects position, closes browser, opens video, position is restored. BLD-385 --- CHANGELOG.rst | 2 + .../contentstore/features/video.feature | 51 +++++ cms/djangoapps/contentstore/features/video.py | 14 ++ common/lib/xmodule/xmodule/fields.py | 5 +- .../xmodule/xmodule/js/fixtures/video.html | 1 + .../xmodule/js/fixtures/video_all.html | 1 + .../xmodule/js/fixtures/video_html5.html | 1 + .../js/fixtures/video_no_captions.html | 1 + .../js/fixtures/video_yt_multiple.html | 1 + common/lib/xmodule/xmodule/js/spec/helper.js | 11 +- .../js/spec/video/cookie_storage_spec.js | 149 -------------- .../xmodule/js/spec/video/events_spec.js | 1 + .../xmodule/js/spec/video/general_spec.js | 17 +- .../xmodule/js/spec/video/html5_video_spec.js | 2 +- .../xmodule/js/spec/video/initialize_spec.js | 154 ++++++++++++++ .../js/spec/video/video_caption_spec.js | 5 +- .../js/spec/video/video_control_spec.js | 1 + .../js/spec/video/video_player_spec.js | 5 +- .../spec/video/video_progress_slider_spec.js | 1 + .../spec/video/video_quality_control_spec.js | 11 +- .../js/spec/video/video_speed_control_spec.js | 1 + .../js/spec/video/video_storage_spec.js | 83 ++++++++ .../spec/video/video_volume_control_spec.js | 1 + common/lib/xmodule/xmodule/js/src/time.coffee | 13 ++ .../xmodule/js/src/video/00_cookie_storage.js | 194 ------------------ .../xmodule/js/src/video/00_video_storage.js | 103 ++++++++++ .../xmodule/js/src/video/01_initialize.js | 108 ++++++---- .../xmodule/js/src/video/03_video_player.js | 92 +++++---- .../js/src/video/06_video_progress_slider.js | 19 +- .../xmodule/xmodule/js/src/video/10_main.js | 19 +- common/lib/xmodule/xmodule/video_module.py | 22 +- .../courseware/features/video.feature | 11 + lms/djangoapps/courseware/features/video.py | 33 +++ .../courseware/tests/test_video_mongo.py | 4 + lms/templates/video.html | 1 + 35 files changed, 656 insertions(+), 482 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/js/spec/video/cookie_storage_spec.js create mode 100644 common/lib/xmodule/xmodule/js/spec/video/initialize_spec.js create mode 100644 common/lib/xmodule/xmodule/js/spec/video/video_storage_spec.js delete mode 100644 common/lib/xmodule/xmodule/js/src/video/00_cookie_storage.js create mode 100644 common/lib/xmodule/xmodule/js/src/video/00_video_storage.js diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 67a39f30d88..73bcfff2b52 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, in roughly chronological order, most recent first. Add your entries at or near the top. Include a label indicating the component affected. +Blades: Persist student progress in video. BLD-385. + Blades: Fix for the list metadata editor that gets into a bad state where "Add" is disabled. BLD-821. diff --git a/cms/djangoapps/contentstore/features/video.feature b/cms/djangoapps/contentstore/features/video.feature index b4a6bfa311f..08ba777505d 100644 --- a/cms/djangoapps/contentstore/features/video.feature +++ b/cms/djangoapps/contentstore/features/video.feature @@ -95,3 +95,54 @@ Feature: CMS.Video Component And I save changes And I click video button "play" Then I see a range on slider + + # 12 + Scenario: Check that position is stored on page refresh, position within start-end range + Given I have created a Video component with subtitles + And Make sure captions are closed + And I edit the component + And I open tab "Advanced" + And I set value "00:00:12" to the field "Start Time" + And I set value "00:00:24" to the field "End Time" + And I save changes + And I click video button "play" + Then I see a range on slider + Then I seek video to "16" seconds + And I click video button "pause" + And I reload the page + And I click video button "play" + Then I see video starts playing from "0:16" position + + # 13 + Scenario: Check that position is stored on page refresh, position before start-end range + Given I have created a Video component with subtitles + And Make sure captions are closed + And I edit the component + And I open tab "Advanced" + And I set value "00:00:12" to the field "Start Time" + And I set value "00:00:24" to the field "End Time" + And I save changes + And I click video button "play" + Then I see a range on slider + Then I seek video to "5" seconds + And I click video button "pause" + And I reload the page + And I click video button "play" + Then I see video starts playing from "0:12" position + + # 14 + Scenario: Check that position is stored on page refresh, position after start-end range + Given I have created a Video component with subtitles + And Make sure captions are closed + And I edit the component + And I open tab "Advanced" + And I set value "00:00:12" to the field "Start Time" + And I set value "00:00:24" to the field "End Time" + And I save changes + And I click video button "play" + Then I see a range on slider + Then I seek video to "30" seconds + And I click video button "pause" + And I reload the page + And I click video button "play" + Then I see video starts playing from "0:12" position diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index 2a862225841..2dc4bccfb39 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -198,3 +198,17 @@ def click_button_video(_step, button_type): button = button_type.strip() world.css_click(VIDEO_BUTTONS[button]) + +@step('I seek video to "([^"]*)" seconds$') +def seek_video_to_n_seconds(_step, seconds): + time = float(seconds.strip()) + jsCode = "$('.video').data('video-player-state').videoPlayer.onSlideSeek({{time: {0:f}}})".format(time) + world.browser.execute_script(jsCode) + + +@step('I see video starts playing from "([^"]*)" position$') +def start_playing_video_from_n_seconds(_step, position): + world.wait_for( + func=lambda _: world.css_html('.vidtime')[:4] == position.strip(), + timeout=5 + ) diff --git a/common/lib/xmodule/xmodule/fields.py b/common/lib/xmodule/xmodule/fields.py index 7eefd060868..74d7a61b7df 100644 --- a/common/lib/xmodule/xmodule/fields.py +++ b/common/lib/xmodule/xmodule/fields.py @@ -140,7 +140,8 @@ class RelativeTime(Field): # Timedeltas are immutable, see http://docs.python.org/2/library/datetime.html#available-types MUTABLE = False - def _isotime_to_timedelta(self, value): + @classmethod + def isotime_to_timedelta(cls, value): """ Validate that value in "HH:MM:SS" format and convert to timedelta. @@ -175,7 +176,7 @@ class RelativeTime(Field): return datetime.timedelta(seconds=value) if isinstance(value, basestring): - return self._isotime_to_timedelta(value) + return self.isotime_to_timedelta(value) msg = "RelativeTime Field {0} has bad value '{1!r}'".format(self._name, value) raise TypeError(msg) diff --git a/common/lib/xmodule/xmodule/js/fixtures/video.html b/common/lib/xmodule/xmodule/js/fixtures/video.html index a0bf54d3639..89b4e360c6e 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video.html @@ -10,6 +10,7 @@ data-speed="1.5" data-start="" data-end="" + data-saved-video-position="0" data-caption-asset-path="/static/subs/" data-autoplay="False" data-yt-test-timeout="1500" diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_all.html b/common/lib/xmodule/xmodule/js/fixtures/video_all.html index 8df39783c66..982aca02328 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_all.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_all.html @@ -9,6 +9,7 @@ data-speed="1.5" data-start="" data-end="" + data-saved-video-position="0" data-caption-asset-path="/static/subs/" data-sub="Z5KLxerq05Y" data-mp4-source="xmodule/include/fixtures/test.mp4" diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_html5.html b/common/lib/xmodule/xmodule/js/fixtures/video_html5.html index 0b1d696e401..c3c61ba3982 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_html5.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_html5.html @@ -9,6 +9,7 @@ data-speed="1.5" data-start="" data-end="" + data-saved-video-position="0" data-caption-asset-path="/static/subs/" data-sub="Z5KLxerq05Y" data-mp4-source="xmodule/include/fixtures/test.mp4" diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html b/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html index 84bd0bc5e10..ee98a3992e0 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html @@ -10,6 +10,7 @@ data-speed="1.5" data-start="" data-end="" + data-saved-video-position="0" data-caption-asset-path="/static/subs/" data-autoplay="False" data-yt-test-timeout="1500" diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html index 53146ba6474..33d304688b8 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html @@ -10,6 +10,7 @@ data-speed="1.5" data-start="" data-end="" + data-saved-video-position="0" data-caption-asset-path="/static/subs/" data-autoplay="False" data-yt-test-timeout="1500" diff --git a/common/lib/xmodule/xmodule/js/spec/helper.js b/common/lib/xmodule/xmodule/js/spec/helper.js index b6801642491..59fe9af25e7 100644 --- a/common/lib/xmodule/xmodule/js/spec/helper.js +++ b/common/lib/xmodule/xmodule/js/spec/helper.js @@ -207,18 +207,17 @@ beforeEach(function () { this.addMatchers({ toHaveAttrs: function (attrs) { - var element = this.actual, - result = true; + var element; if ($.isEmptyObject(attrs)) { return false; } - $.each(attrs, function (name, value) { - return result = result && element.attr(name) === value; - }); + element = this.actual; - return result; + return _.every(attrs, function (value, name) { + return element.attr(name) === value; + }); }, toBeInRange: function (min, max) { return min <= this.actual && this.actual <= max; diff --git a/common/lib/xmodule/xmodule/js/spec/video/cookie_storage_spec.js b/common/lib/xmodule/xmodule/js/spec/video/cookie_storage_spec.js deleted file mode 100644 index 40a092b3434..00000000000 --- a/common/lib/xmodule/xmodule/js/spec/video/cookie_storage_spec.js +++ /dev/null @@ -1,149 +0,0 @@ -(function (requirejs, require, define) { -require( -['video/00_cookie_storage.js'], -function (CookieStorage) { - describe('CookieStorage', function () { - var mostRecentCall; - - beforeEach(function () { - mostRecentCall = $.cookie.mostRecentCall; - }); - - afterEach(function () { - CookieStorage('test_storage').clear(); - }); - - describe('intialize', function () { - it('with namespace', function () { - var storage = CookieStorage('test_storage'); - - storage.setItem('item_1', 'value_1'); - expect(mostRecentCall.args[0]).toBe('test_storage'); - }); - - it('without namespace', function () { - var storage = CookieStorage(); - - storage.setItem('item_1', 'value_1'); - expect(mostRecentCall.args[0]).toBe('cookieStorage'); - }); - }); - - it('unload', function () { - var expected = JSON.stringify({ - storage: { - item_2: { - value: 'value_2', - session: false - } - }, - keys: ['item_2'] - }), - storage = CookieStorage('test_storage'); - - storage.setItem('item_1', 'value_1', true); - storage.setItem('item_2', 'value_2'); - - $(window).trigger('unload'); - expect(mostRecentCall.args[1]).toBe(expected); - }); - - describe('methods: ', function () { - var data = { - storage: { - item_1: { - value: 'value_1', - session: false - } - }, - keys: ['item_1'] - }, - storage; - - beforeEach(function () { - $.cookie.andReturn(JSON.stringify(data)); - storage = CookieStorage('test_storage'); - }); - - describe('setItem', function () { - it('pass correct data', function () { - var expected = JSON.stringify({ - storage: { - item_1: { - value: 'value_1', - session: false - }, - item_2: { - value: 'value_2', - session: false - }, - item_3: { - value: 'value_3', - session: true - }, - }, - keys: ['item_1', 'item_2', 'item_3'] - }); - - storage.setItem('item_2', 'value_2'); - storage.setItem('item_3', 'value_3', true); - expect(mostRecentCall.args[0]).toBe('test_storage'); - expect(mostRecentCall.args[1]).toBe(expected); - }); - - it('pass broken arguments', function () { - $.cookie.reset(); - storage.setItem(null, 'value_1'); - expect($.cookie).not.toHaveBeenCalled(); - }); - }); - - describe('getItem', function () { - it('item exist', function () { - $.each(data['storage'], function(key, value) { - expect(storage.getItem(key)).toBe(value['value']); - }); - }); - - it('item does not exist', function () { - expect(storage.getItem('nonexistent')).toBe(null); - }); - }); - - describe('removeItem', function () { - it('item exist', function () { - var expected = JSON.stringify({ - storage: {}, - keys: [] - }); - - storage.removeItem('item_1'); - expect(mostRecentCall.args[1]).toBe(expected); - }); - - it('item does not exist', function () { - storage.removeItem('nonexistent'); - expect(mostRecentCall.args[1]).toBe(JSON.stringify(data)); - }); - }); - - it('clear', function () { - storage.clear(); - expect(mostRecentCall.args[1]).toBe(null); - }); - - describe('key', function () { - it('key exist', function () { - $.each(data['keys'], function(index, name) { - expect(storage.key(index)).toBe(name); - }); - }); - - it('key is grater than keys list', function () { - expect(storage.key(100)).toBe(null); - }); - }); - }); - }); -}); -}(RequireJS.requirejs, RequireJS.require, RequireJS.define)); diff --git a/common/lib/xmodule/xmodule/js/spec/video/events_spec.js b/common/lib/xmodule/xmodule/js/spec/video/events_spec.js index 20fa4fc00a0..437577c7dd0 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/events_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/events_spec.js @@ -19,6 +19,7 @@ afterEach(function () { $('source').remove(); window.onTouchBasedDevice = oldOTBD; + state.storage.clear(); }); it('initialize', function () { diff --git a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js index 06d42d75206..995c89627ef 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js @@ -8,6 +8,8 @@ afterEach(function () { $('source').remove(); + window.VideoState = {}; + window.VideoState.id = {}; }); describe('constructor', function () { @@ -80,7 +82,7 @@ '0.75': sub, '1.0': sub, '1.25': sub, - '1.5': sub + '1.50': sub }); }); @@ -97,7 +99,7 @@ '0.75': sub, '1.0': sub, '1.25': sub, - '1.5': sub + '1.50': sub }); }); @@ -227,10 +229,17 @@ expect(state.videoPlayer.skipOnEndedStartEndReset).toBe(true); }); + it('when position is not 0: cue is called with stored position value', function () { + state.config.savedVideoPosition = 15; + + state.videoPlayer.updatePlayTime(10); + expect(state.videoPlayer.player.cueVideoById).toHaveBeenCalledWith('cogebirgzzM', 15); + }); + it('Handling cue state', function () { spyOn(state.videoPlayer, 'play'); - state.videoPlayer.startTime = 10; + state.videoPlayer.seekToTimeOnCued = 10; state.videoPlayer.onStateChange({data: 5}); expect(state.videoPlayer.player.seekTo).toHaveBeenCalledWith(10, true); @@ -397,7 +406,7 @@ it('save setting for new speed', function () { expect(state.storage.getItem('general_speed')).toBe('0.75'); - expect(state.storage.getItem('video_speed_' + state.id)).toBe('0.75'); + expect(state.storage.getItem('speed', true)).toBe('0.75'); }); }); diff --git a/common/lib/xmodule/xmodule/js/spec/video/html5_video_spec.js b/common/lib/xmodule/xmodule/js/spec/video/html5_video_spec.js index bfc106bb464..75f8a6e9d3e 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/html5_video_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/html5_video_spec.js @@ -9,7 +9,7 @@ }); afterEach(function () { - state = undefined; + state.storage.clear(); $.fn.scrollTo.reset(); $('.subtitles').remove(); $('source').remove(); diff --git a/common/lib/xmodule/xmodule/js/spec/video/initialize_spec.js b/common/lib/xmodule/xmodule/js/spec/video/initialize_spec.js new file mode 100644 index 00000000000..75c1247061c --- /dev/null +++ b/common/lib/xmodule/xmodule/js/spec/video/initialize_spec.js @@ -0,0 +1,154 @@ +(function (requirejs, require, define, undefined) { + +'use strict'; + +require( +['video/01_initialize.js'], +function (Initialize) { + describe('Initialize', function () { + describe('saveState function', function () { + var state, videoPlayerCurrentTime, newCurrentTime, speed; + + // We make sure that `currentTime` is a float. We need to test + // that Math.round() is called. + videoPlayerCurrentTime = 3.1242; + + // We have two times, because one is stored in + // `videoPlayer.currentTime`, and the other is passed directly to + // `saveState` in `data` object. In each case, there is different + // code that handles these times. They have to be different for + // test completeness sake. Also, make sure it is float, as is the + // time above. + newCurrentTime = 5.4; + + speed = '0.75'; + + beforeEach(function () { + state = { + videoPlayer: { + currentTime: videoPlayerCurrentTime + }, + storage: { + setItem: jasmine.createSpy() + }, + config: { + saveStateUrl: 'http://example.com/save_user_state' + } + }; + + spyOn($, 'ajax'); + spyOn(Time, 'formatFull').andCallThrough(); + }); + + afterEach(function () { + state = undefined; + }); + + it('data is not an object, async is true', function () { + itSpec({ + asyncVal: true, + speedVal: undefined, + positionVal: videoPlayerCurrentTime, + data: undefined, + ajaxData: { + saved_video_position: Time.formatFull(Math.round(videoPlayerCurrentTime)) + } + }); + }); + + it('data contains speed, async is false', function () { + itSpec({ + asyncVal: false, + speedVal: speed, + positionVal: undefined, + data: { + speed: speed + }, + ajaxData: { + speed: speed + } + }); + }); + + it('data contains float position, async is true', function () { + itSpec({ + asyncVal: true, + speedVal: undefined, + positionVal: newCurrentTime, + data: { + saved_video_position: newCurrentTime + }, + ajaxData: { + saved_video_position: Time.formatFull(Math.round(newCurrentTime)) + } + }); + }); + + it('data contains speed and rounded position, async is false', function () { + itSpec({ + asyncVal: false, + speedVal: speed, + positionVal: Math.round(newCurrentTime), + data: { + speed: speed, + saved_video_position: Math.round(newCurrentTime) + }, + ajaxData: { + speed: speed, + saved_video_position: Time.formatFull(Math.round(newCurrentTime)) + } + }); + }); + + it('data contains empty object, async is true', function () { + itSpec({ + asyncVal: true, + speedVal: undefined, + positionVal: undefined, + data: {}, + ajaxData: {} + }); + }); + + return; + + function itSpec(value) { + var asyncVal = value.asyncVal, + speedVal = value.speedVal, + positionVal = value.positionVal, + data = value.data, + ajaxData = value.ajaxData; + + Initialize.prototype.saveState.call(state, asyncVal, data); + + if (speedVal) { + expect(state.storage.setItem).toHaveBeenCalledWith( + 'speed', + speedVal, + true + ); + } + if (positionVal) { + expect(state.storage.setItem).toHaveBeenCalledWith( + 'savedVideoPosition', + positionVal, + true + ); + expect(Time.formatFull).toHaveBeenCalledWith( + positionVal + ); + } + expect($.ajax).toHaveBeenCalledWith({ + url: state.config.saveStateUrl, + type: 'POST', + async: asyncVal, + dataType: 'json', + data: ajaxData + }); + } + }); + }); + +}); + +}(RequireJS.requirejs, RequireJS.require, RequireJS.define)); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js index 20a33e91383..6ad591d74ea 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js @@ -8,10 +8,7 @@ .andReturn(null); state = jasmine.initializePlayer(); - videoControl = state.videoControl; - - $.fn.scrollTo.reset(); }); afterEach(function () { @@ -21,6 +18,8 @@ // had before. Removing of `source` tag, not `video` tag, stops // loading video source and clears the memory. $('source').remove(); + $.fn.scrollTo.reset(); + state.storage.clear(); window.onTouchBasedDevice = oldOTBD; }); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_control_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_control_spec.js index 1691220f513..6e2403a44e8 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_control_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_control_spec.js @@ -10,6 +10,7 @@ afterEach(function () { $('source').remove(); + state.storage.clear(); window.onTouchBasedDevice = oldOTBD; }); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js index 086ea7a8903..3dfba3008d7 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js @@ -11,6 +11,7 @@ afterEach(function () { $('source').remove(); window.onTouchBasedDevice = oldOTBD; + state.storage.clear(); }); describe('constructor', function () { @@ -213,10 +214,6 @@ ); }); - it('pause other video player', function () { - expect(oldState.videoPlayer.onPause).toHaveBeenCalled(); - }); - it('set update interval', function () { expect(window.setInterval).toHaveBeenCalledWith( state.videoPlayer.update, 200 diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js index d99ceee27ad..32dcdc915f0 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_progress_slider_spec.js @@ -11,6 +11,7 @@ afterEach(function () { $('source').remove(); window.onTouchBasedDevice = oldOTBD; + state.storage.clear(); }); describe('constructor', function () { diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js index 3ead4a7c151..3f9b2c2d0d8 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js @@ -12,6 +12,7 @@ afterEach(function () { $('source').remove(); window.onTouchBasedDevice = oldOTBD; + state.storage.clear(); }); describe('constructor', function () { @@ -20,9 +21,15 @@ }); it('render the quality control', function () { - var container = state.videoControl.secondaryControlsEl; + waitsFor(function () { + return state.videoControl; + }, 'videoControl is present', 5000); - expect(container).toContain('a.quality_control'); + runs(function () { + var container = state.videoControl.secondaryControlsEl; + + expect(container).toContain('a.quality_control'); + }); }); it('add ARIA attributes to quality control', function () { diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js index 702b185fc34..73be6ad9646 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js @@ -11,6 +11,7 @@ afterEach(function () { $('source').remove(); window.onTouchBasedDevice = oldOTBD; + state.storage.clear(); }); describe('constructor', function () { diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_storage_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_storage_spec.js new file mode 100644 index 00000000000..4a65e83c6fb --- /dev/null +++ b/common/lib/xmodule/xmodule/js/spec/video/video_storage_spec.js @@ -0,0 +1,83 @@ +(function (requirejs, require, define, undefined) { +require( +['video/00_video_storage.js'], +function (VideoStorage) { + describe('VideoStorage', function () { + var namespace = 'test_storage', + id = 'video_id'; + + afterEach(function () { + VideoStorage(namespace, id).clear(); + }); + + describe('initialize', function () { + it('with namespace and id', function () { + var storage = VideoStorage(namespace, id); + + expect(window[namespace]).toBeDefined(); + expect(window[namespace][id]).toBeDefined(); + }); + + it('without namespace and id', function () { + spyOn(Number.prototype, 'toString').andReturn('0.abcdedg'); + var storage = VideoStorage(); + + expect(window.VideoStorage).toBeDefined(); + expect(window.VideoStorage.abcdedg).toBeDefined(); + }); + }); + + describe('methods: ', function () { + var data, storage; + + beforeEach(function () { + data = { + item_2: 'value_2' + }; + data[id] = { + item_1: 'value_1' + }; + + window[namespace] = data; + storage = VideoStorage(namespace, id); + }); + + it('setItem', function () { + var expected = $.extend(true, {}, data, {item_4: 'value_4'}); + + expected[id]['item_3'] = 'value_3'; + storage.setItem('item_3', 'value_3', true); + storage.setItem('item_4', 'value_4'); + expect(window[namespace]).toEqual(expected); + }); + + it('getItem', function () { + var data = window[namespace], + getItem = storage.getItem; + + expect(getItem('item_1', true)).toBe(data[id]['item_1']); + expect(getItem('item_2')).toBe(data['item_2']); + expect(getItem('item_3')).toBeUndefined(); + }); + + it('removeItem', function () { + var data = window[namespace], + removeItem = storage.removeItem; + + removeItem('item_1', true); + removeItem('item_2'); + expect(data[id]['item_1']).toBeUndefined(); + expect(data['item_2']).toBeUndefined(); + }); + + it('clear', function () { + var expected = {}; + + expected[id] = {}; + storage.clear(); + expect(window[namespace]).toEqual(expected); + }); + }); + }); +}); +}(RequireJS.requirejs, RequireJS.require, RequireJS.define)); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_volume_control_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_volume_control_spec.js index 1c102d518c4..cf2fed8ba0a 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_volume_control_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_volume_control_spec.js @@ -11,6 +11,7 @@ afterEach(function () { $('source').remove(); window.onTouchBasedDevice = oldOTBD; + state.storage.clear(); }); describe('constructor', function () { diff --git a/common/lib/xmodule/xmodule/js/src/time.coffee b/common/lib/xmodule/xmodule/js/src/time.coffee index 9173a7c0b26..e2da2c583d6 100644 --- a/common/lib/xmodule/xmodule/js/src/time.coffee +++ b/common/lib/xmodule/xmodule/js/src/time.coffee @@ -13,5 +13,18 @@ class @Time else "#{minutes}:#{pad(seconds % 60)}" + @formatFull: (time) -> + pad = (number) -> if number < 10 then "0#{number}" else number + + seconds = Math.floor time + minutes = Math.floor seconds / 60 + hours = Math.floor minutes / 60 + seconds = seconds % 60 + minutes = minutes % 60 + + # The returned value will not be user-facing. So no need for + # internationalization. + "#{pad(hours)}:#{pad(minutes)}:#{pad(seconds % 60)}" + @convert: (time, oldSpeed, newSpeed) -> (time * oldSpeed / newSpeed).toFixed(3) diff --git a/common/lib/xmodule/xmodule/js/src/video/00_cookie_storage.js b/common/lib/xmodule/xmodule/js/src/video/00_cookie_storage.js deleted file mode 100644 index e227dfa2d91..00000000000 --- a/common/lib/xmodule/xmodule/js/src/video/00_cookie_storage.js +++ /dev/null @@ -1,194 +0,0 @@ -(function (requirejs, require, define) { - -define( -'video/00_cookie_storage.js', -[], -function() { - "use strict"; -/** - * Provides convenient way to work with cookies. - * - * Maximum 4096 bytes can be stored per namespace. - * - * @TODO: Uses localStorage if available. - * - * @param {string} namespace Namespace that is used to store data. - * @return {object} CookieStorage API. - */ - var CookieStorage = function (namespace) { - var Storage; - - /** - * Returns an empty storage with proper data structure. - * - * @private - * @return {object} Empty storage. - */ - var _getEmptyStorage = function () { - return { - storage: {}, - keys: [] - }; - }; - - /** - * Returns the current value associated with the given namespace. - * If data doesn't exist or has data with incorrect interface, it creates - * an empty storage with proper data structure. - * - * @private - * @param {string} namespace Namespace that is used to store data. - * @return {object} Stored data or an empty storage. - */ - var _getData = function (namespace) { - var data; - - try { - data = JSON.parse($.cookie(namespace)); - } catch (err) { } - - if (!data || !data['storage'] || !data['keys']) { - return _getEmptyStorage(); - } - - return data; - }; - - /** - * Clears cookies that has flag `session` equals true. - * - * @private - */ - var _clearSession = function () { - Storage['keys'] = $.grep(Storage['keys'], function(key_name, index) { - if (Storage['storage'][key_name]['session']) { - delete Storage['storage'][key_name]; - - return false; - } - - return true; - }); - - $.cookie(namespace, JSON.stringify(Storage), { - expires: 3650, - path: '/' - }); - }; - - /** - * Adds new value to the storage or rewrites existent. - * - * @param {string} name Identifier of the data. - * @param {any} value Data to store. - * @param {boolean} useSession Data with this flag will be removed on - * window unload. - */ - var setItem = function (name, value, useSession) { - if (name) { - if ($.inArray(name, Storage['keys']) === -1) { - Storage['keys'].push(name); - } - - Storage['storage'][name] = { - value: value, - session: useSession ? true : false - }; - - $.cookie(namespace, JSON.stringify(Storage), { - expires: 3650, - path: '/' - }); - } - }; - - /** - * Returns the current value associated with the given name. - * - * @param {string} name Identifier of the data. - * @return {any} The current value associated with the given name. - * If the given key does not exist in the list - * associated with the object then this method must return null. - */ - var getItem = function (name) { - try { - return Storage['storage'][name]['value']; - } catch (err) { } - - return null; - }; - - /** - * Removes the current value associated with the given name. - * - * @param {string} name Identifier of the data. - */ - var removeItem = function (name) { - delete Storage['storage'][name]; - - Storage['keys'] = $.grep(Storage['keys'], function(key_name, index) { - return name !== key_name; - }); - - $.cookie(namespace, JSON.stringify(Storage), { - expires: 3650, - path: '/' - }); - }; - - /** - * Empties the storage. - * - */ - var clear = function () { - Storage = _getEmptyStorage(); - $.cookie(namespace, null, { - expires: -1, - path: '/' - }); - }; - - /** - * Returns the name of the `n`th key in the list. - * - * @param {number} n Index of the key. - * @return {string} Name of the `n`th key in the list. - * If `n` is greater than or equal to the number of key/value pairs - * in the object, then this method must return `null`. - */ - var key = function (n) { - if (n >= Storage['keys'].length) { - return null; - } - - return Storage['keys'][n]; - }; - - /** - * Initializes the module: creates a storage with proper namespace, binds - * `unload` event. - * - * @private - */ - (function initialize() { - if (!namespace) { - namespace = 'cookieStorage'; - } - Storage = _getData(namespace); - - $(window).unload(_clearSession); - }()); - - return { - clear: clear, - getItem: getItem, - key: key, - removeItem: removeItem, - setItem: setItem - }; - }; - - return CookieStorage; -}); - -}(RequireJS.requirejs, RequireJS.require, RequireJS.define)); diff --git a/common/lib/xmodule/xmodule/js/src/video/00_video_storage.js b/common/lib/xmodule/xmodule/js/src/video/00_video_storage.js new file mode 100644 index 00000000000..7b1f38a6c64 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/video/00_video_storage.js @@ -0,0 +1,103 @@ +(function (requirejs, require, define) { + +define( +'video/00_video_storage.js', +[], +function() { + "use strict"; +/** + * Provides convenient way to store key value pairs. + * + * @param {string} namespace Namespace that is used to store data. + * @return {object} VideoStorage API. + */ + var VideoStorage = function (namespace, id) { + /** + * Adds new value to the storage or rewrites existent. + * + * @param {string} name Identifier of the data. + * @param {any} value Data to store. + * @param {boolean} instanceSpecific Data with this flag will be added + * to instance specific storage. + */ + var setItem = function (name, value, instanceSpecific) { + if (name) { + if (instanceSpecific) { + window[namespace][id][name] = value; + } else { + window[namespace][name] = value; + } + } + }; + + /** + * Returns the current value associated with the given name. + * + * @param {string} name Identifier of the data. + * @param {boolean} instanceSpecific Data with this flag will be added + * to instance specific storage. + * @return {any} The current value associated with the given name. + * If the given key does not exist in the list + * associated with the object then this method must return null. + */ + var getItem = function (name, instanceSpecific) { + if (instanceSpecific) { + return window[namespace][id][name]; + } else { + return window[namespace][name]; + } + }; + + /** + * Removes the current value associated with the given name. + * + * @param {string} name Identifier of the data. + * @param {boolean} instanceSpecific Data with this flag will be added + * to instance specific storage. + */ + var removeItem = function (name, instanceSpecific) { + if (instanceSpecific) { + delete window[namespace][id][name]; + } else { + delete window[namespace][name]; + } + }; + + /** + * Empties the storage. + * + */ + var clear = function () { + window[namespace] = {}; + window[namespace][id] = {}; + }; + + /** + * Initializes the module: creates a storage with proper namespace. + * + * @private + */ + (function initialize() { + if (!namespace) { + namespace = 'VideoStorage'; + } + if (!id) { + // Generate random alpha-numeric string. + id = Math.random().toString(36).slice(2); + } + + window[namespace] = window[namespace] || {}; + window[namespace][id] = window[namespace][id] || {}; + }()); + + return { + clear: clear, + getItem: getItem, + removeItem: removeItem, + setItem: setItem + }; + }; + + return VideoStorage; +}); +}(RequireJS.requirejs, RequireJS.require, RequireJS.define)); diff --git a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js index 928808c06b1..9b4e6be7bf2 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -14,8 +14,8 @@ define( 'video/01_initialize.js', -['video/03_video_player.js', 'video/00_cookie_storage.js'], -function (VideoPlayer, CookieStorage) { +['video/03_video_player.js', 'video/00_video_storage.js'], +function (VideoPlayer, VideoStorage) { /** * @function * @@ -26,7 +26,7 @@ function (VideoPlayer, CookieStorage) { * available via this object. * @param {DOM element} element Container of the entire Video DOM element. */ - return function (state, element) { + var Initialize = function (state, element) { _makeFunctionsPublic(state); state.initialize(element) @@ -56,8 +56,26 @@ function (VideoPlayer, CookieStorage) { state.el.trigger('initialize', arguments); }); }); + }, + methodsDict = { + bindTo: bindTo, + fetchMetadata: fetchMetadata, + getDuration: getDuration, + getVideoMetadata: getVideoMetadata, + initialize: initialize, + parseSpeed: parseSpeed, + parseVideoSources: parseVideoSources, + parseYoutubeStreams: parseYoutubeStreams, + saveState: saveState, + setSpeed: setSpeed, + trigger: trigger, + youtubeId: youtubeId }; + Initialize.prototype = methodsDict; + + return Initialize; + // *************************************************************** // Private functions start here. Private functions start with underscore. // *************************************************************** @@ -73,21 +91,6 @@ function (VideoPlayer, CookieStorage) { * methods, modules) of the Video player. */ function _makeFunctionsPublic(state) { - var methodsDict = { - bindTo: bindTo, - fetchMetadata: fetchMetadata, - getDuration: getDuration, - getVideoMetadata: getVideoMetadata, - initialize: initialize, - parseSpeed: parseSpeed, - parseVideoSources: parseVideoSources, - parseYoutubeStreams: parseYoutubeStreams, - setSpeed: setSpeed, - stopBuffering: stopBuffering, - trigger: trigger, - youtubeId: youtubeId - }; - bindTo(methodsDict, state, state); } @@ -201,7 +204,7 @@ function (VideoPlayer, CookieStorage) { '0.75': state.config.sub, '1.0': state.config.sub, '1.25': state.config.sub, - '1.5': state.config.sub + '1.50': state.config.sub }; // We must have at least one non-YouTube video source available. @@ -271,13 +274,13 @@ function (VideoPlayer, CookieStorage) { return dfd.promise(); } - function _getConfiguration(data) { + function _getConfiguration(data, storage) { var isBoolean = function (value) { var regExp = /^true$/i; return regExp.test(value.toString()); }, // List of keys that will be extracted form the configuration. - extractKeys = ['speed'], + extractKeys = [], // Compatibility keys used to change names of some parameters in // the final configuration. compatKeys = { @@ -289,6 +292,19 @@ function (VideoPlayer, CookieStorage) { 'showCaptions': isBoolean, 'autoplay': isBoolean, 'autohideHtml5': isBoolean, + 'savedVideoPosition': function (value) { + return storage.getItem('savedVideoPosition', true) || + Number(value) || + 0; + }, + 'speed': function (value) { + return storage.getItem('speed', true) || value; + }, + 'generalSpeed': function (value) { + return storage.getItem('general_speed') || + value || + '1.0'; + }, 'ytTestTimeout': function (value) { value = parseInt(value, 10); @@ -380,12 +396,7 @@ function (VideoPlayer, CookieStorage) { id = el.attr('id').replace(/video_/, ''), __dfd__ = $.Deferred(), isTouch = onTouchBasedDevice() || '', - storage = CookieStorage('video_player'), - speed = storage.getItem('video_speed_' + id) || - el.data('speed') || - storage.getItem('general_speed') || - el.data('general-speed') || - '1.0'; + storage = VideoStorage('VideoState', id); if (isTouch) { el.addClass('is-touch'); @@ -399,7 +410,6 @@ function (VideoPlayer, CookieStorage) { id: id, isFullScreen: false, isTouch: isTouch, - speed: Number(speed).toFixed(2).replace(/\.00$/, '.0'), storage: storage }); @@ -411,7 +421,7 @@ function (VideoPlayer, CookieStorage) { // are "read only", so don't modify them. All variable content lives in // 'state' object. // jQuery .data() return object with keys in lower camelCase format. - this.config = $.extend({}, _getConfiguration(el.data()), { + this.config = $.extend({}, _getConfiguration(el.data(), storage), { element: element, fadeOutTimeout: 1400, captionsFreezeTime: 10000, @@ -422,6 +432,10 @@ function (VideoPlayer, CookieStorage) { this.config.endTime = null; } + this.speed = Number( + this.config.speed || this.config.generalSpeed + ).toFixed(2).replace(/\.00$/, '.0'); + if (!(_parseYouTubeIDs(this))) { // If we do not have YouTube ID's, try parsing HTML5 video sources. @@ -637,8 +651,8 @@ function (VideoPlayer, CookieStorage) { } if (updateStorage) { - this.storage.setItem('video_speed_' + this.id, this.speed, useSession); - this.storage.setItem('general_speed', this.speed, useSession); + this.storage.setItem('speed', this.speed, true); + this.storage.setItem('general_speed', this.speed); } } @@ -659,20 +673,34 @@ function (VideoPlayer, CookieStorage) { return xhr; } - function stopBuffering() { - var video; + function saveState(async, data) { + if (!($.isPlainObject(data))) { + data = { + saved_video_position: this.videoPlayer.currentTime + }; + } - if (this.videoType === 'html5') { - // HTML5 player haven't default way to abort bufferization. - // In this case we simply resetting source and call load(). - video = this.videoPlayer.player.video; - video.src = ''; - video.load(); + if (data.speed) { + this.storage.setItem('speed', data.speed, true); } + + if (data.saved_video_position) { + this.storage.setItem('savedVideoPosition', data.saved_video_position, true); + + data.saved_video_position = Time.formatFull(data.saved_video_position); + } + + $.ajax({ + url: this.config.saveStateUrl, + type: 'POST', + async: async ? true : false, + dataType: 'json', + data: data, + }); } function youtubeId(speed) { - return this.videos[speed || this.speed]; + return this.videos[speed || this.speed] || this.videos['1.0']; } function getDuration() { diff --git a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js index 34e1e1bc2f9..bd80c92aeaa 100644 --- a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js +++ b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js @@ -67,6 +67,8 @@ function (HTML5Video, Resizer) { // metadata is loaded, which normally happens just after the video // starts playing. Just after that configurations can be applied. state.videoPlayer.ready = _.once(function () { + $(window).on('unload', state.saveState); + if (state.currentPlayerMode !== 'flash') { state.videoPlayer.onSpeedChange(state.speed); } @@ -292,8 +294,7 @@ function (HTML5Video, Resizer) { // (currentTime) and its duration. // It is called at a regular interval when the video is playing. function update() { - this.videoPlayer.currentTime = this.videoPlayer.player - .getCurrentTime(); + this.videoPlayer.currentTime = this.videoPlayer.player.getCurrentTime(); if (isFinite(this.videoPlayer.currentTime)) { this.videoPlayer.updatePlayTime(this.videoPlayer.currentTime); @@ -379,14 +380,7 @@ function (HTML5Video, Resizer) { this.el.trigger('speedchange', arguments); - $.ajax({ - url: this.config.saveStateUrl, - type: 'POST', - dataType: 'json', - data: { - speed: newSpeed - }, - }); + this.saveState(true, { speed: newSpeed }); } // Every 200 ms, if the video is playing, we call the function update, via @@ -485,6 +479,7 @@ function (HTML5Video, Resizer) { this.trigger('videoCaption.pause', null); } + this.saveState(true); this.el.trigger('pause', arguments); } @@ -640,7 +635,7 @@ function (HTML5Video, Resizer) { this.videoPlayer.onEnded(); break; case this.videoPlayer.PlayerState.CUED: - this.videoPlayer.player.seekTo(this.videoPlayer.startTime, true); + this.videoPlayer.player.seekTo(this.videoPlayer.seekToTimeOnCued, true); // We need to call play() explicitly because after the call // to functions cueVideoById() followed by seekTo() the video // is in a PAUSED state. @@ -652,28 +647,26 @@ function (HTML5Video, Resizer) { } function updatePlayTime(time) { - var duration = this.videoPlayer.duration(), - durationChange, tempStartTime, tempEndTime, youTubeId; + var videoPlayer = this.videoPlayer, + duration = this.videoPlayer.duration(), + savedVideoPosition = this.config.savedVideoPosition, + isNewSpeed = videoPlayer.seekToStartTimeOldSpeed !== this.speed, + durationChange, tempStartTime, tempEndTime, youTubeId, + startTime, endTime; if ( duration > 0 && - ( - this.videoPlayer.seekToStartTimeOldSpeed !== this.speed || - this.videoPlayer.initialSeekToStartTime - ) + (isNewSpeed || videoPlayer.initialSeekToStartTime) ) { - if ( - this.videoPlayer.seekToStartTimeOldSpeed !== this.speed && - this.videoPlayer.initialSeekToStartTime === false - ) { + if (isNewSpeed && videoPlayer.initialSeekToStartTime === false) { durationChange = true; } else { // this.videoPlayer.initialSeekToStartTime === true - this.videoPlayer.initialSeekToStartTime = false; + videoPlayer.initialSeekToStartTime = false; durationChange = false; } - this.videoPlayer.seekToStartTimeOldSpeed = this.speed; + videoPlayer.seekToStartTimeOldSpeed = this.speed; // Current startTime and endTime could have already been reset. // We will remember their current values, and reset them at the @@ -681,22 +674,20 @@ function (HTML5Video, Resizer) { // times so that the range on the slider gets correctly updated in // the case of speed change in Flash player mode (for YouTube // videos). - tempStartTime = this.videoPlayer.startTime; - tempEndTime = this.videoPlayer.endTime; + tempStartTime = videoPlayer.startTime; + tempEndTime = videoPlayer.endTime; // We retrieve the original times. They could have been changed due // to the fact of speed change (duration change). This happens when // in YouTube Flash mode. There each speed is a different video, // with a different length. - this.videoPlayer.startTime = this.config.startTime; - this.videoPlayer.endTime = this.config.endTime; + videoPlayer.startTime = this.config.startTime; + videoPlayer.endTime = this.config.endTime; - if (this.videoPlayer.startTime > duration) { - this.videoPlayer.startTime = 0; - } else { - if (this.currentPlayerMode === 'flash') { - this.videoPlayer.startTime /= Number(this.speed); - } + if (videoPlayer.startTime > duration) { + videoPlayer.startTime = 0; + } else if (this.currentPlayerMode === 'flash') { + videoPlayer.startTime /= Number(this.speed); } // An `endTime` of `null` means that either the user didn't set @@ -708,13 +699,10 @@ function (HTML5Video, Resizer) { // sometimes in YouTube mode the duration changes slightly during // the course of playback. This would cause the video to pause just // before the actual end of the video. - if ( - this.videoPlayer.endTime !== null && - this.videoPlayer.endTime > duration - ) { - this.videoPlayer.endTime = null; - } else if (this.videoPlayer.endTime !== null) { - if (this.currentPlayerMode === 'flash') { + if (videoPlayer.endTime !== null) { + if (videoPlayer.endTime > duration) { + this.videoPlayer.endTime = null; + } else if (this.currentPlayerMode === 'flash') { this.videoPlayer.endTime /= Number(this.speed); } } @@ -734,9 +722,22 @@ function (HTML5Video, Resizer) { // performed already such a seek. if ( durationChange === false && - this.videoPlayer.startTime > 0 && + (videoPlayer.startTime > 0 || savedVideoPosition !== 0) && !(tempStartTime === 0 && tempEndTime === null) ) { + startTime = this.videoPlayer.startTime; + endTime = this.videoPlayer.endTime; + + if (startTime) { + if (startTime < savedVideoPosition && endTime > savedVideoPosition) { + time = savedVideoPosition; + } else { + time = startTime; + } + } else { + time = savedVideoPosition; + } + // After a bug came up (BLD-708: "In Firefox YouTube video with // start time plays from 00:00:00") the video refused to play // from start time, and only played from the beginning. @@ -765,9 +766,10 @@ function (HTML5Video, Resizer) { // times this.videoPlayer.skipOnEndedStartEndReset = true; - this.videoPlayer.player.cueVideoById(youTubeId, this.videoPlayer.startTime); + this.videoPlayer.seekToTimeOnCued = time; + this.videoPlayer.player.cueVideoById(youTubeId, time); } else { - this.videoPlayer.player.seekTo(this.videoPlayer.startTime); + this.videoPlayer.player.seekTo(time); } } @@ -775,8 +777,8 @@ function (HTML5Video, Resizer) { // already reset (a seek event happened, the video already ended // once, or endTime has already been reached once). if (tempStartTime === 0 && tempEndTime === null) { - this.videoPlayer.startTime = 0; - this.videoPlayer.endTime = null; + videoPlayer.startTime = 0; + videoPlayer.endTime = null; } } diff --git a/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js b/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js index 3016f0c8e58..29e2446d9e6 100644 --- a/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js +++ b/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js @@ -104,7 +104,8 @@ function () { // whole slider). Remember that endTime === null means the end time // is set to the end of video by default. function updateStartEndTimeRegion(params) { - var left, width, start, end, duration, rangeParams; + var isFlashMode = this.currentPlayerMode === 'flash', + left, width, start, end, duration, rangeParams; // We must have a duration in order to determine the area of range. // It also must be non-zero. @@ -119,22 +120,16 @@ function () { if (start > duration) { start = 0; - } else { - if (this.currentPlayerMode === 'flash') { - start /= Number(this.speed); - } + } else if (isFlashMode) { + start /= Number(this.speed); } // If end is set to null, or it is greater than the duration of the // video, then we set it to the end of the video. - if ( - end === null || end > duration - ) { + if (end === null || end > duration) { end = duration; - } else if (end !== null) { - if (this.currentPlayerMode === 'flash') { - end /= Number(this.speed); - } + } else if (isFlashMode) { + end /= Number(this.speed); } // Don't build a range if it takes up the whole slider. diff --git a/common/lib/xmodule/xmodule/js/src/video/10_main.js b/common/lib/xmodule/xmodule/js/src/video/10_main.js index 224f0f4316c..774bee8640d 100644 --- a/common/lib/xmodule/xmodule/js/src/video/10_main.js +++ b/common/lib/xmodule/xmodule/js/src/video/10_main.js @@ -75,22 +75,13 @@ function ( window.Video = function (element) { var state; - // Stop bufferization of previous video on sequence change. - // Problem: multiple video tags with the same src cannot - // play together. The second tag waiting when first video will be fully loaded. - // That's why we abort bufferization forcibly. - $(element).closest('.sequence').bind('sequence:change', function(e){ - if (previousState !== null && typeof previousState.videoPlayer !== 'undefined') { - previousState.stopBuffering(); - $(e.currentTarget).unbind('sequence:change'); - } - }); - // Check for existance of previous state, uninitialize it if necessary, and create a new state. // Store new state for future invocation of this module consturctor function. - if (previousState !== null && typeof previousState.videoPlayer !== 'undefined') { - previousState.videoPlayer.onPause(); + if (previousState && previousState.videoPlayer) { + previousState.saveState(true); + $(window).off('unload', previousState.saveState); } + state = {}; previousState = state; @@ -110,6 +101,8 @@ function ( youtubeXhr = state.youtubeXhr; } + $(element).find('.video').data('video-player-state', state); + // Because the 'state' object is only available inside this closure, we will also make // it available to the caller by returning it. This is necessary so that we can test // Video with Jasmine. diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index 97640661415..8949e525c1e 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -30,7 +30,7 @@ from xmodule.contentstore.django import contentstore from xmodule.contentstore.content import StaticContent from xmodule.exceptions import NotFoundError from xblock.core import XBlock -from xblock.fields import Scope, String, Float, Boolean, List, Integer, ScopeIds +from xblock.fields import Scope, String, Float, Boolean, List, ScopeIds from xmodule.fields import RelativeTime from xmodule.modulestore.inheritance import InheritanceKeyValueStore @@ -46,10 +46,10 @@ class VideoFields(object): default="Video", scope=Scope.settings ) - position = Integer( + saved_video_position = RelativeTime( help="Current position in the video", scope=Scope.user_state, - default=0 + default=datetime.timedelta(seconds=0) ) show_captions = Boolean( help="This controls whether or not captions are shown by default.", @@ -167,7 +167,7 @@ class VideoModule(VideoFields, XModule): # index. We do that to avoid issues that occurs in tests. js = { 'js': [ - resource_string(__name__, 'js/src/video/00_cookie_storage.js'), + resource_string(__name__, 'js/src/video/00_video_storage.js'), resource_string(__name__, 'js/src/video/00_resizer.js'), resource_string(__name__, 'js/src/video/01_initialize.js'), resource_string(__name__, 'js/src/video/025_focus_grabber.js'), @@ -186,15 +186,21 @@ class VideoModule(VideoFields, XModule): js_module_name = "Video" def handle_ajax(self, dispatch, data): - ACCEPTED_KEYS = ['speed'] + accepted_keys = ['speed', 'saved_video_position'] if dispatch == 'save_user_state': for key in data: - if hasattr(self, key) and key in ACCEPTED_KEYS: - setattr(self, key, json.loads(data[key])) + if hasattr(self, key) and key in accepted_keys: + if key == 'saved_video_position': + relative_position = RelativeTime.isotime_to_timedelta(data[key]) + self.saved_video_position = relative_position + else: + setattr(self, key, json.loads(data[key])) if key == 'speed': self.global_speed = self.speed + log.debug(u"Test.") + return json.dumps({'success': True}) log.debug(u"GET {0}".format(data)) @@ -235,6 +241,7 @@ class VideoModule(VideoFields, XModule): 'sources': sources, 'speed': json.dumps(self.speed), 'general_speed': self.global_speed, + 'saved_video_position': self.saved_video_position.total_seconds(), 'start': self.start_time.total_seconds(), 'sub': self.sub, 'track': track_url, @@ -293,6 +300,7 @@ class VideoModule(VideoFields, XModule): return response + class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor): """Descriptor for `VideoModule`.""" module_class = VideoModule diff --git a/lms/djangoapps/courseware/features/video.feature b/lms/djangoapps/courseware/features/video.feature index d040d8c86e7..57aacfbf47a 100644 --- a/lms/djangoapps/courseware/features/video.feature +++ b/lms/djangoapps/courseware/features/video.feature @@ -2,6 +2,17 @@ Feature: LMS.Video component As a student, I want to view course videos in LMS. + # 0 + Scenario: Video component stores position correctly when page is reloaded + Given the course has a Video component in Youtube mode + Then when I view the video it has rendered in Youtube mode + And I click video button "play" + Then I seek video to "10" seconds + And I click video button "pause" + And I reload the page + And I click video button "play" + Then I see video starts playing from "0:10" position + # 1 Scenario: Video component is fully rendered in the LMS in HTML5 mode Given the course has a Video component in HTML5 mode diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index 65933bfc59b..d134bb5f801 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -15,6 +15,16 @@ HTML5_SOURCES_INCORRECT = [ 'https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.mp99' ] +VIDEO_BUTTONS = { + 'CC': '.hide-subtitles', + 'volume': '.volume', + 'play': '.video_control.play', + 'pause': '.video_control.pause', +} + +# We should wait 300 ms for event handler invocation + 200ms for safety. +DELAY = 0.5 + coursenum = 'test_course' sequence = {} @@ -151,3 +161,26 @@ def _change_video_speed(speed): world.browser.execute_script("$('.speeds').addClass('open')") speed_css = 'li[data-speed="{0}"] a'.format(speed) world.css_click(speed_css) + + +@step('I click video button "([^"]*)"$') +def click_button_video(_step, button_type): + world.wait(DELAY) + world.wait_for_ajax_complete() + button = button_type.strip() + world.css_click(VIDEO_BUTTONS[button]) + + +@step('I see video starts playing from "([^"]*)" position$') +def start_playing_video_from_n_seconds(_step, position): + world.wait_for( + func=lambda _: world.css_html('.vidtime')[:4] == position.strip(), + timeout=5 + ) + + +@step('I seek video to "([^"]*)" seconds$') +def seek_video_to_n_seconds(_step, seconds): + time = float(seconds.strip()) + jsCode = "$('.video').data('video-player-state').videoPlayer.onSlideSeek({{time: {0:f}}})".format(time) + world.browser.execute_script(jsCode) diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index e6a24154bd5..e00a0230e60 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -69,6 +69,7 @@ class TestVideoYouTube(TestVideo): 'speed': 'null', 'general_speed': 1.0, 'start': 3603.0, + 'saved_video_position': 0.0, 'sub': u'a_sub_file.srt.sjson', 'track': None, 'youtube_streams': _create_youtube_string(self.item_module), @@ -124,6 +125,7 @@ class TestVideoNonYouTube(TestVideo): 'speed': 'null', 'general_speed': 1.0, 'start': 3603.0, + 'saved_video_position': 0.0, 'sub': u'a_sub_file.srt.sjson', 'track': None, 'youtube_streams': '1.00:OEoXaMPEzfM', @@ -202,6 +204,7 @@ class TestGetHtmlMethod(BaseTestXmodule): u'webm': u'example.webm' }, 'start': 3603.0, + 'saved_video_position': 0.0, 'sub': u'a_sub_file.srt.sjson', 'speed': 'null', 'general_speed': 1.0, @@ -308,6 +311,7 @@ class TestGetHtmlMethod(BaseTestXmodule): 'speed': 'null', 'general_speed': 1.0, 'start': 3603.0, + 'saved_video_position': 0.0, 'sub': u'a_sub_file.srt.sjson', 'track': None, 'youtube_streams': '1.00:OEoXaMPEzfM', diff --git a/lms/templates/video.html b/lms/templates/video.html index e7900c4fd77..00ef4f52bb3 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -22,6 +22,7 @@ data-show-captions="${show_captions}" data-general-speed="${general_speed}" data-speed="${speed}" + data-saved-video-position="${saved_video_position}" data-start="${start}" data-end="${end}" data-caption-asset-path="${caption_asset_path}" -- GitLab