From 7da64db6ccb51535fa6fbf6de6c32f8f3aa253fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Robin?= Date: Fri, 14 Aug 2026 16:42:47 +0200 Subject: [PATCH 1/6] Fix #1235: stop overriding core's resize opt-out on client side uploads WordPress 7.1 switches its own downscaling off while the browser handles an upload, because the browser supplies the scaled file itself. Imagify's filter ignored the incoming value and always won, so the server produced a second scaled file that nothing referenced and `original_image` ended up pointing at it instead of the real upload. Hand a false threshold straight back, and remember the attachment so the later asynchronous optimization does not shrink the original the browser left intact. --- .../WP/FilterBigImageSizeThresholdTest.php | 81 +++++++++++++++++++ classes/Context/WP.php | 70 ++++++++++++++++ classes/Optimization/Process/WP.php | 26 ++++++ inc/classes/class-imagify-options.php | 6 +- inc/common/attachments.php | 2 +- 5 files changed, 182 insertions(+), 3 deletions(-) create mode 100644 Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php diff --git a/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php b/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php new file mode 100644 index 000000000..dc79735da --- /dev/null +++ b/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php @@ -0,0 +1,81 @@ +justReturn( true ); + Functions\expect( 'get_imagify_option' )->never(); + + $this->assertFalse( ( new WP() )->filter_big_image_size_threshold( false, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); + } + + /** + * Test: the attachment is flagged so the later, asynchronous optimization does not resize it. + */ + public function testFlagsTheAttachmentWhenCoreDisabledResizing(): void { + $stored = []; + + Functions\when( 'set_transient' )->alias( + function ( $name, $value ) use ( &$stored ) { + $stored[ $name ] = $value; + return true; + } + ); + + ( new WP() )->filter_big_image_size_threshold( false, [ 3800, 2500 ], '/tmp/big.jpg', 123 ); + + $this->assertSame( [ 'imagify_client_side_scaled_123' => 1 ], $stored ); + } + + /** + * Test: nothing is flagged when no attachment ID is supplied, as happens when the value + * is read outside of an upload. + */ + public function testDoesNotFlagWithoutAnAttachmentId(): void { + Functions\expect( 'set_transient' )->never(); + + $this->assertFalse( ( new WP() )->filter_big_image_size_threshold( false ) ); + } + + /** + * Test: Imagify's own resizing value is still applied when core did not opt out. + */ + public function testReturnsImagifyThresholdWhenResizingIsEnabled(): void { + Functions\when( 'get_imagify_option' )->alias( + function ( $option ) { + return 'resize_larger' === $option ? 1 : 1200; + } + ); + + $this->assertSame( 1200, ( new WP() )->filter_big_image_size_threshold( 2560, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); + } + + /** + * Test: with the setting off, the threshold is 0 and WordPress skips its own resizing. + */ + public function testReturnsZeroWhenResizingIsDisabled(): void { + Functions\when( 'get_imagify_option' )->justReturn( 0 ); + + $this->assertSame( 0, ( new WP() )->filter_big_image_size_threshold( 2560, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); + } +} diff --git a/classes/Context/WP.php b/classes/Context/WP.php index 0b0b658a3..58c7c8e50 100644 --- a/classes/Context/WP.php +++ b/classes/Context/WP.php @@ -69,6 +69,76 @@ public function get_resizing_threshold() { return $this->resizing_threshold; } + /** + * Filter WP's "big images threshold" with Imagify's resizing value. + * + * A `false` value means downscaling has been switched off on purpose, so it is + * returned untouched. WordPress 7.1 does exactly that while the browser handles + * the sub sizes: it supplies its own scaled file through the sideload endpoint, + * and scaling again on the server would leave a conflicting "-scaled" file behind + * and point `original_image` at it instead of the real upload. + * + * The value the browser scales to comes from this same filter + * ({@see WP_REST_Server::get_index()}), so Imagify's setting is still honoured. + * + * @since 2.3.3 + * + * @param int|false $threshold The threshold value in pixels, or false to disable resizing. + * @param array $imagesize Indexed array of the image width and height in pixels. + * @param string $file Full path to the uploaded image file. + * @param int $attachment_id Attachment post ID. + * @return int|false + */ + public function filter_big_image_size_threshold( $threshold, $imagesize = [], $file = '', $attachment_id = 0 ) { + if ( false === $threshold ) { + if ( $attachment_id ) { + self::flag_client_side_scaling( $attachment_id ); + } + + return $threshold; + } + + return $this->get_resizing_threshold(); + } + + /** + * Remember that the browser is supplying the scaled version of an attachment. + * + * Optimization runs in a later, asynchronous request, where the filters WordPress + * set up during the upload are long gone, so the state has to be stored. + * + * @since 2.3.3 + * + * @param int $attachment_id Attachment post ID. + */ + public static function flag_client_side_scaling( $attachment_id ) { + set_transient( self::get_client_side_scaling_flag( $attachment_id ), 1, HOUR_IN_SECONDS ); + } + + /** + * Tell if the browser supplied the scaled version of an attachment. + * + * @since 2.3.3 + * + * @param int $attachment_id Attachment post ID. + * @return bool + */ + public static function is_client_side_scaled( $attachment_id ) { + return (bool) get_transient( self::get_client_side_scaling_flag( $attachment_id ) ); + } + + /** + * Get the transient name used to flag client side scaling for an attachment. + * + * @since 2.3.3 + * + * @param int $attachment_id Attachment post ID. + * @return string + */ + private static function get_client_side_scaling_flag( $attachment_id ) { + return 'imagify_client_side_scaled_' . (int) $attachment_id; + } + /** * Tell if the optimization process is allowed to backup in this context. * diff --git a/classes/Optimization/Process/WP.php b/classes/Optimization/Process/WP.php index 8cbbc9b4b..491c69767 100644 --- a/classes/Optimization/Process/WP.php +++ b/classes/Optimization/Process/WP.php @@ -17,6 +17,32 @@ */ class WP extends AbstractProcess { + /** ----------------------------------------------------------------------------------------- */ + /** RESIZING ================================================================================ */ + /** ----------------------------------------------------------------------------------------- */ + + /** + * Tell if a size should be resized. + * + * When the browser handled the upload it also produced the scaled version, using the + * very threshold Imagify configured ({@see \Imagify\Context\WP::filter_big_image_size_threshold()}), + * so resizing here would only shrink the untouched original that WordPress keeps + * aside as `original_image`. + * + * @since 2.3.3 + * + * @param string $size The size name. + * @param File $file A File instance. + * @return bool + */ + protected function can_resize( $size, $file ) { + if ( ! parent::can_resize( $size, $file ) ) { + return false; + } + + return ! \Imagify\Context\WP::is_client_side_scaled( $this->get_media()->get_id() ); + } + /** ----------------------------------------------------------------------------------------- */ /** MISSING THUMBNAILS ====================================================================== */ /** ----------------------------------------------------------------------------------------- */ diff --git a/inc/classes/class-imagify-options.php b/inc/classes/class-imagify-options.php index 6b1ab300c..f9f48ffcf 100644 --- a/inc/classes/class-imagify-options.php +++ b/inc/classes/class-imagify-options.php @@ -76,7 +76,7 @@ protected function __construct() { if ( function_exists( 'wp_get_original_image_path' ) ) { $this->reset_values['resize_larger'] = 1; - $filter_cb = [ imagify_get_context( 'wp' ), 'get_resizing_threshold' ]; + $filter_cb = [ imagify_get_context( 'wp' ), 'filter_big_image_size_threshold' ]; $filtered = has_filter( 'big_image_size_threshold', $filter_cb ); if ( $filtered ) { @@ -88,7 +88,9 @@ protected function __construct() { $this->reset_values['resize_larger_w'] = $this->sanitize_and_validate_value( 'resize_larger_w', $this->reset_values['resize_larger_w'], $this->default_values['resize_larger_w'] ); if ( $filtered ) { - add_filter( 'big_image_size_threshold', $filter_cb, IMAGIFY_INT_MAX ); + // The argument count has to be repeated here, or the callback would be + // registered with a single one and stop receiving the attachment ID. + add_filter( 'big_image_size_threshold', $filter_cb, IMAGIFY_INT_MAX, 4 ); } } diff --git a/inc/common/attachments.php b/inc/common/attachments.php index 17939c5a5..068393e8d 100755 --- a/inc/common/attachments.php +++ b/inc/common/attachments.php @@ -67,7 +67,7 @@ function imagify_add_avif_type( $ext2type ) { * @since WP 5.3 * @author Grégory Viguier */ -add_filter( 'big_image_size_threshold', [ imagify_get_context( 'wp' ), 'get_resizing_threshold' ], IMAGIFY_INT_MAX ); +add_filter( 'big_image_size_threshold', [ imagify_get_context( 'wp' ), 'filter_big_image_size_threshold' ], IMAGIFY_INT_MAX, 4 ); /** * Add filters to manage images formats that will be generated From f4d440714491a3c418fab586bc8f24482b4af573 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Robin?= Date: Fri, 14 Aug 2026 21:41:55 +0200 Subject: [PATCH 2/6] Cover the resize guard with tests Check the flag before delegating to the parent, so a media the browser already scaled is refused without the rest of the work, and the branch can be exercised directly. --- .../Optimization/Process/WP/CanResizeTest.php | 91 +++++++++++++++++++ classes/Optimization/Process/WP.php | 6 +- 2 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php diff --git a/Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php b/Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php new file mode 100644 index 000000000..86aa8f1b6 --- /dev/null +++ b/Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php @@ -0,0 +1,91 @@ +shouldReceive( 'get_id' )->andReturn( $media_id ); + } + + /* + * A partial mock leaves protected methods alone, so the real can_resize() runs while + * get_media() is stubbed. The media comes from the optimization data, so there is no + * property to set instead. + */ + $process = Mockery::mock( WP::class )->makePartial(); + $process->shouldReceive( 'get_media' )->andReturn( $media ); + $process->shouldReceive( 'is_valid' )->andReturn( false ); + + $method = new \ReflectionMethod( get_class( $process ), 'can_resize' ); + $method->setAccessible( true ); + + return $method->invoke( $process, 'full', Mockery::mock( 'Imagify\Optimization\File' ) ); + } + + /** + * Test: a media the browser already scaled is not resized again. + */ + public function testRefusesToResizeWhenTheBrowserSuppliedTheScaledFile(): void { + Functions\when( 'get_transient' )->alias( + function ( $name ) { + return 'imagify_client_side_scaled_42' === $name ? 1 : false; + } + ); + + $this->assertFalse( $this->canResize( 42 ) ); + } + + /** + * Test: the decision is per attachment, so another media is not caught by the flag. + */ + public function testDoesNotRefuseForAnotherMedia(): void { + Functions\when( 'get_transient' )->alias( + function ( $name ) { + return 'imagify_client_side_scaled_42' === $name ? 1 : false; + } + ); + + /* + * Media 99 carries no flag, so the parent decision applies. The parent bails on an + * invalid media, which is what an instance built without a constructor is, so the + * result is false here too. What matters is that the flag was not what decided it: + * the transient for 99 was consulted and came back empty. + */ + $this->assertFalse( $this->canResize( 99 ) ); + } + + /** + * Test: a process without a media does not blow up looking for an ID. + */ + public function testHandlesAProcessWithoutMedia(): void { + Functions\when( 'get_transient' )->justReturn( false ); + + $this->assertFalse( $this->canResize( null ) ); + } +} diff --git a/classes/Optimization/Process/WP.php b/classes/Optimization/Process/WP.php index 491c69767..b2b43decc 100644 --- a/classes/Optimization/Process/WP.php +++ b/classes/Optimization/Process/WP.php @@ -36,11 +36,13 @@ class WP extends AbstractProcess { * @return bool */ protected function can_resize( $size, $file ) { - if ( ! parent::can_resize( $size, $file ) ) { + $media = $this->get_media(); + + if ( $media && \Imagify\Context\WP::is_client_side_scaled( $media->get_id() ) ) { return false; } - return ! \Imagify\Context\WP::is_client_side_scaled( $this->get_media()->get_id() ); + return parent::can_resize( $size, $file ); } /** ----------------------------------------------------------------------------------------- */ From 533f7af3a239d45e9eda6754eabcfdceb39d01df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Robin?= Date: Mon, 17 Aug 2026 10:17:59 +0200 Subject: [PATCH 3/6] Address review on the resize opt-out fix The can_resize test satisfied everything the parent checks except the guard, so it passed whether or not the guard existed. Every condition is now stubbed to answer true, which makes the guard the only thing that can return false, and a case covers the media the browser did not scale. Register the new transient in InternalStateList so a reset and uninstall clear it, and derive the reset test's query count from that list rather than repeating the number. Note on the flag why it is left to expire instead of deleted. --- .../Optimization/Process/WP/CanResizeTest.php | 38 +++++++++++++------ .../Tools/InternalStateList/sharedList.php | 2 + .../Tools/ResetInternalState/reset.php | 9 +++-- classes/Context/WP.php | 5 +++ classes/Tools/InternalStateList.php | 3 ++ 5 files changed, 43 insertions(+), 14 deletions(-) diff --git a/Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php b/Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php index 86aa8f1b6..a776b02b1 100644 --- a/Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php +++ b/Tests/Unit/classes/Optimization/Process/WP/CanResizeTest.php @@ -13,6 +13,9 @@ * it produced the scaled version itself, using the threshold Imagify configured, so resizing on * the server would only shrink the untouched original WordPress keeps aside as `original_image`. * + * Everything the parent needs is stubbed so it would answer true, which is what makes the + * assertions meaningful: only the guard under test can turn the answer into false. + * * @covers \Imagify\Optimization\Process\WP::can_resize * @group ProcessWP * @since 2.3.3 @@ -20,7 +23,8 @@ class CanResizeTest extends TestCase { /** - * Invoke the protected method on a process whose media reports the given ID. + * Invoke the protected method on a process whose media reports the given ID, with every + * condition the parent checks satisfied. * * @param int|null $media_id Media ID, or null for no media at all. * @return bool @@ -29,10 +33,17 @@ private function canResize( $media_id ): bool { $media = false; if ( null !== $media_id ) { + $context = Mockery::mock( 'Imagify\Context\ContextInterface' ); + $context->shouldReceive( 'can_resize' )->andReturn( true ); + $media = Mockery::mock( 'Imagify\Media\MediaInterface' ); $media->shouldReceive( 'get_id' )->andReturn( $media_id ); + $media->shouldReceive( 'get_context_instance' )->andReturn( $context ); } + $file = Mockery::mock( 'Imagify\Optimization\File' ); + $file->shouldReceive( 'is_image' )->andReturn( true ); + /* * A partial mock leaves protected methods alone, so the real can_resize() runs while * get_media() is stubbed. The media comes from the optimization data, so there is no @@ -40,16 +51,18 @@ private function canResize( $media_id ): bool { */ $process = Mockery::mock( WP::class )->makePartial(); $process->shouldReceive( 'get_media' )->andReturn( $media ); - $process->shouldReceive( 'is_valid' )->andReturn( false ); + // is_valid() is get_media() && get_media()->is_valid(), so it cannot be true without a media. + $process->shouldReceive( 'is_valid' )->andReturn( null !== $media_id ); $method = new \ReflectionMethod( get_class( $process ), 'can_resize' ); $method->setAccessible( true ); - return $method->invoke( $process, 'full', Mockery::mock( 'Imagify\Optimization\File' ) ); + return $method->invoke( $process, 'full', $file ); } /** - * Test: a media the browser already scaled is not resized again. + * Test: a media the browser already scaled is not resized again. Without the guard this + * returns true, since every condition the parent checks is satisfied. */ public function testRefusesToResizeWhenTheBrowserSuppliedTheScaledFile(): void { Functions\when( 'get_transient' )->alias( @@ -61,6 +74,15 @@ function ( $name ) { $this->assertFalse( $this->canResize( 42 ) ); } + /** + * Test: an ordinary media is still resized, so the guard does not block everything. + */ + public function testStillResizesWhenTheBrowserDidNotScaleTheFile(): void { + Functions\when( 'get_transient' )->justReturn( false ); + + $this->assertTrue( $this->canResize( 42 ) ); + } + /** * Test: the decision is per attachment, so another media is not caught by the flag. */ @@ -71,13 +93,7 @@ function ( $name ) { } ); - /* - * Media 99 carries no flag, so the parent decision applies. The parent bails on an - * invalid media, which is what an instance built without a constructor is, so the - * result is false here too. What matters is that the flag was not what decided it: - * the transient for 99 was consulted and came back empty. - */ - $this->assertFalse( $this->canResize( 99 ) ); + $this->assertTrue( $this->canResize( 99 ) ); } /** diff --git a/Tests/Unit/classes/Tools/InternalStateList/sharedList.php b/Tests/Unit/classes/Tools/InternalStateList/sharedList.php index 792abae99..ec6a487ea 100644 --- a/Tests/Unit/classes/Tools/InternalStateList/sharedList.php +++ b/Tests/Unit/classes/Tools/InternalStateList/sharedList.php @@ -68,6 +68,8 @@ public function testGetLockedTransientPatternsReturnsExpectedArray(): void { '_transient_%imagify_rpc_%', '_transient_imagify_%_process_locked', '_site_transient_imagify_%_process_lock%', + '_transient_imagify_client_side_scaled_%', + '_transient_timeout_imagify_client_side_scaled_%', ]; $this->assertSame( $expected, InternalStateList::get_locked_transient_patterns() ); diff --git a/Tests/Unit/classes/Tools/ResetInternalState/reset.php b/Tests/Unit/classes/Tools/ResetInternalState/reset.php index 59ec5e71d..01af2e86f 100644 --- a/Tests/Unit/classes/Tools/ResetInternalState/reset.php +++ b/Tests/Unit/classes/Tools/ResetInternalState/reset.php @@ -4,6 +4,7 @@ namespace Imagify\Tests\Unit\classes\Tools\ResetInternalState; use Imagify\Tests\Unit\TestCase; +use Imagify\Tools\InternalStateList; use Imagify\Tools\ResetInternalState; use Mockery; use Brain\Monkey\Functions; @@ -161,7 +162,7 @@ function ( string $sql, string $pattern ) use ( &$patterns_queried ) { } ); - $this->wpdb->shouldReceive( 'query' )->times( 4 )->andReturn( 0 ); + $this->wpdb->shouldReceive( 'query' )->times( count( InternalStateList::get_locked_transient_patterns() ) )->andReturn( 0 ); ( new ResetInternalState() )->reset(); @@ -171,6 +172,8 @@ function ( string $sql, string $pattern ) use ( &$patterns_queried ) { '\_transient\_%imagify\_rpc\_%', '\_transient\_imagify\_%\_process\_locked', '\_site\_transient\_imagify\_%\_process\_lock%', + '\_transient\_imagify\_client\_side\_scaled\_%', + '\_transient\_timeout\_imagify\_client\_side\_scaled\_%', ]; foreach ( $expected_patterns as $pattern ) { @@ -263,7 +266,7 @@ function () use ( &$query_calls ) { ( new ResetInternalState() )->reset(); - // 4 options-pattern queries prove reset() ran to completion. - $this->assertSame( 4, $query_calls ); + // One options-pattern query per registered pattern proves reset() ran to completion. + $this->assertSame( count( InternalStateList::get_locked_transient_patterns() ), $query_calls ); } } diff --git a/classes/Context/WP.php b/classes/Context/WP.php index 58c7c8e50..1079f7670 100644 --- a/classes/Context/WP.php +++ b/classes/Context/WP.php @@ -107,6 +107,11 @@ public function filter_big_image_size_threshold( $threshold, $imagesize = [], $f * Optimization runs in a later, asynchronous request, where the filters WordPress * set up during the upload are long gone, so the state has to be stored. * + * The flag is left to expire rather than deleted after use: it is read once per size + * being optimized, so deleting it on the first read would let the remaining sizes + * resize the file. An hour is far longer than the queue needs, and the pattern is + * registered in {@see \Imagify\Tools\InternalStateList} so a reset clears it. + * * @since 2.3.3 * * @param int $attachment_id Attachment post ID. diff --git a/classes/Tools/InternalStateList.php b/classes/Tools/InternalStateList.php index b38beee26..72ca93bc5 100644 --- a/classes/Tools/InternalStateList.php +++ b/classes/Tools/InternalStateList.php @@ -64,6 +64,9 @@ public static function get_locked_transient_patterns(): array { '_transient_%imagify_rpc_%', // Legacy/deprecated. '_transient_imagify_%_process_locked', '_site_transient_imagify_%_process_lock%', + // Flags an attachment whose scaled version came from the browser, on WP 7.1+. + '_transient_imagify_client_side_scaled_%', + '_transient_timeout_imagify_client_side_scaled_%', ]; } From a77825a5dbf04471cf3c84179fe9ed976e0450a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Robin?= Date: Mon, 17 Aug 2026 10:58:35 +0200 Subject: [PATCH 4/6] Only stand down for the upload the browser actually scaled Treating every false on the filter as "already scaled" changed behaviour for any site where a third party returns false for its own reasons: WordPress would skip the downscale because it was told to, and Imagify would skip its own because it believed the work was done, leaving an image nobody resized and no scaled file anywhere. Take the state from where WordPress declares it instead. rest_after_insert_attachment runs just before the metadata is generated and carries generate_sub_sizes, which is false exactly when the browser owns the sub sizes. A false from any other source is overridden with Imagify's value, as it was before, so nothing changes outside the 7.1 browser flow. --- .../WP/FilterBigImageSizeThresholdTest.php | 127 +++++++++++++----- classes/Context/WP.php | 54 ++++++-- inc/common/attachments.php | 11 ++ 3 files changed, 150 insertions(+), 42 deletions(-) diff --git a/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php b/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php index dc79735da..40aed847a 100644 --- a/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php +++ b/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php @@ -6,66 +6,92 @@ use Brain\Monkey\Functions; use Imagify\Context\WP; use Imagify\Tests\Unit\TestCase; +use Mockery; /** - * Tests for \Imagify\Context\WP::filter_big_image_size_threshold() — WordPress 7.1 switches - * its own downscaling off while the browser handles the upload, because the browser supplies - * the scaled file itself. Overriding that leaves a conflicting "-scaled" file behind and - * points `original_image` at it, so a `false` threshold has to be handed back untouched. + * Tests for \Imagify\Context\WP::filter_big_image_size_threshold() — WordPress 7.1 switches its + * own downscaling off while the browser handles the upload, because the browser supplies the + * scaled file itself. Overriding that leaves a conflicting "-scaled" file behind and points + * `original_image` at it, so the threshold has to be handed back untouched there. + * + * Only there: a `false` from anywhere else is still overridden, since nothing produced a scaled + * file in that case and the image would end up resized by nobody. * * @covers \Imagify\Context\WP::filter_big_image_size_threshold + * @covers \Imagify\Context\WP::maybe_flag_client_side_scaling * @group ContextWP * @since 2.3.3 */ class FilterBigImageSizeThresholdTest extends TestCase { /** - * Test: a false threshold is returned untouched, so core's opt out wins. + * Stubs the resizing option as enabled with the given width. + * + * @param int $width Configured resizing width. */ - public function testReturnsFalseUntouchedWhenCoreDisabledResizing(): void { - Functions\when( 'set_transient' )->justReturn( true ); - Functions\expect( 'get_imagify_option' )->never(); - - $this->assertFalse( ( new WP() )->filter_big_image_size_threshold( false, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); + private function stubResizingOption( int $width ): void { + Functions\when( 'get_imagify_option' )->alias( + function ( $option ) use ( $width ) { + return 'resize_larger' === $option ? 1 : $width; + } + ); } /** - * Test: the attachment is flagged so the later, asynchronous optimization does not resize it. + * Build a request stub returning the given value for the 'generate_sub_sizes' parameter. + * + * @param mixed $value Value the parameter should return. + * @return Mockery\MockInterface */ - public function testFlagsTheAttachmentWhenCoreDisabledResizing(): void { - $stored = []; + private function requestReturning( $value ) { + $request = Mockery::mock( 'WP_REST_Request' ); + $request->shouldReceive( 'get_param' )->with( 'generate_sub_sizes' )->andReturn( $value ); - Functions\when( 'set_transient' )->alias( - function ( $name, $value ) use ( &$stored ) { - $stored[ $name ] = $value; - return true; + return $request; + } + + /** + * Test: the threshold is handed back untouched for the upload the browser scaled. + */ + public function testReturnsFalseUntouchedForABrowserScaledUpload(): void { + Functions\when( 'get_transient' )->alias( + function ( $name ) { + return 'imagify_client_side_scaled_123' === $name ? 1 : false; } ); + Functions\expect( 'get_imagify_option' )->never(); - ( new WP() )->filter_big_image_size_threshold( false, [ 3800, 2500 ], '/tmp/big.jpg', 123 ); + $this->assertFalse( ( new WP() )->filter_big_image_size_threshold( false, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); + } - $this->assertSame( [ 'imagify_client_side_scaled_123' => 1 ], $stored ); + /** + * Test: a false from anywhere else is overridden with Imagify's value, as it always was. + * Nothing scaled the file in that case, so standing down would leave it unresized. + */ + public function testOverridesAFalseThatDidNotComeFromTheBrowserFlow(): void { + Functions\when( 'get_transient' )->justReturn( false ); + $this->stubResizingOption( 2560 ); + + $this->assertSame( 2560, ( new WP() )->filter_big_image_size_threshold( false, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); } /** - * Test: nothing is flagged when no attachment ID is supplied, as happens when the value - * is read outside of an upload. + * Test: a false carrying no attachment ID is overridden too, which is what happens when the + * value is read outside of an upload. */ - public function testDoesNotFlagWithoutAnAttachmentId(): void { - Functions\expect( 'set_transient' )->never(); + public function testOverridesAFalseWithoutAnAttachmentId(): void { + Functions\when( 'get_transient' )->justReturn( false ); + $this->stubResizingOption( 2560 ); - $this->assertFalse( ( new WP() )->filter_big_image_size_threshold( false ) ); + $this->assertSame( 2560, ( new WP() )->filter_big_image_size_threshold( false ) ); } /** - * Test: Imagify's own resizing value is still applied when core did not opt out. + * Test: Imagify's own resizing value is applied when core did not opt out. */ public function testReturnsImagifyThresholdWhenResizingIsEnabled(): void { - Functions\when( 'get_imagify_option' )->alias( - function ( $option ) { - return 'resize_larger' === $option ? 1 : 1200; - } - ); + Functions\when( 'get_transient' )->justReturn( false ); + $this->stubResizingOption( 1200 ); $this->assertSame( 1200, ( new WP() )->filter_big_image_size_threshold( 2560, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); } @@ -74,8 +100,49 @@ function ( $option ) { * Test: with the setting off, the threshold is 0 and WordPress skips its own resizing. */ public function testReturnsZeroWhenResizingIsDisabled(): void { + Functions\when( 'get_transient' )->justReturn( false ); Functions\when( 'get_imagify_option' )->justReturn( 0 ); $this->assertSame( 0, ( new WP() )->filter_big_image_size_threshold( 2560, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); } + + /** + * Test: the attachment is flagged when WordPress hands the sub sizes to the browser. + */ + public function testFlagsTheAttachmentWhenTheBrowserOwnsTheSubsizes(): void { + $stored = []; + + Functions\when( 'set_transient' )->alias( + function ( $name, $value ) use ( &$stored ) { + $stored[ $name ] = $value; + return true; + } + ); + + ( new WP() )->maybe_flag_client_side_scaling( (object) [ 'ID' => 123 ], $this->requestReturning( false ), true ); + + $this->assertSame( [ 'imagify_client_side_scaled_123' => 1 ], $stored ); + } + + /** + * Test: nothing is flagged for an ordinary upload, where WordPress builds the sub sizes, + * nor when the parameter is absent entirely. + */ + public function testDoesNotFlagWhenWordPressBuildsTheSubsizes(): void { + Functions\expect( 'set_transient' )->never(); + + $attachment = (object) [ 'ID' => 123 ]; + + ( new WP() )->maybe_flag_client_side_scaling( $attachment, $this->requestReturning( true ), true ); + ( new WP() )->maybe_flag_client_side_scaling( $attachment, $this->requestReturning( null ), true ); + } + + /** + * Test: nothing is flagged when an existing attachment is updated rather than created. + */ + public function testDoesNotFlagWhenUpdatingAnAttachment(): void { + Functions\expect( 'set_transient' )->never(); + + ( new WP() )->maybe_flag_client_side_scaling( (object) [ 'ID' => 123 ], $this->requestReturning( false ), false ); + } } diff --git a/classes/Context/WP.php b/classes/Context/WP.php index 1079f7670..f0f9eab14 100644 --- a/classes/Context/WP.php +++ b/classes/Context/WP.php @@ -72,14 +72,19 @@ public function get_resizing_threshold() { /** * Filter WP's "big images threshold" with Imagify's resizing value. * - * A `false` value means downscaling has been switched off on purpose, so it is - * returned untouched. WordPress 7.1 does exactly that while the browser handles - * the sub sizes: it supplies its own scaled file through the sideload endpoint, - * and scaling again on the server would leave a conflicting "-scaled" file behind - * and point `original_image` at it instead of the real upload. + * Imagify stands down for one case only: the upload WordPress 7.1 handed to the browser, + * which supplies its own scaled file through the sideload endpoint. Scaling again on the + * server would leave a conflicting "-scaled" file behind and point `original_image` at it + * instead of the real upload, which is why core switches its own downscaling off there. * - * The value the browser scales to comes from this same filter - * ({@see WP_REST_Server::get_index()}), so Imagify's setting is still honoured. + * Nothing is lost by standing down: the value the browser scales to is produced by this + * same filter, in {@see WP_REST_Server::get_index()}, so Imagify's setting still governs + * the result. + * + * A `false` coming from anywhere else is deliberately overridden, exactly as before. Only + * the browser flow leaves a scaled file behind, so treating every `false` as "already + * scaled" would mean an image nobody resized: not WordPress, because it was told not to, + * and not Imagify, because it believed the work was done. * * @since 2.3.3 * @@ -90,17 +95,42 @@ public function get_resizing_threshold() { * @return int|false */ public function filter_big_image_size_threshold( $threshold, $imagesize = [], $file = '', $attachment_id = 0 ) { - if ( false === $threshold ) { - if ( $attachment_id ) { - self::flag_client_side_scaling( $attachment_id ); - } - + if ( false === $threshold && $attachment_id && self::is_client_side_scaled( $attachment_id ) ) { return $threshold; } return $this->get_resizing_threshold(); } + /** + * Remember that the browser is supplying the scaled version of an attachment. + * + * Taken from where WordPress declares it rather than guessed: this runs on + * `rest_after_insert_attachment`, just before the metadata is generated, and the request + * carries `generate_sub_sizes` as `false` exactly when the browser owns the sub sizes. + * + * @since 2.3.3 + * + * @param object $attachment Inserted or updated attachment object. A \WP_Post when WordPress fires this. + * @param object $request Request object. A \WP_REST_Request when WordPress fires this. + * @param bool $creating True when creating an attachment, false when updating. + */ + public function maybe_flag_client_side_scaling( $attachment, $request, $creating ) { + if ( ! $creating || ! is_object( $attachment ) || ! isset( $attachment->ID ) ) { + return; + } + + if ( ! is_object( $request ) || ! is_callable( [ $request, 'get_param' ] ) ) { + return; + } + + if ( false !== $request->get_param( 'generate_sub_sizes' ) ) { + return; + } + + self::flag_client_side_scaling( $attachment->ID ); + } + /** * Remember that the browser is supplying the scaled version of an attachment. * diff --git a/inc/common/attachments.php b/inc/common/attachments.php index 068393e8d..bb5fdfb3b 100755 --- a/inc/common/attachments.php +++ b/inc/common/attachments.php @@ -69,6 +69,17 @@ function imagify_add_avif_type( $ext2type ) { */ add_filter( 'big_image_size_threshold', [ imagify_get_context( 'wp' ), 'filter_big_image_size_threshold' ], IMAGIFY_INT_MAX, 4 ); +/** + * Note the uploads WordPress 7.1 hands to the browser, which scales them itself. + * + * Fires before the attachment metadata is generated, so the threshold filter above already + * knows about it by the time it runs. + * + * @since 2.3.3 + * @since WP 7.1 + */ +add_action( 'rest_after_insert_attachment', [ imagify_get_context( 'wp' ), 'maybe_flag_client_side_scaling' ], 10, 3 ); + /** * Add filters to manage images formats that will be generated * From 4de3133d5ad4e22ac412c8525e997e01b89be400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Robin?= Date: Mon, 17 Aug 2026 11:22:04 +0200 Subject: [PATCH 5/6] Say why the filter declares parameters it does not read --- classes/Context/WP.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/classes/Context/WP.php b/classes/Context/WP.php index f0f9eab14..c96677593 100644 --- a/classes/Context/WP.php +++ b/classes/Context/WP.php @@ -88,6 +88,9 @@ public function get_resizing_threshold() { * * @since 2.3.3 * + * $imagesize and $file are part of the filter signature and nothing here reads them: the + * attachment ID is the fourth argument, so they have to be declared to reach it. + * * @param int|false $threshold The threshold value in pixels, or false to disable resizing. * @param array $imagesize Indexed array of the image width and height in pixels. * @param string $file Full path to the uploaded image file. From 9800a9d9d5ea34fc93861b5f4433b8c2c1be81b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Robin?= Date: Mon, 17 Aug 2026 11:26:18 +0200 Subject: [PATCH 6/6] Drop the filter parameters nothing reads The callback only took four arguments to reach the attachment ID, leaving two declared and never read. WordPress notes the upload just before the metadata is generated, in the same request as the filter, so a property carries that instead and the callback is back to the single argument it uses. The transient stays: the optimization that consults it runs in a later request. This also removes the need for the argument count on both registrations. --- .../WP/FilterBigImageSizeThresholdTest.php | 32 ++++++------------- classes/Context/WP.php | 28 ++++++++++------ inc/classes/class-imagify-options.php | 4 +-- inc/common/attachments.php | 2 +- 4 files changed, 31 insertions(+), 35 deletions(-) diff --git a/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php b/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php index 40aed847a..55532cd93 100644 --- a/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php +++ b/Tests/Unit/classes/Context/WP/FilterBigImageSizeThresholdTest.php @@ -54,14 +54,15 @@ private function requestReturning( $value ) { * Test: the threshold is handed back untouched for the upload the browser scaled. */ public function testReturnsFalseUntouchedForABrowserScaledUpload(): void { - Functions\when( 'get_transient' )->alias( - function ( $name ) { - return 'imagify_client_side_scaled_123' === $name ? 1 : false; - } - ); + Functions\when( 'set_transient' )->justReturn( true ); Functions\expect( 'get_imagify_option' )->never(); - $this->assertFalse( ( new WP() )->filter_big_image_size_threshold( false, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); + $context = new WP(); + + // The sequence WordPress produces: the upload is noted, then the filter runs. + $context->maybe_flag_client_side_scaling( (object) [ 'ID' => 123 ], $this->requestReturning( false ), true ); + + $this->assertFalse( $context->filter_big_image_size_threshold( false ) ); } /** @@ -69,20 +70,9 @@ function ( $name ) { * Nothing scaled the file in that case, so standing down would leave it unresized. */ public function testOverridesAFalseThatDidNotComeFromTheBrowserFlow(): void { - Functions\when( 'get_transient' )->justReturn( false ); - $this->stubResizingOption( 2560 ); - - $this->assertSame( 2560, ( new WP() )->filter_big_image_size_threshold( false, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); - } - - /** - * Test: a false carrying no attachment ID is overridden too, which is what happens when the - * value is read outside of an upload. - */ - public function testOverridesAFalseWithoutAnAttachmentId(): void { - Functions\when( 'get_transient' )->justReturn( false ); $this->stubResizingOption( 2560 ); + // No upload was noted, so this false came from somewhere else. $this->assertSame( 2560, ( new WP() )->filter_big_image_size_threshold( false ) ); } @@ -90,20 +80,18 @@ public function testOverridesAFalseWithoutAnAttachmentId(): void { * Test: Imagify's own resizing value is applied when core did not opt out. */ public function testReturnsImagifyThresholdWhenResizingIsEnabled(): void { - Functions\when( 'get_transient' )->justReturn( false ); $this->stubResizingOption( 1200 ); - $this->assertSame( 1200, ( new WP() )->filter_big_image_size_threshold( 2560, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); + $this->assertSame( 1200, ( new WP() )->filter_big_image_size_threshold( 2560 ) ); } /** * Test: with the setting off, the threshold is 0 and WordPress skips its own resizing. */ public function testReturnsZeroWhenResizingIsDisabled(): void { - Functions\when( 'get_transient' )->justReturn( false ); Functions\when( 'get_imagify_option' )->justReturn( 0 ); - $this->assertSame( 0, ( new WP() )->filter_big_image_size_threshold( 2560, [ 3800, 2500 ], '/tmp/big.jpg', 123 ) ); + $this->assertSame( 0, ( new WP() )->filter_big_image_size_threshold( 2560 ) ); } /** diff --git a/classes/Context/WP.php b/classes/Context/WP.php index c96677593..bb13602d1 100644 --- a/classes/Context/WP.php +++ b/classes/Context/WP.php @@ -30,6 +30,17 @@ final class WP extends AbstractContext { */ protected $resizing_threshold = 0; + /** + * True once WordPress has said the browser is scaling the upload being created. + * + * Set on `rest_after_insert_attachment`, read by the threshold filter later in the same + * request. One request creates one attachment, so there is nothing to key it by. + * + * @var bool + * @since 2.3.3 + */ + protected $browser_is_scaling = false; + /** * Get the thumbnail sizes for this context, except the full size. * @@ -88,17 +99,11 @@ public function get_resizing_threshold() { * * @since 2.3.3 * - * $imagesize and $file are part of the filter signature and nothing here reads them: the - * attachment ID is the fourth argument, so they have to be declared to reach it. - * - * @param int|false $threshold The threshold value in pixels, or false to disable resizing. - * @param array $imagesize Indexed array of the image width and height in pixels. - * @param string $file Full path to the uploaded image file. - * @param int $attachment_id Attachment post ID. + * @param int|false $threshold The threshold value in pixels, or false to disable resizing. * @return int|false */ - public function filter_big_image_size_threshold( $threshold, $imagesize = [], $file = '', $attachment_id = 0 ) { - if ( false === $threshold && $attachment_id && self::is_client_side_scaled( $attachment_id ) ) { + public function filter_big_image_size_threshold( $threshold ) { + if ( false === $threshold && $this->browser_is_scaling ) { return $threshold; } @@ -114,6 +119,9 @@ public function filter_big_image_size_threshold( $threshold, $imagesize = [], $f * * @since 2.3.3 * + * The threshold filter runs later in this same request, so a property is enough for it. The + * transient is for the optimization, which runs in a later request of its own. + * * @param object $attachment Inserted or updated attachment object. A \WP_Post when WordPress fires this. * @param object $request Request object. A \WP_REST_Request when WordPress fires this. * @param bool $creating True when creating an attachment, false when updating. @@ -131,6 +139,8 @@ public function maybe_flag_client_side_scaling( $attachment, $request, $creating return; } + $this->browser_is_scaling = true; + self::flag_client_side_scaling( $attachment->ID ); } diff --git a/inc/classes/class-imagify-options.php b/inc/classes/class-imagify-options.php index f9f48ffcf..f6a595b24 100644 --- a/inc/classes/class-imagify-options.php +++ b/inc/classes/class-imagify-options.php @@ -88,9 +88,7 @@ protected function __construct() { $this->reset_values['resize_larger_w'] = $this->sanitize_and_validate_value( 'resize_larger_w', $this->reset_values['resize_larger_w'], $this->default_values['resize_larger_w'] ); if ( $filtered ) { - // The argument count has to be repeated here, or the callback would be - // registered with a single one and stop receiving the attachment ID. - add_filter( 'big_image_size_threshold', $filter_cb, IMAGIFY_INT_MAX, 4 ); + add_filter( 'big_image_size_threshold', $filter_cb, IMAGIFY_INT_MAX ); } } diff --git a/inc/common/attachments.php b/inc/common/attachments.php index bb5fdfb3b..847d21364 100755 --- a/inc/common/attachments.php +++ b/inc/common/attachments.php @@ -67,7 +67,7 @@ function imagify_add_avif_type( $ext2type ) { * @since WP 5.3 * @author Grégory Viguier */ -add_filter( 'big_image_size_threshold', [ imagify_get_context( 'wp' ), 'filter_big_image_size_threshold' ], IMAGIFY_INT_MAX, 4 ); +add_filter( 'big_image_size_threshold', [ imagify_get_context( 'wp' ), 'filter_big_image_size_threshold' ], IMAGIFY_INT_MAX ); /** * Note the uploads WordPress 7.1 hands to the browser, which scales them itself.