From @jeherve
I made no edits to the image, it was automatically backed up from my phone to Google Photos, where I downloaded it, and then uploaded it to the editor from the desktop. It seems like it could be caused by Google Photos. I tried with another portrait picture that didn't go through Google Photos, and it worked properly.
Steps to reproduce:
Here is how the image looks like in the editor:
The problem seems similar for the Featured Image view:
From @aduth
The cause appears to be that WordPress strips EXIF rotation data when generating intermediate thumbnail sizes for an image. Since we display the large thumbnail in the editor, it will display in the editor without the proper rotation. On the site, however, Jetpack/Photon instead use the original image passed through Photon and scaled to the desired size. Because it scales the original image, the rotation metadata is correctly preserved.
What are the reasons we shouldn't use Photon URLs for images in the editor? Two I can think of are "What if the site is private?" and "What if the user has disabled Photon?", both of which are testable within Calypso. I could also see an argument being made that we should prefer to use the "original" URL over the Photon URL, though in this case the use of Photon will affect the image's appearance. Should we always use the original (full-size) image in the editor if we determine that the site will use Photon on the front-end?
From @sararosso
Just an update on this. I'm seeing image rotation problems in Chrome but not in Firefox or Safari...
Closed https://github.com/Automattic/wp-desktop/issues/147 as a duplicate, raised by @ryanmarkel
Site I'm testing on is ryanmarkel.com (it's hosted on VIP Go as a test site). App version is 1.3.
When uploading images that were taken with my iPhone 6+, the rotation data is not respected when viewing the post within the post editor:
The Media Library view displays the image with proper rotation:
@gwwar do think think this one is in your wheelhouse?
There's also some related background in 8870-gh-calypso-pre-oss
I tested this and confirmed the incorrect behavior in editor with Jetpack sites. Until this is fixed in core, I think it makes sense to use Photon for editor previews if a blog is using Photon for posts.
Editor preview images uses urls like the following:
https://helpingcat.wpsandbox.me/wp-content/uploads/2016/05/DSC_05005.jpg?w=680
While the correct orientation on the post uses urls like:
https://i0.wp.com/helpingcat.wpsandbox.me/wp-content/uploads/2016/05/DSC_05005.jpg?resize=825
This would have the added benefit (if we use the resize
parameter properly, or if we use ?w=
on a Photon URL) of loading a more optimized image as well, which would speed things up a bit. The ?w=
parameter being added to the "raw" Jetpack URL will do nothing, so a fullsize image will load. Using ?resize=xxx
without a "y" dimension (,yyy
) will also do nothing.
I did a bit of research on this being fixed in core.
The most relevant thread is this one:
https://core.trac.wordpress.org/ticket/14459
TLDR:
GD
library is used for most image operations and that strips EXIF
data on any relevant op because its a bit low-levelThere is no apparent action to fix that.
The ticket is owned by our own @azaozz - could you pitch in ?
Older ticket:
https://core.trac.wordpress.org/ticket/7042
For me, it seems that counting on this being fixed in core cant keep up with our timeline.
Controbutor @n7studios has developed a plugin that kinda fixes this (although I have not tested it yet) - code here and we could try to go similar route and fix it in Jetpack for non-photon sites.
That would "kinda" fix a bug in core in jetpack instead of core itself and that seems to me like trouble down the road - do we generally even consider it?
That would "kinda" fix a bug in core in jetpack instead of core itself and that seems to me like trouble down the road - do we generally even consider it?
We do consider it, carefully commenting the code in question so it can be removed later.
@azaozz Can you take a look also?
Currently core still depends on the user to rotate the images by hand after uploading. This is (finally) set for fixing in 4.7. The best patch for now is https://core.trac.wordpress.org/attachment/ticket/14459/14459.2.diff. It depends on GD removing the EXIF data from rotated JPEGs. IMagick can write EXIF, so the patch resets/removes "Orientation" after rotating.
The plugin mentioned above is not that reliable. It "hacks" the image files directly (binary) and tries to remove the EXIF orientation. Note the inline comment there:
// @TODO I'm not sure this is the best way of changing the EXIF orientation flag, and could potentially affect
// other EXIF data
@azaozz Do you think we should wait until we merge 4.7 to WP.com for this? Or patch / hack it now with a comment?
@lancewillett from the latest discussion in the core-media Slack channel, this is probably not going to be in 4.7. The concern is that on shared hosting it will make some uploads fail because it takes too much memory and processor power to rotate a large image, and that when only GD is available on the server, all of the EXIF data will be stripped from the rotated image.
Looking at this ticket again: it is not really a Calypso problem. I'm thinking we can probably implement something like https://core.trac.wordpress.org/attachment/ticket/14459/14459.2.diff as a plugin and use it when uploading from Calypso.
An elegant solution would be to detect this before uploading the image and rotate it and fix the EXIF orientation from JS. This is quite possible in newer browsers, but not sure if it will work for phones, etc. (same problem, high RAM and CPU requirements).
CC @aduth for a 2nd opinion on what you think would be the most elegant solution.
The JS solution mentioned by @azaozz could use an approach similar to that in #5020, an unfinished pull request which sought to fix the related #318.
For image rotation after the image uploads, we could use Photon to display the image in the editor, which I believe would show with the correct rotation. See original comment for more details where I mentioned this.
We could also/alternatively apply the CSS image-orientation: from-image
style, which still has poor browser support.
@aduth How much time do you think it would take to use Photon in the editor like you mention?
@apeatling Might not be too much work. We could probably hook into the plugin we created to artificially downsize images without affecting the saved src
, but instead always apply the replacement src
(not just for large images) and use Photon:
11963-gh-calypso-pre-oss
Cool, that seems pretty doable. Does anyone on IO have some time to knock that one off? It's one of the top three highest priority issues for Calypso posted by Flow Patrol. It'd be great to close it out.
@apeatling I'll try to take a stab at it, hopefully by end of week, else in the new year.
Apologies for the delay on revisiting this. Shortly after my previous message, I made an attempt to put together a working branch to solve rotation issues based on my thinking that simply passing all images through Photon would be a suitable solution. I encountered a couple issues with this in our resizing utility not being generic enough (related changes in #10264).
From the original issue text, this is now fixed for:
Here is how the image looks like in the editor:
But not:
The problem seems similar for the Featured Image view:
See #11239 for complications around rotation for featured images, specifically around supporting custom thumbnail sizes.
Tested and confirmed that inserting images with rotation EXIF metadata into Jetpack sites are now correctly rotated in the post body but are still incorrectly rotated in the featured image area in the Calypso editor:
Video: 1 min
Seen at https://wordpress.com/post/alittletestblog.com/13593 running WP 4.7.3 and Jetpack Beta (Build 1736160) c142584 using Chrome 57.0.2987.133 on Mac OS X 10.12.4.
Opened a separate issue in https://github.com/Automattic/wp-calypso/issues/13145 to address the featured images and closing this issue as resolved.
Most helpful comment
@apeatling I'll try to take a stab at it, hopefully by end of week, else in the new year.