Merge pull request #132 from edx/peter-fogg/remove-video-xml

Peter fogg/remove video xml

CHANGELOG.rst

@@ -5,6 +5,9 @@ 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.
 
+Studio: Remove XML from the video component editor. All settings are
+moved to be edited as metadata.
+
 XModule: Only write out assets files if the contents have changed.
 
 XModule: Don't delete generated xmodule asset files when compiling (for

cms/djangoapps/contentstore/features/common.py

@@ -171,6 +171,16 @@ def open_new_unit(step):
     world.css_click('a.new-unit-item')
 
 
+@step('when I view the video it (.*) show the 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 world.is_css_not_present('.video.closed')
+
+
 def type_in_codemirror(index, text):
     world.css_click(".CodeMirror", index=index)
     g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea")

cms/djangoapps/contentstore/features/video-editor.feature

@@ -4,10 +4,20 @@ Feature: Video Component Editor
   Scenario: User can view metadata
     Given I have created a Video component
     And I edit and select Settings
-    Then I see only the Video display name setting
+    Then I see the correct settings and default values
 
   Scenario: User can modify display name
     Given I have created a Video component
     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

cms/djangoapps/contentstore/features/video-editor.py

@@ -4,6 +4,20 @@
 from lettuce import world, step
 
 
-@step('I see only the video display name setting$')
-def i_see_only_the_video_display_name(step):
-    world.verify_all_setting_entries([['Display Name', "default", True]])
+@step('I see the correct settings and default values$')
+def i_see_the_correct_settings_and_values(step):
+    world.verify_all_setting_entries([['Default Speed', 'OEoXaMPEzfM', False],
+                                      ['Display Name', 'default', True],
+                                      ['Download Track', '', False],
+                                      ['Download Video', '', False],
+                                      ['Show Captions', 'True', False],
+                                      ['Speed: .75x', '', False],
+                                      ['Speed: 1.25x', '', False],
+                                      ['Speed: 1.5x', '', False]])
+
+
+@step('I have set "show captions" to (.*)')
+def set_show_captions(step, setting):
+    world.css_click('a.edit-button')
+    world.browser.select('Show Captions', setting)
+    world.css_click('a.save-button')

cms/djangoapps/contentstore/features/video.feature

@@ -9,7 +9,16 @@ Feature: Video Component
     Given I have clicked the new unit button
     Then creating a video takes a single click
 
-  Scenario: Captions are shown correctly
+  Scenario: Captions are hidden correctly
     Given I have created a Video component
     And I have hidden captions
     Then when I view the video it does not show the captions
+
+  Scenario: Captions are shown correctly
+    Given I have created a Video component
+    Then when I view the video it does show the captions
+
+  Scenario: Captions are toggled correctly
+    Given I have created a Video component
+    And I have toggled captions
+    Then when I view the video it does show the captions

cms/djangoapps/contentstore/features/video.py

@@ -18,11 +18,16 @@ def video_takes_a_single_click(_step):
     assert(world.is_css_present('.xmodule_VideoModule'))
 
 
-@step('I have hidden captions')
-def set_show_captions_false(step):
-    world.css_click('a.hide-subtitles')
-
-
-@step('when I view the video it does not show the captions')
-def does_not_show_captions(step):
-    assert world.css_find('.video')[0].has_class('closed')
+@step('I have (hidden|toggled) captions')
+def hide_or_show_captions(step, shown):
+    button_css = 'a.hide-subtitles'
+    if shown == 'hidden':
+        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)

common/lib/xmodule/xmodule/js/fixtures/video.html

@@ -2,7 +2,8 @@
   <div id="video_example">
     <div id="example">
       <div id="video_id" class="video"
-        data-streams="0.75:slowerSpeedYoutubeId,1.0:normalSpeedYoutubeId"
+        data-youtube-id-0-75="slowerSpeedYoutubeId"
+        data-youtube-id-1-0="normalSpeedYoutubeId"
         data-show-captions="true"
         data-start=""
         data-end=""
@@ -18,4 +19,4 @@
       </div>
     </div>
   </div>
