Merge pull request #1564 from edx/anton/fix-video-bugs

Video player: fix start-end time bugs.

common/lib/xmodule/xmodule/js/src/video/01_initialize.js

@@ -60,23 +60,23 @@ function (VideoPlayer) {
      *     methods, modules) of the Video player.
      */
     function _makeFunctionsPublic(state) {
-        state.setSpeed      = _.bind(setSpeed, state);
-        state.youtubeId     = _.bind(youtubeId, state);
-        state.getDuration   = _.bind(getDuration, state);
-        state.trigger       = _.bind(trigger, state);
-        state.stopBuffering = _.bind(stopBuffering, state);
-
-        // Old private functions. Now also public so that can be
-        // tested by Jasmine.
-
-        state.initialize          = _.bind(initialize, state);
-        state.parseSpeed          = _.bind(parseSpeed, state);
-        state.fetchMetadata       = _.bind(fetchMetadata, state);
-        state.parseYoutubeStreams = _.bind(parseYoutubeStreams, state);
-        state.parseVideoSources   = _.bind(parseVideoSources, state);
-        state.getVideoMetadata    = _.bind(getVideoMetadata, state);
+        var methodsDict = {
+            bindTo: bindTo,
+            checkStartEndTimes: checkStartEndTimes,
+            fetchMetadata: fetchMetadata,
+            getDuration: getDuration,
+            getVideoMetadata: getVideoMetadata,
+            initialize: initialize,
+            parseSpeed: parseSpeed,
+            parseVideoSources: parseVideoSources,
+            parseYoutubeStreams: parseYoutubeStreams,
+            setSpeed: setSpeed,
+            stopBuffering: stopBuffering,
+            trigger: trigger,
+            youtubeId: youtubeId
+        };
 
-        state.checkStartEndTimes  = _.bind(checkStartEndTimes, state);
+        bindTo(methodsDict, state, state);
     }
 
     // function _renderElements(state)
@@ -97,7 +97,7 @@ function (VideoPlayer) {
         if(state.videoType === 'youtube') {
             YT.ready(function() {
                 VideoPlayer(state);
-            })
+            });
         } else {
             VideoPlayer(state);
         }
