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
This commit is contained in:
Blake Erickson 2024-08-22 13:35:18 -06:00 committed by GitHub
parent 842d2749a1
commit b53df4d884
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 43 additions and 3 deletions

View File

@ -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;

View File

@ -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