diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7f736932fd..f4dac37e94 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: Show the HD button only if there is an HD version available. BLD-937. + Studio: Add edit button to leaf xblocks on the container page. STUD-1306. Blades: Add LTI context_id parameter. BLD-584. diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index ce18abe093..55b7f78a84 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -320,7 +320,7 @@ div.video { a.speed-button, div.volume > a, a.add-fullscreen, - a.quality_control, + a.quality-control, a.hide-subtitles { // overflow is used to bypass Firefox CSS :focus outline bug // http://johndoesdesign.com/blog/2012/css/firefox-and-its-css-focus-outline-bug/ @@ -536,11 +536,10 @@ div.video { } - a.quality_control { + a.quality-control { @extend %video-button; background: url(../images/hd.png) center no-repeat; border-left: none; - display: none; float: left; padding: 0 11px; width: 30px; @@ -550,6 +549,10 @@ div.video { color: #0ff; text-decoration: none; } + + &.is-hidden { + display: none !important; + } } div.lang { diff --git a/common/lib/xmodule/xmodule/js/fixtures/video.html b/common/lib/xmodule/xmodule/js/fixtures/video.html index 2a9c31eb43..e1fe11ae47 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video.html @@ -54,7 +54,7 @@ Fill Browser - HD off + diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_all.html b/common/lib/xmodule/xmodule/js/fixtures/video_all.html index fd986b9437..7ee3f4adb1 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_all.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_all.html @@ -57,7 +57,7 @@ Fill Browser - HD off + 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 d7026d654a..8086c2b269 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html @@ -54,7 +54,7 @@ Fill Browser - HD off + @@ -121,7 +121,7 @@ Fill Browser - HD + @@ -186,7 +186,7 @@ Fill Browser - HD + diff --git a/common/lib/xmodule/xmodule/js/spec/helper.js b/common/lib/xmodule/xmodule/js/spec/helper.js index 193c47ba13..4900fdf8b2 100644 --- a/common/lib/xmodule/xmodule/js/spec/helper.js +++ b/common/lib/xmodule/xmodule/js/spec/helper.js @@ -8,12 +8,16 @@ 'getPlayerState', 'getVolume', 'setVolume', 'loadVideoById', 'getAvailablePlaybackRates', 'playVideo', 'pauseVideo', 'seekTo', 'getDuration', 'setPlaybackRate', - 'getPlaybackQuality', 'destroy' + 'getAvailableQualityLevels', 'getPlaybackQuality', + 'setPlaybackQuality', 'destroy' ] ); Player.getDuration.andReturn(60); Player.getAvailablePlaybackRates.andReturn([0.50, 1.0, 1.50, 2.0]); + Player.getAvailableQualityLevels.andReturn( + ['highres', 'hd1080', 'hd720', 'large', 'medium', 'small'] + ); return Player; }, 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 3f9b2c2d0d..5cc6b7fece 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 @@ -1,41 +1,37 @@ (function (undefined) { describe('VideoQualityControl', function () { - var state, oldOTBD; - - beforeEach(function () { - oldOTBD = window.onTouchBasedDevice; - window.onTouchBasedDevice = jasmine - .createSpy('onTouchBasedDevice') - .andReturn(null); - }); + var state, qualityControl, qualityControlEl, videoPlayer, player; afterEach(function () { $('source').remove(); - window.onTouchBasedDevice = oldOTBD; - state.storage.clear(); + if (state.storage) { + state.storage.clear(); + } }); - describe('constructor', function () { + describe('constructor, YouTube mode', function () { beforeEach(function () { - state = jasmine.initializePlayer('video.html'); - }); + state = jasmine.initializePlayerYouTube(); + qualityControl = state.videoQualityControl; + videoPlayer = state.videoPlayer; + player = videoPlayer.player; - it('render the quality control', function () { - waitsFor(function () { - return state.videoControl; - }, 'videoControl is present', 5000); - - runs(function () { - var container = state.videoControl.secondaryControlsEl; - - expect(container).toContain('a.quality_control'); + // Define empty methods in YouTube stub + player.quality = 'large'; + player.setPlaybackQuality.andCallFake(function (quality){ + player.quality = quality; }); }); - it('add ARIA attributes to quality control', function () { - var qualityControl = $('a.quality_control'); + it('contains the quality control and is initially hidden', + function () { + expect(qualityControl.el).toHaveClass( + 'quality-control is-hidden' + ); + }); - expect(qualityControl).toHaveAttrs({ + it('add ARIA attributes to quality control', function () { + expect(qualityControl.el).toHaveAttrs({ 'role': 'button', 'title': 'HD off', 'aria-disabled': 'false' @@ -43,9 +39,60 @@ }); it('bind the quality control', function () { - var handler = state.videoQualityControl.toggleQuality; + expect(qualityControl.el).toHandleWith('click', + qualityControl.toggleQuality + ); - expect($('a.quality_control')).toHandleWith('click', handler); + expect(state.el).toHandle('play'); + }); + + it('calls fetchAvailableQualities only once', function () { + expect(player.getAvailableQualityLevels.calls.length) + .toEqual(0); + + videoPlayer.onPlay(); + videoPlayer.onPlay(); + + expect(player.getAvailableQualityLevels.calls.length) + .toEqual(1); + }); + + it('shows the quality control on play if HD is available', + function () { + videoPlayer.onPlay(); + + expect(qualityControl.el).not.toHaveClass('is-hidden'); + }); + + it('leaves quality control hidden on play if HD is not available', + function () { + player.getAvailableQualityLevels.andReturn( + ['large', 'medium', 'small'] + ); + + videoPlayer.onPlay(); + expect(qualityControl.el).toHaveClass('is-hidden'); + }); + + it('switch to HD if it is available', function () { + videoPlayer.onPlay(); + + qualityControl.quality = 'large'; + qualityControl.el.click(); + expect(player.setPlaybackQuality) + .toHaveBeenCalledWith('highres'); + + qualityControl.quality = 'highres'; + qualityControl.el.click(); + expect(player.setPlaybackQuality).toHaveBeenCalledWith('large'); + }); + }); + + describe('constructor, HTML5 mode', function () { + it('does not contain the quality control', function () { + state = jasmine.initializePlayer(); + + expect(state.el.find('a.quality-control').length).toBe(0); }); }); }); 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 48c789dc49..f1a63b9c35 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -511,8 +511,10 @@ function (VideoPlayer, VideoStorage) { element: element, fadeOutTimeout: 1400, captionsFreezeTime: 10000, - availableQualities: ['hd720', 'hd1080', 'highres'], - mode: $.cookie('edX_video_player_mode') + mode: $.cookie('edX_video_player_mode'), + // Available HD qualities will only be accessible once the video has + // been played once, via player.getAvailableQualityLevels. + availableHDQualities: [] }); if (this.config.endTime < this.config.startTime) { diff --git a/common/lib/xmodule/xmodule/js/src/video/05_video_quality_control.js b/common/lib/xmodule/xmodule/js/src/video/05_video_quality_control.js index 5f1777ca0b..4ed914044d 100644 --- a/common/lib/xmodule/xmodule/js/src/video/05_video_quality_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/05_video_quality_control.js @@ -12,7 +12,7 @@ function () { // Changing quality for now only works for YouTube videos. if (state.videoType !== 'youtube') { - state.el.find('a.quality_control').remove(); + state.el.find('a.quality-control').remove(); return; } @@ -36,7 +36,9 @@ function () { // get the 'state' object as a context. function _makeFunctionsPublic(state) { var methodsDict = { + fetchAvailableQualities: fetchAvailableQualities, onQualityChange: onQualityChange, + showQualityControl: showQualityControl, toggleQuality: toggleQuality }; @@ -49,17 +51,22 @@ function () { // make the created DOM elements available via the 'state' object. Much easier to work this // way - you don't have to do repeated jQuery element selects. function _renderElements(state) { - state.videoQualityControl.el = state.el.find('a.quality_control'); + state.videoQualityControl.el = state.el.find('a.quality-control'); state.videoQualityControl.el.show(); - state.videoQualityControl.quality = null; + state.videoQualityControl.quality = 'large'; } // function _bindHandlers(state) // // Bind any necessary function callbacks to DOM events (click, mousemove, etc.). function _bindHandlers(state) { - state.videoQualityControl.el.on('click', state.videoQualityControl.toggleQuality); + state.videoQualityControl.el.on('click', + state.videoQualityControl.toggleQuality + ); + state.el.on('play', + _.once(state.videoQualityControl.fetchAvailableQualities) + ); } // *************************************************************** @@ -68,11 +75,42 @@ function () { // The magic private function that makes them available and sets up their context is makeFunctionsPublic(). // *************************************************************** + /* + * @desc Shows quality control. This function will only be called if HD + * qualities are available. + * + * @public + */ + function showQualityControl() { + this.videoQualityControl.el.removeClass('is-hidden'); + } + + // This function can only be called once as _.once has been used. + /* + * @desc Get the available qualities from YouTube API. Possible values are: + ['highres', 'hd1080', 'hd720', 'large', 'medium', 'small']. + HD are: ['highres', 'hd1080', 'hd720']. + * + * @public + */ + function fetchAvailableQualities() { + var qualities = this.videoPlayer.player.getAvailableQualityLevels(); + + this.config.availableHDQualities = _.intersection( + qualities, ['highres', 'hd1080', 'hd720'] + ); + + // HD qualities are available, show video quality control. + if (this.config.availableHDQualities.length > 0) { + this.trigger('videoQualityControl.showQualityControl'); + } + } + function onQualityChange(value) { var controlStateStr; this.videoQualityControl.quality = value; - if (_.indexOf(this.config.availableQualities, value) !== -1) { + if (_.contains(this.config.availableHDQualities, value)) { controlStateStr = gettext('HD on'); this.videoQualityControl.el .addClass('active') @@ -88,24 +126,15 @@ function () { } } - // This function change quality of video. - // Right now we haven't ability to choose quality of HD video, - // 'hd720' will be played by default as HD video(this thing is hardcoded). - // If suggested quality level is not available for the video, - // then the quality will be set to the next lowest level that is available. - // (large -> medium) + // This function toggles the quality of video only if HD qualities are + // available. function toggleQuality(event) { - var newQuality, - value = this.videoQualityControl.quality; + var newQuality, value = this.videoQualityControl.quality, + isHD = _.contains(this.config.availableHDQualities, value); event.preventDefault(); - if (_.indexOf(this.config.availableQualities, value) !== -1) { - newQuality = 'large'; - } else { - newQuality = 'hd720'; - } - + newQuality = isHD ? 'large' : 'highres'; this.trigger('videoPlayer.handlePlaybackQualityChange', newQuality); } diff --git a/lms/djangoapps/courseware/features/video.feature b/lms/djangoapps/courseware/features/video.feature index e4b2972e78..fe37999568 100644 --- a/lms/djangoapps/courseware/features/video.feature +++ b/lms/djangoapps/courseware/features/video.feature @@ -297,3 +297,20 @@ Feature: LMS.Video component # 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" + + # 27 + @skip_firefox + Scenario: Quality button appears on play + Given the course has a Video component in "Youtube" mode + Then I see video button "quality" is hidden + And I click video button "play" + Then I see video button "quality" is visible + + # 28 + @skip_firefox + Scenario: Quality button works correctly + Given the course has a Video component in "Youtube" mode + And I click video button "play" + And I see video button "quality" is inactive + And I click video button "quality" + Then I see video button "quality" is active diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index 583556af80..0cc89514cc 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -6,7 +6,7 @@ import json import os import time import requests -from nose.tools import assert_less, assert_equal +from nose.tools import assert_less, assert_equal, assert_true, assert_false from common import i_am_registered_for_the_course, visit_scenario_item from django.utils.translation import ugettext as _ from django.conf import settings @@ -44,6 +44,7 @@ VIDEO_BUTTONS = { 'pause': '.video_control.pause', 'fullscreen': '.add-fullscreen', 'download_transcript': '.video-tracks > a', + 'quality': '.quality-control', } VIDEO_MENUS = { @@ -542,11 +543,6 @@ def upload_to_assets(_step, filename): upload_file(filename, world.scenario_dict['COURSE'].location) -@step('button "([^"]*)" is hidden$') -def is_hidden_button(_step, button): - assert not world.css_visible(VIDEO_BUTTONS[button]) - - @step('menu "([^"]*)" doesn\'t exist$') def is_hidden_menu(_step, menu): assert world.is_css_not_present(VIDEO_MENUS[menu]) @@ -627,3 +623,30 @@ def click_on_the_caption(_step, index, expected_time): find_caption_line_by_data_index(int(index)).click() actual_time = elapsed_time() assert int(expected_time) == actual_time + + +@step('button "([^"]*)" is (hidden|visible)$') +def is_hidden_button(_step, button, state): + selector = VIDEO_BUTTONS[button] + if state == 'hidden': + world.wait_for_invisible(selector) + assert_false( + world.css_visible(selector), + 'Button {0} is invisible, but should be visible'.format(button) + ) + else: + world.wait_for_visible(selector) + assert_true( + world.css_visible(selector), + 'Button {0} is visible, but should be invisible'.format(button) + ) + + +@step('button "([^"]*)" is (active|inactive)$') +def i_see_active_button(_step, button, state): + selector = VIDEO_BUTTONS[button] + if state == 'active': + assert world.css_has_class(selector, 'active') + else: + assert not world.css_has_class(selector, 'active') + diff --git a/lms/templates/video.html b/lms/templates/video.html index ea6f3a9cb0..b3e1a8a24f 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -85,7 +85,7 @@ ${_('Fill browser')} - ${_('HD off')} +