BLD-1049: Allow the video player to work with redirected links.

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: Fix bug with incorrect link format and redirection. BLD-1049
+
 Blades: Fix bug with incorrect RelativeTime value after XML serialization. BLD-1060
 
 LMS: Update bulk email implementation to lessen load on the database

cms/djangoapps/contentstore/features/transcripts.feature

@@ -34,14 +34,22 @@ Feature: CMS Transcripts
         And I expect inputs are enabled
 
         #User input URL with incorrect format
-        And I enter a "htt://link.c" source to field number 1
+        And I enter a "http://link.c" source to field number 1
         Then I see error message "url_format"
         # Currently we are working with 1st field. It means, that if 1st field
         # contain incorrect value, 2nd and 3rd fields should be disabled until
         # 1st field will be filled by correct correct value
         And I expect 2, 3 inputs are disabled
-        # We are not clearing fields here,
-        # Because we changing same field.
+
+        #User input URL with incorrect format
+        And I enter a "http://goo.gl/pxxZrg" source to field number 1
+        And I enter a "http://goo.gl/pxxZrg" source to field number 2
+        Then I see error message "links_duplication"
+        And I expect 1, 3 inputs are disabled
+
+        And I clear fields
+        And I expect inputs are enabled
+
         And I enter a "http://youtu.be/t_not_exist" source to field number 1
         Then I do not see error message
         And I expect inputs are enabled
@@ -699,3 +707,37 @@ Feature: CMS Transcripts
 
         And I edit the component
         Then I see status message "found"
+
+    #37 Uploading subtitles with different file name than file
+    Scenario: Shortened link: File name and name of subs are different
+        Given I have created a Video component
+        And I edit the component
+
+        And I enter a "http://goo.gl/pxxZrg" source to field number 1
+        And I see status message "not found"
+        And I upload the transcripts file "uk_transcripts.srt"
+        Then I see status message "uploaded_successfully"
+        And I see value "pxxZrg" in the field "Default Timed Transcript"
+
+        And I save changes
+        Then when I view the video it does show the captions
+
+        And I edit the component
+        Then I see status message "found"
+
+    #38 Uploading subtitles with different file name than file
+    Scenario: Relative link: File name and name of subs are different
+        Given I have created a Video component
+        And I edit the component
+
+        And I enter a "/gizmo.webm" source to field number 1
+        And I see status message "not found"
+        And I upload the transcripts file "uk_transcripts.srt"
+        Then I see status message "uploaded_successfully"
+        And I see value "gizmo" in the field "Default Timed Transcript"
+
+        And I save changes
+        Then when I view the video it does show the captions
+
+        And I edit the component
+        Then I see status message "found"

cms/djangoapps/contentstore/features/transcripts.py

