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

Always strip exif data and drop user setting for it #8417

Merged
merged 2 commits into from
Jun 4, 2023

Conversation

SuperTux88
Copy link
Member

Some imagemagick-versions (I tested Ubuntu 22.04 and debian bullseye) always loose exif data when converting from jpg to webp. So this made our CI fail now, but even if it wasn't failing before, some pods always had and have versions which might loose the information anyway. So having a setting to keep exif information is kinda pointless, if we can't guarantee that the information isn't lost. Also, diaspora isn't a photo sharing platform and we don't display exif information anywhere, so I think we should just always strip exif data (which was already the default before), as we don't need them.

Some imagemagick-versions (I tested Ubuntu 22.04 and debian bullseye)
always loose exif data when converting from jpg to webp. So this made
our CI fail now, but even if it wasn't failing before, some pods always
had and have versions which might loose the information anyway. So
having a setting to keep exif information is kinda pointless, if we
can't guarantee that the information isn't lost. Also, diaspora isn't a
photo sharing platform and we don't display exif information anywhere,
so I think we should just always strip exif data (which was already the
default before), as we don't need them.
@SuperTux88 SuperTux88 added this to the 1.0.0 milestone Jun 4, 2023
@Flaburgan
Copy link
Member

Sounds like a reasonable decision to me.
I pulled the code locally, exif is still mentioned in schema.rb but I guess this is expected because you're dropping it with the migration after right? Looks good otherwise.

Yes, I know this is a very ugly workaround, but it works ...

Chrome now requires to add `about:blank` as parameter to open and be
able to use remote debugging. The jasmine-gem isn't supported anymore,
and we need to switch to the `jasmine-browser-runner`, I was working on
that a few months ago, but ran into problems.

As the jasmine-gem doesn't allow to add parameters without `--` infront
of it, lets just add a dummy parameter and add the required
`about:blank` with a space after that. This is ugly, but works for now,
until we can upgrade to the new jasmine version. We could also just
replace the `nil` of the last parameter with that value, but I think
that way it's clearer that this is a workaround and how it works.
@denschub
Copy link
Member

denschub commented Jun 4, 2023

exif is still mentioned in schema.rb

That's a local file that reflects your database state - it's not in this repo.

Copy link
Member

@denschub denschub left a comment

Choose a reason for hiding this comment

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

I'm amazed at our test suite's ability to randomly fail. Thanks for the fix.

@SuperTux88 SuperTux88 merged commit 6510baf into diaspora:develop Jun 4, 2023
8 checks passed
@SuperTux88 SuperTux88 deleted the remove-strip_exif-flag branch June 4, 2023 19:40
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.

None yet

3 participants