@@ -233,6 +233,25 @@ function (VideoPlayer) {
     // them available and sets up their context is makeFunctionsPublic().
     // ***************************************************************
 
+
+    // function bindTo(methodsDict, obj, context, rewrite)
+    // Creates a new function with specific context and assigns it to the provided
+    // object.
+    function bindTo(methodsDict, obj, context, rewrite) {
+        $.each(methodsDict, function(name, method) {
+            if (_.isFunction(method)) {
+
+                if (_.isUndefined(rewrite)) {
+                    rewrite = true;
+                }
+
+                if (_.isUndefined(obj[name]) || rewrite) {
+                    obj[name] = _.bind(method, context);
+                }
+            }
+        });
+    }
+
     // function initialize(element)
     // The function set initial configuration and preparation.
 

common/lib/xmodule/xmodule/js/src/video/025_focus_grabber.js

@@ -44,14 +44,13 @@ function () {
     // Private functions.
 
     function _makeFunctionsPublic(state) {
-        state.focusGrabber.enableFocusGrabber  = _.bind(
-            enableFocusGrabber, state
-        );
-        state.focusGrabber.disableFocusGrabber = _.bind(
-            disableFocusGrabber, state
-        );
-
-        state.focusGrabber.onFocus = _.bind(onFocus, state);
+        var methodsDict = {
+            disableFocusGrabber: disableFocusGrabber,
+            enableFocusGrabber: enableFocusGrabber,
+            onFocus: onFocus
+        };
+
+        state.bindTo(methodsDict, state.focusGrabber, state);
     }
 
     function _renderElements(state) {

common/lib/xmodule/xmodule/js/src/video/03_video_player.js

@@ -24,33 +24,29 @@ function (HTML5Video, Resizer) {
     //     Functions which will be accessible via 'state' object. When called,
     //     these functions will get the 'state' object as a context.
     function _makeFunctionsPublic(state) {
-        state.videoPlayer.pause         = _.bind(pause, state);
-        state.videoPlayer.play          = _.bind(play, state);
-        state.videoPlayer.update        = _.bind(update, state);
-        state.videoPlayer.onSpeedChange = _.bind(onSpeedChange, state);
-        state.videoPlayer.onCaptionSeek = _.bind(onSeek, state);
-        state.videoPlayer.onSlideSeek   = _.bind(onSeek, state);
-        state.videoPlayer.onEnded       = _.bind(onEnded, state);
-        state.videoPlayer.onPause       = _.bind(onPause, state);
-        state.videoPlayer.onPlay        = _.bind(onPlay, state);
-
-        state.videoPlayer.onUnstarted   = _.bind(onUnstarted, state);
-
-        state.videoPlayer.handlePlaybackQualityChange = _.bind(
-            handlePlaybackQualityChange, state
-        );
-
-        state.videoPlayer.onPlaybackQualityChange = _.bind(
-            onPlaybackQualityChange, state
-        );
+        var methodsDict = {
+            duration: duration,
+            handlePlaybackQualityChange: handlePlaybackQualityChange,
+            isPlaying: isPlaying,
+            log: log,
+            onCaptionSeek: onSeek,
+            onEnded: onEnded,
+            onPause: onPause,
+            onPlay: onPlay,
+            onPlaybackQualityChange: onPlaybackQualityChange,
+            onReady: onReady,
+            onSlideSeek: onSeek,
+            onSpeedChange: onSpeedChange,
+            onStateChange: onStateChange,
+            onUnstarted: onUnstarted,
+            onVolumeChange: onVolumeChange,
+            pause: pause,
+            play: play,
+            update: update,
+            updatePlayTime: updatePlayTime
+        };
 
-        state.videoPlayer.onStateChange  = _.bind(onStateChange, state);
-        state.videoPlayer.onReady        = _.bind(onReady, state);
-        state.videoPlayer.updatePlayTime = _.bind(updatePlayTime, state);
-        state.videoPlayer.isPlaying      = _.bind(isPlaying, state);
-        state.videoPlayer.log            = _.bind(log, state);
-        state.videoPlayer.duration       = _.bind(duration, state);
-        state.videoPlayer.onVolumeChange = _.bind(onVolumeChange, state);
+        state.bindTo(methodsDict, state.videoPlayer, state);
     }
 
     // function _initialize(state)
@@ -67,7 +63,9 @@ 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 () {
+            if (state.currentPlayerMode !== 'flash') {
                 state.videoPlayer.onSpeedChange(state.speed);
+            }
         });
 
         if (state.videoType === 'youtube') {
@@ -224,19 +222,23 @@ function (HTML5Video, Resizer) {
     }
 
     function onSpeedChange(newSpeed, updateCookie) {
+        var time = this.videoPlayer.currentTime,
+            methodName, youtubeId;
+
         if (this.currentPlayerMode === 'flash') {
             this.videoPlayer.currentTime = Time.convert(
-                this.videoPlayer.currentTime,
+                time,
                 parseFloat(this.speed),
                 newSpeed
             );
         }
+
         newSpeed = parseFloat(newSpeed).toFixed(2).replace(/\.00$/, '.0');
 
         this.videoPlayer.log(
             'speed_change_video',
             {
-                current_time: this.videoPlayer.currentTime,
+                current_time: time,
                 old_speed: this.speed,
                 new_speed: newSpeed
             }
@@ -259,17 +261,15 @@ function (HTML5Video, Resizer) {
             // speed is 1.0. The second case is necessary to avoid the bug
             // where in Firefox speed switching to 1.0 in HTML5 player mode is
             // handled incorrectly by YouTube API.
+            methodName = 'cueVideoById';
+            youtubeId = this.youtubeId();
+
             if (this.videoPlayer.isPlaying()) {
-                this.videoPlayer.player.loadVideoById(
-                    this.youtubeId(), this.videoPlayer.currentTime
-                );
-            } else {
-                this.videoPlayer.player.cueVideoById(
-                    this.youtubeId(), this.videoPlayer.currentTime
-                );
+                methodName = 'loadVideoById';
             }
 
-            this.videoPlayer.updatePlayTime(this.videoPlayer.currentTime);
+            this.videoPlayer.player[methodName](youtubeId, time);
+            this.videoPlayer.updatePlayTime(time);
         }
     }
 
@@ -318,11 +318,18 @@ function (HTML5Video, Resizer) {
     }
 
     function onEnded() {
+        var time = this.videoPlayer.duration();
+
         this.trigger('videoControl.pause', null);
 
         if (this.config.show_captions) {
             this.trigger('videoCaption.pause', null);
         }
+
+        // Sometimes `onEnded` events fires when `currentTime` not equal
+        // `duration`. In this case, slider doesn't reach the end point of
+        // timeline.
+        this.videoPlayer.updatePlayTime(time);
     }
 
     function onPause() {
@@ -355,6 +362,8 @@ function (HTML5Video, Resizer) {
             this.videoPlayer.updateInterval = setInterval(
                 this.videoPlayer.update, 200
             );
+
+            this.videoPlayer.update();
         }
 
         this.trigger('videoControl.play', null);
@@ -509,9 +518,8 @@ function (HTML5Video, Resizer) {
     }
 
     function updatePlayTime(time) {
-        var duration, durationChange;
-
-        duration = this.videoPlayer.duration();
+        var duration = this.videoPlayer.duration(),
+            durationChange;
 
         if (
             duration > 0 &&
@@ -563,14 +571,7 @@ function (HTML5Video, Resizer) {
             //
             // We seek only if start time differs from zero.
             if (durationChange === false && this.videoPlayer.startTime > 0) {
-                if (this.videoType === 'html5') {
                 this.videoPlayer.player.seekTo(this.videoPlayer.startTime);
-                } else {
-                    this.videoPlayer.player.loadVideoById({
-                        videoId: this.youtubeId(),
-                        startSeconds: this.videoPlayer.startTime
-                    });
-                }
             }
 
             // Rebuild the slider start-end range (if it doesn't take up the
@@ -639,7 +640,7 @@ function (HTML5Video, Resizer) {
             dur = this.getDuration();
         }
 
-        return dur;
+        return Math.floor(dur);
     }
 
     function log(eventName, data) {

common/lib/xmodule/xmodule/js/src/video/04_video_control.js

@@ -24,14 +24,18 @@ function () {
     //     Functions which will be accessible via 'state' object. When called, these functions will
     //     get the 'state' object as a context.
     function _makeFunctionsPublic(state) {
-        state.videoControl.showControls     = _.bind(showControls,state);
-        state.videoControl.hideControls     = _.bind(hideControls,state);
-        state.videoControl.play             = _.bind(play,state);
-        state.videoControl.pause            = _.bind(pause,state);
-        state.videoControl.togglePlayback   = _.bind(togglePlayback,state);
-        state.videoControl.toggleFullScreen = _.bind(toggleFullScreen,state);
-        state.videoControl.exitFullScreen   = _.bind(exitFullScreen,state);
-        state.videoControl.updateVcrVidTime = _.bind(updateVcrVidTime,state);
+        var methodsDict = {
+            exitFullScreen: exitFullScreen,
+            hideControls: hideControls,
+            pause: pause,
+            play: play,
+            showControls: showControls,
+            toggleFullScreen: toggleFullScreen,
+            togglePlayback: togglePlayback,
+            updateVcrVidTime: updateVcrVidTime
+        };
+
+        state.bindTo(methodsDict, state.videoControl, state);
     }
 
     // function _renderElements(state)

common/lib/xmodule/xmodule/js/src/video/05_video_quality_control.js

@@ -29,8 +29,12 @@ function () {
     //     Functions which will be accessible via 'state' object. When called, these functions will
     //     get the 'state' object as a context.
     function _makeFunctionsPublic(state) {
-        state.videoQualityControl.onQualityChange = _.bind(onQualityChange, state);
-        state.videoQualityControl.toggleQuality   = _.bind(toggleQuality, state);
+        var methodsDict = {
+            onQualityChange: onQualityChange,
+            toggleQuality: toggleQuality
+        };
+
+        state.bindTo(methodsDict, state.videoQualityControl, state);
     }
 
     // function _renderElements(state)

common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js

@@ -31,18 +31,15 @@ function () {
     //     Functions which will be accessible via 'state' object. When called,
     //     these functions will get the 'state' object as a context.
     function _makeFunctionsPublic(state) {
-        state.videoProgressSlider.onSlide        = _.bind(onSlide, state);
-        state.videoProgressSlider.onStop         = _.bind(onStop, state);
-        state.videoProgressSlider.updatePlayTime = _.bind(
-            updatePlayTime, state
-        );
-
-        //Added for tests -- JM
-        state.videoProgressSlider.buildSlider = _.bind(buildSlider, state);
+        var methodsDict = {
+            buildSlider: buildSlider,
+            onSlide: onSlide,
+            onStop: onStop,
+            updatePlayTime: updatePlayTime,
+            updateStartEndTimeRegion: updateStartEndTimeRegion
+        };
 
-        state.videoProgressSlider.updateStartEndTimeRegion = _.bind(
-            updateStartEndTimeRegion, state
-        );
+        state.bindTo(methodsDict, state.videoProgressSlider, state);
     }
 
     // function _renderElements(state)
@@ -99,12 +96,14 @@ function () {
     }
 
     function updateStartEndTimeRegion(params) {
-        var left, width, start, end, step;
+        var left, width, start, end, step, duration;
 
         // We must have a duration in order to determine the area of range.
         // It also must be non-zero.
         if (!params.duration) {
             return;
+        } else {
+            duration = params.duration;
         }
 
         // If the range spans the entire length of video, we don't do anything.
@@ -117,7 +116,7 @@ function () {
         // If end is set to null, then we set it to the end of the video. We
         // know that start is not a the beginning, therefore we must build a
         // range.
-        end = this.videoPlayer.endTime || params.duration;
+        end = this.videoPlayer.endTime || duration;
 
         // Because JavaScript has weird rounding rules when a series of
         // mathematical operations are performed in a single statement, we will
@@ -125,11 +124,9 @@ function () {
         //
         // This will ensure that visually, the start-end range aligns nicely
         // with actual starting and ending point of the video.
-        step = 100.0 / params.duration;
+        step = 100.0 / duration;
         left = start * step;
         width = end * step - left;
-        left = left.toFixed(1);
-        width = width.toFixed(1);
 
         if (!this.videoProgressSlider.sliderRange) {
             this.videoProgressSlider.sliderRange = $('<div />', {

common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js

@@ -24,8 +24,12 @@ function () {
     //     Functions which will be accessible via 'state' object. When called, these functions will
     //     get the 'state' object as a context.
     function _makeFunctionsPublic(state) {
-        state.videoVolumeControl.onChange = _.bind(onChange, state);
-        state.videoVolumeControl.toggleMute = _.bind(toggleMute, state);
+        var methodsDict = {
+            onChange: onChange,
+            toggleMute: toggleMute
+        };
+
+        state.bindTo(methodsDict, state.videoVolumeControl, state);
     }
 
     // function _renderElements(state)
@@ -67,14 +71,11 @@ function () {
         // Let screen readers know that:
 
         // This anchor behaves as a button named 'Volume'.
-        var buttonStr = gettext(
-                state.videoVolumeControl.currentVolume === 0
-                ? 'Volume muted'
-                : 'Volume'
-            );
+        var currentVolume = state.videoVolumeControl.currentVolume,
+            buttonStr = (currentVolume === 0) ? 'Volume muted' : 'Volume';
         // We add the aria-label attribute because the title attribute cannot be
         // read.
-        state.videoVolumeControl.buttonEl.attr('aria-label', buttonStr);
+        state.videoVolumeControl.buttonEl.attr('aria-label', gettext(buttonStr));
 
         // Let screen readers know that this anchor, representing the slider
         // handle, behaves as a slider named 'volume'.
@@ -167,8 +168,11 @@ function () {
     // ***************************************************************
 
     function onChange(event, ui) {
-        this.videoVolumeControl.currentVolume = ui.value;
-        this.videoVolumeControl.el.toggleClass('muted', this.videoVolumeControl.currentVolume === 0);
+        var currentVolume = ui.value,
+            ariaLabelText = (currentVolume === 0) ? 'Volume muted' : 'Volume';
+
+        this.videoVolumeControl.currentVolume = currentVolume;
+        this.videoVolumeControl.el.toggleClass('muted', currentVolume === 0);
 
         $.cookie('video_player_volume_level', ui.value, {
             expires: 3650,
@@ -184,9 +188,7 @@ function () {
         });
 
         this.videoVolumeControl.buttonEl.attr(
-            'aria-label', this.videoVolumeControl.currentVolume === 0
-                          ? gettext('Volume muted')
-                          : gettext('Volume')
+            'aria-label', gettext(ariaLabelText)
         );
     }
 

common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js

@@ -44,11 +44,13 @@ function () {
     //     Functions which will be accessible via 'state' object. When called,
     //     these functions will get the 'state' object as a context.
     function _makeFunctionsPublic(state) {
-        state.videoSpeedControl.changeVideoSpeed = _.bind(
-            changeVideoSpeed, state
-        );
-        state.videoSpeedControl.setSpeed = _.bind(setSpeed, state);
-        state.videoSpeedControl.reRender = _.bind(reRender, state);
+        var methodsDict = {
+            changeVideoSpeed: changeVideoSpeed,
+            reRender: reRender,
+            setSpeed: setSpeed
+        };
+
+        state.bindTo(methodsDict, state.videoSpeedControl, state);
     }
 
     // function _renderElements(state)
@@ -229,7 +231,7 @@ function () {
             // Track the focus on intermediary speeds.
             speedLinks
                 .filter(function (index) {
-                    return index === 1 || index === 2
+                    return index === 1 || index === 2;
                 })
                 .on('blur', function () {
                     state.previousFocus = 'otherSpeed';

common/lib/xmodule/xmodule/js/src/video/09_video_caption.js

@@ -37,53 +37,40 @@ function () {
     //     Functions which will be accessible via 'state' object. When called,
     //     these functions will get the 'state' object as a context.
     function _makeFunctionsPublic(state) {
-        state.videoCaption.autoShowCaptions    = _.bind(
-            autoShowCaptions, state
-        );
-        state.videoCaption.autoHideCaptions    = _.bind(
-            autoHideCaptions, state
-        );
-        state.videoCaption.resize              = _.bind(resize, state);
-        state.videoCaption.toggle              = _.bind(toggle, state);
-        state.videoCaption.onMouseEnter        = _.bind(onMouseEnter, state);
-        state.videoCaption.onMouseLeave        = _.bind(onMouseLeave, state);
-        state.videoCaption.onMovement          = _.bind(onMovement, state);
-        state.videoCaption.renderCaption       = _.bind(renderCaption, state);
-        state.videoCaption.captionHeight       = _.bind(captionHeight, state);
-        state.videoCaption.topSpacingHeight    = _.bind(
-            topSpacingHeight, state
-        );
-        state.videoCaption.bottomSpacingHeight = _.bind(
-            bottomSpacingHeight, state
-        );
-        state.videoCaption.scrollCaption       = _.bind(scrollCaption, state);
-        state.videoCaption.search              = _.bind(search, state);
-        state.videoCaption.play                = _.bind(play, state);
-        state.videoCaption.pause               = _.bind(pause, state);
-        state.videoCaption.seekPlayer          = _.bind(seekPlayer, state);
-        state.videoCaption.hideCaptions        = _.bind(hideCaptions, state);
-        state.videoCaption.calculateOffset     = _.bind(
-            calculateOffset, state
-        );
-        state.videoCaption.updatePlayTime      = _.bind(updatePlayTime, state);
-        state.videoCaption.setSubtitlesHeight  = _.bind(
-            setSubtitlesHeight, state
-        );
+        var methodsDict = {
+            autoHideCaptions: autoHideCaptions,
+            autoShowCaptions: autoShowCaptions,
+            bindHandlers: bindHandlers,
+            bottomSpacingHeight: bottomSpacingHeight,
+            calculateOffset: calculateOffset,
+            captionBlur: captionBlur,
+            captionClick: captionClick,
+            captionFocus: captionFocus,
+            captionHeight: captionHeight,
+            captionKeyDown: captionKeyDown,
+            captionMouseDown: captionMouseDown,
+            captionMouseOverOut: captionMouseOverOut,
+            captionURL: captionURL,
+            fetchCaption: fetchCaption,
+            hideCaptions: hideCaptions,
+            onMouseEnter: onMouseEnter,
+            onMouseLeave: onMouseLeave,
+            onMovement: onMovement,
+            pause: pause,
+            play: play,
+            renderCaption: renderCaption,
+            renderElements: renderElements,
+            resize: resize,
+            scrollCaption: scrollCaption,
+            search: search,
+            seekPlayer: seekPlayer,
+            setSubtitlesHeight: setSubtitlesHeight,
+            toggle: toggle,
+            topSpacingHeight: topSpacingHeight,
+            updatePlayTime: updatePlayTime
+        };
 
-        state.videoCaption.renderElements      = _.bind(renderElements, state);
-        state.videoCaption.bindHandlers        = _.bind(bindHandlers, state);
-        state.videoCaption.fetchCaption        = _.bind(fetchCaption, state);
-        state.videoCaption.captionURL          = _.bind(captionURL, state);
-        state.videoCaption.captionMouseOverOut = _.bind(
-            captionMouseOverOut, state
-        );
-        state.videoCaption.captionMouseDown    = _.bind(
-            captionMouseDown, state
-        );
-        state.videoCaption.captionClick        = _.bind(captionClick, state);
-        state.videoCaption.captionFocus        = _.bind(captionFocus, state);
-        state.videoCaption.captionBlur         = _.bind(captionBlur, state);
-        state.videoCaption.captionKeyDown      = _.bind(captionKeyDown, state);
+        state.bindTo(methodsDict, state.videoCaption, state);
     }
 
     // ***************************************************************