From 36bad0c31f6fb1afce2d311b72f5f9661be6af97 Mon Sep 17 00:00:00 2001
From: Penar Musaraj <pmusaraj@gmail.com>
Date: Wed, 15 Jul 2020 18:36:51 -0400
Subject: [PATCH] FIX: Preload metadata for audio/video when secure media
 enabled

Fixes an issue with missing video previews. Should have no side effects now that presigned URLs expire after 5 minutes.
---
 .../addon/engines/discourse-markdown-it.js    | 15 +++------
 test/javascripts/lib/pretty-text-test.js      | 31 ++-----------------
 2 files changed, 6 insertions(+), 40 deletions(-)

diff --git a/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js b/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js
index 358595d2485..5e7ef24c2ac 100644
--- a/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js
+++ b/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js
@@ -144,23 +144,21 @@ export function extractDataAttribute(str) {
 function videoHTML(token, opts) {
   const src = token.attrGet("src");
   const origSrc = token.attrGet("data-orig-src");
-  const preloadType = opts.secureMedia ? "none" : "metadata";
   const dataOrigSrcAttr = origSrc !== null ? `data-orig-src="${origSrc}"` : "";
   return `<div class="video-container">
     <p class="video-description">${opts.alt}</p>
-    <video width="100%" height="100%" preload="${preloadType}" controls>
+    <video width="100%" height="100%" preload="metadata" controls>
       <source src="${src}" ${dataOrigSrcAttr}>
       <a href="${src}">${src}</a>
     </video>
   </div>`;
 }
 
-function audioHTML(token, opts) {
+function audioHTML(token) {
   const src = token.attrGet("src");
   const origSrc = token.attrGet("data-orig-src");
-  const preloadType = opts.secureMedia ? "none" : "metadata";
   const dataOrigSrcAttr = origSrc !== null ? `data-orig-src="${origSrc}"` : "";
-  return `<audio preload="${preloadType}" controls>
+  return `<audio preload="metadata" controls>
     <source src="${src}" ${dataOrigSrcAttr}>
     <a href="${src}">${src}</a>
   </audio>`;
@@ -177,13 +175,12 @@ function renderImageOrPlayableMedia(tokens, idx, options, env, slf) {
   // see https://github.com/markdown-it/markdown-it/blob/master/docs/architecture.md#renderer
   // handles |video and |audio alt transformations for image tags
   const mediaOpts = {
-    secureMedia: options.discourse.limitedSiteSettings.secureMedia,
     alt: split[0]
   };
   if (split[1] === "video") {
     return videoHTML(token, mediaOpts);
   } else if (split[1] === "audio") {
-    return audioHTML(token, mediaOpts);
+    return audioHTML(token);
   }
 
   // parsing ![myimage|500x300]() or ![myimage|75%]() or ![myimage|500x300, 75%]
@@ -342,10 +339,6 @@ export function setup(opts, siteSettings, state) {
   opts.discourse = copy;
   getOptions.f = () => opts.discourse;
 
-  opts.discourse.limitedSiteSettings = {
-    secureMedia: siteSettings.secure_media
-  };
-
   opts.engine = window.markdownit({
     discourse: opts.discourse,
     html: true,
diff --git a/test/javascripts/lib/pretty-text-test.js b/test/javascripts/lib/pretty-text-test.js
index c1304f3657d..393b037a962 100644
--- a/test/javascripts/lib/pretty-text-test.js
+++ b/test/javascripts/lib/pretty-text-test.js
@@ -1025,33 +1025,6 @@ QUnit.test("attachment - mapped url - secure media enabled", assert => {
   );
 });
 
-QUnit.test("video - secure media enabled", assert => {
-  assert.cookedOptions(
-    "![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)",
-    { siteSettings: { secure_media: true } },
-    `<p><div class="video-container">
-    <p class="video-description">baby shark</p>
-    <video width="100%" height="100%" preload="none" controls>
-      <source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4">
-      <a href="/404">/404</a>
-    </video>
-  </div></p>`,
-    "It returns the correct video player HTML"
-  );
-});
-
-QUnit.test("audio - secure media enabled", assert => {
-  assert.cookedOptions(
-    "![young americans|audio](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3)",
-    { siteSettings: { secure_media: true } },
-    `<p><audio preload="none" controls>
-    <source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp3">
-    <a href="/404">/404</a>
-  </audio></p>`,
-    "It returns the correct audio player HTML"
-  );
-});
-
 QUnit.test("video", assert => {
   assert.cooked(
     "![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)",
@@ -1084,7 +1057,7 @@ QUnit.test("video - mapped url - secure media enabled", assert => {
     },
     `<p><div class="video-container">
     <p class="video-description">baby shark</p>
-    <video width="100%" height="100%" preload="none" controls>
+    <video width="100%" height="100%" preload="metadata" controls>
       <source src="/secure-media-uploads/original/3X/c/b/test.mp4">
       <a href="/secure-media-uploads/original/3X/c/b/test.mp4">/secure-media-uploads/original/3X/c/b/test.mp4</a>
     </video>
@@ -1120,7 +1093,7 @@ QUnit.test("audio - mapped url - secure media enabled", assert => {
       siteSettings: { secure_media: true },
       lookupUploadUrls: lookupUploadUrls
     },
-    `<p><audio preload="none" controls>
+    `<p><audio preload="metadata" controls>
     <source src="/secure-media-uploads/original/3X/c/b/test.mp3">
     <a href="/secure-media-uploads/original/3X/c/b/test.mp3">/secure-media-uploads/original/3X/c/b/test.mp3</a>
   </audio></p>`,