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

Editor: Correct image rotation by always passing image through Photon #11239

Merged
merged 1 commit into from
Mar 20, 2017

Conversation

aduth
Copy link
Contributor

@aduth aduth commented Feb 7, 2017

Partial fix for #313, many caveats apply (see section below)

This pull request seeks to correctly display images with rotation EXIF metadata in the post editor for Jetpack sites. The changes here revise the restrict-size TinyMCE sub-plugin to pass all images through Photon. The reason this helps fix the rotation is described in more detail in #313:

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.

Before After
Before After

These changes do not fix rotation issues with featured image. Featured image is more complicated because we respect custom theme size and crop. Because the thumbnail resizing is the cause for the rotation issues, we either must choose (a) to ignore custom thumbnail and pass original image through Photon or (b) to respect custom thumbnail at cost of rotation issues.

Testing instructions:

Using example rotated images, try inserting images into post content. Observe that rotation is restored after image finishes uploading.

  1. Navigate to post editor
  2. Select a Jetpack site
  3. Drag n' drop or otherwise insert image
  4. After image finishes uploading, note that rotation is corrected

Caveats:

  • Rotation is only fixed after the image finishes uploading
  • As can be seen by running through example images, this does not appear to help resolve flipped images, only rotated
  • Keep in mind that your browser will probably fix rotation itself when viewing an image in a standalone tab

@aduth aduth added [Feature] Post/Page Editor The editor for editing posts and pages. [Feature] Media The media screen in Calypso, general media management, or integration with third party media. [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 7, 2017
@matticbot
Copy link
Contributor

@aduth
Copy link
Contributor Author

aduth commented Feb 7, 2017

cc @iamtakashi

@beaulebens
Copy link
Member

Seems to test as suggested by your description for me. So I see it initially "incorrectly rotated", but then once the upload completes, then it rotates properly.

@lancewillett
Copy link
Contributor

@aduth Do you want more testing? Is it ready to merge?

@aduth
Copy link
Contributor Author

aduth commented Mar 16, 2017

@lancewillett I think this could be shippable, though puts us in a weird partly-fixed state with all its caveats. Better overall than what exists currently?

@lancewillett
Copy link
Contributor

@aduth Yes, in my opinion worth the step in the right direction.

@aduth
Copy link
Contributor Author

aduth commented Mar 17, 2017

I'll plan to merge this Monday morning.

@aduth aduth force-pushed the update/editor-photon-always branch from 9b44f6e to 441c4e0 Compare March 20, 2017 12:30
@aduth aduth merged commit 66a3bf4 into master Mar 20, 2017
@aduth aduth deleted the update/editor-photon-always branch March 20, 2017 12:36
@matticbot matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Mar 20, 2017
@matticbot matticbot added the [Size] S Small sized issue label Mar 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Media The media screen in Calypso, general media management, or integration with third party media. [Feature] Post/Page Editor The editor for editing posts and pages. [Size] S Small sized issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants