From b53df4d884aae4624d6655abc0361b2f530bcef2 Mon Sep 17 00:00:00 2001
From: Blake Erickson <o.blakeerickson@gmail.com>
Date: Thu, 22 Aug 2024 13:35:18 -0600
Subject: [PATCH] FIX: Videos not uploading due to thumbnail generation error
(#28493)
If we don't get a `videoWidth` back for a video don't try and generate a
thumbnail for it.
Also as part of this change I switched getImageData, the function
throwing the error, to use video.videoWidth instead of canvas.width
because it's very likely we were setting canvas.width too early before
the width could be read. Now that we are reading the value inside of the
setTimeout hopefully we will actually have a width. Just incase we don't
detect a width we will now exit early instead of throwing an error.
We only need to check for `0` and not null because the value is an
integer and will always return a 0 if it can't be read. https://developer.mozilla.org/en-US/docs/Web/API/HTMLVideoElement/videoWidth
See https://meta.discourse.org/t/322363
---
.../mixins/composer-video-thumbnail-uppy.js | 17 +++++++++--
spec/system/composer_uploads_spec.rb | 29 +++++++++++++++++++
2 files changed, 43 insertions(+), 3 deletions(-)
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 959fdfd38c6..4e19080b77c 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
@@ -70,14 +70,25 @@ export default class ComposerVideoThumbnailUppy extends EmberObject.extend(
video[eventName] = () => {
const canvas = document.createElement("canvas");
const ctx = canvas.getContext("2d");
- canvas.width = video.videoWidth;
- canvas.height = video.videoHeight;
// A timeout is needed on mobile.
setTimeout(() => {
+ // If dimensions can't be read, abort.
+ if (video.videoWidth === 0) {
+ return callback();
+ }
+
+ canvas.width = video.videoWidth;
+ canvas.height = video.videoHeight;
ctx.drawImage(video, 0, 0, video.videoWidth, video.videoHeight);
+
// Detect Empty Thumbnail
- const imageData = ctx.getImageData(0, 0, canvas.width, canvas.height);
+ const imageData = ctx.getImageData(
+ 0,
+ 0,
+ video.videoWidth,
+ video.videoHeight
+ );
const data = imageData.data;
let isEmpty = true;
diff --git a/spec/system/composer_uploads_spec.rb b/spec/system/composer_uploads_spec.rb
index 21a9d68e534..235a268c318 100644
--- a/spec/system/composer_uploads_spec.rb
+++ b/spec/system/composer_uploads_spec.rb
@@ -88,6 +88,35 @@ describe "Uploading files in the composer", type: :system do
end
end
+ it "handles a video where dimensions can't be read gracefully" do
+ visit "/new-topic"
+ expect(composer).to be_opened
+ topic.fill_in_composer_title("Zero Width Video Test")
+
+ # Inject JavaScript to mock video dimensions
+ page.execute_script <<-JS
+ HTMLVideoElement.prototype.__defineGetter__('videoWidth', function() { return 0; });
+ HTMLVideoElement.prototype.__defineGetter__('videoHeight', function() { return 0; });
+ JS
+
+ file_path_1 = file_from_fixtures("small.mp4", "media").path
+ attach_file(file_path_1) { composer.click_toolbar_button("upload") }
+
+ expect(composer).to have_no_in_progress_uploads
+ expect(composer.preview).to have_css(".onebox-placeholder-container")
+
+ composer.submit
+
+ expect(find("#topic-title")).to have_content("Zero Width Video Test")
+
+ selector = topic.post_by_number_selector(1)
+
+ expect(page).to have_css(selector)
+ within(selector) do
+ expect(page).to have_no_css(".video-placeholder-container[data-thumbnail-src]")
+ end
+ end
+
it "shows video player in composer" do
SiteSetting.enable_diffhtml_preview = true