oxen-io / session-desktop

Session Desktop - Onion routing based messenger
https://getsession.org
GNU General Public License v3.0
1.51k stars 193 forks source link

Cannot paste more than one image into a new posting. #2345

Open ianmacd opened 2 years ago

ianmacd commented 2 years ago

Describe the bug

The composition window allows a single image to be pasted directly into it, but not multiple images. The first image may be pasted or selected from a file, but subsequent images can only selected from files.

When making N screenshots to attach to a posting, it's a hindrance to have to save N-1 of them to files, and then select them for attachment.

To Reproduce

  1. Copy an image to the clipboard.
  2. Paste it into the composition box.
  3. Copy a second image to the clipboard.
  4. Attempt to paste it into the composition box. Nothing happens.
Bilb commented 1 year ago

this happens because we filter attachments by name. Because a pasted image as no name, it gets filtered out. We could do some fancy things like computing a hash of the data to avoid inserting a few times the same picture, or just not care about it (I think signal does that)

Should be easy to allow duplicate empty filename (check addStagedAttachmentsInConversation)

ianmacd commented 1 year ago

this happens because we filter attachments by name. Because a pasted image as no name, it gets filtered out.

Actually, it would appear that a pasted image is given the internal name image.png, even though it doesn't exist on a file-system.

We could do some fancy things like computing a hash of the data to avoid inserting a few times the same picture, or just not care about it (I think signal does that)

Should be easy to allow duplicate empty filename (check addStagedAttachmentsInConversation)

Thanks for this pointer. I can see how to fix the problem now.

The easiest way is obviously not to check for duplicate names at all. That seemed lazy, though, so I instead chose to keep the check for duplicate names, but to ignore any named image.png.

That worked, but revealed the full extent of this bug, which is that the deduping function really only considers the file's name, not its content. This means that:

  1. It's actually trivial to post multiple identical attachments, so long as each of them has a different file name.

  2. It's impossible to post different attachments if they have the same base file-name (i.e. minus its directory path).

So, we're actually getting both false positives and false negatives here, which seems as if it's doing more harm than good and inclines me to go back to my original solution of skipping the deduping altogether.

ianmacd commented 1 year ago

@Bilb Do you have any idea where the name image.png is being assigned to image data pasted into the message composition window?

I just cannot find the place in the code where this assignment takes place. Obviously it's not a trivial string assignment or I would have located it immediately.