diff --git a/CHANGELOG.rst b/CHANGELOG.rst index d5daa92d0b..07ce908ba9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,10 @@ 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: When start time and end time are specified for a video, a visual range +will be shown on the time slider to highlight the place in the video that will +be played. + Studio: Bug fix for text loss in Course Updates when the text exists before the first tag. @@ -367,10 +371,10 @@ Studio: Improve link re-writing on imports into a different course-id Studio: course catalog and course outline pages new use course id syntax w/ restful api style -Common: +Common: separate the non-sql db connection configuration from the modulestore (xblock modeling) configuration. in split, separate the the db connection and atomic crud ops into a distinct module & class from modulestore - + Common: location mapper: % encode periods and dollar signs when used as key in the mapping dict Common: location mapper: added a bunch of new helper functions for generating old location style info from a CourseLocator diff --git a/cms/djangoapps/contentstore/features/transcripts.feature b/cms/djangoapps/contentstore/features/transcripts.feature index c88d5da6ad..dad9cbb49e 100644 --- a/cms/djangoapps/contentstore/features/transcripts.feature +++ b/cms/djangoapps/contentstore/features/transcripts.feature @@ -78,7 +78,7 @@ Feature: Video Component Editor And I enter a "http://youtu.be/t__eq_exist" source to field number 1 Then I see status message "not found" And I see button "import" - And I click button "import" + And I click transcript button "import" Then I see status message "found" And I see button "upload_new_timed_transcripts" And I see button "download_to_edit" @@ -111,7 +111,7 @@ Feature: Video Component Editor And I enter a "http://youtu.be/t_neq_exist" source to field number 1 And I see status message "replace" And I see button "replace" - And I click button "replace" + And I click transcript button "replace" And I see status message "found" And I see value "t_neq_exist" in the field "HTML5 Transcript" @@ -153,7 +153,7 @@ Feature: Video Component Editor And I enter a "http://youtu.be/t__eq_exist" source to field number 1 Then I see status message "not found" And I see button "import" - And I click button "import" + And I click transcript button "import" Then I see status message "found" And I enter a "t_not_exist.mp4" source to field number 2 @@ -205,7 +205,7 @@ Feature: Video Component Editor And I enter a "http://youtu.be/t__eq_exist" source to field number 1 Then I see status message "not found" And I see button "import" - And I click button "import" + And I click transcript button "import" Then I see status message "found" And I see button "upload_new_timed_transcripts" @@ -287,7 +287,7 @@ Feature: Video Component Editor And I enter a "http://youtu.be/t__eq_exist" source to field number 1 Then I see status message "not found" And I see button "import" - And I click button "import" + And I click transcript button "import" Then I see status message "found" And I see button "upload_new_timed_transcripts" @@ -309,7 +309,7 @@ Feature: Video Component Editor And I enter a "http://youtu.be/t__eq_exist" source to field number 1 Then I see status message "not found" And I see button "import" - And I click button "import" + And I click transcript button "import" Then I see status message "found" And I see button "upload_new_timed_transcripts" @@ -364,7 +364,7 @@ Feature: Video Component Editor And I see choose button "test_transcripts.mp4" number 1 And I see choose button "t_not_exist.webm" number 2 - And I click button "choose" number 2 + And I click transcript button "choose" number 2 And I see value "test_transcripts|t_not_exist" in the field "HTML5 Transcript" #21 @@ -384,13 +384,13 @@ Feature: Video Component Editor And I enter a "video_name_2.mp4" source to field number 1 Then I see status message "use existing" And I see button "use_existing" - And I click button "use_existing" + And I click transcript button "use_existing" And I see value "video_name_2" in the field "HTML5 Transcript" And I enter a "video_name_3.mp4" source to field number 1 Then I see status message "use existing" And I see button "use_existing" - And I click button "use_existing" + And I click transcript button "use_existing" And I see value "video_name_3" in the field "HTML5 Transcript" #22 @@ -410,7 +410,7 @@ Feature: Video Component Editor And I enter a "video_name_2.mp4" source to field number 1 Then I see status message "use existing" And I see button "use_existing" - And I click button "use_existing" + And I click transcript button "use_existing" And I see value "video_name_2" in the field "HTML5 Transcript" And I enter a "video_name_3.mp4" source to field number 1 @@ -420,7 +420,7 @@ Feature: Video Component Editor And I enter a "video_name_4.mp4" source to field number 1 Then I see status message "use existing" And I see button "use_existing" - And I click button "use_existing" + And I click transcript button "use_existing" And I see value "video_name_4" in the field "HTML5 Transcript" #23 @@ -443,7 +443,7 @@ Feature: Video Component Editor And I enter a "video_name_3.webm" source to field number 2 Then I see status message "use existing" And I see button "use_existing" - And I click button "use_existing" + And I click transcript button "use_existing" And I see value "video_name_2|video_name_3" in the field "HTML5 Transcript" #24 Uploading subtitles with different file name than file diff --git a/cms/djangoapps/contentstore/features/transcripts.py b/cms/djangoapps/contentstore/features/transcripts.py index 15413b52e5..5cbb65dc9d 100644 --- a/cms/djangoapps/contentstore/features/transcripts.py +++ b/cms/djangoapps/contentstore/features/transcripts.py @@ -38,7 +38,7 @@ SELECTORS = { } # button type , button css selector, button message -BUTTONS = { +TRANSCRIPTS_BUTTONS = { 'import': ('.setting-import', 'Import from YouTube'), 'download_to_edit': ('.setting-download', 'Download to Edit'), 'disabled_download_to_edit': ('.setting-download.is-disabled', 'Download to Edit'), @@ -127,9 +127,9 @@ def i_see_button(_step, not_see, button_type): button = button_type.strip() if not_see.strip(): - assert world.is_css_not_present(BUTTONS[button][0]) + assert world.is_css_not_present(TRANSCRIPTS_BUTTONS[button][0]) else: - assert world.css_has_text(BUTTONS[button][0], BUTTONS[button][1]) + assert world.css_has_text(TRANSCRIPTS_BUTTONS[button][0], TRANSCRIPTS_BUTTONS[button][1]) @step('I (.*)see (.*)button "([^"]*)" number (\d+)$') @@ -142,21 +142,21 @@ def i_see_button_with_custom_text(_step, not_see, button_type, custom_text, inde index = int(index.strip()) - 1 if not_see.strip(): - assert world.is_css_not_present(BUTTONS[button][0]) + assert world.is_css_not_present(TRANSCRIPTS_BUTTONS[button][0]) else: - assert world.css_has_text(BUTTONS[button][0], BUTTONS[button][1].format(custom_text), index) + assert world.css_has_text(TRANSCRIPTS_BUTTONS[button][0], TRANSCRIPTS_BUTTONS[button][1].format(custom_text), index) -@step('I click button "([^"]*)"$') -def click_button(_step, button_type): +@step('I click transcript button "([^"]*)"$') +def click_button_transcripts_variant(_step, button_type): world.wait(DELAY) world.wait_for_ajax_complete() button = button_type.strip() - world.css_click(BUTTONS[button][0]) + world.css_click(TRANSCRIPTS_BUTTONS[button][0]) -@step('I click button "([^"]*)" number (\d+)$') +@step('I click transcript button "([^"]*)" number (\d+)$') def click_button_index(_step, button_type, index): world.wait(DELAY) world.wait_for_ajax_complete() @@ -164,7 +164,7 @@ def click_button_index(_step, button_type, index): button = button_type.strip() index = int(index.strip()) - 1 - world.css_click(BUTTONS[button][0], index) + world.css_click(TRANSCRIPTS_BUTTONS[button][0], index) @step('I remove "([^"]+)" transcripts id from store') diff --git a/cms/djangoapps/contentstore/features/video.feature b/cms/djangoapps/contentstore/features/video.feature index 48be51a252..a926302fac 100644 --- a/cms/djangoapps/contentstore/features/video.feature +++ b/cms/djangoapps/contentstore/features/video.feature @@ -77,3 +77,20 @@ Feature: CMS.Video Component Then I focus on caption line with data-index 0 Then I press "enter" button on caption line with data-index 0 And I see caption line with data-index 0 has class "focused" + + # 11 + Scenario: When start end end times are specified, a range on slider is shown + Given I have created a Video component + And Make sure captions are closed + And I edit the component + And I open tab "Advanced" + And I set value "12" to the field "Start Time" + And I set value "24" to the field "End Time" + And I save changes + And I click video button "Play" + + # The below line is a bit flaky. Numbers 73 and 73 were determined rather + # accidentally. They might change in the future as Video player gets CSS + # updates. If this test starts failing, 99.9% cause of failure is the line + # below. + Then I see a range on slider with styles "left" set to 73 px and "width" set to 73 px diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index 4f15bdc9b9..87252a9dc3 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -5,11 +5,15 @@ from xmodule.modulestore import Location from contentstore.utils import get_modulestore from selenium.webdriver.common.keys import Keys -BUTTONS = { +VIDEO_BUTTONS = { 'CC': '.hide-subtitles', 'volume': '.volume', + 'Play': '.video_control.play', } +# We should wait 300 ms for event handler invocation + 200ms for safety. +DELAY = 0.5 + @step('I have created a Video component$') def i_created_a_video_component(step): @@ -131,7 +135,7 @@ def set_captions_visibility_state(_step, captions_state): @step('I hover over button "([^"]*)"$') def hover_over_button(_step, button): - world.css_find(BUTTONS[button.strip()]).mouse_over() + world.css_find(VIDEO_BUTTONS[button.strip()]).mouse_over() @step('Captions (?:are|become) "([^"]*)"$') @@ -173,3 +177,24 @@ def caption_line_has_class(_step, index, className): SELECTOR = ".subtitles > li[data-index='{index}']".format(index=int(index.strip())) world.css_has_class(SELECTOR, className.strip()) + +@step('I see a range on slider with styles "left" set to (.+) px and "width" set to (.+) px$') +def see_a_range_slider_with_proper_range(_step, left, width): + left = int(left.strip()) + width = int(width.strip()) + + world.wait_for_visible(".slider-range") + world.wait(4) + slider_range = world.browser.driver.find_element_by_css_selector(".slider-range") + + assert int(round(float(slider_range.value_of_css_property("left")[:-2]))) == left + assert int(round(float(slider_range.value_of_css_property("width")[:-2]))) == width + + +@step('I click video button "([^"]*)"$') +def click_button_video(_step, button_type): + world.wait(DELAY) + world.wait_for_ajax_complete() + button = button_type.strip() + world.css_click(VIDEO_BUTTONS[button]) + diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index 989ea33a26..fe73fb12ae 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -139,6 +139,24 @@ div.video { box-shadow: inset 0 1px 0 #999; } + div.ui-corner-all.slider-range { + background-color: #1e91d3; + + /* We add opacity so that we can discern the amount of video that has + * been played. The progress will advance as a gray-shaded area. When + * it will overlap with the range, you will see a different shade of + * the range for part that has been played, and for part that is + * still to be played. + * + * For CSS opacity, different browsers are handled differently. + */ + -ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=30)"; + filter: alpha(opacity=30); + -moz-opacity: 0.3; + -khtml-opacity: 0.3; + opacity: 0.3; + } + a.ui-slider-handle { background: $pink url(../images/slider-handle.png) center center no-repeat; background-size: 50%; 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 7803099aa8..16413af317 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js @@ -184,6 +184,62 @@ }); }); + describe('checking start and end times', function () { + var miniTestSuite = [ + { + itDescription: 'both times are proper', + data: {start: 12, end: 24}, + expectData: {start: 12, end: 24} + }, + { + itDescription: 'start time is invalid', + data: {start: '', end: 24}, + expectData: {start: 0, end: 24} + }, + { + itDescription: 'end time is invalid', + data: {start: 12, end: ''}, + expectData: {start: 12, end: null} + }, + { + itDescription: 'start time is less than 0', + data: {start: -12, end: 24}, + expectData: {start: 0, end: 24} + }, + { + itDescription: 'start time is greater than end time', + data: {start: 42, end: 24}, + expectData: {start: 42, end: null} + }, + ]; + + beforeEach(function () { + loadFixtures('video.html'); + + }); + + $.each(miniTestSuite, function (index, test) { + itFabrique(test.itDescription, test.data, test.expectData); + }); + + return; + + function itFabrique(itDescription, data, expectData) { + it(itDescription, function () { + $('#example').find('.video') + .data({ + 'start': data.start, + 'end': data.end + }); + + state = new Video('#example'); + + expect(state.config.start).toBe(expectData.start); + expect(state.config.end).toBe(expectData.end); + }); + } + }); + describe('multiple YT on page', function () { var state1, state2, state3; 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 988d5bdaec..e21faa4a4a 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 @@ -114,7 +114,9 @@ showinfo: 0, enablejsapi: 1, modestbranding: 1, - html5: 1 + html5: 1, + start: 0, + end: null }, videoId: 'cogebirgzzM', events: { 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 67046ccc11..7df3131793 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -75,6 +75,8 @@ function (VideoPlayer) { state.parseYoutubeStreams = _.bind(parseYoutubeStreams, state); state.parseVideoSources = _.bind(parseVideoSources, state); state.getVideoMetadata = _.bind(getVideoMetadata, state); + + state.checkStartEndTimes = _.bind(checkStartEndTimes, state); } // function _renderElements(state) @@ -277,6 +279,10 @@ function (VideoPlayer) { availableQualities: ['hd720', 'hd1080', 'highres'] }; + // Make sure that start end end times are valid. If not, they will be + // set to `null` and will not be used later on. + this.checkStartEndTimes(); + // Check if the YT test timeout has been set. If not, or it is in // improper format, then set to default value. tempYtTestTimeout = parseInt(data['ytTestTimeout'], 10); @@ -360,6 +366,30 @@ function (VideoPlayer) { } } + /* + * function checkStartEndTimes() + * + * Validate config.start and config.end times. + * + * We can check at this time if the times are proper integers, and if they + * make general sense. I.e. if start time is => 0 and <= end time. + * + * An invalid start time will be reset to 0. An invalid end time will be + * set to `null`. It the task for the appropriate player API to figure out + * if start time and/or end time are greater than the length of the video. + */ + function checkStartEndTimes() { + this.config.start = parseInt(this.config.start, 10); + if ((!isFinite(this.config.start)) || (this.config.start < 0)) { + this.config.start = 0; + } + + this.config.end = parseInt(this.config.end, 10); + if ((!isFinite(this.config.end)) || (this.config.end < this.config.start)) { + this.config.end = null; + } + } + // function parseYoutubeStreams(state, youtubeStreams) // // Take a string in the form: diff --git a/common/lib/xmodule/xmodule/js/src/video/02_html5_video.js b/common/lib/xmodule/xmodule/js/src/video/02_html5_video.js index 6cd38b30bd..a82786dab2 100644 --- a/common/lib/xmodule/xmodule/js/src/video/02_html5_video.js +++ b/common/lib/xmodule/xmodule/js/src/video/02_html5_video.js @@ -191,19 +191,8 @@ function () { } // Determine the starting and ending time for the video. - this.start = 0; - this.end = null; - if (config.playerVars) { - this.start = parseFloat(config.playerVars.start); - if ((!isFinite(this.start)) || (this.start < 0)) { - this.start = 0; - } - - this.end = parseFloat(config.playerVars.end); - if ((!isFinite(this.end)) || (this.end < this.start)) { - this.end = null; - } - } + this.start = config.playerVars.start; + this.end = config.playerVars.end; // Create HTML markup for the