-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
Prevent images from being rotated if the original contains EXIF orientation metadata #93
Conversation
This looks good based on the Stack Overflow post you mention. One thing that would make this PR stronger is if we add an image to the test directory that exhibits the issue and is not square. Then a similar test to https://github.com/11ty/eleventy-img/blob/master/test/test.js#L233 -- "Try to use a width larger than original (statsSync)" could demonstrate whether the bug is fixed by checking that the width is longer than the length (or vice versa depending on the test image). The only difficulty is that a test image should be small, an original file from an iPhone would be too large to be good for testing. Perhaps the person who reported issue #49 could help in generating a small image file which has EXIF data that exhibits the bug. |
Below are a couple of images that I have created for testing. The first has no EXIF header, and the second has the following minimal EXIF data that should set the orientation to be upside down:
Although the images display with their correct orientation here, in my file browser, and when opened in Firefox, unfortunately it appears that This issue is not limited to using the Presumably something is missing from my metadata, although I did try copying all fields over from the camera image and it was still ignored. I can't see anything obvious within |
This doesn't seem to work when keeping the original resolution: if I pass |
@@ -292,6 +292,7 @@ async function resizeImage(src, options = {}) { | |||
if(metadata.format !== "svg" || !options.svgAllowUpscale) { | |||
resizeOptions.withoutEnlargement = true; | |||
} | |||
sharpInstance.rotate(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is never reached if stat.width < metadata.width || (options.svgAllowUpscale && metadata.format === "svg"
is false.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's OK. The issue is caused when resizing only, so the code is reached for the same cases when the bug is happening.
If the image is rotated 90 or 270 degrees |
This seems okay to me but needs another |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Closing for #132 |
Fixes: #49
Adds a rotation operation using sharp immediately prior to resizing, as suggested in Stack Overflow post here.