If you upload images from a cell phone or modern digital camera, the EXIF Orientation flag is used to rotate the image according to the orientation of the camera. Sadly, Firefox and Chrome don't honor the flag, and so images can appear to be rotated improperly on the site. There is a drupal module, autorotate that adds an autorotate feature to fix this, but it doesn't work properly with the imagemagick toolkit because it rotates the image but leaves the EXIF Orient flag unchanged.
In trying to fix the issue I discovered it would be easier for imagemagick to support it directly, since ImageMagick has an -auto-orient flag that takes care of rotation and EXIF Orient flags in one step. Attached is a patch that adds the functionality (and fixes some comment/help strings which are wrong in imagemagick_advanced.module).
I think this is a useful feature, and hope you will adopt it into the code base. Thanks.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | imagemagick_advanced.module.patch | 2.64 KB | mikel1 |
| #2 | imagemagick_advanced.module.unified.diff | 2.56 KB | mikel1 |
| #2 | imagemagick_advanced.module.git_.diff | 2.56 KB | mikel1 |
| imagemagick_advanced.module.patch | 2.93 KB | mikel1 |
Comments
Comment #1
sunNice!
Can you provide a regular unified git diff, please?
Comment #2
mikel1 commentedSorry, I'm a git newbie. You mean just a "diff --git" of what against 7.x-1.0-alpha2 or a "git diff" against the head of the repository? Attaching both. Hope that's good.
Comment #3
sunThe git diff one is the right one. :) This applies to all patches on drupal.org. Btw, you might be interested in reading the Configuring Git handbook page (and possibly related pages). :)
That's not meant to confuse you though - the .git.diff patch from #2 is fine. :) (That said, we commonly use .patch as file extension ;))
This looks pretty good already and I think we're close. I only have some smaller nitpicks:
s/image/images/
On the first, also:
s/Strip/Strips/ (third-person form)
Hm — "Auto orient" is not really a term or phrase.
While "Auto-orient" would probably be more correct, that would still be very technical, and I think that the labels are more focused on user interaction.
How about one of these?
1) Automatically correct orientation
2) Fix orientation
3) Adjust orientation
What do you think?
I think I'd opt for the last one, but you definitely have more experience with this effect :)
s/Auto orients/Automatically adjusts the orientation of/
Comment #4
mikel1 commentedWell, I don't have a lot of experience with it. I only discovered yesterday that - surprise! - browsers don't support the flag. There is a multi-year argument about whether to modify Firefox to support it. Ugh.
Anyway, your changes are good. I chose "Auto orient" because I thought it made clear to the user what it was - you can look it up in the ImageMagick documentation. However, you are right in that it should be geared more towards web site designers, not developers. Out of the suggestions you list, I think I'd prefer "Automatically correct orientation" as I think it's the most descriptive. "Autorotate" would be fine, too. The drupal autorotate module already uses that term, so I don't know if that's a plus or minus (what if the user enabled both modules?).
So how does this go from here. Do I need to take any further action? Am I supposed to post a "final" patch? Are you going to commit the change?
Comment #5
sunWould be great if you could provide a final patch that incorporates the tweaks outlined in #3.
Comment #6
mikel1 commentedOK, here it is
Comment #7
sunThanks for reporting, reviewing, and testing! Committed to 7.x-1.x.
A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.