From 40e540284fc4795abf06f244091daabcc308808c Mon Sep 17 00:00:00 2001
From: David Baumgold <david@davidbaumgold.com>
Date: Mon, 14 Oct 2013 10:58:41 -0400
Subject: [PATCH 1/7] wait to load video module until YT API is ready

---
 cms/static/coffee/spec/main.coffee            |  5 ++++-
 cms/static/coffee/spec/main_squire.coffee     |  5 ++++-
 cms/templates/base.html                       | 14 ++++++++++---
 .../xmodule/js/src/video/01_initialize.js     | 21 +++++--------------
 common/static/js/load_youtube.js              |  5 -----
 5 files changed, 24 insertions(+), 26 deletions(-)
 delete mode 100644 common/static/js/load_youtube.js

diff --git a/cms/static/coffee/spec/main.coffee b/cms/static/coffee/spec/main.coffee
index 3fdb216e37..55381d82f8 100644
--- 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"
         },
diff --git a/cms/static/coffee/spec/main_squire.coffee b/cms/static/coffee/spec/main_squire.coffee
index 61e00ab03f..28a3f03ce1 100644
--- 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"
         },
diff --git a/cms/templates/base.html b/cms/templates/base.html
index 30d095087b..152bb40918 100644
--- 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"
         },
diff --git a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js
index 845d79ddb9..b3c5fb91bc 100644
--- 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);
         }
     }
 
diff --git a/common/static/js/load_youtube.js b/common/static/js/load_youtube.js
deleted file mode 100644
index 98f296d06a..0000000000
--- a/common/static/js/load_youtube.js
+++ /dev/null
@@ -1,5 +0,0 @@
-define(["jquery"], function($) {
-    var url = "//www.youtube.com/player_api";
-    $("head").append($("<script/>", {src: url}));
-    return window.YT;
-});

From bcf4fd5f3d153c1afe18466e643354313c2c0102 Mon Sep 17 00:00:00 2001
From: David Baumgold <david@davidbaumgold.com>
Date: Tue, 15 Oct 2013 09:54:58 -0400
Subject: [PATCH 2/7] Mock YT.ready in Xmodule JS tests

and disable tests that fail for unrelated reasons
---
 common/lib/xmodule/xmodule/js/spec/helper.coffee               | 2 ++
 common/lib/xmodule/xmodule/js/spec/video/general_spec.js       | 2 +-
 common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js  | 3 ++-
 .../xmodule/js/spec/video/video_quality_control_spec.js        | 2 +-
 4 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/common/lib/xmodule/xmodule/js/spec/helper.coffee b/common/lib/xmodule/xmodule/js/spec/helper.coffee
index c92cdbe414..1a4f4cbb28 100644
--- 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
 
diff --git a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
index 3444f5389f..440ed2d22c 100644
--- a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
+++ b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
@@ -208,7 +208,7 @@
                 state3 = new Video('#example3');
             });
 
-            it('check for YT availability is performed only once', function () {
+            xit('check for YT availability is performed only once', function () {
                 var numAjaxCalls = 0;
 
                 // Total ajax calls made.
diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js
index a7df088d67..00f51a4c80 100644
--- a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js
+++ b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js
@@ -83,7 +83,8 @@
 
         window.YT = {
             Player: function () { },
-            PlayerState: oldYT.PlayerState
+            PlayerState: oldYT.PlayerState,
+            ready: function(f){f();}
         };
 
         spyOn(window.YT, 'Player');
diff --git 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
index eb2f19aa60..82a8a7ba80 100644
--- 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
@@ -29,7 +29,7 @@
         expect(videoControl.secondaryControlsEl.html()).toContain("<a href=\"#\" class=\"quality_control\" title=\"HD\">");
       });
 
-      it('bind the quality control', function() {
+      xit('bind the quality control', function() {
         expect($('.quality_control')).toHandleWith('click', videoQualityControl.toggleQuality);
       });
     });

From e930b96b52ebbc748bf222645bcd8cbfd5efa99b Mon Sep 17 00:00:00 2001
From: David Baumgold <david@davidbaumgold.com>
Date: Tue, 15 Oct 2013 13:55:06 -0400
Subject: [PATCH 3/7] Remove references to onYouTubePlayerAPIReady and
 onHTML5PlayerAPIReady

---
 common/lib/xmodule/xmodule/js/spec/video/general_spec.js | 2 --
 1 file changed, 2 deletions(-)

diff --git a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
index 440ed2d22c..9105ae0899 100644
--- 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();
         });
 

From cf3ea892896bad2060b39eb6729bec72997c111b Mon Sep 17 00:00:00 2001
From: Valera Rozuvan <valera@edx.org>
Date: Wed, 16 Oct 2013 13:43:58 +0300
Subject: [PATCH 4/7] Rebased. Updated Jasmine describe messages to make them
 more clearer.

---
 .../xmodule/js/spec/video/html5_video_spec.js        | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/common/lib/xmodule/xmodule/js/spec/video/html5_video_spec.js b/common/lib/xmodule/xmodule/js/spec/video/html5_video_spec.js
index 21c2750cac..8851aee4d8 100644
--- 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;

From 269089711e3e4252bb7a75ba29acead4acca8f1e Mon Sep 17 00:00:00 2001
From: Valera Rozuvan <valera@edx.org>
Date: Wed, 16 Oct 2013 14:15:10 +0300
Subject: [PATCH 5/7] Enabled 2 disabled Jasmine tests. Fixed them.

---
 .../spec/video/video_quality_control_spec.js  | 32 ++++++++++++++++---
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git 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
index 82a8a7ba80..d6dd2fe492 100644
--- 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,39 @@
     });
 
     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;
       });
 
-      xit('bind the quality control', function() {
-        expect($('.quality_control')).toHandleWith('click', videoQualityControl.toggleQuality);
+      // Disabled when ARIA markup was added to the anchor
+      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);
       });
     });
   });

From deb36999ca83e05b5ee38ea01f7a40990bb3e6b1 Mon Sep 17 00:00:00 2001
From: David Baumgold <david@davidbaumgold.com>
Date: Wed, 16 Oct 2013 07:56:00 -0400
Subject: [PATCH 6/7] Remove outdated comment

---
 .../xmodule/xmodule/js/spec/video/video_quality_control_spec.js  | 1 -
 1 file changed, 1 deletion(-)

diff --git 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
index d6dd2fe492..c7ff729ceb 100644
--- 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
@@ -36,7 +36,6 @@
         window.YT = oldYT;
       });
 
-      // Disabled when ARIA markup was added to the anchor
       it('render the quality control', function() {
         expect(videoControl.secondaryControlsEl.html())
           .toContain(

From d408d7adec4dfed267b1522a693a299132c5d888 Mon Sep 17 00:00:00 2001
From: Valera Rozuvan <valera@edx.org>
Date: Wed, 16 Oct 2013 15:02:42 +0300
Subject: [PATCH 7/7] Enabled 2nd disabled Jasmine test.

---
 .../xmodule/js/spec/video/general_spec.js       | 17 +++++++++++++----
 .../xmodule/js/src/video/03_video_player.js     |  2 +-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
index 9105ae0899..7803099aa8 100644
--- a/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
+++ b/common/lib/xmodule/xmodule/js/spec/video/general_spec.js
@@ -206,20 +206,29 @@
                 state3 = new Video('#example3');
             });
 
-            xit('check for YT availability is performed only once', function () {
+            it('check for YT availability is performed only once', function () {
                 var numAjaxCalls = 0;
 
                 // 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);
             });
         });
diff --git a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js
index 03fcb19351..0fcbe42578 100644
--- 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() &&