Fix incomplete traversal of clipboard data when an image is present resulting in Not an Image error (#5156)

CHANGELOG.md

@@ -87,6 +87,7 @@
 - Bugfix: Reply contexts now use the color of the replied-to message. (#5145)
 - Bugfix: Fixed top-level window getting stuck after opening settings. (#5161, #5166)
 - Bugfix: Fixed link info not updating without moving the cursor. (#5178)
+- Bugfix: Fixed an upload sometimes failing when copying an image from a browser if it contained extra properties. (#5156)
 - Bugfix: Fixed tooltips getting out of bounds when loading images. (#5186)
 - Dev: Run miniaudio in a separate thread, and simplify it to not manage the device ourselves. There's a chance the simplification is a bad idea. (#4978)
 - Dev: Change clang-format from v14 to v16. (#4929)

src/singletons/ImageUploader.cpp

@@ -238,7 +238,7 @@ void ImageUploader::handleSuccessfulUpload(const NetworkResult &result,
     }
     else
     {
-        QTimer::singleShot(UPLOAD_DELAY, [channel, &textEdit, this]() {
+        QTimer::singleShot(UPLOAD_DELAY, [channel, textEdit, this]() {
             this->sendImageUploadRequest(this->uploadQueue_.front(), channel,
                                          textEdit);
             this->uploadQueue_.pop();
@@ -259,8 +259,11 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel,
     }
 
     channel->addMessage(makeSystemMessage(QString("Started upload...")));
-    if (source->hasUrls())
+    auto tryUploadFromUrls = [&]() -> bool {
+        if (!source->hasUrls())
         {
+            return false;
+        }
         auto mimeDb = QMimeDatabase();
         // This path gets chosen when files are copied from a file manager, like explorer.exe, caja.
         // Each entry in source->urls() is a QUrl pointing to a file that was copied.
@@ -277,8 +280,7 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel,
                 {
                     channel->addMessage(
                         makeSystemMessage(QString("Couldn't load image :(")));
-                    this->uploadMutex_.unlock();
+                    return false;
-                    return;
                 }
 
                 auto imageData = convertToPng(img);
@@ -293,8 +295,7 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel,
                         QString("Cannot upload file: %1. Couldn't convert "
                                 "image to png.")
                             .arg(localPath)));
-                    this->uploadMutex_.unlock();
+                    return false;
-                    return;
                 }
             }
             else if (mime.inherits("image/gif"))
@@ -307,21 +308,12 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel,
                 {
                     channel->addMessage(
                         makeSystemMessage(QString("Failed to open file. :(")));
-                    this->uploadMutex_.unlock();
+                    return false;
-                    return;
                 }
+                // file.readAll() => might be a bit big but it /should/ work
                 RawImageData data = {file.readAll(), "gif", localPath};
                 this->uploadQueue_.push(data);
                 file.close();
-                // file.readAll() => might be a bit big but it /should/ work
-            }
-            else
-            {
-                channel->addMessage(makeSystemMessage(
-                    QString("Cannot upload file: %1. Not an image.")
-                        .arg(localPath)));
-                this->uploadMutex_.unlock();
-                return;
             }
         }
         if (!this->uploadQueue_.empty())
@@ -329,41 +321,53 @@ void ImageUploader::upload(const QMimeData *source, ChannelPtr channel,
             this->sendImageUploadRequest(this->uploadQueue_.front(), channel,
                                          outputTextEdit);
             this->uploadQueue_.pop();
+            return true;
         }
-    }
+        return false;
-    else if (source->hasFormat("image/png"))
+    };
+
+    auto tryUploadDirectly = [&]() -> bool {
+        if (source->hasFormat("image/png"))
         {
             // the path to file is not present every time, thus the filePath is empty
             this->sendImageUploadRequest({source->data("image/png"), "png", ""},
                                          channel, outputTextEdit);
+            return true;
         }
-    else if (source->hasFormat("image/jpeg"))
+        if (source->hasFormat("image/jpeg"))
         {
-        this->sendImageUploadRequest({source->data("image/jpeg"), "jpeg", ""},
+            this->sendImageUploadRequest(
-                                     channel, outputTextEdit);
+                {source->data("image/jpeg"), "jpeg", ""}, channel,
+                outputTextEdit);
+            return true;
         }
-    else if (source->hasFormat("image/gif"))
+        if (source->hasFormat("image/gif"))
         {
             this->sendImageUploadRequest({source->data("image/gif"), "gif", ""},
                                          channel, outputTextEdit);
+            return true;
         }
-
+        // not PNG, try loading it into QImage and save it to a PNG.
-    else
-    {  // not PNG, try loading it into QImage and save it to a PNG.
         auto image = qvariant_cast<QImage>(source->imageData());
         auto imageData = convertToPng(image);
         if (imageData)
         {
             sendImageUploadRequest({*imageData, "png", ""}, channel,
                                    outputTextEdit);
+            return true;
         }
-        else
+        // No direct upload happenned
-        {
         channel->addMessage(makeSystemMessage(
             QString("Cannot upload file, failed to convert to png.")));
+        return false;
+    };
+
+    if (!tryUploadFromUrls() && !tryUploadDirectly())
+    {
+        channel->addMessage(
+            makeSystemMessage(QString("Cannot upload file from clipboard.")));
         this->uploadMutex_.unlock();
     }
-    }
 }
 
 }  // namespace chatterino