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

Correctly handle paste items when pasting images #1078

Merged
merged 1 commit into from
Sep 13, 2021

Conversation

bluec0re
Copy link
Contributor

@bluec0re bluec0re commented Aug 25, 2021

The paste handler does not correctly check for item types.

This results in pasting images to not work if they are also available as {kind: 'string', type: 'image/xpm'} in the clipboard (for example as its the case with flameshot).

The paste handler does not correctly check for item types.

This results in pasting images to not work if they are also available as {kind: 'string', type: 'image/xpm'} in the clipboard (for example as its the case with fireshot).
@CLAassistant
Copy link

CLAassistant commented Aug 25, 2021

CLA assistant check
All committers have signed the CLA.

@threema-danilo
Copy link
Contributor

threema-danilo commented Sep 9, 2021

Thanks! How can I reproduce this issue? When I use flameshot (v0.10.1) on Arch I only get a single DataTransferItem of kind file (and pasting works as intended).

@threema-danilo
Copy link
Contributor

Can you run this in your JS console, and post the output generated if you paste an image from flameshot?

document.getElementById('composeDiv').addEventListener('paste', e => { const items = e.clipboardData.items; for (i=0;i<items.length;i++) { console.log(`item ${i}: ${items[i].kind} => ${items[i].type}`) }})

What version of flamshot do you use? (Output of flameshot --version)

@bluec0re
Copy link
Contributor Author

Sure:

item 0: string => BITMAP
item 1: string => MULTIPLE
item 2: string => PIXMAP
item 3: string => SAVE_TARGETS
item 4: string => TARGETS
item 5: string => TIMESTAMP
item 6: string => application/x-qt-image
item 7: string => image/bmp
item 8: string => image/cur
item 9: string => image/ico
item 10: string => image/jpeg
item 11: string => image/jpg
item 12: string => image/pbm
item 13: string => image/pgm
item 14: file => image/png
item 15: string => image/ppm
item 16: string => image/xbm
item 17: string => image/xpm

The problem for me is that the JS code continues to override the index variable as long as it finds an image mime type. My first approach was to collect all indices which start with image/ and try all until one succeeds, but I then noticed that the API you're using (getAsFile) only supports files anyways.

Flameshot version

Flameshot v0.10.1
Compiled with Qt 5.15.2

Additional info

WM: i3
OS: Debian 11 based
Browser: Chrome 93

@threema-danilo
Copy link
Contributor

Oh wow, I wonder why on your system you get so many entries in the clipboard (and I wonder why binary formats are transferred as a string). On my system (Arch Linux with dwm, identical Flameshot version) I only get one.

With your list of DataTransferItems, your fix makes sense, thank you!

@threema-danilo threema-danilo merged commit 04a8be6 into threema-ch:master Sep 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants