diff --git a/cms/djangoapps/contentstore/features/video-editor.feature b/cms/djangoapps/contentstore/features/video-editor.feature index 9f2d44442b..f28ee568dc 100644 --- a/cms/djangoapps/contentstore/features/video-editor.feature +++ b/cms/djangoapps/contentstore/features/video-editor.feature @@ -11,3 +11,13 @@ Feature: Video Component Editor And I edit and select Settings Then I can modify the display name And my display name change is persisted on save + + Scenario: Captions are hidden when "show captions" is false + Given I have created a Video component + And I have set "show captions" to False + Then when I view the video it does not show the captions + + Scenario: Captions are shown when "show captions" is true + Given I have created a Video component + And I have set "show captions" to True + Then when I view the video it does show the captions diff --git a/cms/djangoapps/contentstore/features/video.feature b/cms/djangoapps/contentstore/features/video.feature index b13ce76b52..e4caa70ef6 100644 --- a/cms/djangoapps/contentstore/features/video.feature +++ b/cms/djangoapps/contentstore/features/video.feature @@ -16,15 +16,9 @@ Feature: Video Component Scenario: Captions are shown correctly Given I have created a Video component - And I have shown captions Then when I view the video it does show the captions - Scenario: Captions are hidden when "show captions" is false + Scenario: Captions are toggled correctly Given I have created a Video component - And I have set "show captions" to False - Then when I view the video it does not show the captions - - Scenario: Captions are shown when "show captions" is true - Given I have created a Video component - And I have set "show captions" to True + And I have toggled captions Then when I view the video it does show the captions diff --git a/cms/djangoapps/contentstore/features/video.py b/cms/djangoapps/contentstore/features/video.py index 0b50d0946f..03b63d4137 100644 --- a/cms/djangoapps/contentstore/features/video.py +++ b/cms/djangoapps/contentstore/features/video.py @@ -18,31 +18,33 @@ def video_takes_a_single_click(step): assert(world.is_css_present('.xmodule_VideoModule')) -@step('I have (hidden|shown) captions') +@step('I have (hidden|toggled) captions') def hide_or_show_captions(step, shown): + button_css = 'a.hide-subtitles' if shown == 'hidden': - world.css_click('a.hide-subtitles') + world.css_click(button_css) + if shown == 'toggled': + world.css_click(button_css) + # When we click the first time, a tooltip shows up. We want to + # click the button rather than the tooltip, so move the mouse + # away to make it disappear. + button = world.css_find(button_css) + button.mouse_out() + world.css_click(button_css) @step('when I view the video it (.*) show the captions') -def does_not_show_captions(step, show_captions): +def shows_captions(step, show_captions): # Prevent cookies from overriding course settings world.browser.cookies.delete('hide_captions') if show_captions == 'does not': assert world.css_find('.video')[0].has_class('closed') else: - assert not world.css_find('.video')[0].has_class('closed') - - -# @step('when I view the video it does show the captions') -# def shows_captions(step): -# # Prevent cookies from overriding course settings -# world.browser.cookies.delete('hide_captions') -# assert not world.css_find('.video')[0].has_class('closed') + assert world.is_css_not_present('.video.closed') @step('I have set "show captions" to (.*)') -def set_show_captions_false(step, setting): +def set_show_captions(step, setting): world.css_click('a.edit-button') world.browser.select('Show Captions', setting) world.css_click('a.save-button') diff --git a/common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee b/common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee index de3cdbf0c1..0ba0cc85f8 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee +++ b/common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee @@ -59,7 +59,7 @@ describe 'Video', -> @originalYT = window.YT window.YT = { Player: true } spyOn(window, 'VideoPlayer').andReturn(@stubVideoPlayer) - @video = new Video '#example', @videosDefinition + @video = new Video '#example' afterEach -> window.YT = @originalYT @@ -72,7 +72,7 @@ describe 'Video', -> beforeEach -> @originalYT = window.YT window.YT = {} - @video = new Video '#example', @videosDefinition + @video = new Video '#example' afterEach -> window.YT = @originalYT @@ -85,7 +85,7 @@ describe 'Video', -> @originalYT = window.YT window.YT = {} spyOn(window, 'VideoPlayer').andReturn(@stubVideoPlayer) - @video = new Video '#example', @videosDefinition + @video = new Video '#example' window.onYouTubePlayerAPIReady() afterEach -> @@ -98,7 +98,7 @@ describe 'Video', -> describe 'youtubeId', -> beforeEach -> $.cookie.andReturn '1.0' - @video = new Video '#example', @videosDefinition + @video = new Video '#example' describe 'with speed', -> it 'return the video id for given speed', -> @@ -111,7 +111,7 @@ describe 'Video', -> describe 'setSpeed', -> beforeEach -> - @video = new Video '#example', @videosDefinition + @video = new Video '#example' describe 'when new speed is available', -> beforeEach -> @@ -132,14 +132,14 @@ describe 'Video', -> describe 'getDuration', -> beforeEach -> - @video = new Video '#example', @videosDefinition + @video = new Video '#example' it 'return duration for current video', -> expect(@video.getDuration()).toEqual 200 describe 'log', -> beforeEach -> - @video = new Video '#example', @videosDefinition + @video = new Video '#example' @video.setSpeed '1.0' spyOn Logger, 'log' @video.player = { currentTime: 25 } diff --git a/common/lib/xmodule/xmodule/tests/test_video_module.py b/common/lib/xmodule/xmodule/tests/test_video_module.py index 4902e6d7f8..f8ae54eee1 100644 --- a/common/lib/xmodule/xmodule/tests/test_video_module.py +++ b/common/lib/xmodule/xmodule/tests/test_video_module.py @@ -57,3 +57,20 @@ class VideoDescriptorImportTestCase(unittest.TestCase): self.assertEquals(output.end_time, 0.0) self.assertEquals(output.track, 'http://www.example.com/track') self.assertEquals(output.source, 'http://www.example.com/source.mp4') + + def test_from_xml_no_attributes(self): + ''' + Make sure settings are correct if none are explicitly set in XML. + ''' + module_system = DummySystem(load_error_modules=True) + xml_data = '' + output = VideoDescriptor.from_xml(xml_data, module_system) + self.assertEquals(output.youtube_id_0_75, '') + self.assertEquals(output.youtube_id_1_0, 'OEoXaMPEzfM') + self.assertEquals(output.youtube_id_1_25, '') + self.assertEquals(output.youtube_id_1_5, '') + self.assertEquals(output.show_captions, True) + self.assertEquals(output.start_time, 0.0) + self.assertEquals(output.end_time, 0.0) + self.assertEquals(output.track, '') + self.assertEquals(output.source, '') diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index 222df7417a..8d9a77f207 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -14,8 +14,7 @@ from django.http import Http404 from xmodule.x_module import XModule from xmodule.raw_module import RawDescriptor from xmodule.editing_module import MetadataOnlyEditingDescriptor -from xblock.core import Integer, Scope, String -from fields import StringyFloat, StringyBoolean +from xblock.core import Integer, Scope, String, Float, Boolean log = logging.getLogger(__name__) @@ -23,13 +22,13 @@ log = logging.getLogger(__name__) class VideoFields(object): """Fields for `VideoModule` and `VideoDescriptor`.""" position = Integer(help="Current position in the video", scope=Scope.user_state, default=0) - show_captions = StringyBoolean(help="This controls whether or not captions are shown by default.", display_name="Show Captions", scope=Scope.settings, default=True) - youtube_id_1_0 = String(help="This is the Youtube ID reference for the normal speed video.", display_name="Default Speed", scope=Scope.settings, default="") + show_captions = Boolean(help="This controls whether or not captions are shown by default.", display_name="Show Captions", scope=Scope.settings, default=True) + youtube_id_1_0 = String(help="This is the Youtube ID reference for the normal speed video.", display_name="Default Speed", scope=Scope.settings, default="OEoXaMPEzfM") youtube_id_0_75 = String(help="The Youtube ID for the .75x speed video.", display_name="Speed: .75x", scope=Scope.settings, default="") youtube_id_1_25 = String(help="The Youtube ID for the 1.25x speed video.", display_name="Speed: 1.25x", scope=Scope.settings, default="") youtube_id_1_5 = String(help="The Youtube ID for the 1.5x speed video.", display_name="Speed: 1.5x", scope=Scope.settings, default="") - start_time = StringyFloat(help="Time the video starts", display_name="Start Time", scope=Scope.settings, default=0.0) - end_time = StringyFloat(help="Time the video ends", display_name="End Time", scope=Scope.settings, default=0.0) + start_time = Float(help="Time the video starts", display_name="Start Time", scope=Scope.settings, default=0.0) + end_time = Float(help="Time the video ends", display_name="End Time", scope=Scope.settings, default=0.0) source = String(help="The external URL to download the video. This appears as a link beneath the video.", display_name="Download Video", scope=Scope.settings, default="") track = String(help="The external URL to download the subtitle track. This appears as a link beneath the video.", display_name="Download Track", scope=Scope.settings, default="") @@ -125,7 +124,9 @@ class VideoDescriptor(VideoFields, video.youtube_id_1_25 = speeds['1.25'] if speeds['1.50']: video.youtube_id_1_5 = speeds['1.50'] - video.show_captions = True if xml.get('show_captions') == 'true' else False + show_captions = xml.get('show_captions') + if show_captions: + video.show_captions = json.loads(show_captions) source = _get_first_external(xml, 'source') if source: video.source = source @@ -177,7 +178,7 @@ def _parse_youtube(data): def _parse_time(str_time): """Converts s in '12:34:45' format to seconds. If s is None, returns empty string""" - if str_time is None: + if str_time is None or str_time == '': return '' else: obj_time = time.strptime(str_time, '%H:%M:%S') diff --git a/lms/djangoapps/courseware/tests/test_video_xml.py b/lms/djangoapps/courseware/tests/test_video_xml.py index 57282ed39a..169e5be2d9 100644 --- a/lms/djangoapps/courseware/tests/test_video_xml.py +++ b/lms/djangoapps/courseware/tests/test_video_xml.py @@ -68,13 +68,23 @@ class VideoModuleLogicTest(LogicTest): } def test_parse_time(self): - """Ensure that times are parse correctly into seconds.""" + """Ensure that times are parsed correctly into seconds.""" output = _parse_time('00:04:07') self.assertEqual(output, 247) + def test_parse_time_none(self): + """Check parsing of None.""" + output = _parse_time(None) + self.assertEqual(output, '') + + def test_parse_time_empty(self): + """Check parsing of the empty string.""" + output = _parse_time('') + self.assertEqual(output, '') + def test_parse_youtube(self): """Test parsing old-style Youtube ID strings into a dict.""" - youtube_str = '0.75:jNCf2gIqpeE,1.0:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg' + youtube_str = '0.75:jNCf2gIqpeE,1.00:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg' output = _parse_youtube(youtube_str) self.assertEqual(output, {'0.75': 'jNCf2gIqpeE', '1.00': 'ZwkTiUPN0mg', @@ -92,3 +102,11 @@ class VideoModuleLogicTest(LogicTest): '1.00': '', '1.25': '', '1.50': ''}) + + def test_parse_youtube_key_format(self): + """ + Make sure that inconsistent speed keys are parsed correctly. + """ + youtube_str = '1.00:p2Q6BrNhdh8' + youtube_str_hack = '1.0:p2Q6BrNhdh8' + self.assertEqual(_parse_youtube(youtube_str), _parse_youtube(youtube_str_hack))