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

Using ImageEditor removes images Metadata #4183

Closed
2 tasks done
thommor opened this issue Nov 3, 2022 · 13 comments · Fixed by #4926
Closed
2 tasks done

Using ImageEditor removes images Metadata #4183

thommor opened this issue Nov 3, 2022 · 13 comments · Fixed by #4926
Assignees
Labels
Help Wanted Indicates that we’d especially appreciate community input in this issue Improvement

Comments

@thommor
Copy link

thommor commented Nov 3, 2022

Initial checklist

  • I understand this is a bug report and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Link to runnable example

No response

Steps to reproduce

  • Enable ImageEditor plugin
  • Upload an image to somewhere (I.E S3)
  • Extract the image metadata using a tool such as exiv2

Expected behavior

Ideally when modifying an image using the ImageEditor plugin, the metadata from the initial image should be maintained in the altered image. This would allow systems using Uppy to extract this metadata as required.

Actual behavior

No metadata is stored with the altered image when uploaded

@arturi arturi added Help Wanted Indicates that we’d especially appreciate community input in this issue Improvement and removed Bug Triage labels Nov 4, 2022
@fluxthedev
Copy link

@thommor @arturi I can work on this. I think a core dev needs to assign the ticket to me. Or is that not a thing anymore?

@arturi
Copy link
Contributor

arturi commented Oct 1, 2023

@fluxthedev we welcome PRs, no need to assign the ticket. How do you plan to implement this, extract metadata before editing, then apply it back?

@fluxthedev
Copy link

@arturi thanks for the info. I'm not 100% sure yet on the implementation until I can replicate the problem. So I'll get back to you on that.

@fluxthedev
Copy link

@arturi Still looking into this. I'll be completely honest, I'm struggling to get it set up on AWS, so I'll try one of the other methods. I'm very knowledgeable with react / node, but my experience with AWS isn't great. I'll keep digging.

@fluxthedev
Copy link

I finally got S3 bucket / local Uppy setup and successfully uploaded it to S3. I used exiftool on my MacBook to compare the metadata before and after the upload through Uppy. I used the ImageEditor plugin with the following settings:

.use(ImageEditor, {
    quality: 0.8,
    cropperOptions: {
      viewMode: 1,
      background: false,
      autoCropArea: 1,
      responsive: true,
    },
  });

Here is the console output using exiftool before and after I uploaded it.
exiftool_before_and_after

I observed that the metadata does in fact stay. @arturi I'm not convinced yet that there isn't a problem. I will run tests on different images with a variety of metadata using the exact metadata extraction tool (exiv2) that was mentioned in the original post. Will report back sometime this weekend.

@fluxthedev
Copy link

@arturi I did another test just now using exiv2 to extract the metadata of this exif test sample and confirmed the data was not stripped using the same ImageEditor parameters I used in my previous post. Here's the txt files of the extraction. They are the same. Let me know if you need anything else from me to close this issue. Thanks for all your good work!
metadata_after_upload_10_08_2023.txt
metadata_before_upload_10_08_2023.txt

@lakesare
Copy link
Contributor

Thanks for the checks, @fluxthedev!
The checks you ran - did you only add .use(ImageEditor, {...},}); to the uppy settings, or did you also edit the image?
So, did you try to 1. edit the image using that ImageEditor, 2. save it 3. upload it 4. check the metadata?

@lakesare
Copy link
Contributor

@thommor, could you tell us what metadata you were missing?

@fluxthedev
Copy link

Thanks for the checks, @fluxthedev! The checks you ran - did you only add .use(ImageEditor, {...},}); to the uppy settings, or did you also edit the image? So, did you try to 1. edit the image using that ImageEditor, 2. save it 3. upload it 4. check the metadata?


@lakesare Hello! I don't recall if I manually edited the image using the imageEditor UI, so I'll do that. As far as the image editor config and other settings,yes that's all I added. Looking back I'm thinking the images weren't actually modified by uppy. The quality should of 0.8 should of at least changed the file size a bit for the jpg I uploaded. I'll make sure I make an edit in the UI and try multiple modifications, as it could be just one piece of functionality, or no issue anymore. Stay tuned!

@lakesare
Copy link
Contributor

lakesare commented Feb 15, 2024

@fluxthedev, thank you!
The most likely place where this metadata stripping happens is in the .toBlob call:

const saveBlobCallback: BlobCallback = (blob) => {

So, this happens after we edit the image and press SAVE. I recently had to readd the filename to the blob (using the new File([blob!], currentImage!.name, { type: blob!.type }) line), because the filename was getting lost, so I assume the same might be happening to other metadata.

@thommor
Copy link
Author

thommor commented Feb 15, 2024

Hi, thanks for people looking into this.

As I recall it was directly editing the image that appeared to strip the metadata out as is mentioned above.

If I recall correctly most (if not all) metadata was removed but would have to run the tests again to check

@lakesare
Copy link
Contributor

lakesare commented Feb 19, 2024

I suspect this might be happening because we add exif data as a side-effect after thumbnail generation, and we weren't regenerating a thumbnail after image editing.

So, this PR, once merged, might fix it #4926.

@fluxthedev, if you will be checking this, then please check it both before PR #4926 and after PR #4926 (to see if we had this issue before this PR, and to see if this PR did fix the issue).

@Murderlon
Copy link
Member

Fix is released. Let us know if the problem still occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted Indicates that we’d especially appreciate community input in this issue Improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants