allusion-app / Allusion

A free and open source desktop application for managing your visual library
GNU General Public License v3.0
662 stars 42 forks source link

Drag-and-dropping images from chromium-based browsers creates undeletable files #605

Open BartHageman opened 10 months ago

BartHageman commented 10 months ago

Describe the bug Dragging and dropping an image from a chromium based browser (Tested on Brave and MS Edge) sometimes creates an undeletable file, created with a number in the following format: ' 1.' ,' 2.', '3.'

Directory listing:

Directory of C:\Users\barth\Sync\Visual Library\Miscellaneous

12/09/2023  20:41    <DIR>          .
11/09/2023  00:29    <DIR>          ..
12/09/2023  20:40                 7  1.
12/09/2023  20:41                 7  2.
12/09/2023  20:41                 0 restofthefiles.txt

Adding a new file and incrementing with each drag and drop. When attempting to delete these files using file explorer, it claims the files cannot be found (even though it is still displaying them. Likely because of it's abnormal naming scheme (no extension after the period). Files must be deleted using the command line. Firefox does not seem to produce this behavior, and dragging and dropping works as expected.

To Reproduce Steps to reproduce the behavior:

  1. Open Microsoft Edge
  2. Navigate to any twitter page (twitter usually produces this bug, although it's not the only place it happens.)
  3. Drag an image over to a folder in your locations in Allusion and drop it.
  4. Instead of an image, a generic empty file is created without an extension.

Expected behavior The image that was dragged and dropped should be added to the folder.

Screenshots image

Allusion version

Desktop OS

BartHageman commented 10 months ago

Issue appears to be a happening in a few places:

https://github.com/allusion-app/Allusion/blob/631cfb5fb9c3bb62677e5fd37be28dbfaf4b8e2f/src/frontend/containers/Outliner/LocationsPanel/dnd.ts#L140-L149

Apparently the "file.path" value in the file can be empty. If this is the case, the file should probably be skipped. If we patch it to do so, we move onto the 'text/html' datatransfer, which works as expected.

https://github.com/allusion-app/Allusion/blob/631cfb5fb9c3bb62677e5fd37be28dbfaf4b8e2f/src/frontend/containers/Outliner/LocationsPanel/dnd.ts#L168-L181

Then this section, we filter out URLs without image extensions and perform the check. However, our obtained URL from twitter comes in a different format: https://pbs.twimg.com/media/[identifier]?format=jpg&name=medium, and gets missed. So we move to fetch it and check the type of the resulting blob.

https://github.com/allusion-app/Allusion/blob/631cfb5fb9c3bb62677e5fd37be28dbfaf4b8e2f/src/frontend/containers/Outliner/LocationsPanel/dnd.ts#L101-L109

Unfortunately, by the looks of things the blob returned has type basic, and is therefore not recognized.

Haven't gotten to the bottom of this yet, but it's a start.

BartHageman commented 10 months ago

Incidentally, if we apply a 'naive' fix of removing the dot from

else if (IMG_EXTENSIONS.some((ext) => item.toLowerCase().includes(`.${ext}`))) { 

Then the image gets pulled in as expected. However, I feel like this is not ideal.