From 0dea99115677a21a06ae1a9007289b22fd8bd14f Mon Sep 17 00:00:00 2001
From: Blake Erickson <o.blakeerickson@gmail.com>
Date: Fri, 28 Apr 2023 12:14:36 -0600
Subject: [PATCH] FIX: Empty video thumbnails (#21290)

* FIX: Empty video thumbnails

This fix ensures that topic video thumbnail generation is completed
before the composer is allowed to submit which should prevent some bugs
around missing thumbnails on video topics.

* move callback to on upload-success
---
 .../app/mixins/composer-upload-uppy.js        | 42 +++++++++----------
 .../mixins/composer-video-thumbnail-uppy.js   | 12 +++---
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
index 9b5856178b2..b0a7ac5fef5 100644
--- a/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
+++ b/app/assets/javascripts/discourse/app/mixins/composer-upload-uppy.js
@@ -325,32 +325,28 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
 
         cacheShortUploadUrl(upload.short_url, upload);
 
-        // video/mp4, video/webm, video/quicktime, etc.
-        if (file.type.split("/")[0] === "video") {
-          this._generateVideoThumbnail(file, upload.url);
-        }
-
-        if (this.useUploadPlaceholders) {
+        this._generateVideoThumbnail(file, upload.url, () => {
+          if (this.useUploadPlaceholders) {
+            this.appEvents.trigger(
+              `${this.composerEventPrefix}:replace-text`,
+              this.placeholders[file.id].uploadPlaceholder.trim(),
+              markdown
+            );
+          }
+          this._resetUpload(file, { removePlaceholder: false });
           this.appEvents.trigger(
-            `${this.composerEventPrefix}:replace-text`,
-            this.placeholders[file.id].uploadPlaceholder.trim(),
-            markdown
+            `${this.composerEventPrefix}:upload-success`,
+            file.name,
+            upload
           );
-        }
 
-        this._resetUpload(file, { removePlaceholder: false });
-        this.appEvents.trigger(
-          `${this.composerEventPrefix}:upload-success`,
-          file.name,
-          upload
-        );
-
-        if (this.inProgressUploads.length === 0) {
-          this.appEvents.trigger(
-            `${this.composerEventPrefix}:all-uploads-complete`
-          );
-          this._reset();
-        }
+          if (this.inProgressUploads.length === 0) {
+            this.appEvents.trigger(
+              `${this.composerEventPrefix}:all-uploads-complete`
+            );
+            this._reset();
+          }
+        });
       });
     });
 
diff --git a/app/assets/javascripts/discourse/app/mixins/composer-video-thumbnail-uppy.js b/app/assets/javascripts/discourse/app/mixins/composer-video-thumbnail-uppy.js
index d4f2f5c7834..7781149d38d 100644
--- a/app/assets/javascripts/discourse/app/mixins/composer-video-thumbnail-uppy.js
+++ b/app/assets/javascripts/discourse/app/mixins/composer-video-thumbnail-uppy.js
@@ -17,9 +17,12 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
   useUploadPlaceholders: true,
 
   @bind
-  _generateVideoThumbnail(videoFile, uploadUrl) {
+  _generateVideoThumbnail(videoFile, uploadUrl, callback) {
     if (!this.siteSettings.video_thumbnails_enabled) {
-      return;
+      return callback();
+    }
+    if (videoFile.type.split("/")[0] !== "video") {
+      return callback();
     }
     let video = document.createElement("video");
     video.src = URL.createObjectURL(videoFile.data);
@@ -45,10 +48,7 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
       // A timeout is needed on mobile.
       setTimeout(() => {
         ctx.drawImage(video, 0, 0, video.videoWidth, video.videoHeight);
-      }, 100);
 
-      // A timeout is needed on mobile.
-      setTimeout(() => {
         // upload video thumbnail
         canvas.toBlob((blob) => {
           this._uppyInstance = new Uppy({
@@ -77,6 +77,7 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
 
           this._uppyInstance.on("upload-success", () => {
             this.set("uploading", false);
+            callback();
           });
 
           this._uppyInstance.on("upload-error", (file, error, response) => {
@@ -88,6 +89,7 @@ export default Mixin.create(ExtendableUploader, UppyS3Multipart, {
             // eslint-disable-next-line no-console
             console.error(message);
             this.set("uploading", false);
+            callback();
           });
 
           try {