-</div>
\ No newline at end of file
+</div>

common/lib/xmodule/xmodule/js/spec/video/display_spec.coffee

@@ -5,7 +5,6 @@ describe 'Video', ->
     loadFixtures 'video.html'
     jasmine.stubRequests()
 
-    @videosDefinition = '0.75:slowerSpeedYoutubeId,1.0:normalSpeedYoutubeId'
     @slowerSpeedYoutubeId = 'slowerSpeedYoutubeId'
     @normalSpeedYoutubeId = 'normalSpeedYoutubeId'
     metadata =
@@ -30,7 +29,7 @@ describe 'Video', ->
       beforeEach ->
         spyOn(window.Video.prototype, 'fetchMetadata').andCallFake ->
           @metadata = metadata
-        @video = new Video '#example', @videosDefinition
+        @video = new Video '#example'
       it 'reset the current video player', ->
         expect(window.player).toBeNull()
 
@@ -60,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
@@ -73,7 +72,7 @@ describe 'Video', ->
       beforeEach ->
         @originalYT = window.YT
         window.YT = {}
-        @video = new Video '#example', @videosDefinition
+        @video = new Video '#example'
 
       afterEach ->
         window.YT = @originalYT
@@ -86,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 ->
@@ -99,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', ->
@@ -112,7 +111,7 @@ describe 'Video', ->
 
   describe 'setSpeed', ->
     beforeEach ->
-      @video = new Video '#example', @videosDefinition
+      @video = new Video '#example'
 
     describe 'when new speed is available', ->
       beforeEach ->
@@ -133,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 }

common/lib/xmodule/xmodule/js/src/video/display.coffee

@@ -8,7 +8,7 @@ class @Video
     @show_captions = @el.data('show-captions')
     window.player = null
     @el = $("#video_#{@id}")
-    @parseVideos @el.data('streams')
+    @parseVideos()
     @fetchMetadata()
     @parseSpeed()
     $("#video_#{@id}").data('video', this).addClass('video-load-complete')
@@ -27,10 +27,14 @@ class @Video
 
   parseVideos: (videos) ->
     @videos = {}
-    $.each videos.split(/,/), (index, video) =>
-      video = video.split(/:/)
-      speed = parseFloat(video[0]).toFixed(2).replace /\.00$/, '.0'
-      @videos[speed] = video[1]
+    if @el.data('youtube-id-0-75')
+      @videos['0.75'] = @el.data('youtube-id-0-75')
+    if @el.data('youtube-id-1-0')
+      @videos['1.0'] = @el.data('youtube-id-1-0')
+    if @el.data('youtube-id-1-25')
+      @videos['1.25'] = @el.data('youtube-id-1-25')
+    if @el.data('youtube-id-1-5')
+      @videos['1.50'] = @el.data('youtube-id-1-5')
 
   parseSpeed: ->
     @setSpeed($.cookie('video_speed'))

common/lib/xmodule/xmodule/templates/video/default.yaml

 ---
 metadata:
     display_name: default
-    data_dir: a_made_up_name
-data: |
-  <video  youtube="0.75:JMD_ifUUfsU,1.0:OEoXaMPEzfM,1.25:AKqURZnYqpk,1.50:DYpADpL7jAY"/>
+data: ""
 children: []

common/lib/xmodule/xmodule/tests/test_import.py

@@ -336,8 +336,8 @@ class ImportTestCase(BaseCourseTestCase):
         location = Location(["i4x", "edX", "toy", "video", "Welcome"])
         toy_video = modulestore.get_instance(toy_id, location)
         two_toy_video = modulestore.get_instance(two_toy_id, location)
-        self.assertEqual(etree.fromstring(toy_video.data).get('youtube'), "1.0:p2Q6BrNhdh8")
-        self.assertEqual(etree.fromstring(two_toy_video.data).get('youtube'), "1.0:p2Q6BrNhdh9")
+        self.assertEqual(toy_video.youtube_id_1_0, "p2Q6BrNhdh8")
+        self.assertEqual(two_toy_video.youtube_id_1_0, "p2Q6BrNhdh9")
 
     def test_colon_in_url_name(self):
         """Ensure that colons in url_names convert to file paths properly"""

