Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force video player in flash mode. #3195

Merged
merged 1 commit into from
Apr 4, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cms/djangoapps/contentstore/features/transcripts.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,8 @@ def upload_file(_step, file_name):

@step('I see "([^"]*)" text in the captions')
def check_text_in_the_captions(_step, text):
world.wait_for(lambda _: world.css_text('.subtitles'), 30)
world.wait_for_present('.video.is-captions-rendered')
world.wait_for(lambda _: world.css_text('.subtitles'), timeout=30)
actual_text = world.css_text('.subtitles')
assert (text in actual_text)

Expand Down
5 changes: 4 additions & 1 deletion cms/djangoapps/contentstore/features/video.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,15 @@ def find_caption_line_by_data_index(index):

@step('I focus on caption line with data-index "([^"]*)"$')
def focus_on_caption_line(_step, index):
world.wait_for_present('.video.is-captions-rendered')
world.wait_for(lambda _: world.css_text('.subtitles'), timeout=30)
find_caption_line_by_data_index(int(index.strip()))._element.send_keys(Keys.TAB)


@step('I press "enter" button on caption line with data-index "([^"]*)"$')
def click_on_the_caption(_step, index):
world.wait_for_present('.video.is-captions-rendered')
world.wait_for(lambda _: world.css_text('.subtitles'), timeout=30)
find_caption_line_by_data_index(int(index.strip()))._element.send_keys(Keys.ENTER)


Expand All @@ -214,7 +218,6 @@ def caption_line_has_class(_step, index, className):
@step('I see a range on slider$')
def see_a_range_slider_with_proper_range(_step):
world.wait_for_visible(VIDEO_BUTTONS['pause'])

assert world.css_visible(".slider-range")


Expand Down
83 changes: 83 additions & 0 deletions common/lib/xmodule/xmodule/js/spec/video/initialize_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,89 @@ function (Initialize) {
});
});
});

describe('setPlayerMode', function () {
beforeEach(function () {
state = {
currentPlayerMode: 'flash',
};
});

it('updates player mode', function () {
var setPlayerMode = Initialize.prototype.setPlayerMode;

setPlayerMode.call(state, 'html5');
expect(state.currentPlayerMode).toBe('html5');
setPlayerMode.call(state, 'flash');
expect(state.currentPlayerMode).toBe('flash');
});

it('sets default mode if passed is not supported', function () {
var setPlayerMode = Initialize.prototype.setPlayerMode;

setPlayerMode.call(state, '77html77');
expect(state.currentPlayerMode).toBe('html5');
});
});

describe('getPlayerMode', function () {
beforeEach(function () {
state = {
currentPlayerMode: 'flash',
};
});

it('returns current player mode', function () {
var getPlayerMode = Initialize.prototype.getPlayerMode,
actual = getPlayerMode.call(state);

expect(actual).toBe(state.currentPlayerMode);
});
});

describe('isFlashMode', function () {
it('returns `true` if player in `flash` mode', function () {
var state = {
getPlayerMode: jasmine.createSpy().andReturn('flash'),
},
isFlashMode = Initialize.prototype.isFlashMode,
actual = isFlashMode.call(state);

expect(actual).toBeTruthy();
});

it('returns `false` if player is not in `flash` mode', function () {
var state = {
getPlayerMode: jasmine.createSpy().andReturn('html5'),
},
isFlashMode = Initialize.prototype.isFlashMode,
actual = isFlashMode.call(state);

expect(actual).toBeFalsy();
});
});

describe('isHtml5Mode', function () {
it('returns `true` if player in `html5` mode', function () {
var state = {
getPlayerMode: jasmine.createSpy().andReturn('html5'),
},
isHtml5Mode = Initialize.prototype.isHtml5Mode,
actual = isHtml5Mode.call(state);

expect(actual).toBeTruthy();
});

it('returns `false` if player is not in `html5` mode', function () {
var state = {
getPlayerMode: jasmine.createSpy().andReturn('flash'),
},
isHtml5Mode = Initialize.prototype.isHtml5Mode,
actual = isHtml5Mode.call(state);

expect(actual).toBeFalsy();
});
});
});
});

