Take into account image exif data to determine which way is up

[jenlampton]

Describe your issue or idea

When uploading a file from his computer, one of my Backdrop students today immediately notices that his photo was sideways. If there is exif data attached to an image, does Backdrop use that to make the image right-side-up?

Here's a contrib module that solves the problem: https://github.com/backdrop-contrib/exif_orientation

I think we should consider moving this functionality into core.

---

PR by @Graham-72 backdrop/backdrop#2224 is a small addition to core file.inc. now revised extensively.

[Graham-72]

This is included in #1307 as 'Nice to have' 'Image operations', but where are we on that issue?
See comment at #1307 (comment)

[jlfranklin]

This is of use to sites that encourage users to post images taken with their iPhone camera, which doesn't rotate the image, but instead uses the Exif data to record orientation. Unless someone can cite more fundamental uses for it, I don't think Exif Orientation meets the "80% of sites" threshold. The module statistics back this up: the most popular module, GA, has 118 installations, Exif Orientation has 2.

I've seen a few of this kind of issue over the last year, the "here's a really useful contirb module, let's pull it into core." I'm worried that we'll pull in half of contrib into core, each time saying it's useful and not that much code, and bloat core over time.

In the past, modules have moved from contrib to core when the major number bumps. CCK in 7.x, Views in 8.x being two of the most prominent examples. Perhaps it would be better for us to leave contrib modules as contrib in 1.x, and nominate them for inclusion in 2.x.

[docwilmot]

I'm worried that we'll pull in half of contrib into core

Agreed

[Graham-72]

I understand your point but this is an important issue, and not just for iPhone users. My Android smartphone also has this problem. If I upload a portrait photo directly into a BD image file field it is displayed on its side as if it is a landscape image.

I have three operational BD sites where my clients can upload their photos into pages/articles and they experience this problem. Initially I thought it was just that they use iPhones and iPads but it seems now to be a general problem which in my view we must fix wherever users can upload image files.

[olafgrabienski]

Afaik, getting a feature in core doesn't mean necessarily to put exactly the respective contrib module in core, often the functionality can be integrated in a different (better, less bloated) way. And if the Backdrop editorial interface is meant to also be used on mobile phones, it makes sense to display images uploaded with such a device correctly out of the box.

[docwilmot]

Point is that the (very) few people who want this can install the contrib module.

[jlfranklin]

I have three operational BD sites where my clients ... experience this problem

Then, Exif Orientation will soon have a "sites using" count of 5, right?

Afaik, getting a feature in core doesn't mean necessarily to put exactly the respective contrib module in core, often the functionality can be integrated in a different (better, less bloated) way.

Depends on how deeply integrated the module is with core. The simplest way is to drop the module in the core/modules directory. Adding this module more deeply than that will make the php_exif extension a mandatory requirement for all Backdrop sites. In this case, bloat isn't just code bloat.

[olafgrabienski]

Point is that the (very) few people who want this can install the contrib module.

I guess few people (site builders) who install a module doesn't mean that few people (editors, authenticated users) want to use the functionality. Besides, certain things should just work even if not 80% use it.

Okay, "certain" is vague, but think for example of Backdrop trainings and demo sessions where some people just upload an image via phone or tablet, and that doesn't work. Sure, the trainer could install the module but does he expect that he has to in a modern CMS?

[jlfranklin]

Okay, "certain" is vague, but think for example of Backdrop trainings and demo sessions where some people just upload an image via phone or tablet, and that doesn't work. Sure, the trainer could install the module but does he expect that he has to in a modern CMS?

What a great opportunity for the trainer to show how easy it is to add a feature in Backdrop that doesn't show something overt in the site! "Oh, yeah, there's a module that handles that. Go install Exif Orientation via the Project interface, and it'll be upright again."

[olafgrabienski]

What a great opportunity for the trainer to show how easy it is to add a feature in Backdrop (...)

Good point ... if the trainer knows that the module exists (as a trainer he might).

Apart from that, I think it's however good to support mobile devices out of the box nowadays.

[jenlampton]

as a trainer he might

or she, in this case.

I don't think Exif Orientation meets the "80% of sites" threshold. ... Point is that the (very) few people who want this can install the contrib module.

I would imagine that 100% of people who upload an image would expect it to be the right way up. When it's not, it seems like a bug in the software. I would guess the currently low usage for the module is because most people haven't noticed the problem yet. I would bet that most sites will eventually attempt to upload an image taken by a smart phone.

Most people (including myself) don't check contrib projects for fixes to bugs. It's a suitable way to work around various problems, but it would be better if we could avoid the problem all together :) My vote would be to add the rotation into one of the core file, image, or system modules so that any time an image file is uploaded, it's correctly oriented.

The one thing that is of concern is the hard requirement to use the php_exif extension. In core, that should be a soft requirement. We could check to see if the extension is present, and if so, automatically rotate images when necessary. If its not present, we could add a message on the status report explaining that without it, images taken with smart phones or tablets might not be properly oriented.

[laryn]

I agree with Jen's assessment, particularly that basic mobile support for this is expected to just work these days, especially when you can already upload images to an image field directly from a phone. I think her proposal is great.

[tomgrandy]

Agree that there is an assumption that a vertically shot photo would be uploaded as such and not end up on its side as a horizontal. Any site that has a photo gallery runs into this problem.

[herbdool]

Tested and works well. I can see from the code it'll only run if the function exists.

[quicksketch]

I'm not sure doing the rotation in file_save_upload() is the appropriate place, but we should at least separate out the file rotation into a stand-alone function. I also think we need the ability to disable this functionality, for a few reasons:

• Rotating a full size JPG can easily exhaust the server memory.
• Overwriting a file with a modified version will wipe out EXIF data.

As this happens immediately on upload, this means that modules that track EXIF data would be unable to pull out any further EXIF data on any images that were rotated.

I think it would be best to move this feature into image.module and make it part of the image upload widget, same as the max dimensions setting (which also wipes out EXIF data for the same reason -- and has description indicating it will do that).

[Graham-72]

@quicksketch thanks for your comments and advice. I will produce a new PR, this time for image.module.

[Graham-72]

@quicksketch I have extensively revised the PR and the rotation (if needed) is now done by a validate function.

This validate function is then used when uploading an 'inline' image in CKEditor, uploading an image file to an image field, and also when a user uploads their image (which might well be a 'selfie')'

I have added admin settings so that the 'orientate' action needs to be set for each field instance, and also for the editing format for CKEditor.

[klonos]

...comment moved to its own issue: #3333

[quicksketch]

Looks good @Graham-72! I posted a review to the PR: backdrop/backdrop#2224 (review)

This is really close!

[Graham-72]

@quicksketch Thanks for the review. I have amended the PR.

[quicksketch]

An additional review: backdrop/backdrop#2224 (review)

We also need test coverage for automatically rotating images. We can just add the new testing to ImageFieldValidateTestCase, where image resizing is already covered. We'll need to include a sample image that needs rotation, either that or write EXIF data to one of the existing sample images and then test uploading it.

[Graham-72]

We now have test coverage, thanks to @herbdool for help with this, and an amended PR backdrop/backdrop#2224 .

[quicksketch]

Thanks @Graham-72! I've merged backdrop/backdrop#2224 into 1.x for 1.11.0. Thanks @herbdool for the reviews as well!