DEV: Prevent videos from preloading metadata (#23807)

Preloading just metadata is not always respected by browsers, and sometimes the whole video will be downloaded. This switches to using a placeholder image for the video and only loads the video when the play button is clicked.
2023-10-12 13:47:48 -06:00 · 2023-10-12 13:47:48 -06:00 · 2443446e62
parent 460e702887
commit 2443446e62
11 changed files with 215 additions and 70 deletions
--- a/app/assets/javascripts/discourse/app/instance-initializers/post-decorations.js
+++ b/app/assets/javascripts/discourse/app/instance-initializers/post-decorations.js
@ -9,7 +9,6 @@ import { SELECTORS } from "discourse/lib/lightbox/constants";
 import { withPluginApi } from "discourse/lib/plugin-api";
 import { setTextDirections } from "discourse/lib/text-direction";
 import { iconHTML, iconNode } from "discourse-common/lib/icon-library";
-import discourseLater from "discourse-common/lib/later";
 import I18n from "I18n";

 export default {
@ -82,30 +81,6 @@ export default {
        });
      });

-      const caps = owner.lookup("service:capabilities");
-      if (caps.isSafari || caps.isIOS) {
-        api.decorateCookedElement(
-          (elem) => {
-            elem.querySelectorAll("video").forEach((video) => {
-              if (video.poster && video.poster !== "" && !video.autoplay) {
-                return;
-              }
-
-              const source = video.querySelector("source");
-              if (source) {
-                // In post-cooked.js, we create the video element in a detached DOM
-                // then adopt it into to the real DOM.
-                // This confuses safari, and preloading/autoplay do not happen.
-
-                // Calling `.load()` tricks Safari into loading the video element correctly
-                source.parentElement.load();
-              }
-            });
-          },
-          { afterAdopt: true, onlyStream: true }
-        );
-      }
-
      const oneboxTypes = {
        amazon: "discourse-amazon",
        githubactions: "fab-github",
@ -131,28 +106,6 @@ export default {
        });
      });