common/lib/xmodule/xmodule/tests/test_video_module.py

+# -*- coding: utf-8 -*-
+import unittest
+
+from xmodule.video_module import VideoDescriptor
+from .test_import import DummySystem
+
+
+class VideoDescriptorImportTestCase(unittest.TestCase):
+    """
+    Make sure that VideoDescriptor can import an old XML-based video correctly.
+    """
+
+    def test_from_xml(self):
+        module_system = DummySystem(load_error_modules=True)
+        xml_data = '''
+            <video display_name="Test Video"
+                   youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8"
+                   show_captions="false"
+                   from="00:00:01"
+                   to="00:01:00">
+              <source src="http://www.example.com/source.mp4"/>
+              <track src="http://www.example.com/track"/>
+            </video>
+        '''
+        output = VideoDescriptor.from_xml(xml_data, module_system)
+        self.assertEquals(output.youtube_id_0_75, 'izygArpw-Qo')
+        self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8')
+        self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA')
+        self.assertEquals(output.youtube_id_1_5, 'rABDYkeK0x8')
+        self.assertEquals(output.show_captions, False)
+        self.assertEquals(output.start_time, 1.0)
+        self.assertEquals(output.end_time, 60)
+        self.assertEquals(output.track, 'http://www.example.com/track')
+        self.assertEquals(output.source, 'http://www.example.com/source.mp4')
+
+    def test_from_xml_missing_attributes(self):
+        """
+        Ensure that attributes have the right values if they aren't
+        explicitly set in XML.
+        """
+        module_system = DummySystem(load_error_modules=True)
+        xml_data = '''
+            <video display_name="Test Video"
+                   youtube="1.0:p2Q6BrNhdh8,1.25:1EeWXzPdhSA"
+                   show_captions="true">
+              <source src="http://www.example.com/source.mp4"/>
+              <track src="http://www.example.com/track"/>
+            </video>
+        '''
+        output = VideoDescriptor.from_xml(xml_data, module_system)
+        self.assertEquals(output.youtube_id_0_75, '')
+        self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8')
+        self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA')
+        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, '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 = '<video></video>'
+        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, '')

common/lib/xmodule/xmodule/tests/test_video_xml.py

@@ -18,7 +18,7 @@ import unittest
 from mock import Mock
 from lxml import etree
 
-from xmodule.video_module import VideoDescriptor, VideoModule
+from xmodule.video_module import VideoDescriptor, VideoModule, _parse_time, _parse_youtube
 from xmodule.modulestore import Location
 from xmodule.tests import get_test_system
 from xmodule.tests.test_logic import LogicTest
@@ -49,7 +49,7 @@ class VideoFactory(object):
                              "SampleProblem1"])
         model_data = {'data': VideoFactory.sample_problem_xml_youtube, 'location': location}
 
-        descriptor = Mock(weight="1")
+        descriptor = Mock(weight="1", url_name="SampleProblem1")
 
         system = get_test_system()
         system.render_template = lambda template, context: context
@@ -67,69 +67,57 @@ class VideoModuleLogicTest(LogicTest):
         'data': '<video />'
     }
 
