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

#779 Strip EXIF data as user preference #5510

Merged
merged 1 commit into from Jan 18, 2015
Merged

Conversation

@margori
Copy link
Contributor

margori commented Dec 31, 2014

My solution for issue #779 including

  • Strip EXIF data from uploaded images.
  • Privacy user preference to allow not striping.
  • DB migration for new user pref.
  • Testing both cases.

I need a review of photo_spec file.

Included image from Flickr

@Flaburgan
Copy link
Member

Flaburgan commented Dec 31, 2014

Awesome! Thank you for working on this!

Can you please add strip_exif to the user data exported in the json file? See e174514

Also, you decide here to set the default value to true (= strip exif data by default). When this is nice from a privacy point of view, this is changing the default behavior without telling anything to the user. So I'm wondering what we should do.

@jhass
Copy link
Member

jhass commented Dec 31, 2014

What again were the arguments for keeping it after we're done with doing any processing based on it?

@margori
Copy link
Contributor Author

margori commented Jan 1, 2015

What again were the arguments for keeping it after we're done with doing any processing based on it?

I see this question full of agresivity. So I gonna translate it to a peaceful language, so I can answer it

I see that you're maintaining EXIF data in processed images. We already have an unpleasant conversation, which I cannot find now, where was decided to strip them. Why did you decide to maintain them?

Since strip EXIF data decision would be in user realm, if he/she decide to maintain them, it's spread to processed images too. If not, exif is striped from original and processed.

@jhass
Copy link
Member

jhass commented Jan 1, 2015

I didn't mean to be aggressive, I'm sorry if it seemed that way, I'm truly wondering.

We do make privacy (by default) decisions elsewhere, for example by not having a setting for posting public by default, to keep that a conscious decision every single time. So, I still do wonder what arguments are there for storing images with EXIF data on the server at all, remember we're not primarily a photo sharing or gallery software.

@margori
Copy link
Contributor Author

margori commented Jan 2, 2015

I still do wonder what arguments are there for storing images with EXIF data on the server at all

Because d* says that user is in control. If user decide to make his/her photo EXIF data public, as developer I'm honoring his/her decision, I'm at their service.

If he/she doesn't know about EXIF or metadata, let's suggest to strip it, from every uploaded image to the server.

@margori margori force-pushed the margori:779_remove_exif branch from 5cf4bb8 to 067bca2 Jan 2, 2015
@margori
Copy link
Contributor Author

margori commented Jan 2, 2015

@Flaburgan Strip_exif included en JSON exporter

this is changing the default behavior without telling anything to the user.

Default behavior is: leave EXIF data in original image file, strip in reduced size image files. Example public post, processed image, original file. Original file has exif, processed doesn't.

@jhass 'Recommended' caption included.

@goobertron
Copy link

goobertron commented Jan 2, 2015

I still do wonder what arguments are there for storing images with EXIF data on the server at all, remember we're not primarily a photo sharing or gallery software.

@jhass There are some arguments in the discussion on #779, for example retaining copyright information for photographers who want this attached to their photos. I agree Diaspora isn't primarily for sharing or publishing photos; however, unless it's going to cause database, performance or privacy problems, I think it's a good idea to give people the choice whether EXIF data are retained with or stripped from their uploaded photos.

@@ -1291,8 +1291,10 @@ en:
privacy_settings:
title: "Privacy Settings"
ignored_users: "Ignored Users"
strip_exif: "Strip uploaded images metadata (geopos, author, camera, etc.) (Recommended)"

This comment has been minimized.

Copy link
@goobertron

goobertron Jan 2, 2015

I'd like to suggest a slight rewording of this sentence:

"Strip metadata such as location, author, and camera model from uploaded images (recommended)"

@margori margori force-pushed the margori:779_remove_exif branch 2 times, most recently from 9939ecd to 54771bb Jan 2, 2015
@margori
Copy link
Contributor Author

margori commented Jan 4, 2015

Last week my words were

...as developer I'm honoring his/her decision, I'm at their service.

and a read this thread in Loomio. It was decided to use full resolution images, but this create a lot of technical issues, use a lot of resources of servers and can put some pods out of business.

So I change my words:

This pull request gives freedom of choice to user without using additional resources.

@margori margori force-pushed the margori:779_remove_exif branch from 54771bb to cb0d37c Jan 6, 2015
@margori margori force-pushed the margori:779_remove_exif branch from cb0d37c to 63eca44 Jan 18, 2015
@margori
Copy link
Contributor Author

margori commented Jan 18, 2015

Request rebased.

Dear mergers, I really appreciate feedback. It's been 2 weeks of silence.

@jhass
Copy link
Member

jhass commented Jan 18, 2015

I'm not sure if the privacy tab is the right place for the setting, currently it's more targeted at managing what falls under the term "privacy lists", which is used for blocking parts of communication.

But then I feel like we need a reorganization of the settings pages in the mid-term anyway, especially as more settings are added, so I'm willing to glance over that for now.

Sorry for the silence and thank you for your contribution!

@jhass jhass merged commit 63eca44 into diaspora:develop Jan 18, 2015
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
jhass added a commit that referenced this pull request Jan 18, 2015
@jhass jhass added this to the next-major milestone Jan 18, 2015
@margori
Copy link
Contributor Author

margori commented Jan 18, 2015

I agree, privacy tab should be reviewed for UI & UX design. Let's see what alpha testers say.

@margori margori deleted the margori:779_remove_exif branch Jan 18, 2015
@marienfressinaud
Copy link
Contributor

marienfressinaud commented Jan 18, 2015

I think strip EXIF data preference is at the right place (that's totally related to personal privacy). On the other side, I always found "ignored users" preference was not at its place (I may prefer "Contact" page). Preference organization is a hard work and it could be interesting to see how the other social networks handle it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.