Skip to content

Commit 744b10b

Browse files
CopilotswissspidyCopilot
authored
Fix fix-orientation command for Imagick and add WP 5.3+ tests (#233)
* Initial plan * Fix flip_rotate_image orientation check and update feature tests for WP 5.3+ Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Fix Imagick flip-only orientation failure: use DB metadata for detection, normalize after save Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Update src/Media_Command.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add @require-extension-imagick scenario for flip-only orientations and fix PHPStan type error Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Fix flip_rotate_image: guard metadata regeneration behind successful save, validate non-empty metadata Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Fix WP 5.3+ false-positive: verify file EXIF when DB orientation > 1 to detect already-auto-rotated images Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> * Update src/Media_Command.php Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com> Co-authored-by: Pascal Birchler <pascal.birchler@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent e0dc04d commit 744b10b

2 files changed

Lines changed: 166 additions & 22 deletions

File tree

‎features/media-fix-orientation.feature‎

Lines changed: 104 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,8 @@ Feature: Fix WordPress attachments orientation
1111
Error: No images found.
1212
"""
1313

14-
# On WP 4.9 tests this results in "Couldn't fix orientation".
15-
# Todo: Revisit this test and improve or potentially remove it if useless.
16-
@require-extension-exif @require-wp-4.0 @less-than-wp-4.9
14+
# On WP 5.3+, images are auto-rotated by WordPress during import, so fix-orientation reports them as already fixed.
15+
@require-extension-exif @require-wp-4.0 @less-than-wp-5.3
1716
Scenario: Fix orientation for all images
1817
Given download:
1918
| path | url |
@@ -96,10 +95,8 @@ Feature: Fix WordPress attachments orientation
9695
Success: Images already fixed.
9796
"""
9897

99-
# On newer versions (5.3+) the image is already considered fixed.
100-
# On WP 4.9 tests this results in "Couldn't fix orientation".
101-
# Todo: Revisit this test and improve or potentially remove it if useless.
102-
@require-extension-exif @require-wp-4.0 @less-than-wp-4.9
98+
# On WP 5.3+, images are auto-rotated by WordPress during import, so fix-orientation reports them as already fixed.
99+
@require-extension-exif @require-wp-4.0 @less-than-wp-5.3
103100
Scenario: Fix orientation for single image
104101
Given download:
105102
| path | url |
@@ -122,10 +119,110 @@ Feature: Fix WordPress attachments orientation
122119
Success: Image already fixed.
123120
"""
124121

122+
# This specifically tests the Imagick flip-only path (orientations 2, 4) where
123+
# WP_Image_Editor_Imagick::flip() does not update the EXIF orientation tag, requiring
124+
# explicit metadata normalization after the fix.
125+
@require-extension-exif @require-extension-imagick @require-wp-4.0 @less-than-wp-5.3
126+
Scenario: Fix flip-only orientation with Imagick
127+
Given download:
128+
| path | url |
129+
| {CACHE_DIR}/landscape-2.jpg | https://raw.githubusercontent.com/thrijith/test-images/master/Landscape_2.jpg |
130+
| {CACHE_DIR}/portrait-4.jpg | https://raw.githubusercontent.com/thrijith/test-images/master/Portrait_4.jpg |
131+
And I run `wp option update uploads_use_yearmonth_folders 0`
132+
133+
When I run `wp media import {CACHE_DIR}/landscape-2.jpg --title="Landscape Two" --porcelain`
134+
Then save STDOUT as {LANDSCAPE_TWO}
135+
136+
When I run `wp media import {CACHE_DIR}/portrait-4.jpg --title="Portrait Four" --porcelain`
137+
Then save STDOUT as {PORTRAIT_FOUR}
138+
139+
When I run `wp media fix-orientation`
140+
Then STDOUT should contain:
141+
"""
142+
Fixing orientation for "Landscape Two" (ID {LANDSCAPE_TWO}).
143+
"""
144+
And STDOUT should contain:
145+
"""
146+
Fixing orientation for "Portrait Four" (ID {PORTRAIT_FOUR}).
147+
"""
148+
And STDOUT should contain:
149+
"""
150+
Success: Fixed 2 of 2 images.
151+
"""
152+
153+
# Verify that a second run reports no fix required (metadata normalized after save).
154+
When I run `wp media fix-orientation`
155+
Then STDOUT should contain:
156+
"""
157+
No orientation fix required for "Landscape Two" (ID {LANDSCAPE_TWO}).
158+
"""
159+
And STDOUT should contain:
160+
"""
161+
No orientation fix required for "Portrait Four" (ID {PORTRAIT_FOUR}).
162+
"""
163+
And STDOUT should contain:
164+
"""
165+
Success: Images already fixed.
166+
"""
167+
125168
@require-wp-4.0
126169
Scenario: Fix orientation for non existent image
127170
When I try `wp media fix-orientation 9999`
128171
Then STDERR should be:
129172
"""
130173
Error: No images found.
131174
"""
175+
176+
@require-extension-exif @require-wp-5.3
177+
Scenario: Fix orientation for all images already auto-rotated by WordPress
178+
Given download:
179+
| path | url |
180+
| {CACHE_DIR}/landscape-2.jpg | https://raw.githubusercontent.com/thrijith/test-images/master/Landscape_2.jpg |
181+
| {CACHE_DIR}/landscape-5.jpg | https://raw.githubusercontent.com/thrijith/test-images/master/Landscape_5.jpg |
182+
| {CACHE_DIR}/portrait-4.jpg | https://raw.githubusercontent.com/thrijith/test-images/master/Portrait_4.jpg |
183+
And I run `wp option update uploads_use_yearmonth_folders 0`
184+
185+
When I run `wp media import {CACHE_DIR}/landscape-2.jpg --title="Landscape Two" --porcelain`
186+
Then save STDOUT as {LANDSCAPE_TWO}
187+
188+
When I run `wp media import {CACHE_DIR}/landscape-5.jpg --title="Landscape Five" --porcelain`
189+
Then save STDOUT as {LANDSCAPE_FIVE}
190+
191+
When I run `wp media import {CACHE_DIR}/portrait-4.jpg --title="Portrait Four" --porcelain`
192+
Then save STDOUT as {PORTRAIT_FOUR}
193+
194+
When I run `wp media fix-orientation`
195+
Then STDOUT should contain:
196+
"""
197+
No orientation fix required for "Portrait Four" (ID {PORTRAIT_FOUR}).
198+
"""
199+
200+
And STDOUT should contain:
201+
"""
202+
No orientation fix required for "Landscape Five" (ID {LANDSCAPE_FIVE}).
203+
"""
204+
205+
And STDOUT should contain:
206+
"""
207+
No orientation fix required for "Landscape Two" (ID {LANDSCAPE_TWO}).
208+
"""
209+
210+
And STDOUT should contain:
211+
"""
212+
Success: Images already fixed.
213+
"""
214+
215+
@require-extension-exif @require-wp-5.3
216+
Scenario: Fix orientation for single image already auto-rotated by WordPress
217+
Given download:
218+
| path | url |
219+
| {CACHE_DIR}/portrait-6.jpg | https://raw.githubusercontent.com/thrijith/test-images/master/Portrait_6.jpg |
220+
When I run `wp media import {CACHE_DIR}/portrait-6.jpg --title="Portrait Six" --porcelain`
221+
Then save STDOUT as {PORTRAIT_SIX}
222+
223+
When I run `wp media fix-orientation {PORTRAIT_SIX}`
224+
Then STDOUT should be:
225+
"""
226+
1/1 No orientation fix required for "Portrait Six" (ID {PORTRAIT_SIX}).
227+
Success: Image already fixed.
228+
"""

‎src/Media_Command.php‎

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1462,14 +1462,48 @@ private function process_orientation_fix( $id, $progress, &$successes, &$errors,
14621462
return;
14631463
}
14641464

1465-
// Get current metadata of the attachment.
1466-
$metadata = wp_generate_attachment_metadata( $id, $full_size_path );
1467-
$image_meta = ! empty( $metadata['image_meta'] ) ? $metadata['image_meta'] : [];
1465+
// Get current metadata of the attachment from the database.
1466+
$metadata = wp_get_attachment_metadata( $id );
1467+
$image_meta = is_array( $metadata ) && ! empty( $metadata['image_meta'] ) ? $metadata['image_meta'] : [];
14681468

1469-
if ( isset( $image_meta['orientation'] ) && absint( $image_meta['orientation'] ) > 1 ) {
1469+
// Determine orientation from DB metadata first.
1470+
$orientation = isset( $image_meta['orientation'] ) ? absint( $image_meta['orientation'] ) : 0;
1471+
1472+
if ( $orientation > 1 ) {
1473+
// DB shows orientation > 1, but WP 5.3+ may have already auto-rotated the image
1474+
// on import (via wp_maybe_exif_rotate()), storing the original EXIF value before
1475+
// rotating. On WP < 5.3 this behavior does not occur, so skip the extra EXIF read.
1476+
if ( Utils\wp_version_compare( '5.3', '>=' ) ) {
1477+
// Verify against the file's current EXIF: if it is <= 1 the image is already
1478+
// correctly oriented and no fix is needed.
1479+
$file_image_meta = wp_read_image_metadata( $full_size_path );
1480+
if ( is_array( $file_image_meta ) && isset( $file_image_meta['orientation'] ) ) {
1481+
$raw_orientation = $file_image_meta['orientation'];
1482+
$file_orientation = is_scalar( $raw_orientation ) ? absint( $raw_orientation ) : 0;
1483+
if ( $file_orientation <= 1 ) {
1484+
$orientation = $file_orientation;
1485+
}
1486+
}
1487+
}
1488+
} elseif ( empty( $image_meta ) || ! isset( $image_meta['orientation'] ) ) {
1489+
// DB has no orientation data at all (stale/absent metadata). Fall back to reading
1490+
// from the file's EXIF so the command still works for such attachments.
1491+
$file_image_meta = wp_read_image_metadata( $full_size_path );
1492+
if ( is_array( $file_image_meta ) && isset( $file_image_meta['orientation'] ) ) {
1493+
$raw_orientation = $file_image_meta['orientation'];
1494+
$file_orientation = is_scalar( $raw_orientation ) ? absint( $raw_orientation ) : 0;
1495+
if ( $file_orientation > 1 ) {
1496+
// Merge file-based metadata so flip_rotate_image() has the orientation.
1497+
$image_meta = array_merge( $image_meta, $file_image_meta );
1498+
$orientation = $file_orientation;
1499+
}
1500+
}
1501+
}
1502+
1503+
if ( $orientation > 1 ) {
14701504
if ( ! $dry_run ) {
14711505
WP_CLI::log( "{$progress} Fixing orientation for {$att_desc}." );
1472-
if ( false !== $this->flip_rotate_image( $id, $metadata, $image_meta, $full_size_path ) ) {
1506+
if ( false !== $this->flip_rotate_image( $id, $image_meta, $full_size_path ) ) {
14731507
++$successes;
14741508
} else {
14751509
++$errors;
@@ -1488,13 +1522,12 @@ private function process_orientation_fix( $id, $progress, &$successes, &$errors,
14881522
* Perform image rotate operations on the image.
14891523
*
14901524
* @param int $id Attachment Id.
1491-
* @param array $metadata Attachment Metadata.
14921525
* @param array $image_meta `image_meta` information for the attachment.
14931526
* @param string $full_size_path Path to original image.
14941527
*
14951528
* @return bool Whether the image rotation operation succeeded.
14961529
*/
1497-
private function flip_rotate_image( $id, $metadata, $image_meta, $full_size_path ) {
1530+
private function flip_rotate_image( $id, $image_meta, $full_size_path ) {
14981531
$editor = wp_get_image_editor( $full_size_path );
14991532

15001533
if ( ! is_wp_error( $editor ) ) {
@@ -1510,17 +1543,31 @@ private function flip_rotate_image( $id, $metadata, $image_meta, $full_size_path
15101543
$editor->flip( $operations['flip'][0], $operations['flip'][1] );
15111544
}
15121545

1513-
// Save the image and generate metadata.
1514-
$editor->save( $full_size_path );
1515-
$metadata = wp_generate_attachment_metadata( $id, $full_size_path );
1516-
$image_meta = empty( $metadata['image_meta'] ) ? [] : $metadata['image_meta'];
1546+
$saved = $editor->save( $full_size_path );
15171547

1518-
// Update attachment metadata with newly generated data.
1519-
wp_update_attachment_metadata( $id, $metadata );
1548+
if ( is_wp_error( $saved ) ) {
1549+
return false;
1550+
}
15201551

1521-
if ( isset( $image_meta['orientation'] ) && absint( $image_meta['orientation'] ) === 0 ) {
1522-
return true;
1552+
// Regenerate attachment metadata after the corrected image is saved.
1553+
$metadata = wp_generate_attachment_metadata( $id, $full_size_path );
1554+
1555+
if ( empty( $metadata ) ) {
1556+
return false;
1557+
}
1558+
1559+
// Normalize the stored orientation to prevent re-detection on subsequent runs.
1560+
// WP_Image_Editor_Imagick::flip() does not reset the EXIF orientation tag in the
1561+
// file, so the file may still report a non-normal orientation even though the pixels
1562+
// have been corrected. Forcing orientation to 0 in the stored metadata ensures the
1563+
// next run reports "No orientation fix required".
1564+
if ( isset( $metadata['image_meta']['orientation'] ) ) {
1565+
$metadata['image_meta']['orientation'] = 0;
15231566
}
1567+
1568+
wp_update_attachment_metadata( $id, $metadata );
1569+
1570+
return true;
15241571
}
15251572

15261573
return false;

0 commit comments

Comments
 (0)