-    def test_get_timeframe_no_parameters(self):
-        """Make sure that timeframe() works correctly w/o parameters"""
-        xmltree = etree.fromstring('<video>test</video>')
-        output = self.xmodule.get_timeframe(xmltree)
-        self.assertEqual(output, ('', ''))
-
-    def test_get_timeframe_with_one_parameter(self):
-        """Make sure that timeframe() works correctly with one parameter"""
-        xmltree = etree.fromstring(
-            '<video from="00:04:07">test</video>'
-        )
-        output = self.xmodule.get_timeframe(xmltree)
-        self.assertEqual(output, (247, ''))
-
-    def test_get_timeframe_with_two_parameters(self):
-        """Make sure that timeframe() works correctly with two parameters"""
-        xmltree = etree.fromstring(
-            '''<video
-                    from="00:04:07"
-                    to="13:04:39"
-                >test</video>'''
-        )
-        output = self.xmodule.get_timeframe(xmltree)
-        self.assertEqual(output, (247, 47079))
-
-
-class VideoModuleUnitTest(unittest.TestCase):
-    """Unit tests for Video Xmodule."""
-
-    def test_video_constructor(self):
-        """Make sure that all parameters extracted correclty from xml"""
-        module = VideoFactory.create()
-
-        # `get_html` return only context, cause we
-        # overwrite `system.render_template`
-        context = module.get_html()
-        expected_context = {
-            'track': None,
-            'show_captions': 'true',
-            'display_name': 'SampleProblem1',
-            'id': module.location.html_id(),
-            'end': 3610.0,
-            'caption_asset_path': '/static/subs/',
-            'source': '.../mit-3091x/M-3091X-FA12-L21-3_100.mp4',
-            'streams': '0.75:jNCf2gIqpeE,1.0:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg',
-            'normal_speed_video_id': 'ZwkTiUPN0mg',
-            'position': 0,
-            'start': 3603.0
-        }
-        self.assertDictEqual(context, expected_context)
-
-        self.assertEqual(
-            module.youtube,
-            '0.75:jNCf2gIqpeE,1.0:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg')
-
-        self.assertEqual(
-            module.video_list(),
-            module.youtube)
-
-        self.assertEqual(
-            module.position,
-            0)
-
-        self.assertDictEqual(
-            json.loads(module.get_instance_state()),
-            {'position': 0})
+    def test_parse_time(self):
+        """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.00:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg'
+        output = _parse_youtube(youtube_str)
+        self.assertEqual(output, {'0.75': 'jNCf2gIqpeE',
+                                  '1.00': 'ZwkTiUPN0mg',
+                                  '1.25': 'rsq9auxASqI',
+                                  '1.50': 'kMyNdzVHHgg'})
+
+    def test_parse_youtube_one_video(self):
+        """
+        Ensure that all keys are present and missing speeds map to the
+        empty string.
+        """
+        youtube_str = '0.75:jNCf2gIqpeE'
+        output = _parse_youtube(youtube_str)
+        self.assertEqual(output, {'0.75': 'jNCf2gIqpeE',
+                                  '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))
+
+    def test_parse_youtube_empty(self):
+        """
+        Some courses have empty youtube attributes, so we should handle
+        that well.
+        """
+        self.assertEqual(_parse_youtube(''),
+                         {'0.75': '',
+                          '1.00': '',
+                          '1.25': '',
+                          '1.50': ''})

common/lib/xmodule/xmodule/video_module.py

@@ -6,23 +6,31 @@ import logging
 
 from lxml import etree
 from pkg_resources import resource_string, resource_listdir
+import datetime
+import time
 
 from django.http import Http404
 
 from xmodule.x_module import XModule
 from xmodule.raw_module import RawDescriptor
-from xblock.core import Integer, Scope, String
-
-import datetime
-import time
+from xmodule.editing_module import MetadataOnlyEditingDescriptor
+from xblock.core import Integer, Scope, String, Float, Boolean
 
 log = logging.getLogger(__name__)
 
 
 class VideoFields(object):
     """Fields for `VideoModule` and `VideoDescriptor`."""
-    data = String(help="XML data for the problem", scope=Scope.content)
     position = Integer(help="Current position in the video", scope=Scope.user_state, default=0)
+    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 = 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="")
 
 
 class VideoModule(VideoFields, XModule):
@@ -46,54 +54,6 @@ class VideoModule(VideoFields, XModule):
     def __init__(self, *args, **kwargs):
         XModule.__init__(self, *args, **kwargs)
 
-        xmltree = etree.fromstring(self.data)
-        self.youtube = xmltree.get('youtube')
-        self.show_captions = xmltree.get('show_captions', 'true')
-        self.source = self._get_source(xmltree)
-        self.track = self._get_track(xmltree)
-        self.start_time, self.end_time = self.get_timeframe(xmltree)
-
-    def _get_source(self, xmltree):
-        """Find the first valid source."""
-        return self._get_first_external(xmltree, 'source')
-
-    def _get_track(self, xmltree):
-        """Find the first valid track."""
-        return self._get_first_external(xmltree, 'track')
-
-    def _get_first_external(self, xmltree, tag):
-        """
-        Will return the first valid element
-        of the given tag.
-        'valid' means has a non-empty 'src' attribute
-        """
-        result = None
-        for element in xmltree.findall(tag):
-            src = element.get('src')
-            if src:
-                result = src
-                break
-        return result
-
-    def get_timeframe(self, xmltree):
-        """ Converts 'from' and 'to' parameters in video tag to seconds.
-        If there are no parameters, returns empty string. """
-
-        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:
-                return ''
-            else:
-                obj_time = time.strptime(str_time, '%H:%M:%S')
-                return datetime.timedelta(
-                    hours=obj_time.tm_hour,
-                    minutes=obj_time.tm_min,
-                    seconds=obj_time.tm_sec
-                ).total_seconds()
-
-        return parse_time(xmltree.get('from')), parse_time(xmltree.get('to'))
-
     def handle_ajax(self, dispatch, get):
         """This is not being called right now and we raise 404 error."""
         log.debug(u"GET {0}".format(get))
@@ -104,38 +64,135 @@ class VideoModule(VideoFields, XModule):
         """Return information about state (position)."""
         return json.dumps({'position': self.position})
 
-    def video_list(self):
-        """Return video list."""
-        return self.youtube
-
     def get_html(self):
-        # We normally let JS parse this, but in the case that we need a hacked
-        # out <object> player because YouTube has broken their <iframe> API for
-        # the third time in a year, we need to extract it server side.
-        normal_speed_video_id = None  # The 1.0 speed video
-
-        # video_list() example:
-        #   "0.75:nugHYNiD3fI,1.0:7m8pab1MfYY,1.25:3CxdPGXShq8,1.50:F-D7bOFCnXA"
-        for video_id_str in self.video_list().split(","):
-            if video_id_str.startswith("1.0:"):
-                normal_speed_video_id = video_id_str.split(":")[1]
-
         return self.system.render_template('video.html', {
-            'streams': self.video_list(),
+            'youtube_id_0_75': self.youtube_id_0_75,
+            'youtube_id_1_0': self.youtube_id_1_0,
+            'youtube_id_1_25': self.youtube_id_1_25,
+            'youtube_id_1_5': self.youtube_id_1_5,
             'id': self.location.html_id(),
             'position': self.position,
             'source': self.source,
             'track': self.track,
             'display_name': self.display_name_with_default,
             'caption_asset_path': "/static/subs/",
-            'show_captions': self.show_captions,
+            'show_captions': 'true' if self.show_captions else 'false',
             'start': self.start_time,
-            'end': self.end_time,
-            'normal_speed_video_id': normal_speed_video_id
+            'end': self.end_time
         })
 
 
-class VideoDescriptor(VideoFields, RawDescriptor):
-    """Descriptor for `VideoModule`."""
+class VideoDescriptor(VideoFields,
+                      MetadataOnlyEditingDescriptor,
+                      RawDescriptor):
     module_class = VideoModule
     template_dir_name = "video"
+
+    @property
+    def non_editable_metadata_fields(self):
+        non_editable_fields = super(MetadataOnlyEditingDescriptor, self).non_editable_metadata_fields
+        non_editable_fields.extend([VideoModule.start_time,
+                                    VideoModule.end_time])
+        return non_editable_fields
+
+    @classmethod
+    def from_xml(cls, xml_data, system, org=None, course=None):
+        """
+        Creates an instance of this descriptor from the supplied xml_data.
+        This may be overridden by subclasses
+
+        xml_data: A string of xml that will be translated into data and children for
+            this module
+        system: A DescriptorSystem for interacting with external resources
+        org and course are optional strings that will be used in the generated modules
+            url identifiers
+        """
+        video = super(VideoDescriptor, cls).from_xml(xml_data, system, org, course)
+        xml = etree.fromstring(xml_data)
+
+        display_name = xml.get('display_name')
+        if display_name:
+            video.display_name = display_name
+
+        youtube = xml.get('youtube')
+        if youtube:
+            speeds = _parse_youtube(youtube)
+            if speeds['0.75']:
+                video.youtube_id_0_75 = speeds['0.75']
+            if speeds['1.00']:
+                video.youtube_id_1_0 = speeds['1.00']
+            if speeds['1.25']:
+                video.youtube_id_1_25 = speeds['1.25']
+            if speeds['1.50']:
+                video.youtube_id_1_5 = speeds['1.50']
+
+        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
+
+        track = _get_first_external(xml, 'track')
+        if track:
+            video.track = track
+
+        start_time = _parse_time(xml.get('from'))
+        if start_time:
+            video.start_time = start_time
+
+        end_time = _parse_time(xml.get('to'))
+        if end_time:
+            video.end_time = end_time
+
+        return video
+
+
+def _get_first_external(xmltree, tag):
+    """
+    Returns the src attribute of the nested `tag` in `xmltree`, if it
+    exists.
+    """
+    for element in xmltree.findall(tag):
+        src = element.get('src')
+        if src:
+            return src
+    return None
+
+
+def _parse_youtube(data):
+    """
+    Parses a string of Youtube IDs such as "1.0:AXdE34_U,1.5:VO3SxfeD"
+    into a dictionary. Necessary for backwards compatibility with
+    XML-based courses.
+    """
+    ret = {'0.75': '', '1.00': '', '1.25': '', '1.50': ''}
+    if data == '':
+        return ret
+    videos = data.split(',')
+    for video in videos:
+        pieces = video.split(':')
+        # HACK
+        # To elaborate somewhat: in many LMS tests, the keys for
+        # Youtube IDs are inconsistent. Sometimes a particular
+        # speed isn't present, and formatting is also inconsistent
+        # ('1.0' versus '1.00'). So it's necessary to either do
+        # something like this or update all the tests to work
+        # properly.
+        ret['%.2f' % float(pieces[0])] = pieces[1]
+    return ret
+
+
+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 or str_time == '':
+        return ''
+    else:
+        obj_time = time.strptime(str_time, '%H:%M:%S')
+        return datetime.timedelta(
+            hours=obj_time.tm_hour,
+            minutes=obj_time.tm_min,
+            seconds=obj_time.tm_sec
+        ).total_seconds()

common/test/data/full/sequential/Administrivia_and_Circuit_Elements.xml

@@ -16,13 +16,16 @@
     <vertical slug="vertical_94" graceperiod="1 day 12 hours 59 minutes 59 seconds"
         showanswer="attempted" rerandomize="never">
         <video
-            youtube="0.75:XNh13VZhThQ,1.0:XbDRmF6J0K0,1.25:JDty12WEQWk,1.50:wELKGj-5iyM"
-            slug="What_s_next" name="What's next" />
+            youtube_id_0_75="&quot;XNh13VZhThQ&quot;"
+            youtube_id_1_0="&quot;XbDRmF6J0K0&quot;"
+            youtube_id_1_25="&quot;JDty12WEQWk&quot;"
+            youtube_id_1_5="&quot;wELKGj-5iyM&quot;"
+            slug="What_s_next"
+            name="What's next"/>
         <html slug="html_95">Minor correction: Six elements (five resistors)…
         </html>
         <customtag tag="S1" slug="discuss_96" impl="discuss" />
     </vertical>
-
     <randomize url_name="PS1_Q4" display_name="Problem 4: Read a Molecule">
         <vertical>
             <html slug="html_900">

common/test/data/full/vertical/vertical_58.xml

 <sequential>
-  <video youtube="1.50:8kARlsUt9lM,1.25:4cLA-IME32w,1.0:pFOrD8k9_p4,0.75:CcgAYu0n0bg" slug="S1V9_Demo_Setup_-_Lumped_Elements" name="S1V9: Demo Setup - Lumped Elements"/>
+  <video youtube_id_1_5="&quot;8kARlsUt9lM&quot;" youtube_id_1_25="&quot;4cLA-IME32w&quot;" youtube_id_1_0="&quot;pFOrD8k9_p4&quot;" youtube_id_0_75="&quot;CcgAYu0n0bg&quot;" slug="S1V9_Demo_Setup_-_Lumped_Elements" name="S1V9: Demo Setup - Lumped Elements"/>
   <customtag tag="S1" slug="discuss_59" impl="discuss"/>
   <customtag page="29" slug="book_60" impl="book"/>
   <customtag lecnum="1" slug="slides_61" impl="slides"/>

common/test/data/full/vertical/vertical_89.xml

@@ -3,7 +3,7 @@
     <!-- UTF-8 characters are acceptable… HTML entities are not -->
     <h1>Inline content…</h1>
   </html>
-  <video youtube="1.50:vl9xrfxcr38,1.25:qxNX4REGqx4,1.0:BGU1poJDgOY,0.75:8rK9vnpystQ" slug="S1V14_Summary" name="S1V14: Summary"/>
+  <video youtube_id_1_5="&quot;vl9xrfxcr38&quot;" youtube_id_1_25="&quot;qxNX4REGqx4&quot;" youtube_id_1_0="&quot;BGU1poJDgOY&quot;" youtube_id_0_75="&quot;8rK9vnpystQ&quot;" slug="S1V14_Summary" name="S1V14: Summary"/>
   <customtag tag="S1" slug="discuss_91" impl="discuss"/>
   <customtag page="70" slug="book_92" impl="book"/>
   <customtag lecnum="1" slug="slides_93" impl="slides"/>

common/test/data/full/vertical/vertical_94.xml

 <sequential>
-  <video youtube="0.75:3NIegrCmA5k,1.0:eLAyO33baQ8,1.25:m1zWi_sh4Aw,1.50:EG-fRTJln_E" slug="S2V1_Review_KVL_KCL" name="S2V1: Review KVL, KCL"/>
+  <video youtube_id_0_75="&quot;3NIegrCmA5k&quot;" youtube_id_1_0="&quot;eLAyO33baQ8&quot;" youtube_id_1_25="&quot;m1zWi_sh4Aw&quot;" youtube_id_1_5="&quot;EG-fRTJln_E&quot;" slug="S2V1_Review_KVL_KCL" name="S2V1: Review KVL, KCL"/>
   <customtag tag="S2" slug="discuss_95" impl="discuss"/>
   <customtag page="54" slug="book_96" impl="book"/>
   <customtag lecnum="2" slug="slides_97" impl="slides"/>

common/test/data/full/vertical/vertical_98.xml

 <sequential>
-  <video youtube="0.75:S_1NaY5te8Q,1.0:G_2F9wivspM,1.25:b-r7dISY-Uc,1.50:jjxHom0oXWk" slug="S2V2_Demo-_KVL_KCL" name="S2V2: Demo- KVL, KCL"/>
+  <video youtube_id_0_75="&quot;S_1NaY5te8Q&quot;" youtube_id_1_0="&quot;G_2F9wivspM&quot;" youtube_id_1_25="&quot;b-r7dISY-Uc&quot;" youtube_id_1_5="&quot;jjxHom0oXWk&quot;" slug="S2V2_Demo-_KVL_KCL" name="S2V2: Demo- KVL, KCL"/>
   <customtag tag="S2" slug="discuss_99" impl="discuss"/>
   <customtag page="56" slug="book_100" impl="book"/>
   <customtag lecnum="2" slug="slides_101" impl="slides"/>

common/test/data/full/video/welcome.xml

-<video youtube="0.75:izygArpw-Qo,1.0:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" format="Video" display_name="Welcome…"/>
+<video youtube_id_0_75="&quot;izygArpw-Qo&quot;" youtube_id_1_0="&quot;p2Q6BrNhdh8&quot;" youtube_id_1_25="&quot;1EeWXzPdhSA&quot;" youtube_id_1_5="&quot;rABDYkeK0x8&quot;" format="Video" display_name="Welcome…"/>