From 335e1b4d47e92ff28f674bf9d5cf22c72a8882ee Mon Sep 17 00:00:00 2001 From: selul Date: Fri, 9 Oct 2026 19:55:21 +0300 Subject: [PATCH 1/2] fix: request the CSS width for Retina srcset entries and use it in sizes With Retina images on, the page profiler stores a 1x and a 2x entry per breakpoint. The 2x entry carries its delivered width (e.g. 824, advertised as "824w") and dpr 2, and the CDN multiplies w by dpr, so the URL asked for w:824/dpr:2 and delivered 1648px for an 824w candidate (#1111). The sizes attribute also used that doubled width, so a breakpoint got two max-width conditions, one with the 2x width (#1113). The URL now asks for the CSS width (width / dpr) with the dpr, and the sizes slot uses the CSS width, so the 1x and 2x entries give the same condition and array_unique() keeps one. Doing this in PHP also corrects profiles already stored. Co-Authored-By: Claude Opus 5.5 --- inc/tag_replacer.php | 9 +++++-- tests/test-srcset.php | 61 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/inc/tag_replacer.php b/inc/tag_replacer.php index 3ec4741e..74d2d6e3 100644 --- a/inc/tag_replacer.php +++ b/inc/tag_replacer.php @@ -500,8 +500,13 @@ public function add_missing_srcset_attributes( $tag, $missing_srcsets, $new_url, } $breakpoint = isset( $srcset_data['b'] ) ? (int) $srcset_data['b'] : 0; + // A dpr entry advertises its delivered width ("824w"): the CSS width times the dpr. The CDN + // multiplies w by dpr, so it gets the CSS width, which is also the slot in the sizes attribute. + $css_width = $dpr > 1 ? (int) round( $width / $dpr ) : $width; + $css_height = $dpr > 1 ? (int) round( $height / $dpr ) : $height; + // Generate optimized URL for this size - $optimized_url = $this->change_url_for_size( $new_url, $width, $height, $dpr ); + $optimized_url = $this->change_url_for_size( $new_url, $css_width, $css_height, $dpr ); if ( $optimized_url ) { $escaped_url = esc_url( $optimized_url ); @@ -512,7 +517,7 @@ public function add_missing_srcset_attributes( $tag, $missing_srcsets, $new_url, // Add sizes attribute entry for responsive breakpoints if ( $breakpoint > 0 ) { - $new_sizes_entries[] = '(max-width: ' . $breakpoint . 'px) ' . $width . 'px'; + $new_sizes_entries[] = '(max-width: ' . $breakpoint . 'px) ' . $css_width . 'px'; } } } diff --git a/tests/test-srcset.php b/tests/test-srcset.php index befee5d9..0d8c268c 100644 --- a/tests/test-srcset.php +++ b/tests/test-srcset.php @@ -191,6 +191,67 @@ public function test_add_missing_srcset_attributes_retina_disabled() { $this->assertStringNotContainsString( '400w', $result ); } + /** + * With Retina enabled, the profiler sends a 1x and a 2x entry per breakpoint. The 2x entry + * advertises its delivered width (824w); the CDN multiplies w by dpr, so its URL asks for the + * CSS width (w:412/dpr:2), not w:824/dpr:2, which would deliver 1648px. + */ + public function test_add_missing_srcset_attributes_retina_entry_delivers_its_advertised_width() { + $tag_replacer = new Optml_Tag_Replacer(); + $tag_replacer->settings = $this->retina_settings(); + + $result = $tag_replacer->add_missing_srcset_attributes( + 'Test', + [ + [ 'w' => 412, 'h' => 300, 'd' => 1, 's' => '412w', 'b' => 768 ], + [ 'w' => 824, 'h' => 600, 'd' => 2, 's' => '824w', 'b' => 768 ], + ], + 'https://example.i.optimole.com/w:auto/h:auto/q:mauto/https://example.com/image.jpg', + false + ); + + $this->assertStringContainsString( 'w:412/h:300/q:mauto/https://example.com/image.jpg 412w', $result ); + $this->assertStringContainsString( 'w:412/dpr:2/h:300/q:mauto/https://example.com/image.jpg 824w', $result ); + $this->assertStringNotContainsString( 'w:824', $result ); + } + + /** + * The 1x and 2x entries of a breakpoint share one slot: the sizes attribute gets one + * condition for it, with the CSS width, not a second one with the doubled width. + */ + public function test_add_missing_srcset_attributes_retina_entries_share_one_sizes_condition() { + $tag_replacer = new Optml_Tag_Replacer(); + $tag_replacer->settings = $this->retina_settings(); + + $result = $tag_replacer->add_missing_srcset_attributes( + 'Test', + [ + [ 'w' => 300, 'h' => 200, 'd' => 1, 's' => '300w', 'b' => 480 ], + [ 'w' => 600, 'h' => 400, 'd' => 2, 's' => '600w', 'b' => 480 ], + [ 'w' => 412, 'h' => 300, 'd' => 1, 's' => '412w', 'b' => 768 ], + [ 'w' => 824, 'h' => 600, 'd' => 2, 's' => '824w', 'b' => 768 ], + ], + 'https://example.i.optimole.com/w:auto/h:auto/q:mauto/https://example.com/image.jpg', + false + ); + + $this->assertSame( 1, substr_count( $result, '(max-width: 480px)' ) ); + $this->assertSame( 1, substr_count( $result, '(max-width: 768px)' ) ); + $this->assertStringContainsString( 'sizes="(max-width: 480px) 300px, (max-width: 768px) 412px"', $result ); + } + + /** + * Settings with Retina images enabled. + * + * @return Optml_Settings + */ + private function retina_settings() { + $settings = $this->getMockBuilder( 'Optml_Settings' )->getMock(); + $settings->method( 'get' )->with( 'retina_images' )->willReturn( 'enabled' ); + + return $settings; + } + /** * Test enhance_existing_srcset method */ From 13fb6a0d701ecd5093bed06b8debbddfa3815370 Mon Sep 17 00:00:00 2001 From: selul Date: Sun, 11 Oct 2026 16:59:49 +0300 Subject: [PATCH 2/2] test: type the new srcset tests and share the Retina settings mock Co-Authored-By: Claude Opus 5.5 --- tests/test-srcset.php | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/test-srcset.php b/tests/test-srcset.php index 0d8c268c..255116eb 100644 --- a/tests/test-srcset.php +++ b/tests/test-srcset.php @@ -175,9 +175,7 @@ public function test_add_missing_srcset_attributes_retina_disabled() { $tag_replacer = new Optml_Tag_Replacer(); // Mock settings to disable retina images - $settings = $this->getMockBuilder( 'Optml_Settings' )->getMock(); - $settings->method( 'get' )->with( 'retina_images' )->willReturn( 'disabled' ); - $tag_replacer->settings = $settings; + $tag_replacer->settings = $this->retina_settings( 'disabled' ); $tag = 'Test'; $missing_srcsets = [ @@ -196,9 +194,9 @@ public function test_add_missing_srcset_attributes_retina_disabled() { * advertises its delivered width (824w); the CDN multiplies w by dpr, so its URL asks for the * CSS width (w:412/dpr:2), not w:824/dpr:2, which would deliver 1648px. */ - public function test_add_missing_srcset_attributes_retina_entry_delivers_its_advertised_width() { + public function test_add_missing_srcset_attributes_retina_entry_delivers_its_advertised_width(): void { $tag_replacer = new Optml_Tag_Replacer(); - $tag_replacer->settings = $this->retina_settings(); + $tag_replacer->settings = $this->retina_settings( 'enabled' ); $result = $tag_replacer->add_missing_srcset_attributes( 'Test', @@ -219,9 +217,9 @@ public function test_add_missing_srcset_attributes_retina_entry_delivers_its_adv * The 1x and 2x entries of a breakpoint share one slot: the sizes attribute gets one * condition for it, with the CSS width, not a second one with the doubled width. */ - public function test_add_missing_srcset_attributes_retina_entries_share_one_sizes_condition() { + public function test_add_missing_srcset_attributes_retina_entries_share_one_sizes_condition(): void { $tag_replacer = new Optml_Tag_Replacer(); - $tag_replacer->settings = $this->retina_settings(); + $tag_replacer->settings = $this->retina_settings( 'enabled' ); $result = $tag_replacer->add_missing_srcset_attributes( 'Test', @@ -241,13 +239,15 @@ public function test_add_missing_srcset_attributes_retina_entries_share_one_size } /** - * Settings with Retina images enabled. + * Settings mock for the Retina images option. + * + * @param string $retina_images The option value, 'enabled' or 'disabled'. * * @return Optml_Settings */ - private function retina_settings() { + private function retina_settings( string $retina_images ): Optml_Settings { $settings = $this->getMockBuilder( 'Optml_Settings' )->getMock(); - $settings->method( 'get' )->with( 'retina_images' )->willReturn( 'enabled' ); + $settings->method( 'get' )->with( 'retina_images' )->willReturn( $retina_images ); return $settings; }