Make WordPress Core

Opened 12 years ago

Last modified 11 months ago

#9257 assigned enhancement


Reported by: B-Scan Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 2.7
Component: Media Keywords: has-patch needs-testing dev-feedback
Focuses: Cc:


Attached patch adds GPS longitude and latitude to image meta data.
Changed: wp_read_image_metadata function (file: wp-admin/includes/image.php).

It complies with exif standard:
http://www.exif.org/Exif2-2.PDF (page 46)

Commented on wp-hackers list:

Attachments (7)

image.php (11.6 KB) - added by B-Scan 12 years ago.
9257.patch (1.7 KB) - added by Viper007Bond 12 years ago.
Patch version of previous attachment
9257.2.patch (2.1 KB) - added by kraftner 7 years ago.
Refreshed and slightly changed to actually make the values float instead of string
9257_3.patch (2.1 KB) - added by kraftner 7 years ago.
works with non-float coordinates (thanks alanjcastonguay), checks for values to be <= 180°
9257.4.diff (3.2 KB) - added by Mte90 3 years ago.
new refreshed patch with unit test
9257.diff (5.0 KB) - added by desrosj 3 years ago.
test-gps-location.jpg (286.7 KB) - added by desrosj 3 years ago.
Unit test image

Download all attachments as: .zip

Change History (40)

12 years ago

12 years ago

Patch version of previous attachment

#1 @alanjcastonguay
12 years ago

is_float(50) == false

In the incredibly unlikely case* that the number coming out of wp_exif_gpsconvert() is an integer, it will return 999.

*Or a GPS unit with sufficiently low resolution to only return degrees and not minutes or seconds.

#2 @Denis-de-Bernardy
12 years ago

  • Keywords has-patch needs-testing added; exif gps longitude latitude image removed
  • Milestone changed from Unassigned to 2.9
  • Version set to 2.7

#3 @Denis-de-Bernardy
12 years ago

  • Component changed from Administration to Upload

#4 @ryan
11 years ago

  • Milestone changed from 2.9 to Future Release

#5 @kraftner
7 years ago

I would appreciate if this was being picked up again. Also have a look at #5353.

#6 @markoheijnen
7 years ago

If you would love that this is going to be picked up then you can help out. There is already a patch and it's most likely needs a refresh.

#7 @kraftner
7 years ago

I hope this patch is formally okay as it is my first one ever here.
If not I am more than willing to learn! ;)

7 years ago

Refreshed and slightly changed to actually make the values float instead of string

7 years ago

works with non-float coordinates (thanks alanjcastonguay), checks for values to be <= 180°

#8 @kraftner
7 years ago

  • Cc thomas@… added

#9 @chriscct7
5 years ago

  • Keywords needs-unit-tests added

3 years ago

new refreshed patch with unit test

#10 @Mte90
3 years ago

  • Keywords dev-feedback added; needs-unit-tests removed

Find an image with GPS data in Exif was not easy but I refreshed the patch and added an unit test that check that is reading rightly the data from the file.
The diff cannot contain binary stuff but I taken the image from ttps://github.com/mkttanabe/exif/blob/master/test.jpg

Looking at the code there is also a sanitization of the exif data and conversion to a better value but there are no comments or image to test it.

This ticket was mentioned in Slack in #core by mte90. View the logs.

3 years ago

#12 @desrosj
3 years ago

  • Component changed from Upload to Media

Thanks for the refresh, @Mte90.

I am moving this to the Media component to get better visibility. This is similar to some other enhancements that have been worked on recently for media. I will also mention this ticket in the next component meeting to get more eyes on it.

