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
window.addEventListener('load', functionName) not working properly dev	1	9694	March 30, 2018
Trouble with `tests/test_helper`? bug	2	687	April 26, 2021
Category-selector.js:73 - Uncaught TypeError: this.attrs.onChange is not a function bug	11	666	November 12, 2023
Pixel jump whenever page refreshes (mobile & desktop) ux	2	644	July 13, 2022
Where's the pumpkin? support halloween-decorations	3	138	November 8, 2023

Small error in the console after clicking inside a loaded video

Related Topics