From 50eaf8e881539980666febea6ea52b0aa3029803 Mon Sep 17 00:00:00 2001 From: Akshit Sethi Date: Tue, 12 Jul 2022 16:28:41 +0530 Subject: [PATCH 1/7] Add replacing of images only in frontend context --- modules/images/webp-uploads/load.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 1098a40908..4889cb3c0f 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -367,6 +367,15 @@ function webp_uploads_remove_sources_files( $attachment_id ) { * @return string The content with the updated references to the images. */ function webp_uploads_update_image_references( $content ) { + /** + * Bail early if request is not for the frontend. + * + * The is_feed() function is to check if the query is for feeds (rdf, rss, rss2, atom). + */ + if ( ! did_action( 'template_redirect' ) || is_feed() ) { + return $content; + } + // This content does not have any tag on it, move forward. if ( ! preg_match_all( '/<(img)\s[^>]+>/', $content, $img_tags, PREG_SET_ORDER ) ) { return $content; From 6a2e8b0a845fb00f6cb4d56d4b413e6e73dac9a3 Mon Sep 17 00:00:00 2001 From: Akshit Sethi Date: Tue, 12 Jul 2022 16:34:27 +0530 Subject: [PATCH 2/7] Check if the request made is inside wp_head hook --- modules/images/webp-uploads/load.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 4889cb3c0f..80a6189044 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -372,7 +372,7 @@ function webp_uploads_update_image_references( $content ) { * * The is_feed() function is to check if the query is for feeds (rdf, rss, rss2, atom). */ - if ( ! did_action( 'template_redirect' ) || is_feed() ) { + if ( ! did_action( 'template_redirect' ) || doing_action( 'wp_head' ) || is_feed() ) { return $content; } From 41ed9a37f60262f949b496f03742e55215f58b9a Mon Sep 17 00:00:00 2001 From: Akshit Sethi Date: Tue, 12 Jul 2022 19:25:32 +0530 Subject: [PATCH 3/7] Add tests to verify request check for frontend context --- modules/images/webp-uploads/helper.php | 16 ++++++ modules/images/webp-uploads/load.php | 8 +-- .../images/webp-uploads/load-tests.php | 55 +++++++++++++++++++ 3 files changed, 73 insertions(+), 6 deletions(-) diff --git a/modules/images/webp-uploads/helper.php b/modules/images/webp-uploads/helper.php index 2ca66c60a6..bd91ecc95b 100644 --- a/modules/images/webp-uploads/helper.php +++ b/modules/images/webp-uploads/helper.php @@ -240,3 +240,19 @@ function webp_uploads_get_attachment_sources( $attachment_id, $size = 'thumbnail // Return an empty array if no sources found. return array(); } + +/** + * Verify if the request if for the frontend context. + * The is_feed() function is to check if the query is for feeds (rdf, rss, rss2, atom). + * + * @since n.e.x.t + * + * @return bool + */ +function webp_uploads_is_frontend_context() { + if ( ! did_action( 'template_redirect' ) || doing_action( 'wp_head' ) || doing_action( 'wp_footer' ) || is_feed() ) { + return false; + } + + return true; +} diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 80a6189044..6fb888490f 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -367,12 +367,8 @@ function webp_uploads_remove_sources_files( $attachment_id ) { * @return string The content with the updated references to the images. */ function webp_uploads_update_image_references( $content ) { - /** - * Bail early if request is not for the frontend. - * - * The is_feed() function is to check if the query is for feeds (rdf, rss, rss2, atom). - */ - if ( ! did_action( 'template_redirect' ) || doing_action( 'wp_head' ) || is_feed() ) { + // Bail early if request is not for the frontend. + if ( ! webp_uploads_is_frontend_context() ) { return $content; } diff --git a/tests/modules/images/webp-uploads/load-tests.php b/tests/modules/images/webp-uploads/load-tests.php index 4cf4f2fd8c..2b3fed7bf7 100644 --- a/tests/modules/images/webp-uploads/load-tests.php +++ b/tests/modules/images/webp-uploads/load-tests.php @@ -484,6 +484,61 @@ public function data_provider_not_supported_webp_images() { yield 'GIFT image' => array( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/earth.gif' ); } + /** + * Ensure jpg image is replaced if the request is for the frontend context. + * + * @group webp_uploads_update_image_references + * + * @test + */ + public function it_should_not_replace_image_references_when_request_is_for_frontend_context() { + $this->mock_action_template_redirect(); + + $attachment_id = $this->factory->attachment->create_upload_object( + TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' + ); + + $tag = wp_get_attachment_image( $attachment_id, 'medium', false, array( 'class' => "wp-image-{$attachment_id}" ) ); + $this->assertNotSame( $tag, webp_uploads_update_image_references( $tag ) ); + } + + /** + * Prevent replacing a jpg image if the request is not for the frontend context. + * In this test case, the request if for the `feed` page. + * + * @group webp_uploads_update_image_references + * + * @test + */ + public function it_should_not_replace_image_references_when_request_is_not_for_frontend_context() { + $this->mock_action_template_redirect(); + $this->mock_is_feed_page(); + + $attachment_id = $this->factory->attachment->create_upload_object( + TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' + ); + + $tag = wp_get_attachment_image( $attachment_id, 'medium', false, array( 'class' => "wp-image-{$attachment_id}" ) ); + + $this->assertSame( $tag, webp_uploads_update_image_references( $tag ) ); + } + + /** + * Mock is_home in $wp_query. + */ + public function mock_is_feed_page() { + global $wp_query; + $wp_query->is_feed = true; + } + + /** + * Execute an empty `template_redirect` action. + */ + public function mock_action_template_redirect() { + remove_all_actions( 'template_redirect' ); + do_action( 'template_redirect' ); + } + /** * Use the original image to generate all the image sizes * From 2655c7f68f5548a6c426679da3c87e61079e5fbc Mon Sep 17 00:00:00 2001 From: Akshit Sethi Date: Wed, 13 Jul 2022 13:28:01 +0530 Subject: [PATCH 4/7] Implement PR suggestions --- modules/images/webp-uploads/helper.php | 11 +++++++++-- modules/images/webp-uploads/load.php | 2 +- tests/modules/images/webp-uploads/load-tests.php | 6 +++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/modules/images/webp-uploads/helper.php b/modules/images/webp-uploads/helper.php index bd91ecc95b..9764f4b365 100644 --- a/modules/images/webp-uploads/helper.php +++ b/modules/images/webp-uploads/helper.php @@ -249,8 +249,15 @@ function webp_uploads_get_attachment_sources( $attachment_id, $size = 'thumbnail * * @return bool */ -function webp_uploads_is_frontend_context() { - if ( ! did_action( 'template_redirect' ) || doing_action( 'wp_head' ) || doing_action( 'wp_footer' ) || is_feed() ) { +function webp_uploads_in_frontend_body() { + global $wp_query; + + // Check if this request is generally outside (or before) any frontend context. + if ( ! isset( $wp_query ) || defined( 'REST_REQUEST' ) || is_feed() ) { + return false; + } + + if ( ! did_action( 'template_redirect' ) || doing_action( 'wp_head' ) ) { return false; } diff --git a/modules/images/webp-uploads/load.php b/modules/images/webp-uploads/load.php index 6fb888490f..c76d213849 100644 --- a/modules/images/webp-uploads/load.php +++ b/modules/images/webp-uploads/load.php @@ -368,7 +368,7 @@ function webp_uploads_remove_sources_files( $attachment_id ) { */ function webp_uploads_update_image_references( $content ) { // Bail early if request is not for the frontend. - if ( ! webp_uploads_is_frontend_context() ) { + if ( ! webp_uploads_in_frontend_body() ) { return $content; } diff --git a/tests/modules/images/webp-uploads/load-tests.php b/tests/modules/images/webp-uploads/load-tests.php index 2b3fed7bf7..178dac3e18 100644 --- a/tests/modules/images/webp-uploads/load-tests.php +++ b/tests/modules/images/webp-uploads/load-tests.php @@ -491,7 +491,7 @@ public function data_provider_not_supported_webp_images() { * * @test */ - public function it_should_not_replace_image_references_when_request_is_for_frontend_context() { + public function it_should_replace_image_references_when_in_frontend_context() { $this->mock_action_template_redirect(); $attachment_id = $this->factory->attachment->create_upload_object( @@ -510,7 +510,7 @@ public function it_should_not_replace_image_references_when_request_is_for_front * * @test */ - public function it_should_not_replace_image_references_when_request_is_not_for_frontend_context() { + public function it_should_not_replace_image_references_when_not_in_frontend_context() { $this->mock_action_template_redirect(); $this->mock_is_feed_page(); @@ -524,7 +524,7 @@ public function it_should_not_replace_image_references_when_request_is_not_for_f } /** - * Mock is_home in $wp_query. + * Mock is_feed in $wp_query. */ public function mock_is_feed_page() { global $wp_query; From 14e1a0e7e368d565ede1e601e38647e68d9532f4 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 14 Jul 2022 11:48:44 -0700 Subject: [PATCH 5/7] Revised logic for webp_uploads_in_frontend_body() and added more test coverage for the function specifically. --- modules/images/webp-uploads/helper.php | 10 ++-- .../images/webp-uploads/helper-tests.php | 38 +++++++++++++ .../images/webp-uploads/load-tests.php | 55 ------------------- 3 files changed, 43 insertions(+), 60 deletions(-) diff --git a/modules/images/webp-uploads/helper.php b/modules/images/webp-uploads/helper.php index 2d03a4c2bd..f619fb2867 100644 --- a/modules/images/webp-uploads/helper.php +++ b/modules/images/webp-uploads/helper.php @@ -242,22 +242,22 @@ function webp_uploads_get_attachment_sources( $attachment_id, $size = 'thumbnail } /** - * Verify if the request if for the frontend context. - * The is_feed() function is to check if the query is for feeds (rdf, rss, rss2, atom). + * Verifies if the request is for a frontend context within the tag. * * @since n.e.x.t * - * @return bool + * @return bool True if in the within a frontend request, false otherwise. */ function webp_uploads_in_frontend_body() { global $wp_query; // Check if this request is generally outside (or before) any frontend context. - if ( ! isset( $wp_query ) || defined( 'REST_REQUEST' ) || is_feed() ) { + if ( ! isset( $wp_query ) || defined( 'REST_REQUEST' ) || defined( 'XMLRPC_REQUEST' ) || is_feed() ) { return false; } - if ( ! did_action( 'template_redirect' ) || doing_action( 'wp_head' ) ) { + // Check if we're anywhere before or within the 'wp_head' action. + if ( ! did_action( 'template_redirect' ) || ! did_action( 'wp_head' ) || doing_action( 'wp_head' ) ) { return false; } diff --git a/tests/modules/images/webp-uploads/helper-tests.php b/tests/modules/images/webp-uploads/helper-tests.php index bff6dac2ea..ceb330bf14 100644 --- a/tests/modules/images/webp-uploads/helper-tests.php +++ b/tests/modules/images/webp-uploads/helper-tests.php @@ -454,4 +454,42 @@ public function it_should_never_discard_additional_image_if_filter_is_false( $or $output = webp_uploads_should_discard_additional_image_file( $original_filesize, $additional_filesize ); $this->assertFalse( $output ); } + + public function test_webp_uploads_in_frontend_body_without_wp_query() { + unset( $GLOBALS['wp_query'] ); + + $this->assertFalse( webp_uploads_in_frontend_body() ); + } + + public function test_webp_uploads_in_frontend_body_with_feed() { + $this->mock_empty_action( 'template_redirect' ); + $GLOBALS['wp_query']->is_feed = true; + + $this->assertFalse( webp_uploads_in_frontend_body() ); + } + + public function test_webp_uploads_in_frontend_body_without_template_redirect() { + $this->assertFalse( webp_uploads_in_frontend_body() ); + } + + public function test_webp_uploads_in_frontend_body_before_wp_head() { + $this->mock_empty_action( 'template_redirect' ); + $result = webp_uploads_in_frontend_body(); + $this->mock_empty_action( 'wp_head' ); + + $this->assertFalse( $result ); + } + + public function test_webp_uploads_in_frontend_body_after_wp_head() { + $this->mock_empty_action( 'template_redirect' ); + $this->mock_empty_action( 'wp_head' ); + $result = webp_uploads_in_frontend_body(); + + $this->assertTrue( $result ); + } + + private function mock_empty_action( $action ) { + remove_all_actions( $action ); + do_action( $action ); + } } diff --git a/tests/modules/images/webp-uploads/load-tests.php b/tests/modules/images/webp-uploads/load-tests.php index 5a2dde1102..e4bc666433 100644 --- a/tests/modules/images/webp-uploads/load-tests.php +++ b/tests/modules/images/webp-uploads/load-tests.php @@ -504,61 +504,6 @@ public function data_provider_not_supported_webp_images() { yield 'GIFT image' => array( TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/earth.gif' ); } - /** - * Ensure jpg image is replaced if the request is for the frontend context. - * - * @group webp_uploads_update_image_references - * - * @test - */ - public function it_should_replace_image_references_when_in_frontend_context() { - $this->mock_action_template_redirect(); - - $attachment_id = $this->factory->attachment->create_upload_object( - TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' - ); - - $tag = wp_get_attachment_image( $attachment_id, 'medium', false, array( 'class' => "wp-image-{$attachment_id}" ) ); - $this->assertNotSame( $tag, webp_uploads_update_image_references( $tag ) ); - } - - /** - * Prevent replacing a jpg image if the request is not for the frontend context. - * In this test case, the request if for the `feed` page. - * - * @group webp_uploads_update_image_references - * - * @test - */ - public function it_should_not_replace_image_references_when_not_in_frontend_context() { - $this->mock_action_template_redirect(); - $this->mock_is_feed_page(); - - $attachment_id = $this->factory->attachment->create_upload_object( - TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' - ); - - $tag = wp_get_attachment_image( $attachment_id, 'medium', false, array( 'class' => "wp-image-{$attachment_id}" ) ); - - $this->assertSame( $tag, webp_uploads_update_image_references( $tag ) ); - } - - /** - * Mock is_feed in $wp_query. - */ - public function mock_is_feed_page() { - global $wp_query; - $wp_query->is_feed = true; - } - - /** - * Execute an empty `template_redirect` action. - */ - public function mock_action_template_redirect() { - remove_all_actions( 'template_redirect' ); - do_action( 'template_redirect' ); - } - /** * Use the original image to generate all the image sizes * From 510e07bc0273fc67880801fc646dedd243568b0b Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 14 Jul 2022 12:09:11 -0700 Subject: [PATCH 6/7] Run necessary hooks to mock frontend body for relevant content replacement tests to still pass. --- .../images/webp-uploads/load-tests.php | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/modules/images/webp-uploads/load-tests.php b/tests/modules/images/webp-uploads/load-tests.php index e4bc666433..ba28c798ea 100644 --- a/tests/modules/images/webp-uploads/load-tests.php +++ b/tests/modules/images/webp-uploads/load-tests.php @@ -330,6 +330,9 @@ public function it_should_remove_the_backup_sizes_and_sources_if_the_attachment_ * @test */ public function it_should_avoid_the_change_of_urls_of_images_that_are_not_part_of_the_media_library() { + // Run critical hooks to satisfy webp_uploads_in_frontend_body() conditions. + $this->mock_frontend_body_hooks(); + $paragraph = '

Donec accumsan, sapien et , id commodo nisi sapien et est. Mauris nisl odio, iaculis vitae pellentesque nec.

'; $this->assertSame( $paragraph, webp_uploads_update_image_references( $paragraph ) ); @@ -343,6 +346,9 @@ public function it_should_avoid_the_change_of_urls_of_images_that_are_not_part_o * @test */ public function it_should_avoid_replacing_not_existing_attachment_i_ds() { + // Run critical hooks to satisfy webp_uploads_in_frontend_body() conditions. + $this->mock_frontend_body_hooks(); + $paragraph = '

Donec accumsan, sapien et , id commodo nisi sapien et est. Mauris nisl odio, iaculis vitae pellentesque nec.

'; $this->assertSame( $paragraph, webp_uploads_update_image_references( $paragraph ) ); @@ -384,6 +390,9 @@ public function it_should_prevent_replacing_a_jpg_image_if_the_image_does_not_ha TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + // Run critical hooks to satisfy webp_uploads_in_frontend_body() conditions. + $this->mock_frontend_body_hooks(); + $tag = wp_get_attachment_image( $attachment_id, 'medium' ); $this->assertSame( $tag, webp_uploads_update_image_references( $tag ) ); @@ -729,6 +738,9 @@ public function it_should_add_fallback_script_if_content_has_updated_images() { TESTS_PLUGIN_DIR . '/tests/testdata/modules/images/leafs.jpg' ); + // Run critical hooks to satisfy webp_uploads_in_frontend_body() conditions. + $this->mock_frontend_body_hooks(); + apply_filters( 'the_content', sprintf( @@ -803,4 +815,14 @@ public function it_should_create_mime_types_for_allowed_sizes_only_via_global_va $this->assertImageHasSizeSource( $attachment_id, 'allowed_size_400x300', 'image/webp' ); $this->assertImageNotHasSizeSource( $attachment_id, 'not_allowed_size_200x150', 'image/webp' ); } + + /** + * Runs (empty) hooks to satisfy webp_uploads_in_frontend_body() conditions. + */ + private function mock_frontend_body_hooks() { + remove_all_actions( 'template_redirect' ); + do_action( 'template_redirect' ); + remove_all_actions( 'wp_head' ); + do_action( 'wp_head' ); + } } From 10e9fbebbd94b3aabb42063b48a1b58c6e9a6347 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Thu, 14 Jul 2022 12:31:10 -0700 Subject: [PATCH 7/7] Remove condition for whether wp_head was run to satisfy FSE. --- modules/images/webp-uploads/helper.php | 4 +-- .../images/webp-uploads/helper-tests.php | 25 +++++++++++++++---- .../images/webp-uploads/load-tests.php | 2 -- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/modules/images/webp-uploads/helper.php b/modules/images/webp-uploads/helper.php index f619fb2867..af5c82068c 100644 --- a/modules/images/webp-uploads/helper.php +++ b/modules/images/webp-uploads/helper.php @@ -256,8 +256,8 @@ function webp_uploads_in_frontend_body() { return false; } - // Check if we're anywhere before or within the 'wp_head' action. - if ( ! did_action( 'template_redirect' ) || ! did_action( 'wp_head' ) || doing_action( 'wp_head' ) ) { + // Check if we're anywhere before 'template_redirect' or within the 'wp_head' action. + if ( ! did_action( 'template_redirect' ) || doing_action( 'wp_head' ) ) { return false; } diff --git a/tests/modules/images/webp-uploads/helper-tests.php b/tests/modules/images/webp-uploads/helper-tests.php index ceb330bf14..a9338782db 100644 --- a/tests/modules/images/webp-uploads/helper-tests.php +++ b/tests/modules/images/webp-uploads/helper-tests.php @@ -472,22 +472,37 @@ public function test_webp_uploads_in_frontend_body_without_template_redirect() { $this->assertFalse( webp_uploads_in_frontend_body() ); } - public function test_webp_uploads_in_frontend_body_before_wp_head() { - $this->mock_empty_action( 'template_redirect' ); + public function test_webp_uploads_in_frontend_body_before_template_redirect() { $result = webp_uploads_in_frontend_body(); - $this->mock_empty_action( 'wp_head' ); + $this->mock_empty_action( 'template_redirect' ); $this->assertFalse( $result ); } - public function test_webp_uploads_in_frontend_body_after_wp_head() { + public function test_webp_uploads_in_frontend_body_after_template_redirect() { $this->mock_empty_action( 'template_redirect' ); - $this->mock_empty_action( 'wp_head' ); $result = webp_uploads_in_frontend_body(); $this->assertTrue( $result ); } + public function test_webp_uploads_in_frontend_body_within_wp_head() { + $this->mock_empty_action( 'template_redirect' ); + + // Call function within a 'wp_head' callback. + remove_all_actions( 'wp_head' ); + $result = null; + add_action( + 'wp_head', + function() use ( &$result ) { + $result = webp_uploads_in_frontend_body(); + } + ); + do_action( 'wp_head' ); + + $this->assertFalse( $result ); + } + private function mock_empty_action( $action ) { remove_all_actions( $action ); do_action( $action ); diff --git a/tests/modules/images/webp-uploads/load-tests.php b/tests/modules/images/webp-uploads/load-tests.php index ba28c798ea..7fcbc35a2b 100644 --- a/tests/modules/images/webp-uploads/load-tests.php +++ b/tests/modules/images/webp-uploads/load-tests.php @@ -822,7 +822,5 @@ public function it_should_create_mime_types_for_allowed_sizes_only_via_global_va private function mock_frontend_body_hooks() { remove_all_actions( 'template_redirect' ); do_action( 'template_redirect' ); - remove_all_actions( 'wp_head' ); - do_action( 'wp_head' ); } }