From fbb36ed1e60cd85a7aa1e10e304f31c8bf107ccd Mon Sep 17 00:00:00 2001
From: Marina Glancy <marina@moodle.com>
Date: Mon, 7 Nov 2016 11:33:06 +0800
Subject: [PATCH] MDL-38158 media: some players do not want fallback to other
 players

there is already logic to determine whether html5 is supported;
otherwise we may end up with nested <audio> or <video> tags
---
 lib/tests/medialib_test.php                |  2 +-
 media/classes/manager.php                  | 15 ++++----
 media/classes/player.php                   |  7 ++++
 media/player/html5audio/classes/plugin.php |  4 +-
 media/player/html5video/classes/plugin.php |  4 +-
 media/player/videojs/classes/plugin.php    |  4 +-
 media/player/videojs/tests/player_test.php | 43 ++++++++++++++++++++++
 7 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/lib/tests/medialib_test.php b/lib/tests/medialib_test.php
index 6720de9f397..d2ccdc2a9dd 100644
--- a/lib/tests/medialib_test.php
+++ b/lib/tests/medialib_test.php
@@ -217,7 +217,7 @@ class core_medialib_testcase extends advanced_testcase {
         $this->assertContains($link, $t);
 
         // Enable media players that can play the same media formats. (ie. test & html5audio for mp3 files, etc.)
-        \core\plugininfo\media::set_enabled_plugins('html5video,html5audio,test,swf');
+        \core\plugininfo\media::set_enabled_plugins('test,html5video,html5audio,swf');
         $manager = core_media_manager::instance();
 
         // Test media formats that can be played by 2 or more players.
diff --git a/media/classes/manager.php b/media/classes/manager.php
index 4982f2679be..40e7777a2ec 100644
--- a/media/classes/manager.php
+++ b/media/classes/manager.php
@@ -256,14 +256,15 @@ class core_media_manager {
             }
         }
 