-      api.decorateCookedElement((element) => {
-        element
-          .querySelectorAll(".video-container")
-          .forEach((videoContainer) => {
-            const video = videoContainer.getElementsByTagName("video")[0];
-            video.addEventListener("loadeddata", () => {
-              discourseLater(() => {
-                if (video.videoWidth === 0 || video.videoHeight === 0) {
-                  const notice = document.createElement("div");
-                  notice.className = "notice";
-                  notice.innerHTML =
-                    iconHTML("exclamation-triangle") +
-                    " " +
-                    I18n.t("cannot_render_video");
-
-                  videoContainer.appendChild(notice);
-                }
-              }, 500);
-            });
-          });
-      });
-
      function _createButton() {
        const openPopupBtn = document.createElement("button");
        openPopupBtn.classList.add(
--- a/app/assets/javascripts/discourse/app/instance-initializers/video-placeholder.js
+++ b/app/assets/javascripts/discourse/app/instance-initializers/video-placeholder.js
@ -0,0 +1,92 @@
+import { withPluginApi } from "discourse/lib/plugin-api";
+import { iconHTML } from "discourse-common/lib/icon-library";
+import discourseLater from "discourse-common/lib/later";
+import I18n from "I18n";
+
+export default {
+  initialize(owner) {
+    withPluginApi("0.8.7", (api) => {
+      function handleVideoPlaceholderClick(helper, event) {
+        const parentDiv = event.target.closest(".video-placeholder-container");
+        const wrapper = event.target.closest(".video-placeholder-wrapper");
+
+        const videoHTML = `
+        <video width="100%" height="100%" preload="metadata" controls style="display:none">
+          <source src="${parentDiv.dataset.videoSrc}" ${parentDiv.dataset.origSrc}>
+          <a href="${parentDiv.dataset.videoSrc}">${parentDiv.dataset.videoSrc}</a>
+        </video>`;
+        parentDiv.insertAdjacentHTML("beforeend", videoHTML);
+        parentDiv.classList.add("video-container");
+
+        const video = parentDiv.querySelector("video");
+
+        const caps = owner.lookup("service:capabilities");
+        if (caps.isSafari || caps.isIOS) {
+          const source = video.querySelector("source");
+          if (source) {
+            // In post-cooked.js, we create the video element in a detached DOM
+            // then adopt it into to the real DOM.
+            // This confuses safari, and preloading/autoplay do not happen.
+
+            // Calling `.load()` tricks Safari into loading the video element correctly
+            source.parentElement.load();
+          }
+        }
+
+        video.addEventListener("loadeddata", () => {
+          discourseLater(() => {
+            if (video.videoWidth === 0 || video.videoHeight === 0) {
+              const notice = document.createElement("div");
+              notice.className = "notice";
+              notice.innerHTML =
+                iconHTML("exclamation-triangle") +
+                " " +
+                I18n.t("cannot_render_video");
+
+              parentDiv.appendChild(notice);
+            }
+          }, 500);
+        });
+
+        video.addEventListener("canplay", function () {
+          video.play();
+          wrapper.remove();
+          video.style.display = "";
+          parentDiv.classList.remove("video-placeholder-container");
+        });
+      }
+
+      function applyVideoPlaceholder(post, helper) {
+        if (!helper) {
+          return;
+        }
+
+        const containers = post.querySelectorAll(
+          ".video-placeholder-container"
+        );
+
+        containers.forEach((container) => {
+          const wrapper = document.createElement("div"),
+            overlay = document.createElement("div");
+
+          wrapper.classList.add("video-placeholder-wrapper");
+          container.appendChild(wrapper);
+
+          overlay.classList.add("video-placeholder-overlay");
+          overlay.style.cursor = "pointer";
+          overlay.addEventListener(
+            "click",
+            handleVideoPlaceholderClick.bind(null, helper),
+            false
+          );
+          overlay.innerHTML = `${iconHTML("play")}`;
+          wrapper.appendChild(overlay);
+        });
+      }
+
+      api.decorateCookedElement(applyVideoPlaceholder, {
+        onlyStream: true,
+      });
+    });
+  },
+};
--- a/app/assets/javascripts/discourse/tests/acceptance/video-placeholder-test.js
+++ b/app/assets/javascripts/discourse/tests/acceptance/video-placeholder-test.js
@ -0,0 +1,27 @@
+import { click, visit } from "@ember/test-helpers";
+import { test } from "qunit";
+import { acceptance } from "discourse/tests/helpers/qunit-helpers";
+
+acceptance("Video Placeholder Test", function () {
+  test("placeholder shows up on posts with videos", async function (assert) {
+    await visit("/t/54081");
+
+    const postWithVideo = document.querySelector(
+      ".video-placeholder-container"
+    );
+    assert.ok(
+      postWithVideo.hasAttribute("data-video-src"),
+      "Video placeholder should have the 'data-video-src' attribute"
+    );
+
+    const overlay = postWithVideo.querySelector(".video-placeholder-overlay");
+
+    assert.dom("video").doesNotExist("The video element does not exist yet");
+
+    await click(overlay);
+
+    assert.dom(".video-container").exists("The video container appears");
+
+    assert.dom("video").exists("The video element appears");
+  });
+});
--- a/app/assets/javascripts/discourse/tests/fixtures/topic.js
+++ b/app/assets/javascripts/discourse/tests/fixtures/topic.js
@ -6887,6 +6887,55 @@ export default {
        can_view_edit_history: true,
        wiki: false,
      },
+      {
+        id: 21,
+        username: "eviltrout",
+        avatar_template: "/images/avatar.png",
+        name: "Evil Trout",
+        uploaded_avatar_id: 9,
+        created_at: "2015-08-13T14:49:23.927Z",
+        cooked:
+          '<div class="video-placeholder-container" data-video-src="/uploads/default/original/1X/55508bc98a00f615dbe9bd4c84a253ba4238b021.mp4"></div>',
+        post_number: 4,
+        post_type: 1,
+        updated_at: "2015-08-13T14:49:23.927Z",
+        reply_count: 0,
+        reply_to_post_number: null,
+        quote_count: 0,
+        incoming_link_count: 0,
+        reads: 1,
+        score: 0,
+        yours: true,
+        topic_id: 9,
+        topic_slug: "this-is-a-test-topic",
+        display_username: "",
+        primary_group_name: null,
+        version: 1,
+        can_edit: true,
+        can_delete: true,
+        can_recover: true,
+        read: true,
+        user_title: null,
+        actions_summary: [
+          { id: 3, can_act: true },
+          { id: 4, can_act: true },
+          { id: 5, hidden: true, can_act: true },
+          { id: 7, can_act: true },
+          { id: 8, can_act: true },
+        ],
+        moderator: false,
+        admin: true,
+        staff: true,
+        user_id: 1,
+        hidden: false,
+        hidden_reason_id: null,
+        trust_level: 4,
+        deleted_at: null,
+        user_deleted: false,
+        edit_reason: null,
+        can_view_edit_history: true,
+        wiki: false,
+      },
      ],
      stream: [398, 419],
      gaps: { before: {}, after: { 398: [419] } },
@ -6894,7 +6943,7 @@ export default {
    id: 54081,
    title: "This is a topic with tables!",
    fancy_title: "This is a topic with tables!",
-    posts_count: 2,
+    posts_count: 4,
    created_at: "2013-02-05T21:29:00.174Z",
    views: 5211,
    reply_count: 1,
--- a/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js
+++ b/app/assets/javascripts/discourse/tests/unit/lib/pretty-text-test.js
@ -1017,11 +1017,7 @@ eviltrout</p>
  test("video", function (assert) {
    assert.cooked(
      "![baby shark|video](upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4)",
-      `<p><div class="video-container">
-    <video width="100%" height="100%" preload="metadata" controls>
-      <source src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4">
-      <a href="/404">/404</a>
-    </video>
+      `<p><div class="video-placeholder-container" data-video-src="/404" data-orig-src="upload://eyPnj7UzkU0AkGkx2dx8G4YM1Jx.mp4">
  </div></p>`,
      "It returns the correct video player HTML"
    );
@ -1043,11 +1039,7 @@ eviltrout</p>
        siteSettings: { secure_uploads: true },
        lookupUploadUrls,
      },
-      `<p><div class="video-container">
-    <video width="100%" height="100%" preload="metadata" controls>
-      <source src="/secure-uploads/original/3X/c/b/test.mp4">
-      <a href="/secure-uploads/original/3X/c/b/test.mp4">/secure-uploads/original/3X/c/b/test.mp4</a>
-    </video>
+      `<p><div class="video-placeholder-container" data-video-src="/secure-uploads/original/3X/c/b/test.mp4">
  </div></p>`,
      "It returns the correct video HTML when the URL is mapped with secure uploads, removing data-orig-src"
    );
--- a/app/assets/javascripts/pretty-text/addon/allow-lister.js
+++ b/app/assets/javascripts/pretty-text/addon/allow-lister.js
@ -197,6 +197,7 @@ export const DEFAULT_LIST = [
  "span.excerpt",
  "div.excerpt",
  "div.video-container",
+  "div.video-placeholder-container",
  "div.onebox-placeholder-container",
  "span.placeholder-icon video",
  "span.hashtag",
--- a/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js
+++ b/app/assets/javascripts/pretty-text/addon/engines/discourse-markdown-it.js
@ -159,11 +159,7 @@ function videoHTML(token) {
  const src = token.attrGet("src");
  const origSrc = token.attrGet("data-orig-src");
  const dataOrigSrcAttr = origSrc !== null ? `data-orig-src="${origSrc}"` : "";
-  return `<div class="video-container">
-    <video width="100%" height="100%" preload="metadata" controls>
-      <source src="${src}" ${dataOrigSrcAttr}>
-      <a href="${src}">${src}</a>
-    </video>
+  return `<div class="video-placeholder-container" data-video-src="${src}" ${dataOrigSrcAttr}>
  </div>`;
 }

@ -191,13 +187,26 @@ function renderImageOrPlayableMedia(tokens, idx, options, env, slf) {
  if (split[1] === "video") {
    if (
      options.discourse.previewing &&
-      !options.discourse.limitedSiteSettings.enableDiffhtmlPreview
+      options.discourse.limitedSiteSettings.enableDiffhtmlPreview
    ) {
-      return `<div class="onebox-placeholder-container">
+      const src = token.attrGet("src");
+      const origSrc = token.attrGet("data-orig-src");
+      const dataOrigSrcAttr =
+        origSrc !== null ? `data-orig-src="${origSrc}"` : "";
+      return `<div class="video-container">
+        <video width="100%" height="100%" preload="metadata" controls>
+          <source src="${src}" ${dataOrigSrcAttr}>
+          <a href="${src}">${src}</a>
+        </video>
+        </div>`;
+    } else {
+      if (options.discourse.previewing) {
+        return `<div class="onebox-placeholder-container">
        <span class="placeholder-icon video"></span>
      </div>`;
-    } else {
-      return videoHTML(token);
+      } else {
+        return videoHTML(token);
+      }
    }
  } else if (split[1] === "audio") {
    return audioHTML(token);
--- a/app/assets/stylesheets/common/base/onebox.scss
+++ b/app/assets/stylesheets/common/base/onebox.scss
@ -919,6 +919,23 @@ aside.onebox.mixcloud-preview {
    height: 100%;
  }
 }
+.video-placeholder-container {
+  position: relative;
+  padding: 0 0 56.25% 0;
+  width: 100%;
+  background-color: black;
+
+  .video-placeholder-overlay {
+    position: absolute;
+    left: 50%;
+    top: 50%;
+    transform: translate(-50%, -50%);
+    transition: 150ms;
+    background-repeat: no-repeat;
+    background-position: center;
+    max-width: 30%;
+  }
+}

 iframe.vimeo-onebox {
  width: 100%;
--- a/lib/pretty_text.rb
+++ b/lib/pretty_text.rb
@ -342,11 +342,14 @@ module PrettyText
    allowed_pattern = allowed_src_pattern

    doc
-      .css("img[src], source[src], source[srcset], track[src]")
+      .css("img[src], source[src], source[srcset], track[src], div[data-video-src]")
      .each do |el|
        if el["src"] && !el["src"].match?(allowed_pattern)
          el[PrettyText::BLOCKED_HOTLINKED_SRC_ATTR] = el.delete("src")
        end
+        if el["data-video-src"] && !el["data-video-src"].match?(allowed_pattern)
+          el[PrettyText::BLOCKED_HOTLINKED_SRC_ATTR] = el["data-video-src"]
+        end

        if el["srcset"]
          srcs = el["srcset"].split(",").map { |e| e.split(" ", 2)[0].presence }
--- a/spec/integration/blocked_hotlinked_media_spec.rb
+++ b/spec/integration/blocked_hotlinked_media_spec.rb
@ -48,7 +48,7 @@ RSpec.describe "hotlinked media blocking" do
      post = Fabricate(:post, raw: "![alt text|video](#{hotlinked_url})")
      expect(post.cooked).not_to have_tag("video source[src]")
      expect(post.cooked).to have_tag(
-        "video source",
+        "div",
        with: {
          PrettyText::BLOCKED_HOTLINKED_SRC_ATTR => hotlinked_url,
        },
--- a/spec/system/composer/category_templates_spec.rb
+++ b/spec/system/composer/category_templates_spec.rb
@ -325,7 +325,9 @@ describe "Composer Form Templates", type: :system do
    )
    expect(find("#{topic_page.post_by_number_selector(1)} .cooked")).to have_css("a.attachment")
    expect(find("#{topic_page.post_by_number_selector(1)} .cooked")).to have_css("audio")
-    expect(find("#{topic_page.post_by_number_selector(1)} .cooked")).to have_css("video")
+    expect(find("#{topic_page.post_by_number_selector(1)} .cooked")).to have_css(
+      ".video-placeholder-container",
+    )
  end

  it "shows labels and descriptions when a form template is assigned to the category" do