@@ -19,6 +19,7 @@ DELAY = 0.5
 ERROR_MESSAGES = {
     'url_format': u'Incorrect url format.',
     'file_type': u'Link types should be unique.',
+    'links_duplication': u'Links should be unique.',
 }
 
 STATUSES = {
@@ -118,8 +119,7 @@ def i_see_status_message(_step, status):
     assert world.css_has_text(SELECTORS['status_bar'], STATUSES[status])
 
     DOWNLOAD_BUTTON = TRANSCRIPTS_BUTTONS["download_to_edit"][0]
-    if world.is_css_present(DOWNLOAD_BUTTON, wait_time=1) \
-    and not world.css_find(DOWNLOAD_BUTTON)[0].has_class('is-disabled'):
+    if world.is_css_present(DOWNLOAD_BUTTON, wait_time=1) and not world.css_find(DOWNLOAD_BUTTON)[0].has_class('is-disabled'):
         assert _transcripts_are_downloaded()
 
 
@@ -210,7 +210,7 @@ def check_text_in_the_captions(_step, text):
 @step('I see value "([^"]*)" in the field "([^"]*)"$')
 def check_transcripts_field(_step, values, field_name):
     world.select_editor_tab('Advanced')
-    tab = world.css_find('#settings-tab').first;
+    tab = world.css_find('#settings-tab').first
     field_id = '#' + tab.find_by_xpath('.//label[text()="%s"]' % field_name.strip())[0]['for']
     values_list = [i.strip() == world.css_value(field_id) for i in values.split('|')]
     assert any(values_list)
@@ -229,7 +229,7 @@ def open_tab(_step, tab_name):
 
 @step('I set value "([^"]*)" to the field "([^"]*)"$')
 def set_value_transcripts_field(_step, value, field_name):
-    tab = world.css_find('#settings-tab').first;
+    tab = world.css_find('#settings-tab').first
     XPATH = './/label[text()="{name}"]'.format(name=field_name)
     SELECTOR = '#' + tab.find_by_xpath(XPATH)[0]['for']
     element = world.css_find(SELECTOR).first
@@ -241,7 +241,7 @@ def set_value_transcripts_field(_step, value, field_name):
         world.browser.execute_script(SCRIPT)
         assert world.css_has_value(SELECTOR, value)
     else:
-        assert False, 'Incorrect element type.';
+        assert False, 'Incorrect element type.'
     world.wait_for_ajax_complete()
 
 

cms/static/js/spec/video/transcripts/utils_spec.js

@@ -26,7 +26,7 @@ describe('Transcripts.Utils', function () {
         } (videoId)),
         html5FileName = 'file_name',
         html5LinksList =  (function (videoName) {
-            var videoTypes = ['mp4', 'webm'],
+            var videoTypes = ['mp4', 'webm', 'm4v', 'ogv'],
                 links = [
                     'http://somelink.com/%s.%s?param=1&param=2#hash',
                     'http://somelink.com/%s.%s#hash',
@@ -34,6 +34,7 @@ describe('Transcripts.Utils', function () {
                     'http://somelink.com/%s.%s',
                     'ftp://somelink.com/%s.%s',
                     'https://somelink.com/%s.%s',
+                    'https://somelink.com/sub/sub/%s.%s',
                     'http://cdn.somecdn.net/v/%s.%s',
                     'somelink.com/%s.%s',
                      '%s.%s'
@@ -48,7 +49,25 @@ describe('Transcripts.Utils', function () {
 
             return data;
 
-        } (html5FileName));
+        } (html5FileName)),
+        otherLinkId = 'other_link_id',
+        otherLinksList =  (function (linkId) {
+            var links = [
+                    'http://goo.gl/%s?param=1&param=2#hash',
+                    'http://goo.gl/%s?param=1&param=2',
+                    'http://goo.gl/%s#hash',
+                    'http://goo.gl/%s',
+                    'http://goo.gl/%s',
+                    'ftp://goo.gl/%s',
+                    'https://goo.gl/%s',
+                     '%s'
+                ];
+
+            return $.map(links, function (link) {
+                return _str.sprintf(link, linkId);
+            });
+
+        } (otherLinkId));
 
     describe('Method: getField', function (){
         var collection,
@@ -107,7 +126,6 @@ describe('Transcripts.Utils', function () {
         });
 
         describe('Wrong arguments ', function () {
-
             beforeEach(function(){
                 spyOn(console, 'log');
             });
@@ -124,18 +142,9 @@ describe('Transcripts.Utils', function () {
                 expect(result).toBeUndefined();
             });
 
-            it('videoId is wrong', function () {
-                var videoId = 'wrong_id',
-                    link = 'http://youtu.be/' + videoId,
-                    result = Utils.parseYoutubeLink(link);
-
-                expect(result).toBeUndefined();
-            });
-
             var wrongUrls = [
-                'http://youtu.bee/' + videoId,
                 'http://youtu.be/',
-                'example.com',
+                '/static/example',
                 'http://google.com/somevideo.mp4'
             ];
 
@@ -163,10 +172,20 @@ describe('Transcripts.Utils', function () {
                     });
                 });
             });
+
+            $.each(otherLinksList, function (index, link) {
+                it(link, function () {
+                    var result = Utils.parseHTML5Link(link);
+
+                    expect(result).toEqual({
+                        video: otherLinkId,
+                        type: 'other'
+                    });
+                });
+            });
         });
 
         describe('Wrong arguments ', function () {
-
             beforeEach(function(){
                 spyOn(console, 'log');
             });
@@ -184,15 +203,11 @@ describe('Transcripts.Utils', function () {
             });
 
             var html5WrongUrls = [
-                'http://youtu.bee/' + videoId,
                 'http://youtu.be/',
-                'example.com',
-                'http://google.com/somevideo.mp1',
-                'http://google.com/somevideomp4',
-                'http://google.com/somevideo_mp4',
-                'http://google.com/somevideo:mp4',
-                'http://google.com/somevideo',
-                'http://google.com/somevideo.webm_'
+                'http://example.com/.mp4',
+                'http://example.com/video_name.',
+                'http://example.com/',
+                'http://example.com'
             ];
 
             $.each(html5WrongUrls, function (index, link) {
@@ -248,6 +263,13 @@ describe('Transcripts.Utils', function () {
         });
 
         describe('Wrong arguments ', function () {
+            it('youtube videoId is wrong', function () {
+                var videoId = 'wrong_id',
+                    link = 'http://youtu.be/' + videoId,
+                    result = Utils.parseLink(link);
+
+                expect(result).toEqual({ mode : 'incorrect' });
+            });
 
             it('no arguments', function () {
                 var result = Utils.parseLink();

cms/static/js/views/video/transcripts/utils.js

@@ -13,7 +13,6 @@ return (function () {
      */
     Storage.set = function (data_id, data) {
         Storage[data_id] = data;
-
         return this;
     };
 
@@ -24,11 +23,9 @@ return (function () {
      * @return {Any} Stored data.
      */
     Storage.get= function (data_id) {
-
         return Storage[data_id];
     };
 
-
     /**
      * Deletes data from the Storage object by identifier.
      * @function
@@ -36,7 +33,6 @@ return (function () {
      * @return {Boolean} Boolean value that indicate if data is removed.
      */
     Storage.remove = function (data_id) {
-
         return (delete Storage[data_id]);
     };
 
@@ -83,11 +79,10 @@ return (function () {
      */
     var _youtubeParser = (function () {
         var cache = {},
-            regExp = /(?:http|https|)(?:\:\/\/|)(?:www.|)(?:youtu\.be\/|youtube\.com(?:\/embed\/|\/v\/|\/watch\?v=|\/ytscreeningroom\?v=|\/feeds\/api\/videos\/|\/user\S*[^\w\-\s]|\S*[^\w\-\s]))([\w\-]{11})[a-z0-9;:@#?&%=+\/\$_.-]*/i;
+            regExp = /(?:http|https|)(?:\:\/\/|)(?:www.|)(?:youtu\.be\/|youtube\.com(?:\/embed\/|\/v\/|\/watch\?v=|\/ytscreeningroom\?v=|\/feeds\/api\/videos\/|\/user\S*[^\w\-\s]|\S*[^\w\-\s]))([\w\-]+)/i;
 
         return function (url) {
             if (typeof url !== 'string') {
-
                 return void(0);
             }
 
@@ -96,9 +91,7 @@ return (function () {
             }
 
             var match = url.match(regExp);
-            cache[url] = (match && match[1].length === 11) ?
-                match[1] :
-                void(0);
+            cache[url] = (match) ? match[1] : void(0);
 
             return cache[url];
         };
@@ -109,9 +102,9 @@ return (function () {
      * @function
      * @param {String} url Url that should be parsed.
      * @return {
-    *    object:        Object with information about the video
+     *    Object:        Object with information about the video
      *                   (file name, video type),
-    *    undefined:     when url has incorrect format or argument is
+     *    Undefined:     when url has incorrect format or argument is
      *                   non-string.
      * }
      */
@@ -132,24 +125,26 @@ return (function () {
                 match;
 
             link.href = url;
-            match = link.pathname
-                        .split('/')
-                        .pop()
-                        .match(/(.+)\.(mp?4v?|webm)$/);
-
+            // The regular expression try catches file name and file extension.
+            // '[scheme://hostname/pathname/]filename.extension[?query#hash]'
+            match = link.pathname.match(/\/{1}([^\/]+)\.([^\/]+)$/);
             if (match) {
                 cache[url] = {
                     video: match[1],
                     type: match[2]
                 };
-            } /*else {
+            } else {
+                // Links like http://goo.gl/pxxZrg
+                // The regular expression try catches file name.
+                // '[scheme://hostname/pathname/]filename[?query#hash]'
+                match = link.pathname.match(/\/{1}([^\/\.]+)$/);
+                if (match) {
                     cache[url] = {
-                    video: link.pathname
-                            .split('/')
-                            .pop(),
+                        video: match[1],
                         type: 'other'
                     };
-            }*/
+                }
+            }
 
             return cache[url];
         };
@@ -164,25 +159,31 @@ return (function () {
      *                   {
      *                       mode: "youtube|html5|incorrect",
      *                       video: "file_name|youtube_id",
-    *                       type: "youtube|mp4|webm"
+     *                       type: "youtube|mp4|webm|other"
      *                   },
      *    undefined:     when argument is non-string.
      * }
      */
     var _linkParser = function (url) {
-        var result;
+        var youtubeIdLength = 11,
+            result;
 
         if (typeof url !== 'string') {
-
             return void(0);
         }
 
         if (_youtubeParser(url)) {
+            if (_youtubeParser(url).length === youtubeIdLength) {
                 result = {
                     mode: 'youtube',
                     video: _youtubeParser(url),
                     type: 'youtube'
                 };
+            } else {
+                result = {
+                    mode: 'incorrect'
+                };
+            }
         } else if (_videoLinkParser(url)) {
             result = $.extend({mode: 'html5'}, _videoLinkParser(url));
         } else {
@@ -197,9 +198,8 @@ return (function () {
     /**
      * Returns short-hand youtube url.
      * @function
-    * @param {string} video_id Youtube Video Id that will be added to the
-    * link.
-    * @return {string} Short-hand Youtube url.
+     * @param {String} video_id Youtube Video Id that will be added to the link.
+     * @return {String} Short-hand Youtube url.
      * @examples
      * _getYoutubeLink('OEoXaMPEzfM'); => 'http://youtu.be/OEoXaMPEzfM'
      */
@@ -210,8 +210,8 @@ return (function () {
     /**
      * Returns list of objects with information about the passed links.
      * @function
-    * @param {array} links List of links that will be processed.
-    * @returns {array} List of objects.
+     * @param {Array} links List of links that will be processed.
+     * @returns {Array} List of objects.
      * @examples
      * var links = [
      *      'http://youtu.be/OEoXaMPEzfM',
@@ -243,7 +243,6 @@ return (function () {
         }
     };
 
-
     /**
      * Synchronizes 2 Backbone collections by 'field_name' property.
      * @function

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

@@ -15,9 +15,7 @@
         data-transcript-translation-url="/transcript/translation"
         data-transcript-available-translations-url="/transcript/available_translations"
         data-sub="Z5KLxerq05Y"
-        data-mp4-source="xmodule/include/fixtures/test.mp4"
-        data-webm-source="xmodule/include/fixtures/test.webm"
-        data-ogg-source="xmodule/include/fixtures/test.ogv"
+        data-sources='["xmodule/include/fixtures/test.mp4","xmodule/include/fixtures/test.webm","xmodule/include/fixtures/test.ogv"]'
         data-autoplay="False"
         data-yt-test-timeout="1500"
         data-yt-api-url="www.youtube.com/iframe_api"

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

@@ -15,9 +15,7 @@
         data-transcript-translation-url="/transcript/translation"
         data-transcript-available-translations-url="/transcript/available_translations"
         data-sub="Z5KLxerq05Y"
-        data-mp4-source="xmodule/include/fixtures/test.mp4"
-        data-webm-source="xmodule/include/fixtures/test.webm"
-        data-ogg-source="xmodule/include/fixtures/test.ogv"
+        data-sources='["xmodule/include/fixtures/test.mp4","xmodule/include/fixtures/test.webm","xmodule/include/fixtures/test.ogv"]'
         data-autoplay="False"
         data-yt-test-timeout="1500"
         data-yt-api-url="www.youtube.com/iframe_api"

common/lib/xmodule/xmodule/js/spec/video/general_spec.js

@@ -79,53 +79,6 @@
                         expect(state.videos).toBeUndefined();
                     });
 
-                    it('parse Html5 sources', function () {
-                        var html5Sources = {
-                                mp4: null,
-                                webm: null,
-                                ogg: null
-                            }, v = document.createElement('video');
-
-                        if (
-                            !!(
-                                v.canPlayType &&
-                                v.canPlayType(
-                                    'video/webm; codecs="vp8, vorbis"'
-                                ).replace(/no/, '')
-                            )
-                        ) {
-                            html5Sources['webm'] =
-                                'xmodule/include/fixtures/test.webm';
-                        }
-
-                        if (
-                            !!(
-                                v.canPlayType &&
-                                v.canPlayType(
-                                    'video/mp4; codecs="avc1.42E01E, ' +
-                                    'mp4a.40.2"'
-                                ).replace(/no/, '')
-                            )
-                        ) {
-                            html5Sources['mp4'] =
-                                'xmodule/include/fixtures/test.mp4';
-                        }
-
-                        if (
-                            !!(
-                                v.canPlayType &&
-                                v.canPlayType(
-                                    'video/ogg; codecs="theora"'
-                                ).replace(/no/, '')
-                            )
-                        ) {
-                            html5Sources['ogg'] =
-                                'xmodule/include/fixtures/test.ogv';
-                        }
-
-                        expect(state.html5Sources).toEqual(html5Sources);
-                    });
-
                     it('parse available video speeds', function () {
                         var speeds = jasmine.stubbedHtml5Speeds;
 

common/lib/xmodule/xmodule/js/src/video/01_initialize.js

@@ -70,7 +70,6 @@ function (VideoPlayer, VideoStorage, i18n) {
         isFlashMode: isFlashMode,
         isYoutubeType: isYoutubeType,
         parseSpeed: parseSpeed,
-        parseVideoSources: parseVideoSources,
         parseYoutubeStreams: parseYoutubeStreams,
         saveState: saveState,
         setPlayerMode: setPlayerMode,
@@ -280,32 +279,17 @@ function (VideoPlayer, VideoStorage, i18n) {
     // The function prepare HTML5 video, parse HTML5
     // video sources etc.
     function _prepareHTML5Video(state) {
-        state.parseVideoSources(
-            {
-                mp4: state.config.mp4Source,
-                webm: state.config.webmSource,
-                ogg: state.config.oggSource
-            }
-        );
-
         state.speeds = ['0.75', '1.0', '1.25', '1.50'];
-
-        // We must have at least one non-YouTube video source available.
-        // Otherwise, return a negative.
-        if (
-            state.html5Sources.webm === null &&
-            state.html5Sources.mp4 === null &&
-            state.html5Sources.ogg === null
-        ) {
-
-            // TODO: use 1 class to work with.
-            state.el.find('.video-player div').addClass('hidden');
-            state.el.find('.video-player h3').removeClass('hidden');
+        // If none of the supported video formats can be played and there is no
+        // short-hand video links, than hide the spinner and show error message.
+        if (!state.config.sources.length) {
             _hideWaitPlaceholder(state);
-
-            console.log(
-                '[Video info]: Non-youtube video sources aren\'t available.'
-            );
+            state.el
+                .find('.video-player div')
+                    .addClass('hidden')
+                .end()
+                .find('.video-player h3')
+                    .removeClass('hidden');
 
             return false;
         }
@@ -642,48 +626,6 @@ function (VideoPlayer, VideoStorage, i18n) {
         return _.isString(this.videos['1.0']);
     }
 
-    // function parseVideoSources(, mp4Source, webmSource, oggSource)
-    //
-    //     Take the HTML5 sources (URLs of videos), and make them available
-    //     explictly for each type of video format (mp4, webm, ogg).
-    function parseVideoSources(sources) {
-        var _this = this,
-            v = document.createElement('video'),
-            sourceCodecs = {
-                mp4: 'video/mp4; codecs="avc1.42E01E, mp4a.40.2"',
-                webm: 'video/webm; codecs="vp8, vorbis"',
-                ogg: 'video/ogg; codecs="theora"'
-            };
-
-        this.html5Sources = {
-            mp4: null,
-            webm: null,
-            ogg: null
-        };
-
-        $.each(sources, function (name, source) {
-            if (source && source.length) {
-                if (
-                    Boolean(
-                        v.canPlayType &&
-                        v.canPlayType(sourceCodecs[name]).replace(/no/, '')
-                    )
-                ) {
-                    _this.html5Sources[name] = source;
-                }
-            }
-        });
-
-        // None of the supported video formats can be played. Hide the spinner.
-        if (!(_.compact(_.values(this.html5Sources)))) {
-            _hideWaitPlaceholder(state);
-            console.log(
-                '[Video info]: This browser cannot play .mp4, .ogg, or .webm ' +
-                'files'
-            );
-        }
-    }
-
     // function fetchMetadata()
     //
     //     When dealing with YouTube videos, we must fetch meta data that has
@@ -763,7 +705,10 @@ function (VideoPlayer, VideoStorage, i18n) {
         }
         successHandler = ($.isFunction(callback)) ? callback : null;
         xhr = $.ajax({
-            url: document.location.protocol + '//'  + this.config.ytTestUrl + url + '?v=2&alt=jsonc',
+            url: [
+                document.location.protocol, '//', this.config.ytTestUrl, url,
+                '?v=2&alt=jsonc'
+            ].join(''),
             dataType: 'jsonp',
             timeout: this.config.ytTestTimeout,
             success: successHandler

common/lib/xmodule/xmodule/js/src/video/02_html5_video.js

@@ -94,6 +94,22 @@ function () {
             return this.logs;
         };
 
+        Player.prototype.showErrorMessage = function () {
+            this.el
+                .find('.video-player div')
+                    .addClass('hidden')
+                .end()
+                .find('.video-player h3')
+                    .removeClass('hidden')
+                .end()
+                    .addClass('is-initialized')
+                .find('.spinner')
+                    .attr({
+                        'aria-hidden': 'true',
+                        'tabindex': -1
+                    });
+        };
+
         return Player;
 
         /*
@@ -113,7 +129,7 @@ function () {
          *
          *     config = {
          *
-         *        videoSources: {},   // An object with properties being video
+         *        videoSources: [],   // An array with properties being video
          *                            // sources. The property name is the
          *                            // video format of the source. Supported
          *                            // video formats are: 'mp4', 'webm', and
@@ -134,7 +150,7 @@ function () {
          */
         function Player(el, config) {
             var isTouch = onTouchBasedDevice() || '',
-                sourceStr, _this, errorMessage;
+                sourceList, _this, errorMessage, lastSource;
 
             this.logs = [];
             // Initially we assume that el is a DOM element. If jQuery selector
@@ -167,63 +183,50 @@ function () {
 
             // We should have at least one video source. Otherwise there is no
             // point to continue.
-            if (!config.videoSources) {
+            if (!config.videoSources && !config.videoSources.length) {
                 return;
             }
 
-            // From the start, all sources are empty. We will populate this
-            // object below.
-            sourceStr = {
-                mp4: ' ',
-                webm: ' ',
-                ogg: ' '
-            };
 
             // Will be used in inner functions to point to the current object.
             _this = this;
 
             // Create HTML markup for individual sources of the HTML5 <video>
             // element.
-            $.each(sourceStr, function (videoType, videoSource) {
-                var url = _this.config.videoSources[videoType];
-                if (url && url.length) {
-                    sourceStr[videoType] =
-                        '<source ' +
-                            'src="' + url +
+            sourceList = $.map(config.videoSources, function (source) {
+               return [
+                    '<source ',
+                        'src="', source,
             // Following hack allows to open the same video twice
             // https://code.google.com/p/chromium/issues/detail?id=31014
             // Check whether the url already has a '?' inside, and if so,
             // use '&' instead of '?' to prevent breaking the url's integrity.
-                            (url.indexOf('?') == -1 ? '?' : '&') + (new Date()).getTime() +
-                            '" ' + 'type="video/' + videoType + '" ' +
-                        '/> ';
-                }
+                        (source.indexOf('?') === -1 ? '?' : '&'),
+                        (new Date()).getTime(), '" />'
+                ].join('');
             });
 
-            // We should have at least one video source. Otherwise there is no
-            // point to continue.
-            if (
-                sourceStr.mp4 === ' ' &&
-                sourceStr.webm === ' ' &&
-                sourceStr.ogg === ' '
-            ) {
-                return;
-            }
 
             // Create HTML markup for the <video> element, populating it with
             // sources from previous step. Because of problems with creating
             // video element via jquery (http://bugs.jquery.com/ticket/9174) we
             // create it using native JS.
             this.video = document.createElement('video');
-            errorMessage = gettext('This browser cannot play .mp4, .ogg, or .webm files.')
-                         + gettext('Try using a different browser, such as Google Chrome.');
-            this.video.innerHTML = _.values(sourceStr).join('') + errorMessage;
+
+            errorMessage = [
+                gettext('This browser cannot play .mp4, .ogg, or .webm files.'),
+                gettext('Try using a different browser, such as Google Chrome.')
+            ].join('');
+            this.video.innerHTML = sourceList.join('') + errorMessage;
 
             // Get the jQuery object, and set the player state to UNSTARTED.
             // The player state is used by other parts of the VideoPlayer to
             // determine what the video is currently doing.
             this.videoEl = $(this.video);
 
+            lastSource = this.videoEl.find('source').last();
+            lastSource.on('error', this.showErrorMessage.bind(this));
+
             if (/iP(hone|od)/i.test(isTouch[0])) {
                 this.videoEl.prop('controls', true);
             }
@@ -253,6 +256,7 @@ function () {
                 'durationchange', 'volumechange'
             ];
 
+            this.debug = false;
             $.each(events, function(index, eventName) {
                 _this.video.addEventListener(eventName, function () {
                     _this.logs.push({
@@ -260,6 +264,15 @@ function () {
                         'state': _this.playerState
                     });
 
+                    if (_this.debug) {
+                        console.log(
+                            'event name:', eventName,
+                            'state:', _this.playerState,
+                            'readyState:', _this.video.readyState,
+                            'networkState:', _this.video.networkState
+                        );
+                    }
+
                     el.trigger('html5:' + eventName, arguments);
                 });
             });

common/lib/xmodule/xmodule/js/src/video/03_video_player.js

@@ -142,7 +142,7 @@ function (HTML5Video, Resizer) {
         if (state.videoType === 'html5') {
             state.videoPlayer.player = new HTML5Video.Player(state.el, {
                 playerVars:   state.videoPlayer.playerVars,
-                videoSources: state.html5Sources,
+                videoSources: state.config.sources,
                 events: {
                     onReady:       state.videoPlayer.onReady,
                     onStateChange: state.videoPlayer.onStateChange

common/lib/xmodule/xmodule/tests/test_video.py

@@ -20,7 +20,7 @@ from mock import Mock
 from . import LogicTest
 from lxml import etree
 from opaque_keys.edx.locations import Location
-from xmodule.video_module import VideoDescriptor, create_youtube_string, get_ext
+from xmodule.video_module import VideoDescriptor, create_youtube_string
 from .test_import import DummySystem
 from xblock.field_data import DictFieldData
 from xblock.fields import ScopeIds
@@ -107,18 +107,6 @@ class VideoModuleTest(LogicTest):
              '1.50': ''}
         )
 
-    def test_get_ext(self):
-        """Test get the file's extension in a url without query string."""
-        filename_str = 'http://www.example.com/path/video.mp4'
-        output = get_ext(filename_str)
-        self.assertEqual(output, 'mp4')
-
-    def test_get_ext_with_query_string(self):
-        """Test get the file's extension in a url with query string."""
-        filename_str = 'http://www.example.com/path/video.mp4?param1=1&p2=2'
-        output = get_ext(filename_str)
-        self.assertEqual(output, 'mp4')
-
 
 class VideoDescriptorTest(unittest.TestCase):
     """Test for VideoDescriptor"""

common/lib/xmodule/xmodule/video_module/video_module.py

@@ -35,14 +35,6 @@ from .video_utils import create_youtube_string
 from .video_xfields import VideoFields
 from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers
 
-from urlparse import urlparse
-
-
-def get_ext(filename):
-    # Prevent incorrectly parsing urls like 'http://abc.com/path/video.mp4?xxxx'.
-    path = urlparse(filename).path
-    return path.rpartition('.')[-1]
-
 
 log = logging.getLogger(__name__)
 _ = lambda text: text
@@ -97,15 +89,15 @@ class VideoModule(VideoFields, VideoStudentViewHandlers, XModule):
 
     def get_html(self):
         track_url = None
+        download_video_link = None
         transcript_download_format = self.transcript_download_format
-
-        sources = {get_ext(src): src for src in self.html5_sources}
+        sources = filter(None, self.html5_sources)
 
         if self.download_video:
             if self.source:
-                sources['main'] = self.source
+                download_video_link = self.source
             elif self.html5_sources:
-                sources['main'] = self.html5_sources[0]
+                download_video_link = self.html5_sources[0]
 
         if self.download_track:
             if self.track:
@@ -149,7 +141,8 @@ class VideoModule(VideoFields, VideoStudentViewHandlers, XModule):
             'handout': self.handout,
             'id': self.location.html_id(),
             'show_captions': json.dumps(self.show_captions),
-            'sources': sources,
+            'download_video_link': download_video_link,
+            'sources': json.dumps(sources),
             'speed': json.dumps(self.speed),
             'general_speed': self.global_speed,
             'saved_video_position': self.saved_video_position.total_seconds(),

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -26,12 +26,7 @@ class TestVideoYouTube(TestVideo):
     def test_video_constructor(self):
         """Make sure that all parameters extracted correctly from xml"""
         context = self.item_descriptor.render('student_view').content
-
-        sources = {
-            'main': u'example.mp4',
-            u'mp4': u'example.mp4',
-            u'webm': u'example.webm',
-        }
+        sources = json.dumps([u'example.mp4', u'example.webm'])
 
         expected_context = {
             'ajax_url': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state',
@@ -42,6 +37,7 @@ class TestVideoYouTube(TestVideo):
             'id': self.item_descriptor.location.html_id(),
             'show_captions': 'true',
             'handout': None,
+            'download_video_link': u'example.mp4',
             'sources': sources,
             'speed': 'null',
             'general_speed': 1.0,
@@ -93,13 +89,8 @@ class TestVideoNonYouTube(TestVideo):
         """Make sure that if the 'youtube' attribute is omitted in XML, then
             the template generates an empty string for the YouTube streams.
         """
-        sources = {
-            'main': u'example.mp4',
-            u'mp4': u'example.mp4',
-            u'webm': u'example.webm',
-        }
-
         context = self.item_descriptor.render('student_view').content
+        sources = json.dumps([u'example.mp4', u'example.webm'])
 
         expected_context = {
             'ajax_url': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state',
@@ -107,6 +98,7 @@ class TestVideoNonYouTube(TestVideo):
             'show_captions': 'true',
             'handout': None,
             'display_name': u'A Name',
+            'download_video_link': u'example.mp4',
             'end': 3610.0,
             'id': self.item_descriptor.location.html_id(),
             'sources': sources,
@@ -148,7 +140,7 @@ class TestGetHtmlMethod(BaseTestXmodule):
     METADATA = {}
 
     def setUp(self):
-        self.setup_course();
+        self.setup_course()
 
     def test_get_html_track(self):
         SOURCE_XML = """
@@ -201,19 +193,17 @@ class TestGetHtmlMethod(BaseTestXmodule):
                 'transcripts': '<transcript language="uk" src="ukrainian.srt" />',
             },
         ]
+        sources = json.dumps([u'example.mp4', u'example.webm'])
 
         expected_context = {
             'data_dir': getattr(self, 'data_dir', None),
             'show_captions': 'true',
             'handout': None,
             'display_name': u'A Name',
+            'download_video_link': u'example.mp4',
             'end': 3610.0,
             'id': None,
-            'sources': {
-                'main': u'example.mp4',
-                u'mp4': u'example.mp4',
-                u'webm': u'example.webm'
-            },
+            'sources': sources,
             'start': 3603.0,
             'saved_video_position': 0.0,
             'sub': u'a_sub_file.srt.sjson',
@@ -284,9 +274,8 @@ class TestGetHtmlMethod(BaseTestXmodule):
                     <source src="example.webm"/>
                 """,
                 'result': {
-                    'main': u'example_source.mp4',
-                    u'mp4': u'example.mp4',
-                    u'webm': u'example.webm',
+                    'download_video_link': u'example_source.mp4',
+                    'sources': json.dumps([u'example.mp4', u'example.webm']),
                 },
             },
             {
@@ -297,9 +286,8 @@ class TestGetHtmlMethod(BaseTestXmodule):
                     <source src="example.webm"/>
                 """,
                 'result': {
-                    'main': u'example.mp4',
-                    u'mp4': u'example.mp4',
-                    u'webm': u'example.webm',
+                    'download_video_link': u'example.mp4',
+                    'sources': json.dumps([u'example.mp4', u'example.webm']),
                 },
             },
             {
@@ -318,20 +306,20 @@ class TestGetHtmlMethod(BaseTestXmodule):
                     <source src="example.webm"/>
                 """,
                 'result': {
-                    u'mp4': u'example.mp4',
-                    u'webm': u'example.webm',
+                    'sources': json.dumps([u'example.mp4', u'example.webm']),
                 },
             },
         ]
 
-        expected_context = {
+        initial_context = {
             'data_dir': getattr(self, 'data_dir', None),
             'show_captions': 'true',
             'handout': None,
             'display_name': u'A Name',
+            'download_video_link': None,
             'end': 3610.0,
             'id': None,
-            'sources': None,
+            'sources': '[]',
             'speed': 'null',
             'general_speed': 1.0,
             'start': 3603.0,
@@ -358,6 +346,7 @@ class TestGetHtmlMethod(BaseTestXmodule):
             self.initialize_module(data=DATA)
             context = self.item_descriptor.render('student_view').content
 
+            expected_context = dict(initial_context)
             expected_context.update({
                 'transcript_translation_url': self.item_descriptor.xmodule_runtime.handler_url(
                     self.item_descriptor, 'transcript', 'translation'
@@ -366,9 +355,9 @@ class TestGetHtmlMethod(BaseTestXmodule):
                     self.item_descriptor, 'transcript', 'available_translations'
                 ).rstrip('/?'),
                 'ajax_url': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state',
-                'sources': data['result'],
                 'id': self.item_descriptor.location.html_id(),
             })
+            expected_context.update(data['result'])
 
             self.assertEqual(
                 context,
@@ -385,7 +374,7 @@ class TestVideoDescriptorInitialization(BaseTestXmodule):
     METADATA = {}
 
     def setUp(self):
-        self.setup_course();
+        self.setup_course()
 
     def test_source_not_in_html5sources(self):
         metadata = {

lms/templates/video.html

@@ -13,10 +13,7 @@
     ${'data-sub="{}"'.format(sub) if sub else ''}
     ${'data-autoplay="{}"'.format(autoplay) if autoplay else ''}
 
-    ${'data-mp4-source="{}"'.format(sources.get('mp4')) if sources.get('mp4') else ''}
-    ${'data-webm-source="{}"'.format(sources.get('webm')) if sources.get('webm') else ''}
-    ${'data-ogg-source="{}"'.format(sources.get('ogv')) if sources.get('ogv') else ''}
-
+    data-sources='${sources}'
     data-save-state-url="${ajax_url}"
     data-caption-data-dir="${data_dir}"
     data-show-captions="${show_captions}"
@@ -106,9 +103,9 @@
 
     <div class="focus_grabber last"></div>
   <ul class="wrapper-downloads">
-    % if sources.get('main'):
+    % if download_video_link:
         <li class="video-sources video-download-button">
-            ${('<a href="%s">' + _('Download video') + '</a>') % sources.get('main')}
+            ${('<a href="%s">' + _('Download video') + '</a>') % download_video_link}
         </li>
     % endif
     % if track: