Editor: drag-n-drop upload can show incorrect rotation until upload is complete

[designsimply]

Raised by @nickmomrik

What I Did: Dragged a portrait-oriented image into the editor.

What I Expected: To see it displayed portrait.

What Actually Happened: It first showed up sideways while uploading, then when completed it refreshed and displayed correctly.

I'm using Firefox on a Mac.

Example screencast https://cloudup.com/c-ERYMHtjSY and I'm attaching the image used.

/hat tip nickmomrik for the original report

Notes from @aduth

This would be an easy fix if image-orientation: from-image had good browser support. Unfortunately that's not the case. There are a few node packages that help with parsing exif data.

https://www.npmjs.com/browse/keyword/exif

I spent a decent part of my afternoon investigating solutions here. It's a tricky problem, especially when considering transitioning from the temporary media item to the persisted copy, and accounting for changes in width/height in the oriented media item.

There were two promising libraries for assisting with parsing metadata:

• https://github.com/blueimp/JavaScript-Load-Image
• https://github.com/bwindels/exif-parser

I tried a few strategies for applying the orientation in the editor:

• Parsing the metadata using exif-parser, updating the temporary media copy with REST API-formatted attributes (width, height, exif.orientation), then applying CSS styles using transform.
  • Downside: width and height aren't recalculated when applying transform. Possibility that we'd need to strip additional classes or attributes before pulling content on save, or when replacing the item with the persisted copy.
• Attempting to generate a new image to replace the one in the editor
  • TinyMCE has some built-in utilities to help here: https://github.com/tinymce/tinymce/blob/2a58327/js/tinymce/plugins/imagetools/classes/ImageTools.js#L21
  • blueimp-load-image can generate a rotated canvas object to insert into the editor, though I had encountered some trouble with it respecting orientation

More information about EXIF rotation: http://sylvana.net/jpegcrop/exif_orientation.html

[krisk]

Cheers all :)

Attempting to generate a new image to replace the one in the editor

This would work for images of small size, but potentially large images will slow rendering performance to a debilitating state.

I did some preliminary testing on this, and if a temporary canvas element is used (just as how TinyMCE does blobToImage conversion), the following problems may occur for large images:

• Execution of toDataURL on the canvas, assigning it to an image source, and rendering it using renderToStaticMarkup is perceptibly long, therefore, waiting for its completion before the image is rendered seems to go against the point of quickly showing a preview.
• The data conversion and rendering may take longer to complete than the actual call to upload the image itself and render it (assuming an average connection speed), which, as above, may defeat the purpose of a quick preview.
• When the image source contains a large data URI, opening up the HTML tab of TinyMCE's editor may result in a "RangeError: Maximum call stack size exceeded"

Note: for small images (in which the data URI is of acceptable length),blobToImage conversion works quite nicely.

As @aduth mentioned, I would go with the first solution instead:

• Read orientation information.
• If it needs rotation, apply necessary styling changes using transform, transform-origin, etc...
• Once the image is rotated, the height and width are now inverted, therefore its max-height would have to be the width of the parent container.
  • This poses problems during container resizing, as the container width may change, so the value for max-height would have to be recalculated. That's a nuisance, but necessary.
  • Some additional problems with aspect ratio during image resizing needs to be addressed.

Note: different orientations will behave similarly, but their transformation will be different.

If the above sounds good, I'm ready to tackle it.

[aduth]

Hey @krisk ! Thanks for taking a look at this.

I think the path you outlined should be a reasonably simple approach, unforeseen complications notwithstanding.

A few other items to consider, which may or may not affect the intended direction:

• In the case of generating a new image, we may consider looking at TinyMCE's built-in image editing support. If it's capable of handling large images, we should see if there's anything we can reuse there or there are tricks to improved performance we can use as inspiration. The demo on the TinyMCE homepage includes image rotation.
• There's been some past consideration of indicating that an inserted image is still uploading via a progress indicator. Would it be less jarring if at least it were obvious that the image had transitioned from a pending state to a final state? I don't think this is an ideal end-solution, but could improve the current situation. Keep in mind that last I had attempted this, I found it was not as simple as it might seem to add a loading placeholder 😄

I was disappointed to see that there's been little progress in browser support for image-orientation 😞 I had hoped that there may have been a polyfill, but my recent search again came up dry. Would be interesting to try to build this in such a way that could be externalized as a polyfill, if not too much trouble.

---

In a related note, I had already drafted a response before realizing that there's two issues with image rotation, the other concerning generated thumbnails on Jetpack sites (#313). The text below applies only to #313, but in case it overlaps or is otherwise in scope of what you're looking at, may be useful:

It's been some time since my original observations, so I think it'd be good to take a moment and clarify the circumstances in which this is an issue. For example, does this affect both Jetpack and WordPress.com sites? Where are the images rotated incorrectly: modal? featured image sidebar? featured image header? post content? front-end?

I seem to recall Photon playing a role in whether the image would be rotated. If I recall correctly, this is because when WordPress generates thumbnails from an image, it strips the EXIF metadata tags, but when a Photon-ized image is rendered, it regenerates an appropriately resized image from the original with metadata intact. Keep in mind that Photon is always in use on WordPress.com, but is a module that can be toggled on-and-off for a Jetpack site.

I've been using some of the images from this repository (and insight from the related blog post) in testing: https://github.com/recurser/exif-orientation-examples

[mdawaffe]

Thanks for looking into this @krisk. Sounds like a good approach. I also like @aduth's thoughts about making a generalized solution. I propose we think more about whether or not to do that once we have something working for this specific case, but it's good to keep in mind even now.

[gwwar]

I investigated this a bit and the following is possible, though I would argue not really worth it for the added complexity since this only affects transient images:

• Check for exif data and parse it
• If orientation is set to a value > 1, we can attempt to generate a small temporary canvas, apply the correct transformation and export this as a blob

If the image is large enough, we may run into the case where generating the thumbnail is more work than the actual upload. I wonder if it would be enough to add a simple text or image placeholder for images we detect that have orientation set.

This also won't work for images uploaded from urls that are from a different origin.

[lancewillett]

@rralian Checking in on the progress here — is there a relate pull request, does it need help along (testing, review)?

[lancewillett]

Re-ping @rralian I know things are busy — could you pass to someone else if you can't get to this one soon? It's a high priority issue.

[designsimply]

Re-tested and confirmed this is still an issue. Tested with Firefox 53.0.3 on Mac OS X 10.12.5 on production with a regular user account. (39s)

Semi-related, there is also a bug filed regarding the image "jumping" when the transient to permanent image switch kicks in: #14252.

[rralian]

I missed the previous pings about this and didn't realize this was still assigned to me. Sorry, I believe I was in the throes of planning Automated Transfer at the time and things got a little crazy. I'm not actively working on this, and it's probably not going to be on my team's list anytime soon as this is a transient issues that's annoying, but not a showstopper level bug. So I've un-assigned myself in case someone else is able to take it.

[lancewillett]

Going to punt this one — low priority. We can pick it up later if we get requests for it when talking to customers.