Merge pull request #3643 from edx/anton/fix-video-in-ff

Anton/fix video in ff

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 = [
                 {

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);

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);
             });

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;
+        var time = params.time,
+            type = params.type;
 
-        if (
-            (typeof newTime !== 'number') ||
-            (newTime > duration) ||
-            (newTime < 0)
-        ) {
-            return;
+        // After the user seeks, the video will start playing from
+        // the sought point, and stop playing at the end.
+        this.videoPlayer.goToStartTime = false;
+        if (time > this.videoPlayer.endTime || this.videoPlayer.endTime === null) {
+            this.videoPlayer.stopAtEndTime = false;
         }
 
-        this.el.off('play.seek');
+        this.videoPlayer.seekTo(time);
+
         this.videoPlayer.log(
             'seek_video',
             {
                 old_time: this.videoPlayer.currentTime,
-                new_time: newTime,
-                type: params.type
+                new_time: time,
+                type: 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) {
-            this.videoPlayer.stopAtEndTime = false;
+    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',
             {

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,