From f6d5b34cca21493931e8243476a5dbecd0685221 Mon Sep 17 00:00:00 2001
From: drigato <denise.rigato@sourcefabric.org>
Date: Tue, 7 Apr 2015 17:49:21 -0400
Subject: [PATCH] Embed player - code review fixes

---
 .../application/controllers/ApiController.php |  2 +-
 .../EmbeddableplayerController.php            | 17 +++-
 .../application/forms/EmbeddablePlayer.php    | 19 +++--
 .../application/models/StreamSetting.php      | 16 ----
 .../scripts/embeddableplayer/embed-code.phtml | 83 ++++++++++++++-----
 5 files changed, 89 insertions(+), 48 deletions(-)

diff --git a/airtime_mvc/application/controllers/ApiController.php b/airtime_mvc/application/controllers/ApiController.php
index 80429db08..5ecd5f1da 100644
--- a/airtime_mvc/application/controllers/ApiController.php
+++ b/airtime_mvc/application/controllers/ApiController.php
@@ -523,7 +523,7 @@ class ApiController extends Zend_Controller_Action
             $result["description"] = Application_Model_Preference::GetStationDescription();
             $result["timezone"] = Application_Model_Preference::GetDefaultTimezone();
             $result["locale"] = Application_Model_Preference::GetDefaultLocale();
-            $result["enabled_stream_urls"] = Application_Model_StreamSetting::getEnabledStreamUrls();
+            $result["stream_data"] = Application_Model_StreamSetting::getEnabledStreamData();
             
             // used by caller to determine if the airtime they are running or widgets in use is out of date.
             $result['AIRTIME_API_VERSION'] = AIRTIME_API_VERSION;
diff --git a/airtime_mvc/application/controllers/EmbeddableplayerController.php b/airtime_mvc/application/controllers/EmbeddableplayerController.php
index 3552ee47e..43e8be2d5 100644
--- a/airtime_mvc/application/controllers/EmbeddableplayerController.php
+++ b/airtime_mvc/application/controllers/EmbeddableplayerController.php
@@ -30,18 +30,29 @@ class EmbeddablePlayerController extends Zend_Controller_Action
 
     }
 
+    /**
+     * This is the action that is called to insert the player onto a web page.
+     * It passes all the js and css files to the view, as well as all the
+     * stream customization information.
+     *
+     * The view for this action contains all the inline javascript needed to
+     * create the player.
+     */
     public function embedCodeAction()
     {
         $this->view->layout()->disableLayout();
 
+        $CC_CONFIG = Config::getConfig();
+
         $request = $this->getRequest();
 
-        $this->view->css = Application_Common_HTTPHelper::getStationUrl() . "css/embeddable-player.css";
-        $this->view->mrp_js = Application_Common_HTTPHelper::getStationUrl() . "js/airtime/embeddableplayer/mrp.js";
+        $this->view->css = Application_Common_HTTPHelper::getStationUrl() . "css/embeddable-player.css?".$CC_CONFIG['airtime_version'];
+        $this->view->mrp_js = Application_Common_HTTPHelper::getStationUrl() . "js/airtime/embeddableplayer/mrp.js?".$CC_CONFIG['airtime_version'];
         $this->view->jquery = Application_Common_HTTPHelper::getStationUrl() . "js/libs/jquery-1.10.2.js";
         $this->view->muses_swf = Application_Common_HTTPHelper::getStationUrl() . "js/airtime/embeddableplayer/muses.swf";
         $this->view->metadata_api_url = Application_Common_HTTPHelper::getStationUrl() . "api/live-info";
-        $this->view->station_name = Application_Model_Preference::GetStationName();
+        $this->view->station_name = addslashes(Application_Model_Preference::GetStationName());
+
         $stream = $request->getParam('stream');
         $streamData = Application_Model_StreamSetting::getEnabledStreamData();
         $availableMobileStreams = array();
diff --git a/airtime_mvc/application/forms/EmbeddablePlayer.php b/airtime_mvc/application/forms/EmbeddablePlayer.php
index e5086716a..ca8ca45c9 100644
--- a/airtime_mvc/application/forms/EmbeddablePlayer.php
+++ b/airtime_mvc/application/forms/EmbeddablePlayer.php
@@ -1,5 +1,7 @@
 <?php
 
+define("OPUS", "opus");
+
 class Application_Form_EmbeddablePlayer extends Zend_Form_SubForm
 {
     public function init()
@@ -8,17 +10,19 @@ class Application_Form_EmbeddablePlayer extends Zend_Form_SubForm
             array('ViewScript', array('viewScript' => 'form/embeddableplayer.phtml'))
         ));
 
+        /* We will use this option in the future
         $displayTrackMetadata = new Zend_Form_Element_Checkbox('player_display_track_metadata');
         $displayTrackMetadata->setValue(true);
         $displayTrackMetadata->setLabel(_('Display track metadata?'));
         $this->addElement($displayTrackMetadata);
+        */
 
         $streamMode = new Zend_Form_Element_Radio('player_stream_mode');
         $streamMode->setLabel(_('Select Stream:'));
         $streamMode->setMultiOptions(
             array(
-                "auto" => "Auto detect the most appropriate stream to use.",
-                "manual" => "Select a stream:"
+                "auto" => _("Auto detect the most appropriate stream to use."),
+                "manual" => _("Select a stream:")
             )
         );
         $streamMode->setValue("auto");
