Small error in the console after clicking inside a loaded video

Arkshine · October 26, 2023, 11:15pm

Related to:

DEV: Prevent videos from preloading metadata by oblakeerickson · Pull Request #23807 · discourse/discourse · GitHub
DEV: Show a loading spinner on video placeholders by oblakeerickson · Pull Request #24039 · discourse/discourse · GitHub

Here is a small video showing the issue:

It looks like removing the click listener here doesn’t work. The next time a click happens, the video-placeholder-container doesn’t exist anymore:

github.com

discourse/discourse/blob/main/app/assets/javascripts/discourse/app/instance-initializers/video-placeholder.js#L16


      
          
          export default {
            initialize(owner) {
              withPluginApi("0.8.7", (api) => {
                function handleVideoPlaceholderClick(helper, event) {
                  const parentDiv = event.target.closest(".video-placeholder-container");
                  const wrapper = parentDiv.querySelector(".video-placeholder-wrapper");
                  const overlay = wrapper.querySelector(".video-placeholder-overlay");
          
                  parentDiv.style.cursor = "";
                  overlay.innerHTML = spinnerHTML;
          
                  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");

Probably because of bind usage here:

github.com

discourse/discourse/blob/main/app/assets/javascripts/discourse/app/instance-initializers/video-placeholder.js#L85


      
          if (!helper) {
            return;
          }
          
          const containers = post.querySelectorAll(
            ".video-placeholder-container"
          );
          
          containers.forEach((container) => {
            // Add video thumbnail image
            if (container.dataset.thumbnailSrc) {
              const thumbnail = new Image();
              thumbnail.onload = function () {
                container.style.backgroundImage = "url('" + thumbnail.src + "')";
              };
              thumbnail.src = container.dataset.thumbnailSrc;
            }
          
            const wrapper = document.createElement("div"),
              overlay = document.createElement("div");

Maybe an easy solution would be to use { once: true } in addEventListener instead?

Arkshine · October 27, 2023, 4:05pm

Thanks for the fix @blake!

blake · October 27, 2023, 4:07pm

My pleasure! Thank you for reporting it and for the suggestion

Topic		Replies	Views
Paused/finished YouTube videos automatically start playing when scrolling up far enough to load new posts Bug	40	10021	February 13, 2023
Media Overlay component Theme component	40	6728	January 12, 2024
Pixel jump whenever page refreshes (mobile & desktop) UX	2	649	July 13, 2022
[PAID] Discourse Chat Bug Fixes + Enhancements Marketplace	9	226	January 10, 2025
Render a component within a Widget. (Using select-kit components within plugin code) Dev	31	3194	March 6, 2024

Small error in the console after clicking inside a loaded video

Related topics