Expand Down
11 changes: 7 additions & 4 deletions common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -1070,6 +1070,9 @@ function (VideoPlayer) {
beforeEach(function () {
state = {
youtubeId: jasmine.createSpy().andReturn('videoId'),
isFlashMode: jasmine.createSpy().andReturn(false),
isHtml5Mode: jasmine.createSpy().andReturn(true),
setPlayerMode: jasmine.createSpy(),
videoPlayer: {
currentTime: 60,
isPlaying: jasmine.createSpy(),
Expand All @@ -1083,7 +1086,8 @@ function (VideoPlayer) {
});

it('in Flash mode and video is playing', function () {
state.currentPlayerMode = 'flash';
state.isFlashMode.andReturn(true);
state.isHtml5Mode.andReturn(false);
state.videoPlayer.isPlaying.andReturn(true);
VideoPlayer.prototype.setPlaybackRate.call(state, '0.75');
expect(state.videoPlayer.updatePlayTime).toHaveBeenCalledWith(60);
Expand All @@ -1092,7 +1096,8 @@ function (VideoPlayer) {
});

it('in Flash mode and video not started', function () {
state.currentPlayerMode = 'flash';
state.isFlashMode.andReturn(true);
state.isHtml5Mode.andReturn(false);
state.videoPlayer.isPlaying.andReturn(false);
VideoPlayer.prototype.setPlaybackRate.call(state, '0.75');
expect(state.videoPlayer.updatePlayTime).toHaveBeenCalledWith(60);
Expand All @@ -1101,13 +1106,11 @@ function (VideoPlayer) {
});

it('in HTML5 mode', function () {
state.currentPlayerMode = 'html5';
VideoPlayer.prototype.setPlaybackRate.call(state, '0.75');
expect(state.videoPlayer.player.setPlaybackRate).toHaveBeenCalledWith('0.75');
});

it('Youtube video in FF, with new speed equal 1.0', function () {
state.currentPlayerMode = 'html5';
state.videoType = 'youtube';
state.browserIsFirefox = true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ function() {
* @param {array} list Array to process.
* @param {function} process Calls this function on each item in the list.
* @return {array} Returns a Promise object to observe when all actions of a
certain type bound to the collection, queued or not, have finished.
* certain type bound to the collection, queued or not, have finished.
*/
var AsyncProcess = {
array: function (list, process) {
Expand Down
61 changes: 45 additions & 16 deletions common/lib/xmodule/xmodule/js/src/video/01_initialize.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,16 @@ function (VideoPlayer, VideoStorage) {
fetchMetadata: fetchMetadata,
getCurrentLanguage: getCurrentLanguage,
getDuration: getDuration,
getPlayerMode: getPlayerMode,
getVideoMetadata: getVideoMetadata,
initialize: initialize,
isHtml5Mode: isHtml5Mode,
isFlashMode: isFlashMode,
parseSpeed: parseSpeed,
parseVideoSources: parseVideoSources,
parseYoutubeStreams: parseYoutubeStreams,
saveState: saveState,
setPlayerMode: setPlayerMode,
setSpeed: setSpeed,
trigger: trigger,
youtubeId: youtubeId
Expand Down Expand Up @@ -250,18 +253,6 @@ function (VideoPlayer, VideoStorage) {
}
}

// function _setPlayerMode(state)
// By default we will be forcing HTML5 player mode. Only in the case
// when, after initializtion, we will get one available playback rate,
// we will change to Flash player mode. There is a need to store this
// setting in cookies because otherwise we will have to change from
// HTML5 to Flash on every page load in a browser that doesn't fully
// support HTML5. When we have this setting in cookies, we can select
// the proper mode from the start (not having to change mode later on).
function _setPlayerMode(state) {
state.currentPlayerMode = 'html5';
}

// function _parseYouTubeIDs(state)
// The function parse YouTube stream ID's.
// @return
Expand Down Expand Up @@ -339,8 +330,7 @@ function (VideoPlayer, VideoStorage) {

function _setConfigurations(state) {
_configureCaptions(state);
_setPlayerMode(state);

state.setPlayerMode(state.config.mode);
// Possible value are: 'visible', 'hiding', and 'invisible'.
state.controlState = 'visible';
state.controlHideTimeout = null;
Expand Down Expand Up @@ -520,7 +510,8 @@ function (VideoPlayer, VideoStorage) {
element: element,
fadeOutTimeout: 1400,
captionsFreezeTime: 10000,
availableQualities: ['hd720', 'hd1080', 'highres']
availableQualities: ['hd720', 'hd1080', 'highres'],
mode: $.cookie('edX_video_player_mode')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cookie is defined only in acc.tests. What will be not in tests? mode will be undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will be null and setPlayerMode method will be called with this value. See line 814.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, thanks for the clarification.

});

if (this.config.endTime < this.config.startTime) {
Expand Down Expand Up @@ -811,8 +802,46 @@ function (VideoPlayer, VideoStorage) {
}
}

/**
* Sets player mode.
*
* @param {string} mode Mode to set for the video player if it is supported.
* Otherwise, `html5` is used by default.
*/
function setPlayerMode(mode) {
var supportedModes = ['html5', 'flash'];

mode = _.contains(supportedModes, mode) ? mode : 'html5';
this.currentPlayerMode = mode;
}

/**
* Returns current player mode.
*
* @return {string} Returns string that describes player mode
*/
function getPlayerMode() {
return this.currentPlayerMode;
}

/**
* Checks if current player mode is Flash.
*
* @return {boolean} Returns `true` if current mode is `flash`, otherwise
* it returns `false`
*/
function isFlashMode() {
return this.currentPlayerMode === 'flash';
return this.getPlayerMode() === 'flash';
}

/**
* Checks if current player mode is Html5.
*
* @return {boolean} Returns `true` if current mode is `html5`, otherwise
* it returns `false`
*/
function isHtml5Mode() {
return this.getPlayerMode() === 'html5';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not isMode(mode_name)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By using current helpers you shouldn't remember possible values for the player mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense.

}

function getCurrentLanguage() {
Expand Down
10 changes: 5 additions & 5 deletions common/lib/xmodule/xmodule/js/src/video/03_video_player.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ function (HTML5Video, Resizer) {
// Remove from the page current iFrame with HTML5 video.
state.videoPlayer.player.destroy();

state.currentPlayerMode = 'flash';
state.setPlayerMode('flash');

console.log('[Video info]: Changing YouTube player mode to "flash".');

Expand Down Expand Up @@ -334,7 +334,7 @@ function (HTML5Video, Resizer) {
methodName, youtubeId;

if (
this.currentPlayerMode === 'html5' &&
this.isHtml5Mode() &&
!(
this.browserIsFirefox &&
newSpeed === '1.0' &&
Expand Down Expand Up @@ -554,7 +554,7 @@ function (HTML5Video, Resizer) {
// For more information, please see the PR that introduced this change:
// https://github.com/edx/edx-platform/pull/2841
if (
(this.currentPlayerMode === 'html5' || availablePlaybackRates.length > 1) &&
(this.isHtml5Mode() || availablePlaybackRates.length > 1) &&
this.videoType === 'youtube'
) {
if (availablePlaybackRates.length === 1 && !this.isTouch) {
Expand All @@ -568,7 +568,7 @@ function (HTML5Video, Resizer) {

_restartUsingFlash(this);
} else if (availablePlaybackRates.length > 1) {
this.currentPlayerMode = 'html5';
this.setPlayerMode('html5');

// We need to synchronize available frame rates with the ones
// that the user specified.
Expand Down Expand Up @@ -607,7 +607,7 @@ function (HTML5Video, Resizer) {
this.trigger('videoSpeedControl.setSpeed', this.speed);
}

if (this.currentPlayerMode === 'html5') {
if (this.isHtml5Mode()) {
this.videoPlayer.player.setPlaybackRate(this.speed);
}

Expand Down
3 changes: 2 additions & 1 deletion common/lib/xmodule/xmodule/js/src/video/09_video_caption.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ function (Sjson, AsyncProcess) {
};
}

state.el.removeClass('is-captions-rendered');
// Fetch the captions file. If no file was specified, or if an error
// occurred, then we hide the captions panel, and the "CC" button
this.fetchXHR = $.ajaxWithPrefix({
Expand Down Expand Up @@ -447,9 +448,9 @@ function (Sjson, AsyncProcess) {
// outline has to be drawn (tabbing) or not (mouse click).
self.isMouseFocus = false;
self.rendered = true;
self.state.el.addClass('is-captions-rendered');
};


this.rendered = false;
this.subtitlesEl.empty();
this.setSubtitlesHeight();
Expand Down
3 changes: 0 additions & 3 deletions common/lib/xmodule/xmodule/video_module/video_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
- Navigational subtitles can be disabled altogether via an attribute
in XML.
"""
import os
import json
import logging
from operator import itemgetter
Expand All @@ -36,8 +35,6 @@
from .video_xfields import VideoFields
from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers

from xmodule.modulestore.inheritance import InheritanceKeyValueStore
from xblock.runtime import KvsFieldData
from urlparse import urlparse

def get_ext(filename):
Expand Down
31 changes: 31 additions & 0 deletions lms/djangoapps/courseware/features/video.feature
Original file line number Diff line number Diff line change
Expand Up @@ -310,3 +310,34 @@ Feature: LMS Video component
When I open video "D"
Then the video has rendered in "HTML5" mode
And the video does not show the captions

# 27
Scenario: Transcripts are available on different speeds of Flash mode
Given I am registered for the course "test_course"
And I have a "subs_OEoXaMPEzfM.srt.sjson" transcript file in assets
And it has a video in "Flash" mode
Then the video has rendered in "Flash" mode
And I make sure captions are opened
And I see "Hi, welcome to Edx." text in the captions
Then I select the "1.50" speed
And I see "Hi, welcome to Edx." text in the captions
Then I select the "0.75" speed
And I see "Hi, welcome to Edx." text in the captions
Then I select the "1.25" speed
And I see "Hi, welcome to Edx." text in the captions

# 28
Scenario: Elapsed time calculates correctly on different speeds of Flash mode
Given I am registered for the course "test_course"
And I have a "subs_OEoXaMPEzfM.srt.sjson" transcript file in assets
And it has a video in "Flash" mode
And I make sure captions are opened
Then I select the "1.50" speed
And I click video button "pause"
And I click on caption line "4", video module shows elapsed time "7"
Then I select the "0.75" speed
And I click video button "pause"
And I click on caption line "3", video module shows elapsed time "9"
Then I select the "1.25" speed
And I click video button "pause"
And I click on caption line "2", video module shows elapsed time "4"
Loading