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
Sticky see new or updated topic notification Support	3	652	October 28, 2021
Paused/finished YouTube videos automatically start playing when scrolling up far enough to load new posts Bug	40	9951	February 13, 2023
Media Overlay component Theme component	40	6693	January 12, 2024
Found a workable CSS hack to show spinner/loader on slow initial load UX	6	3032	May 15, 2018
Pixel jump whenever page refreshes (mobile & desktop) UX	2	648	July 13, 2022

Small error in the console after clicking inside a loaded video

Related topics