The patch works as expected. There are just a few minor coding standards issues with 9257.4.diff that should be fixed (See https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#space-usage). The test image also needs to be attached to the ticket.

I do have a question regarding the GPS precision, though. I did some researching and it seems that 6 points of precision is the practical limit of commercial surveying, and anything over that is only used for tectonic plate mapping. Should we trim numbers, or just allow whatever precision is calculated? I assume that mobile devices will not have better than 3-5 points of precision.

I think it would be great to add this support for video, and maybe audio as well. These will require separate tickets though.

#13 @melchoyce
3 years ago

This is a cool idea — we can do a lot of fun stuff with EXIF data. As I mentioned in #42479, let's just be sure to be cautious about how we're potentially exposing this data, and make sure there's no easy way folks could use it for malicious purposes. Sounds from @dd32 that it likely won't be an issue, but let's still keep it in mind all the same.

#14 @Mte90
3 years ago

I will improve the coding standards, for the file as I written svn doesn't support binary stuff so the patch cannot contain the image itself.

For 6 limit I can implement that part to trim that is the most simple way, maybe implement also a filter for that so if someone need to set a custom limit will be possible.

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.

3 years ago

#16 @wonderboymusic
3 years ago

#42479 was marked as a duplicate.

3 years ago

3 years ago

Unit test image

#17 @desrosj
3 years ago

  • Owner set to desrosj
  • Status changed from new to assigned

9257.diff updates the previous patch to apply cleanly after [42343]. It also has the following adjustments:

  • Coding standards fixes.
  • Moves the latitude & longitude keys into an array stored in the location key.
  • Add the wp_read_image_geo_data filter. Returning a falsey value to this filter will disable all processing of location data.
  • Only adds the location key to the meta array if the parsing location data is not disabled with the wp_read_image_geo_data filter.
  • Limits the GPS decimal precision to 7 (practical limit of commercial surveying).
  • Updated other image meta tests to check for empty location info.

To address @melchoyce's concerns, these changes will only parse the location data into image meta so that plugins and themes can utilize it. We could expose the data in the admin on each image, but I think that is best for a separate ticket.

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.

3 years ago

#19 @mikeschroder
3 years ago

  • Milestone changed from Future Release to 5.0

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.

3 years ago

#21 @desrosj
3 years ago

Opened #43454 to discuss informing the user appropriately about the presence of geolocation data.

#22 @desrosj
3 years ago

Talked this over in the #gdpr-compliance room in Slack (https://wordpress.slack.com/archives/C9695RJBW/p1520020560000530). It seems that this functionality needs to be off by default unless the user is made aware that the location data is being used.

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.

3 years ago

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.

3 years ago

This ticket was mentioned in Slack in #core-media by desrosj. View the logs.

3 years ago

#26 @desrosj
3 years ago

  • Milestone 5.0 deleted

I am removing this from the 5.0 milestone. This feature will most likely need to be off by default to be GDPR compliant, and after discussing in the weekly #core-media chat, that may make it plugin material. It all depends on what the opt-in process ends up being.

Still would love to see this land, but need to let that dust settle first.

#27 @ocean90
3 years ago

  • Resolution set to maybelater
  • Status changed from assigned to closed

This ticket was mentioned in Slack in #core-media by dshanske. View the logs.

12 months ago

#29 @dshanske
12 months ago

  • Resolution maybelater deleted
  • Status changed from closed to reopened

I would like to propose revisiting this issue.

We already store the manufacturer of the device and other information...and we are already storing and delivering pictures that are holding this location data already.

Rant aside, exposing this information in the admin educates individuals about the fact that the image contains this data... protecting their privacy and allowing them to decide to delete the image entirely.

Maybe we should address the fact that the original has this information by storing it in the WordPress database and removing it from the file itself on upload, which is a harder issue.

Suggest to comply with the requirement as described, the admin should at minimum store whether or not the image file has location data at all as not disclosing this seems the bigger issue.

At the same time, it should optionally allow for storing the location in lieu of just the fact that the location is in the file, as described in the prior filter, and display one or the other accordingly.

I think the admin should display any image data we store by default, if the argument is protecting user data.

To the purge argument, store the location or the fact there is data in the geo_ prefix cited in the Codex allowing it to be queried and purged with a meta query.

In my opinion, displaying it on the frontend is plugin or theme territory. Handling it on the backend belongs in Core.

I see storing it more as protecting people's data, as opposed to harming it

#30 @dshanske
12 months ago

To add to this discussion, https://make.wordpress.org/core/2019/10/09/introducing-handling-of-big-images-in-wordpress-5-3/ actually ensures that the original version of the file isn't shared in additional cases, and EXIF can be stripped during processing.

#31 @dshanske
12 months ago

Also want to cite this comment https://core.trac.wordpress.org/ticket/40782#comment:6 as indicative of the common user reaction to the fact that EXIF data is tored.

#32 @dshanske
12 months ago

Been reading http://www.cipa.jp/std/documents/download_e.html?DC-X010-2020 which is EXIF 2.32 adopted last year and not really being used yet.

Appears that in version 2.32 of EXIF, GPSLongitudeRef and GPSLatitudeRef are being merged into GPSLatitude and GPSLongitude.

Value type of GPSCoordinate is a Text value in the form “DDD,MM,SSk” or “DDD,MM.mmk”, where:

  • DDD is a number of degrees
  • MM is a number of minutes
  • SS is a number of seconds
  • mm is a fraction of minutes
  • k is a single character N, S, E, or W indicating a direction (north, south, east, west)

#33 @desrosj
11 months ago

  • Milestone set to Awaiting Review
  • Owner desrosj deleted
  • Status changed from reopened to assigned
Note: See TracTickets for help on using tickets.