From 18edab35f3e71af7d009ecd9be625a4285cb89c4 Mon Sep 17 00:00:00 2001 From: polesye Date: Thu, 8 May 2014 14:47:20 +0300 Subject: [PATCH] BLD-1057: Fix Video player in FF. --- .../xmodule/js/spec/video/general_spec.js | 53 ------ .../js/spec/video/video_control_spec.js | 7 +- .../js/spec/video/video_player_spec.js | 26 +-- .../xmodule/js/src/video/03_video_player.js | 155 +++++++----------- .../xmodule/xmodule/js/src/video/10_main.js | 21 ++- 5 files changed, 88 insertions(+), 174 deletions(-) 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 892a0e33d1..fed16ddfa2 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js @@ -177,59 +177,6 @@ }); }); - describe('YouTube video in FireFox will cue first', function () { - var oldUserAgent; - - beforeEach(function () { - oldUserAgent = window.navigator.userAgent; - window.navigator.userAgent = 'firefox'; - - state = jasmine.initializePlayer('video.html', { - start: 10, - end: 30 - }); - }); - - afterEach(function () { - window.navigator.userAgent = oldUserAgent; - }); - - it('cue is called, skipOnEndedStartEndReset is set', function () { - state.videoPlayer.updatePlayTime(10); - expect(state.videoPlayer.player.cueVideoById).toHaveBeenCalledWith('cogebirgzzM', 10); - 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.seekToTimeOnCued = 10; - state.videoPlayer.onStateChange({data: 5}); - - expect(state.videoPlayer.player.seekTo).toHaveBeenCalledWith(10, true); - expect(state.videoPlayer.play).toHaveBeenCalled(); - }); - - it('even when cued, onEnded does not resets start and end time', function () { - state.videoPlayer.skipOnEndedStartEndReset = true; - state.videoPlayer.onEnded(); - expect(state.videoPlayer.startTime).toBe(10); - expect(state.videoPlayer.endTime).toBe(30); - - state.videoPlayer.skipOnEndedStartEndReset = undefined; - state.videoPlayer.onEnded(); - expect(state.videoPlayer.startTime).toBe(10); - expect(state.videoPlayer.endTime).toBe(30); - }); - }); - describe('checking start and end times', function () { var miniTestSuite = [ { 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 e78ef752f4..21cd0587ef 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 @@ -13,6 +13,7 @@ afterEach(function () { $('source').remove(); state.storage.clear(); + window.Video.previousState = null; window.onTouchBasedDevice = oldOTBD; }); @@ -37,7 +38,7 @@ }); it('add ARIA attributes to time control', function () { - var timeControl = $('div.slider>a'); + var timeControl = $('div.slider > a'); expect(timeControl).toHaveAttrs({ 'role': 'slider', @@ -135,8 +136,6 @@ expectedValue = sliderEl.slider('option', 'value'); expect(expectedValue).toBe(10); - - state.storage.clear(); }); }); @@ -389,7 +388,7 @@ runs(function () { state = jasmine.initializePlayer({ end: 20, - savedVideoPosition: 'a' + savedVideoPosition: 'a' }); sliderEl = state.videoProgressSlider.slider; spyOn(state.videoPlayer, 'duration').andReturn(60); 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 f404a47d0a..3b7cd29f49 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 @@ -17,6 +17,7 @@ function (VideoPlayer) { afterEach(function () { $('source').remove(); window.onTouchBasedDevice = oldOTBD; + window.Video.previousState = null; if (state.storage) { state.storage.clear(); } @@ -179,6 +180,11 @@ function (VideoPlayer) { it('autoplay the first video', function () { expect(state.videoPlayer.play).not.toHaveBeenCalled(); }); + + + it('invalid endTime is reset to null', function () { + expect(state.videoPlayer.endTime).toBe(null); + }); }); describe('onReady YouTube', function () { @@ -752,17 +758,6 @@ function (VideoPlayer) { isFlashMode: jasmine.createSpy().andReturn(false) }; }); - - it('invalid endTime is reset to null', function () { - VideoPlayer.prototype.updatePlayTime.call(state, 0); - - expect(state.videoPlayer.figureOutStartingTime).toHaveBeenCalled(); - - VideoPlayer.prototype.figureOutStartEndTime.call(state, 60); - VideoPlayer.prototype.figureOutStartingTime.call(state, 60); - - expect(state.videoPlayer.endTime).toBe(null); - }); }); describe('toggleFullScreen', function () { @@ -1087,9 +1082,12 @@ function (VideoPlayer) { isHtml5Mode: jasmine.createSpy().andReturn(true), isYoutubeType: jasmine.createSpy().andReturn(true), setPlayerMode: jasmine.createSpy(), + trigger: jasmine.createSpy(), videoPlayer: { currentTime: 60, isPlaying: jasmine.createSpy(), + seekTo: jasmine.createSpy(), + duration: jasmine.createSpy().andReturn(60), updatePlayTime: jasmine.createSpy(), setPlaybackRate: jasmine.createSpy(), player: jasmine.createSpyObj('player', [ @@ -1115,6 +1113,12 @@ function (VideoPlayer) { state.videoPlayer.isPlaying.andReturn(false); VideoPlayer.prototype.setPlaybackRate.call(state, '0.75'); expect(state.videoPlayer.updatePlayTime).toHaveBeenCalledWith(60); + expect(state.videoPlayer.seekTo).toHaveBeenCalledWith(60); + expect(state.trigger).toHaveBeenCalledWith( + 'videoProgressSlider.updateStartEndTimeRegion', + { + duration: 60 + }); expect(state.videoPlayer.player.cueVideoById) .toHaveBeenCalledWith('videoId', 60); }); 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 cda33180c3..07dd005366 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 @@ -44,6 +44,7 @@ function (HTML5Video, Resizer) { onVolumeChange: onVolumeChange, pause: pause, play: play, + seekTo: seekTo, setPlaybackRate: setPlaybackRate, update: update, figureOutStartEndTime: figureOutStartEndTime, @@ -94,7 +95,7 @@ function (HTML5Video, Resizer) { state.videoPlayer.ready = _.once(function () { $(window).on('unload', state.saveState); - if (!state.isFlashMode()) { + if (!state.isFlashMode() && state.speed != '1.0') { state.videoPlayer.setPlaybackRate(state.speed); } state.videoPlayer.player.setVolume(state.currentVolume); @@ -352,7 +353,8 @@ function (HTML5Video, Resizer) { } function setPlaybackRate(newSpeed) { - var time = this.videoPlayer.currentTime, + var duration = this.videoPlayer.duration(), + time = this.videoPlayer.currentTime, methodName, youtubeId; if ( @@ -378,7 +380,22 @@ function (HTML5Video, Resizer) { } this.videoPlayer.player[methodName](youtubeId, time); + + // We need to call play() explicitly because after the call + // to functions cueVideoById() followed by seekTo() the video + // is in a PAUSED state. + // + // Why? This is how the YouTube API is implemented. this.videoPlayer.updatePlayTime(time); + if (time > 0 && this.isFlashMode()) { + this.videoPlayer.seekTo(time); + this.trigger( + 'videoProgressSlider.updateStartEndTimeRegion', + { + duration: duration + } + ); + } } } @@ -414,59 +431,62 @@ function (HTML5Video, Resizer) { // It is created on a onPlay event. Cleared on a onPause event. // Reinitialized on a onSeek event. function onSeek(params) { - var duration = this.videoPlayer.duration(), - newTime = params.time; - - if ( - (typeof newTime !== 'number') || - (newTime > duration) || - (newTime < 0) - ) { - return; - } - - this.el.off('play.seek'); - this.videoPlayer.log( - 'seek_video', - { - old_time: this.videoPlayer.currentTime, - new_time: newTime, - type: params.type - } - ); + var time = params.time, + type = params.type; // After the user seeks, the video will start playing from // the sought point, and stop playing at the end. this.videoPlayer.goToStartTime = false; - if (newTime > this.videoPlayer.endTime || this.videoPlayer.endTime === null) { + if (time > this.videoPlayer.endTime || this.videoPlayer.endTime === null) { this.videoPlayer.stopAtEndTime = false; } + this.videoPlayer.seekTo(time); + + this.videoPlayer.log( + 'seek_video', + { + old_time: this.videoPlayer.currentTime, + new_time: time, + type: type + } + ); + } + + function seekTo(time) { + var duration = this.videoPlayer.duration(); + + if ((typeof time !== 'number') || (time > duration) || (time < 0)) { + return false; + } + + this.el.off('play.seek'); + if (this.videoPlayer.isPlaying()) { this.videoPlayer.stopTimer(); } else { - this.videoPlayer.currentTime = newTime; + this.videoPlayer.currentTime = time; } var isUnplayed = this.videoPlayer.isUnstarted() || this.videoPlayer.isCued(); // Use `cueVideoById` method for youtube video that is not played before. if (isUnplayed && this.isYoutubeType()) { - this.videoPlayer.player.cueVideoById(this.youtubeId(), newTime); + this.videoPlayer.player.cueVideoById(this.youtubeId(), time); } else { // Youtube video cannot be rewinded during bufferization, so wait to // finish bufferization and then rewind the video. if (this.isYoutubeType() && this.videoPlayer.isBuffering()) { this.el.on('play.seek', function () { - this.videoPlayer.player.seekTo(newTime, true); + this.videoPlayer.player.seekTo(time, true); }.bind(this)); } else { // Otherwise, just seek the video - this.videoPlayer.player.seekTo(newTime, true); + this.videoPlayer.player.seekTo(time, true); } } - this.videoPlayer.updatePlayTime(newTime, true); + this.videoPlayer.updatePlayTime(time, true); this.el.trigger('seek', arguments); } @@ -609,6 +629,7 @@ function (HTML5Video, Resizer) { // have 1 speed available, we fall back to Flash. _restartUsingFlash(this); + return false; } else if (availablePlaybackRates.length > 1) { this.setPlayerMode('html5'); @@ -646,16 +667,15 @@ function (HTML5Video, Resizer) { this.videoPlayer.player.setPlaybackRate(this.speed); } - this.el.trigger('ready', arguments); - /* The following has been commented out to make sure autoplay is - disabled for students. - if ( - !this.isTouch && - $('.video:first').data('autoplay') === 'True' - ) { - this.videoPlayer.play(); + + var duration = this.videoPlayer.duration(), + time = this.videoPlayer.figureOutStartingTime(duration); + + if (time > 0 && this.videoPlayer.goToStartTime) { + this.videoPlayer.seekTo(time); } - */ + + this.el.trigger('ready', arguments); } function onStateChange(event) { @@ -687,13 +707,9 @@ function (HTML5Video, Resizer) { break; case this.videoPlayer.PlayerState.CUED: this.el.addClass('is-cued'); - 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. - // - // Why? This is how the YouTube API is implemented. - this.videoPlayer.play(); + if (this.isFlashMode()) { + this.videoPlayer.play(); + } break; } } @@ -769,57 +785,6 @@ function (HTML5Video, Resizer) { duration = this.videoPlayer.duration(), youTubeId; - if (duration > 0 && videoPlayer.goToStartTime && !skip_seek) { - videoPlayer.goToStartTime = false; - - // The duration might have changed. Update the start-end time region to - // reflect this fact. - this.trigger( - 'videoProgressSlider.updateStartEndTimeRegion', - { - duration: duration - } - ); - - time = videoPlayer.figureOutStartingTime(duration); - - // When the video finishes playing, we will start from the - // start-time, or from the beginning (rather than from the remembered - // position). - this.config.savedVideoPosition = 0; - - if (time > 0) { - // 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. - // - // It turned out that for some reason if Firefox you couldn't - // seek beyond some amount of time before the video loaded. - // Very strange, but in Chrome there is no such bug. - // - // HTML5 video sources play fine from start-time in both Chrome - // and Firefox. - if (this.browserIsFirefox && this.isYoutubeType()) { - youTubeId = this.youtubeId(); - - // When we will call cueVideoById() for some strange reason - // an ENDED event will be fired. It really does no damage - // except for the fact that the end-time is reset to null. - // We do not want this. - // - // The flag `skipOnEndedStartEndReset` will notify the - // onEnded() callback for the ENDED event that there - // is no need in resetting the start-time and end-time. - videoPlayer.skipOnEndedStartEndReset = true; - - videoPlayer.seekToTimeOnCued = time; - videoPlayer.player.cueVideoById(youTubeId, time); - } else { - videoPlayer.player.seekTo(time); - } - } - } - this.trigger( 'videoProgressSlider.updatePlayTime', { 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 038b5f3577..5e8d9f2811 100644 --- a/common/lib/xmodule/xmodule/js/src/video/10_main.js +++ b/common/lib/xmodule/xmodule/js/src/video/10_main.js @@ -57,18 +57,11 @@ VideoCaption ) { var youtubeXhr = null, - oldVideo = window.Video, - - // Because this constructor can be called multiple times on a single page (when the user switches - // verticals, the page doesn't reload, but the content changes), we must will check each time if there - // is a previous copy of 'state' object. If there is, we will make sure that copy exists cleanly. We - // have to do this because when verticals switch, the code does not handle any Xmodule JS code that is - // running - it simply removes DOM elements from the page. Any functions that were running during this, - // and that will run afterwards (expecting the DOM elements to be present) must be stopped by hand. - previousState = null; + oldVideo = window.Video; window.Video = function (element) { - var state; + var previousState = window.Video.previousState, + state; // 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. @@ -78,7 +71,13 @@ } state = {}; - previousState = state; + // Because this constructor can be called multiple times on a single page (when the user switches + // verticals, the page doesn't reload, but the content changes), we must will check each time if there + // is a previous copy of 'state' object. If there is, we will make sure that copy exists cleanly. We + // have to do this because when verticals switch, the code does not handle any Xmodule JS code that is + // running - it simply removes DOM elements from the page. Any functions that were running during this, + // and that will run afterwards (expecting the DOM elements to be present) must be stopped by hand. + window.Video.previousState = state; state.modules = [ FocusGrabber,