Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Feb 24, 2024

Conversation

hemirt
Copy link
Contributor

@hemirt hemirt commented Feb 5, 2024

Current clipboard data traversal incorrectly stops the traversal when URLs are present, alongside with an image.

This fixes using "Copy Image" in Google Chrome sometimes being unable to upload the image and failing with message: "Cannot upload file. . Not an image.". This error happens when an tag has more data to it, than just plain src attribute, the "alt" property seems to be interfering with it, yet when the image is opened directly in a new window, the copy worked fine.

"InsideClipboard" program, which shows the clipboard contents, shows that when the image has the alt tag, an extra item into the clipboard gets added, of type UniformResourceLocatorW (which is an URL, so the code path goes into the hasUrls() part) with the image url as data -> but obviously, the string cannot be converted into image data.

Nothing much in the actual code was changed.

The changes are:

  1. moved the upload code into two lambdas, one that tries the URLs, and the other is trying it directly from clipboard
  2. early returns now instead of returning from the function, return from the lambda, and indicate whether an upload succesfully happened
  3. when upload was unsuccessful (e.g. no image in data, invalid format, or unable to convert, ...), release the uploadMutex_ as was done before

@hemirt
Copy link
Contributor Author

hemirt commented Feb 5, 2024

fixes #5157

src/singletons/ImageUploader.cpp Outdated Show resolved Hide resolved
src/singletons/ImageUploader.cpp Outdated Show resolved Hide resolved
src/singletons/ImageUploader.cpp Outdated Show resolved Hide resolved
src/singletons/ImageUploader.cpp Outdated Show resolved Hide resolved
src/singletons/ImageUploader.cpp Outdated Show resolved Hide resolved
@hemirt
Copy link
Contributor Author

hemirt commented Feb 10, 2024

@Nerixyz I opted to not change the code unless absolutely necessary, so that the logic reasoning could be preserved and just the necessary change for the fix was added.

I could make these changes (or I believe maintainers should have access to my repo).

Your changes are mostly cosmetic and wouldn't add any extra new reasoning or logic paths, so I guess I could do them, should I then?

@Nerixyz
Copy link
Contributor

Nerixyz commented Feb 10, 2024

Your changes are mostly cosmetic and wouldn't add any extra new reasoning or logic paths, so I guess I could do them, should I then?

Imo, only the early-return and the moved comment are somewhat important - the no-else-after-return is really cosmetic (if you use clangd, then it should offer you an option to automatically fix that). The logic-issue and else-if in loop can be marked as resolved (I can't do that 🙃).

@hemirt
Copy link
Contributor Author

hemirt commented Feb 11, 2024

with the latest indent change, the git changes are now easier to read

@pajlada
Copy link
Member

pajlada commented Feb 23, 2024

My plan is to look into this PR this weekend

Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected for me on Arch Linux (btw) - thanks for looking into this & fixing it!

@pajlada pajlada enabled auto-merge (squash) February 24, 2024 12:28
@pajlada pajlada merged commit 6691050 into Chatterino:master Feb 24, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clipboard data sometimes does not paste the image - Cannot upload file: . Not an image.
3 participants