-        if (empty($options[self::OPTION_FALLBACK_TO_BLANK]) || $out !== core_media_player::PLACEHOLDER) {
-            // Fallback to the link. Exception: in case of OPTION_FALLBACK_TO_BLANK and no other player matched do not fallback.
-            $text = $this->fallback_to_link($alternatives, $name, $options);
-            $out = str_replace(core_media_player::PLACEHOLDER, $text, $out);
+        if (!empty($options[self::OPTION_FALLBACK_TO_BLANK]) && $out === core_media_player::PLACEHOLDER) {
+            // In case of OPTION_FALLBACK_TO_BLANK and no player matched do not fallback to link, just return empty string.
+            return '';
         }
 
         // Remove 'fallback' slot from final version and return it.
-        $out = str_replace(core_media_player::PLACEHOLDER, '', $out);
+        $fallback = $this->fallback_to_link($alternatives, $name, $options);
+        $out = str_replace(core_media_player::PLACEHOLDER, $fallback, $out);
+        $out = str_replace(core_media_player::LINKPLACEHOLDER, $fallback, $out);
         if (!empty($options[self::OPTION_BLOCK]) && $out !== '') {
             $out = html_writer::tag('div', $out, array('class' => 'resourcecontent'));
         }
@@ -280,7 +281,7 @@ class core_media_manager {
      */
     protected function fallback_to_link($urls, $name, $options) {
         // If link is turned off, return empty.
-        if (!empty($options[core_media_manager::OPTION_NO_LINK])) {
+        if (!empty($options[self::OPTION_NO_LINK])) {
             return '';
         }
 
@@ -290,7 +291,7 @@ class core_media_manager {
             if (strval($name) !== '' && $output === '') {
                 $title = $name;
             } else {
-                $title = core_media_manager::instance()->get_filename($url);
+                $title = $this->get_filename($url);
             }
             $printlink = html_writer::link($url, $title, array('class' => 'mediafallbacklink'));
             if ($output) {
diff --git a/media/classes/player.php b/media/classes/player.php
index 9afdce0962f..f7d50ada966 100644
--- a/media/classes/player.php
+++ b/media/classes/player.php
@@ -53,6 +53,13 @@ abstract class core_media_player {
      */
     const PLACEHOLDER = '<!--FALLBACK-->';
 
+    /**
+     * Placeholder text used to indicate where the link fallback is placed.
+     * No other players will apply to it but it will be converted to the link in the
+     * end (unless prevented by OPTION_NO_LINK).
+     */
+    const LINKPLACEHOLDER = '<!--LINKFALLBACK-->';
+
     /**
      * Generates code required to embed the player.
      *
diff --git a/media/player/html5audio/classes/plugin.php b/media/player/html5audio/classes/plugin.php
index f501c81472a..2b375075169 100644
--- a/media/player/html5audio/classes/plugin.php
+++ b/media/player/html5audio/classes/plugin.php
@@ -65,7 +65,9 @@ class media_html5audio_plugin extends core_media_player_native {
             $size = 'width="' . $width . '"';
         }
 
-        $fallback = core_media_player::PLACEHOLDER;
+        // We don't want fallback to another player because list_supported_urls() is already smart.
+        // Otherwise we could end up with nested <audio> tags. Fallback to link only.
+        $fallback = self::LINKPLACEHOLDER;
 
         return <<<OET
 <audio controls="true" $size class="mediaplugin mediaplugin_html5audio" preload="none" title="$title">
diff --git a/media/player/html5video/classes/plugin.php b/media/player/html5video/classes/plugin.php
index 8a871c74b0c..6c62b185337 100644
--- a/media/player/html5video/classes/plugin.php
+++ b/media/player/html5video/classes/plugin.php
@@ -93,7 +93,9 @@ document.getElementById('$id').addEventListener('click', function() {
 OET;
         }
 
-        $fallback = core_media_player::PLACEHOLDER;
+        // We don't want fallback to another player because list_supported_urls() is already smart.
+        // Otherwise we could end up with nested <video> tags. Fallback to link only.
+        $fallback = self::LINKPLACEHOLDER;
         return <<<OET
 <span class="mediaplugin mediaplugin_html5video">
 <video $idtag controls="true" $size preload="metadata" title="$title">
diff --git a/media/player/videojs/classes/plugin.php b/media/player/videojs/classes/plugin.php
index 91bb5311ae6..f9885714ff7 100644
--- a/media/player/videojs/classes/plugin.php
+++ b/media/player/videojs/classes/plugin.php
@@ -159,8 +159,10 @@ class media_videojs_plugin extends core_media_player_native {
             $text = self::replace_sources($text, $sources);
         } else {
             // Create <video> or <audio> tag with necessary attributes and all sources.
+            // We don't want fallback to another player because list_supported_urls() is already smart.
+            // Otherwise we could end up with nested <audio> or <video> tags. Fallback to link only.
             $attributes += ['preload' => 'auto', 'controls' => 'true', 'title' => $title];
-            $text = html_writer::tag($isaudio ? 'audio' : 'video', $sources . self::PLACEHOLDER, $attributes);
+            $text = html_writer::tag($isaudio ? 'audio' : 'video', $sources . self::LINKPLACEHOLDER, $attributes);
         }
 
         // Limit the width of the video if width is specified.
diff --git a/media/player/videojs/tests/player_test.php b/media/player/videojs/tests/player_test.php
index 3b404d42d63..43d698c2745 100644
--- a/media/player/videojs/tests/player_test.php
+++ b/media/player/videojs/tests/player_test.php
@@ -133,6 +133,49 @@ class media_videojs_testcase extends advanced_testcase {
         $this->assertRegExp('~style="max-width:' . $CFG->media_default_width . 'px;~', $content);
     }
 
+    /**
+     * Test that only supported URLs are listed as sources but all URLs are present in links fallbacks.
+     */
+    public function test_fallback() {
+
+        $urls = [
+            new moodle_url('http://example.org/1.rv'), // Not supported.
+            new moodle_url('http://example.org/2.webm'), // Supported.
+            new moodle_url('http://example.org/3.ogv'), // Supported.
+        ];
+
+        $manager = core_media_manager::instance();
+        $content = $manager->embed_alternatives($urls, '', 0, 0, []);
+
+        $this->assertRegExp('~mediaplugin_videojs~', $content);
+        $this->assertRegExp('~</video>~', $content);
+        // Title is taken from the name of the first supported file.
+        $this->assertRegExp('~title="2"~', $content);
+        // Only supported files are in <source>'s.
+        $this->assertNotRegExp('~<source src="http://example.org/1.rv"~', $content);
+        $this->assertRegExp('~<source src="http://example.org/2.webm"~', $content);
+        $this->assertRegExp('~<source src="http://example.org/3.ogv"~', $content);
+        // Links to all files are included.
+        $this->assertRegExp('~<a class="mediafallbacklink" href="http://example.org/1.rv">1.rv</a>~', $content);
+        $this->assertRegExp('~<a class="mediafallbacklink" href="http://example.org/2.webm">2.webm</a>~', $content);
+        $this->assertRegExp('~<a class="mediafallbacklink" href="http://example.org/3.ogv">3.ogv</a>~', $content);
+    }
+
+    /**
+     * Assert other players do not apply after videojs was applied.
+     */
+    public function test_prevent_other_players() {
+        \core\plugininfo\media::set_enabled_plugins('videojs,html5video');
+        $url = new moodle_url('http://example.org/some_filename.webm');
+        $text = html_writer::link($url, 'Apply one player only');
+        $content = format_text($text, FORMAT_HTML);
+
+        $this->assertRegExp('~mediaplugin_videojs~', $content);
+        $this->assertEquals(1, substr_count($content, '</video>'));
+        $this->assertNotRegExp('~mediaplugin_html5video~', $content);
+        $this->assertRegExp('~<a class="mediafallbacklink" href="http://example.org/some_filename.webm">Apply one player only</a>~', $content);
+    }
+
     /**
      * Test that mediaplugin filter adds player code on top of <video> tags.
      *