@@ -30,19 +34,20 @@ class Application_Form_EmbeddablePlayer extends Zend_Form_SubForm
         foreach(Application_Model_StreamSetting::getEnabledStreamData() as $stream => $data) {
             $urlOptions[$stream] = strtoupper($data["codec"])." - ".$data["bitrate"]."kbps";
             if ($data["mobile"]) {
-                $urlOptions[$stream] .= " - Mobile friendly";
+                $urlOptions[$stream] .= _(" - Mobile friendly");
             }
-            if ($data["codec"] == "opus") {
+            if ($data["codec"] == OPUS) {
                 $opusStreamCount += 1;
-                $urlOptions[$stream] .=" - The player does not support Opus streams.";
+                $urlOptions[$stream] .= _(" - The player does not support Opus streams.");
             }
         }
         $streamURL->setMultiOptions(
             $urlOptions
         );
 
+        // Set default value to the first non-opus stream we find
         foreach ($urlOptions as $o => $v) {
-            if (strpos(strtolower($v), "opus") !== false) {
+            if (strpos(strtolower($v), OPUS) !== false) {
                 continue;
             } else {
                 $streamURL->setValue($o);
@@ -62,7 +67,7 @@ class Application_Form_EmbeddablePlayer extends Zend_Form_SubForm
         $this->addElement($embedSrc);
 
         $previewLabel = new Zend_Form_Element_Text('player_preview_label');
-        $previewLabel->setLabel("Preview:");
+        $previewLabel->setLabel(_("Preview:"));
         $this->addElement($previewLabel);
 
     }
diff --git a/airtime_mvc/application/models/StreamSetting.php b/airtime_mvc/application/models/StreamSetting.php
index c51995d6d..688ddc397 100644
--- a/airtime_mvc/application/models/StreamSetting.php
+++ b/airtime_mvc/application/models/StreamSetting.php
@@ -65,22 +65,6 @@ class Application_Model_StreamSetting
         return $result ? $result : $default;
     }
 
-    public static function getEnabledStreamUrls()
-    {
-        $urls = Array();
-        $streamIds = Application_Model_StreamSetting::getEnabledStreamIds();
-        foreach ($streamIds as $id) {
-            $prefix = $id."_";
-            $streamData = Application_Model_StreamSetting::getStreamData($id);
-            $host = $streamData[$prefix."host"];
-            $port = $streamData[$prefix."port"];
-            $mount = $streamData[$prefix."mount"];
-            $type = $streamData[$prefix."type"];
-            $urls[$type] = "http://$host:$port/$mount";
-        }
-        return $urls;
-    }
-
     public static function getEnabledStreamData()
     {
         $streams = Array();
diff --git a/airtime_mvc/application/views/scripts/embeddableplayer/embed-code.phtml b/airtime_mvc/application/views/scripts/embeddableplayer/embed-code.phtml
index 66117609a..f079a4564 100644
--- a/airtime_mvc/application/views/scripts/embeddableplayer/embed-code.phtml
+++ b/airtime_mvc/application/views/scripts/embeddableplayer/embed-code.phtml
@@ -7,6 +7,17 @@
     <script src="<?php echo $this->jquery?>" type="text/javascript"></script>
     <script type="text/javascript">
 
+        var MAX_MOBILE_SCREEN_WIDTH = 760;
+
+        // We are creating a custom player object that acts as a wrapper
+        // around the player object we get from Muses. We are doing this
+        // for a couple reasons:
+        //
+        // 1. It will be easier to swap out Muses for a different player engine
+        //    in the future - if we ever decide to do that.
+        // 2. We had to add in some custom behaviour depending on the player
+        //    customizations and whether or not the player is in Flash or HTML5
+        //    mode.
         var MusesPlayer = function() {
             this.mobileDetect = this.mobileDetect();
             this.playerMode = "<?php echo $this->playerMode ?>";
@@ -34,11 +45,12 @@
                 this.settings.codec = stream["codec"];
             }
 
+            // Create the Muses player object
             MRP.insert(this.settings);
 
-            $("p.station_name").html("<?php echo $this->station_name?>");
+            $("p.station_name").html(htmlEscape("<?php echo $this->station_name?>"));
 
-            getMetadata();
+            attachStreamMetadataToPlayer();
 
             // detects events in HTML5 mode
             if (!this.flashDetect) {
@@ -62,13 +74,14 @@
         };
 
         MusesPlayer.prototype.mobileDetect = function() {
-            if ( screen.width <= 760) {
-                return true;
-            } else {
-                return false;
-            }
+            return (screen.width <= MAX_MOBILE_SCREEN_WIDTH);
         }
 
+        // This function is called if an error occurs while a client is
+        // attempting to connect to a stream (An error would occur if
+        // the streams listener count has been maxed out or if the stream is down).
+        // It checks if the client is a mobile device or not and returns the next
+        // best available stream.
         MusesPlayer.prototype.getNextAvailableStream = function() {
             if (this.mobileDetect && this.availableMobileStreamQueue.length > 0) {
                 return this.getNextAvailableMobileStream();
@@ -78,9 +91,9 @@
                 return this.getNextAvailableDesktopStream();
             }
 
-            //if we get to this point there are no available streams for the
-            //type of device the client has connected with so just return
-            //the next available stream - first we'll try the desktop streams
+            // If we get to this point there are no available streams for the
+            // type of device the client has connected with so just return
+            // the next available stream - first we'll try the desktop streams
             var desktopStream = this.getNextAvailableDesktopStream();
             if (desktopStream) {
                 return desktopStream;
@@ -90,6 +103,8 @@
 
         }
 
+        // Gets and returns the next available mobile stream from the queue,
+        // but adds it back to the end of the queue to be recycled.
         MusesPlayer.prototype.getNextAvailableMobileStream = function() {
             var stream = this.availableMobileStreamQueue.shift();
             //add to end of queue
@@ -97,6 +112,8 @@
             return stream;
         }
 
+        // Gets and returns the next available desktop stream from the queue,
+        // but adds it back to the end of the queue to be recycled.
         MusesPlayer.prototype.getNextAvailableDesktopStream = function() {
             var stream = this.availableDesktopStreamQueue.shift();
             //add to end of queue
@@ -114,10 +131,6 @@
             togglePlayStopButton();
         };
 
-        MusesPlayer.prototype.setVolume = function(value) {
-            //this.flashDetect ? MRP.setVolume(value) : null;
-        };
-
         MusesPlayer.prototype.setURL = function(url) {
             console.log("setURL");
             MRP.setUrl(url);
@@ -129,7 +142,6 @@
                 case "ioError":
                     // connection limit reached or problem connecting to stream
                     if (value === "0") {
-                        console.log("ioError");
                         var stream = musesPlayer.getNextAvailableStream();
                         musesPlayer.setURL(stream["url"]);
                         musesPlayer.play();
@@ -137,14 +149,23 @@
             }
         }
 
+        // Triggers the play function on the Muses player object in HTML5 mode
         function musesHTMLPlayClick() {
+            /*if (MRP.html === undefined) {
+                console.log("inserting player");
+                MRP.insert(musesPlayer.settings);
+            }*/
             MRP.html.audio.src = MRP.html.src;
 
             MRP.html.audio.play();
         }
 
+        // Triggers the stop function on the Muses player object in HTML5 mode
+        // NOTE: The HTML5 audio element doesn't have stop functionality. It
+        // can only be paused.
         function musesHTMLStopClick() {
             MRP.html.audio.pause();
+            //delete MRP.html;
         }
 
         function togglePlayStopButton() {
@@ -155,12 +176,15 @@
         // default how often to fetch metadata to 20 seconds
         var time_to_next_track_starts = 20000;
 
-        function getMetadata(){
+        // Fetches the streams metadata from the Airtime live-info API
+        // and attaches it to the player UI.
+        //
+        // The metadata is fetched when the current track is about to end.
+        function attachStreamMetadataToPlayer(){
             $.ajax({url: "<?php echo $this->metadata_api_url?>",
                 data: {type:"interval",limit:"5"},
                 dataType: "jsonp",
                 success: function(data) {
-                    //console.log("hello");
 
                     if (data.current === null) {
                         $("p.now_playing").html("Offline");
@@ -191,14 +215,30 @@
 
                 }
             });
-            setTimeout(getMetadata, time_to_next_track_starts);
+            setTimeout(attachStreamMetadataToPlayer, time_to_next_track_starts);
+        }
+
+        function htmlEscape(str) {
+            return String(str)
+                .replace(/&/g, '&amp;')
+                .replace(/"/g, '&quot;')
+                .replace(/'/g, '&#39;')
+                .replace(/</g, '&lt;')
+                .replace(/>/g, '&gt;');
         }
 
     </script>
 
     <style type="text/css">
+        /*
+        We have to have the default Muses skin displayed on the page or else
+        the player will not work. Setting the display to none and hidden does
+        not work. It has to be "visible" on the page. As a hacky work around we
+        set the height and width to 1px so users will not see it.
+        */
         #muses_skin{width:1px; height:1px; overflow-x: hidden; overflow-y: hidden;}
     </style>
+
 </head>
 
 <body>
@@ -219,7 +259,8 @@
 
     </div>
 
-    <!--<div class="airtime_volume">
+    <!--
+    <div class="airtime_volume">
 
         <div class="volume_control">
             <span class="mute"></span>
@@ -229,7 +270,8 @@
             <div class="airtime_volume_bar_value" style="width: 80%;"></div>
         </div>
 
-    </div>-->
+    </div>
+    -->
     <div style="clear:both"></div>
 
     <div class="airtime_schedule">
@@ -240,7 +282,6 @@
     </div>
     <a class="airtime_pro" target="_blank" href="https://www.airtime.pro/">Powered by Airtime Pro<span class="airtime_pro_logo"></span></a>
 
-
 </div>
 
 <div id="muses_skin">