Merge pull request #1337 from edx/db/fix-video-requirejs

fix video module issue with require.js
2013-10-16 09:40:48 -07:00
parent 637b66197b d408d7adec
commit e363e5a72d
11 changed files with 72 additions and 43 deletions
--- a/cms/static/coffee/spec/main.coffee
+++ b/cms/static/coffee/spec/main.coffee
@@ -22,7 +22,7 @@ requirejs.config({
        "underscore.string": "xmodule_js/common_static/js/vendor/underscore.string.min",
        "backbone": "xmodule_js/common_static/js/vendor/backbone-min",
        "backbone.associations": "xmodule_js/common_static/js/vendor/backbone-associations-min",
-        "youtube": "xmodule_js/common_static/js/load_youtube",
+        "youtube": "//www.youtube.com/player_api?noext",
        "tinymce": "xmodule_js/common_static/js/vendor/tiny_mce/tiny_mce",
        "jquery.tinymce": "xmodule_js/common_static/js/vendor/tiny_mce/jquery.tinymce",
        "mathjax": "https://edx-static.s3.amazonaws.com/mathjax-MathJax-727332c/MathJax.js?config=TeX-MML-AM_HTMLorMML-full",
@@ -102,6 +102,9 @@ requirejs.config({
            deps: ["backbone"],
            exports: "Backbone.Associations"
        },
+        "youtube": {
+            exports: "YT"
+        },
        "codemirror": {
            exports: "CodeMirror"
        },
--- a/cms/static/coffee/spec/main_squire.coffee
+++ b/cms/static/coffee/spec/main_squire.coffee
@@ -22,7 +22,7 @@ requirejs.config({
        "underscore.string": "xmodule_js/common_static/js/vendor/underscore.string.min",
        "backbone": "xmodule_js/common_static/js/vendor/backbone-min",
        "backbone.associations": "xmodule_js/common_static/js/vendor/backbone-associations-min",
-        "youtube": "xmodule_js/common_static/js/load_youtube",
+        "youtube": "//www.youtube.com/player_api?noext",
        "tinymce": "xmodule_js/common_static/js/vendor/tiny_mce/tiny_mce",
        "jquery.tinymce": "xmodule_js/common_static/js/vendor/tiny_mce/jquery.tinymce",
        "mathjax": "https://edx-static.s3.amazonaws.com/mathjax-MathJax-727332c/MathJax.js?config=TeX-MML-AM_HTMLorMML-full",
@@ -100,6 +100,9 @@ requirejs.config({
            deps: ["backbone"],
            exports: "Backbone.Associations"
        },
+        "youtube": {
+            exports: "YT"
+        },
        "codemirror": {
            exports: "CodeMirror"
        },
--- a/cms/templates/base.html
+++ b/cms/templates/base.html
@@ -61,13 +61,18 @@ var require = {
        "underscore.string": "js/vendor/underscore.string.min",
        "backbone": "js/vendor/backbone-min",
        "backbone.associations": "js/vendor/backbone-associations-min",
-        "youtube": "js/load_youtube",
        "tinymce": "js/vendor/tiny_mce/tiny_mce",
        "jquery.tinymce": "js/vendor/tiny_mce/jquery.tinymce",
-        "mathjax": "https://edx-static.s3.amazonaws.com/mathjax-MathJax-727332c/MathJax.js?config=TeX-MML-AM_HTMLorMML-full",
        "xmodule": "/xmodule/xmodule",
        "utility": "js/src/utility",
-        "draggabilly": "js/vendor/draggabilly.pkgd"
+        "draggabilly": "js/vendor/draggabilly.pkgd",
+
+        // externally hosted files
+        "mathjax": "//edx-static.s3.amazonaws.com/mathjax-MathJax-727332c/MathJax.js?config=TeX-MML-AM_HTMLorMML-full",
+        // youtube URL does not end in ".js". We add "?noext" to the path so
+        // that require.js adds the ".js" to the query component of the URL,
+        // and leaves the path component intact.
+        "youtube": "//www.youtube.com/player_api?noext"
    },
    shim: {
        "gettext": {
@@ -138,6 +143,9 @@ var require = {
            deps: ["backbone"],
            exports: "Backbone.Associations"
        },
+        "youtube": {
+            exports: "YT"
+        },
        "codemirror": {
            exports: "CodeMirror"
        },
--- a/common/lib/xmodule/xmodule/js/spec/helper.coffee
+++ b/common/lib/xmodule/xmodule/js/spec/helper.coffee
@@ -1,5 +1,6 @@
 # Stub Youtube API
 window.YT =
+  Player: ->
  PlayerState:
    UNSTARTED: -1
    ENDED: 0
@@ -7,6 +8,7 @@ window.YT =
    PAUSED: 2
    BUFFERING: 3
    CUED: 5
+  ready: (f) -> f()

 window.STATUS = window.YT.PlayerState

--- a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
+++ b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
@@ -11,8 +11,6 @@

        afterEach(function () {
            window.OldVideoPlayer = undefined;
-            window.onYouTubePlayerAPIReady = undefined;
-            window.onHTML5PlayerAPIReady = undefined;
            $('source').remove();
        });

@@ -214,14 +212,23 @@
                // Total ajax calls made.
                numAjaxCalls = $.ajax.calls.length;

-                // Subtract ajax calls to get captions.
+                // Subtract ajax calls to get captions via
+                // state.videoCaption.fetchCaption() function.
                numAjaxCalls -= $.ajaxWithPrefix.calls.length;

-                // Subtract ajax calls to get metadata for each video.
+                // Subtract ajax calls to get metadata for each video via
+                // state.getVideoMetadata() function.
+                numAjaxCalls -= 3;
+
+                // Subtract ajax calls to log event 'pause_video' via
+                // state.videoPlayer.log() function.
                numAjaxCalls -= 3;

                // This should leave just one call. It was made to check
-                // for YT availability.
+                // for YT availability. This is done in state.initialize()
+                // function. SPecifically, with the statement
+                //
+                //     this.youtubeXhr = this.getVideoMetadata();
                expect(numAjaxCalls).toBe(1);
            });
        });
--- a/common/lib/xmodule/xmodule/js/spec/video/html5_video_spec.js
+++ b/common/lib/xmodule/xmodule/js/spec/video/html5_video_spec.js
@@ -28,7 +28,7 @@
                spyOn(player, 'callStateChangeCallback').andCallThrough();
            });

-            describe('click', function () {
+            describe('[click]', function () {
                describe('when player is paused', function () {
                    beforeEach(function () {
                        spyOn(player.video, 'play').andCallThrough();
@@ -61,7 +61,7 @@
                    });
                });

-                describe('when player is playing', function () {
+                describe('[player is playing]', function () {
                    beforeEach(function () {
                        spyOn(player.video, 'pause').andCallThrough();
                        player.playerState  = STATUS.PLAYING;
@@ -94,7 +94,7 @@
                });
            });

-            describe('play', function () {
+            describe('[play]', function () {
                beforeEach(function () {
                    spyOn(player.video, 'play').andCallThrough();
                    player.playerState = STATUS.PAUSED;
@@ -126,7 +126,7 @@
                });
            });

-            describe('pause', function () {
+            describe('[pause]', function () {
                beforeEach(function () {
                    spyOn(player.video, 'pause').andCallThrough();
                    player.playerState = STATUS.UNSTARTED;
@@ -161,7 +161,7 @@
                });
            });

-            describe('canplay', function () {
+            describe('[canplay]', function () {
                beforeEach(function () {
                    waitsFor(function () {
                        return player.getPlayerState() !== STATUS.UNSTARTED;
@@ -193,7 +193,7 @@
                });
            });

-            describe('ended', function () {
+            describe('[ended]', function () {
                beforeEach(function () {
                    waitsFor(function () {
                        return player.getPlayerState() !== STATUS.UNSTARTED;
--- a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js
+++ b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js
@@ -83,7 +83,8 @@

        window.YT = {
            Player: function () { },
-            PlayerState: oldYT.PlayerState
+            PlayerState: oldYT.PlayerState,
+            ready: function(f){f();}
        };

        spyOn(window.YT, 'Player');
--- a/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js
+++ b/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js
@@ -20,17 +20,38 @@
    });

    describe('constructor', function() {
+      var oldYT = window.YT;
+
      beforeEach(function() {
+        window.YT = {
+            Player: function () { },
+            PlayerState: oldYT.PlayerState,
+            ready: function(f){f();}
+        };
+
        initialize();
      });

-      // Disabled when ARIA markup was added to the anchor
-      xit('render the quality control', function() {
-        expect(videoControl.secondaryControlsEl.html()).toContain("<a href=\"#\" class=\"quality_control\" title=\"HD\">");
+      afterEach(function () {
+        window.YT = oldYT;
+      });
+
+      it('render the quality control', function() {
+        expect(videoControl.secondaryControlsEl.html())
+          .toContain(
+            '<a ' +
+              'href="#" ' +
+              'class="quality_control" ' +
+              'title="HD" ' +
+              'role="button" ' +
+              'aria-disabled="false"' +
+            '>HD</a>'
+          );
      });

      it('bind the quality control', function() {
-        expect($('.quality_control')).toHandleWith('click', videoQualityControl.toggleQuality);
+        expect($('.quality_control'))
+          .toHandleWith('click', videoQualityControl.toggleQuality);
      });
    });
  });
--- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js
+++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js
@@ -92,23 +92,12 @@ function (VideoPlayer) {
        // Require JS. At the time when we reach this code, the stand alone
        // HTML5 player is already loaded, so no further testing in that case
        // is required.
-        var onPlayerReadyFunc;
-        if (
-            (
-                (state.videoType === 'youtube') &&
-                (window.YT) &&
-                (window.YT.Player)
-            ) ||
-            (state.videoType === 'html5')
-        ) {
-            VideoPlayer(state);
+        if(state.videoType === 'youtube') {
+            YT.ready(function() {
+                VideoPlayer(state);
+            })
        } else {
-            if (state.videoType === 'youtube') {
-                onPlayerReadyFunc = 'onYouTubePlayerAPIReady';
-            } else {
-                onPlayerReadyFunc = 'onHTML5PlayerAPIReady';
-            }
-            window[onPlayerReadyFunc] = _.bind(VideoPlayer, window, state);
+            VideoPlayer(state);
        }
    }

--- a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js
+++ b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js
@@ -420,7 +420,7 @@ function (HTML5Video) {
            this.videoPlayer.player.setPlaybackRate(this.speed);
        }

-        /* The following has been commented out to make sure autoplay is 
+        /* The following has been commented out to make sure autoplay is
           disabled for students.
        if (
            !onTouchBasedDevice() &&
--- a/common/static/js/load_youtube.js
+++ b/common/static/js/load_youtube.js
@@ -1,5 +0,0 @@
-define(["jquery"], function($) {
-    var url = "//www.youtube.com/player_api";
-    $("head").append($("<script/>", {src: url}));
-    return window.YT;
-});