From f19d670672cd5e8c5eb77154c5b95f259fa04ca3 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Fri, 20 Dec 2024 19:06:02 +0530 Subject: [PATCH 01/10] Change protected visibility to private as it is a final class anyway Signed-off-by: Shyamsundar Gadde --- plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index 9c30bdfc7..d82e048da 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -26,7 +26,7 @@ final class Embed_Optimizer_Tag_Visitor { * * @var bool */ - protected $added_lazy_script = false; + private $added_lazy_script = false; /** * Determines whether the processor is currently at a figure.wp-block-embed tag. From 6778acce19b54ae5b2b9e42a693d93583b032d84 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Fri, 20 Dec 2024 19:25:49 +0530 Subject: [PATCH 02/10] Extract preconnect logic to its own method Signed-off-by: Shyamsundar Gadde --- .../class-embed-optimizer-tag-visitor.php | 171 ++++++++++-------- 1 file changed, 93 insertions(+), 78 deletions(-) diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index d82e048da..a27f60689 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -104,7 +104,9 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { $this->reduce_layout_shifts( $context ); - // Preconnect links and lazy-loading can only be done once there are URL Metrics collected for both mobile and desktop. + $this->add_preconnect_links( $context ); + + // Lazy-loading can only be done once there are URL Metrics collected for both mobile and desktop. if ( $context->url_metric_group_collection->get_first_group()->count() > 0 && @@ -112,83 +114,7 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { ) { $embed_wrapper_xpath = self::get_embed_wrapper_xpath( $processor->get_xpath() ); $max_intersection_ratio = $context->url_metric_group_collection->get_element_max_intersection_ratio( $embed_wrapper_xpath ); - if ( $max_intersection_ratio > 0 ) { - /* - * The following embeds have been chosen for optimization due to their relative popularity among all embed types. - * See . - * The list of hosts being preconnected to was obtained by inserting an embed into a post and then looking - * at the network log on the frontend as the embed renders. Each should include the host of the iframe src - * as well as URLs for assets used by the embed, _if_ the URL looks like it is not geotargeted (e.g. '-us') - * or load-balanced (e.g. 's0.example.com'). For the load balancing case, attempt to load the asset by - * incrementing the number appearing in the subdomain (e.g. s1.example.com). If the asset still loads, then - * it is a likely case of a load balancing domain name which cannot be safely preconnected since it could - * not end up being the load balanced domain used for the embed. Lastly, these domains are only for the URLs - * for GET requests, as POST requests are not likely to be part of the critical rendering path. - */ - $preconnect_hrefs = array(); - $has_class = static function ( string $wanted_class ) use ( $processor ): bool { - return true === $processor->has_class( $wanted_class ); - }; - if ( $has_class( 'wp-block-embed-youtube' ) ) { - $preconnect_hrefs[] = 'https://www.youtube.com'; - $preconnect_hrefs[] = 'https://i.ytimg.com'; - } elseif ( $has_class( 'wp-block-embed-twitter' ) ) { - $preconnect_hrefs[] = 'https://syndication.twitter.com'; - $preconnect_hrefs[] = 'https://pbs.twimg.com'; - } elseif ( $has_class( 'wp-block-embed-vimeo' ) ) { - $preconnect_hrefs[] = 'https://player.vimeo.com'; - $preconnect_hrefs[] = 'https://f.vimeocdn.com'; - $preconnect_hrefs[] = 'https://i.vimeocdn.com'; - } elseif ( $has_class( 'wp-block-embed-spotify' ) ) { - $preconnect_hrefs[] = 'https://apresolve.spotify.com'; - $preconnect_hrefs[] = 'https://embed-cdn.spotifycdn.com'; - $preconnect_hrefs[] = 'https://encore.scdn.co'; - $preconnect_hrefs[] = 'https://i.scdn.co'; - } elseif ( $has_class( 'wp-block-embed-videopress' ) || $has_class( 'wp-block-embed-wordpress-tv' ) ) { - $preconnect_hrefs[] = 'https://video.wordpress.com'; - $preconnect_hrefs[] = 'https://public-api.wordpress.com'; - $preconnect_hrefs[] = 'https://videos.files.wordpress.com'; - $preconnect_hrefs[] = 'https://v0.wordpress.com'; // This does not appear to be a load-balanced domain since v1.wordpress.com is not valid. - } elseif ( $has_class( 'wp-block-embed-instagram' ) ) { - $preconnect_hrefs[] = 'https://www.instagram.com'; - $preconnect_hrefs[] = 'https://static.cdninstagram.com'; - $preconnect_hrefs[] = 'https://scontent.cdninstagram.com'; - } elseif ( $has_class( 'wp-block-embed-tiktok' ) ) { - $preconnect_hrefs[] = 'https://www.tiktok.com'; - // Note: The other domains used for TikTok embeds include https://lf16-tiktok-web.tiktokcdn-us.com, - // https://lf16-cdn-tos.tiktokcdn-us.com, and https://lf16-tiktok-common.tiktokcdn-us.com among others - // which either appear to be geo-targeted ('-us') _or_ load-balanced ('lf16'). So these are not added - // to the preconnected hosts. - } elseif ( $has_class( 'wp-block-embed-amazon' ) ) { - $preconnect_hrefs[] = 'https://read.amazon.com'; - $preconnect_hrefs[] = 'https://m.media-amazon.com'; - } elseif ( $has_class( 'wp-block-embed-soundcloud' ) ) { - $preconnect_hrefs[] = 'https://w.soundcloud.com'; - $preconnect_hrefs[] = 'https://widget.sndcdn.com'; - // Note: There is also https://i1.sndcdn.com which is for the album art, but the '1' indicates it may be geotargeted/load-balanced. - } elseif ( $has_class( 'wp-block-embed-pinterest' ) ) { - $preconnect_hrefs[] = 'https://assets.pinterest.com'; - $preconnect_hrefs[] = 'https://widgets.pinterest.com'; - $preconnect_hrefs[] = 'https://i.pinimg.com'; - } - - foreach ( $preconnect_hrefs as $preconnect_href ) { - foreach ( $context->url_metric_group_collection as $group ) { - if ( ! ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) > 0.0 ) ) { - continue; - } - - $context->link_collection->add_link( - array( - 'rel' => 'preconnect', - 'href' => $preconnect_href, - ), - $group->get_minimum_viewport_width(), - $group->get_maximum_viewport_width() - ); - } - } - } elseif ( embed_optimizer_update_markup( $processor, false ) && ! $this->added_lazy_script ) { + if ( ! ( $max_intersection_ratio > 0.0 ) && embed_optimizer_update_markup( $processor, false ) && ! $this->added_lazy_script ) { $processor->append_body_html( wp_get_inline_script_tag( embed_optimizer_get_lazy_load_script(), array( 'type' => 'module' ) ) ); $this->added_lazy_script = true; } @@ -283,4 +209,93 @@ private function reduce_layout_shifts( OD_Tag_Visitor_Context $context ): void { $processor->append_head_html( sprintf( "\n", join( "\n", $style_rules ) ) ); } } + + /** + * Adds preconnect links for embed resources. + * + * @since n.e.x.t + * + * @param OD_Tag_Visitor_Context $context Tag visitor context, with the cursor currently at an embed block. + */ + private function add_preconnect_links( OD_Tag_Visitor_Context $context ): void { + $processor = $context->processor; + $embed_wrapper_xpath = self::get_embed_wrapper_xpath( $processor->get_xpath() ); + + /* + * The following embeds have been chosen for optimization due to their relative popularity among all embed types. + * See . + * The list of hosts being preconnected to was obtained by inserting an embed into a post and then looking + * at the network log on the frontend as the embed renders. Each should include the host of the iframe src + * as well as URLs for assets used by the embed, _if_ the URL looks like it is not geotargeted (e.g. '-us') + * or load-balanced (e.g. 's0.example.com'). For the load balancing case, attempt to load the asset by + * incrementing the number appearing in the subdomain (e.g. s1.example.com). If the asset still loads, then + * it is a likely case of a load balancing domain name which cannot be safely preconnected since it could + * not end up being the load balanced domain used for the embed. Lastly, these domains are only for the URLs + * for GET requests, as POST requests are not likely to be part of the critical rendering path. + */ + $preconnect_hrefs = array(); + $has_class = static function ( string $wanted_class ) use ( $processor ): bool { + return true === $processor->has_class( $wanted_class ); + }; + + if ( $has_class( 'wp-block-embed-youtube' ) ) { + $preconnect_hrefs[] = 'https://www.youtube.com'; + $preconnect_hrefs[] = 'https://i.ytimg.com'; + } elseif ( $has_class( 'wp-block-embed-twitter' ) ) { + $preconnect_hrefs[] = 'https://syndication.twitter.com'; + $preconnect_hrefs[] = 'https://pbs.twimg.com'; + } elseif ( $has_class( 'wp-block-embed-vimeo' ) ) { + $preconnect_hrefs[] = 'https://player.vimeo.com'; + $preconnect_hrefs[] = 'https://f.vimeocdn.com'; + $preconnect_hrefs[] = 'https://i.vimeocdn.com'; + } elseif ( $has_class( 'wp-block-embed-spotify' ) ) { + $preconnect_hrefs[] = 'https://apresolve.spotify.com'; + $preconnect_hrefs[] = 'https://embed-cdn.spotifycdn.com'; + $preconnect_hrefs[] = 'https://encore.scdn.co'; + $preconnect_hrefs[] = 'https://i.scdn.co'; + } elseif ( $has_class( 'wp-block-embed-videopress' ) || $has_class( 'wp-block-embed-wordpress-tv' ) ) { + $preconnect_hrefs[] = 'https://video.wordpress.com'; + $preconnect_hrefs[] = 'https://public-api.wordpress.com'; + $preconnect_hrefs[] = 'https://videos.files.wordpress.com'; + $preconnect_hrefs[] = 'https://v0.wordpress.com'; // This does not appear to be a load-balanced domain since v1.wordpress.com is not valid. + } elseif ( $has_class( 'wp-block-embed-instagram' ) ) { + $preconnect_hrefs[] = 'https://www.instagram.com'; + $preconnect_hrefs[] = 'https://static.cdninstagram.com'; + $preconnect_hrefs[] = 'https://scontent.cdninstagram.com'; + } elseif ( $has_class( 'wp-block-embed-tiktok' ) ) { + $preconnect_hrefs[] = 'https://www.tiktok.com'; + // Note: The other domains used for TikTok embeds include https://lf16-tiktok-web.tiktokcdn-us.com, + // https://lf16-cdn-tos.tiktokcdn-us.com, and https://lf16-tiktok-common.tiktokcdn-us.com among others + // which either appear to be geo-targeted ('-us') _or_ load-balanced ('lf16'). So these are not added + // to the preconnected hosts. + } elseif ( $has_class( 'wp-block-embed-amazon' ) ) { + $preconnect_hrefs[] = 'https://read.amazon.com'; + $preconnect_hrefs[] = 'https://m.media-amazon.com'; + } elseif ( $has_class( 'wp-block-embed-soundcloud' ) ) { + $preconnect_hrefs[] = 'https://w.soundcloud.com'; + $preconnect_hrefs[] = 'https://widget.sndcdn.com'; + // Note: There is also https://i1.sndcdn.com which is for the album art, but the '1' indicates it may be geotargeted/load-balanced. + } elseif ( $has_class( 'wp-block-embed-pinterest' ) ) { + $preconnect_hrefs[] = 'https://assets.pinterest.com'; + $preconnect_hrefs[] = 'https://widgets.pinterest.com'; + $preconnect_hrefs[] = 'https://i.pinimg.com'; + } + + foreach ( $preconnect_hrefs as $preconnect_href ) { + foreach ( $context->url_metric_group_collection as $group ) { + if ( ! ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) > 0.0 ) ) { + continue; + } + + $context->link_collection->add_link( + array( + 'rel' => 'preconnect', + 'href' => $preconnect_href, + ), + $group->get_minimum_viewport_width(), + $group->get_maximum_viewport_width() + ); + } + } + } } From 7f08b4e3caaf7ab04553f7b29e79a9cbe201d37c Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Fri, 20 Dec 2024 19:48:41 +0530 Subject: [PATCH 03/10] Extract lazy-loading logic into `lazy_load_embeds` method Signed-off-by: Shyamsundar Gadde --- .../class-embed-optimizer-tag-visitor.php | 44 ++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index a27f60689..65f4c25a4 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -103,22 +103,8 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { } $this->reduce_layout_shifts( $context ); - $this->add_preconnect_links( $context ); - - // Lazy-loading can only be done once there are URL Metrics collected for both mobile and desktop. - if ( - $context->url_metric_group_collection->get_first_group()->count() > 0 - && - $context->url_metric_group_collection->get_last_group()->count() > 0 - ) { - $embed_wrapper_xpath = self::get_embed_wrapper_xpath( $processor->get_xpath() ); - $max_intersection_ratio = $context->url_metric_group_collection->get_element_max_intersection_ratio( $embed_wrapper_xpath ); - if ( ! ( $max_intersection_ratio > 0.0 ) && embed_optimizer_update_markup( $processor, false ) && ! $this->added_lazy_script ) { - $processor->append_body_html( wp_get_inline_script_tag( embed_optimizer_get_lazy_load_script(), array( 'type' => 'module' ) ) ); - $this->added_lazy_script = true; - } - } + $this->lazy_load_embeds( $context ); /* * At this point the tag is a figure.wp-block-embed, and we can return false because this does not need to be @@ -298,4 +284,32 @@ private function add_preconnect_links( OD_Tag_Visitor_Context $context ): void { } } } + + /** + * Optimizes an embed based on whether it is displayed in any initial viewport. + * + * @since n.e.x.t + * + * @param OD_Tag_Visitor_Context $context Tag visitor context, with the cursor currently at an embed block. + */ + private function lazy_load_embeds( OD_Tag_Visitor_Context $context ): void { + $processor = $context->processor; + + // Lazy-loading can only be done once there are URL Metrics collected for both mobile and desktop. + if ( + $context->url_metric_group_collection->get_first_group()->count() === 0 + || + $context->url_metric_group_collection->get_last_group()->count() === 0 + ) { + return; + } + + $embed_wrapper_xpath = self::get_embed_wrapper_xpath( $processor->get_xpath() ); + + $max_intersection_ratio = $context->url_metric_group_collection->get_element_max_intersection_ratio( $embed_wrapper_xpath ); + if ( ! ( $max_intersection_ratio > 0.0 ) && embed_optimizer_update_markup( $processor, false ) && ! $this->added_lazy_script ) { + $processor->append_body_html( wp_get_inline_script_tag( embed_optimizer_get_lazy_load_script(), array( 'type' => 'module' ) ) ); + $this->added_lazy_script = true; + } + } } From d9fc859bf2959ce66cd51a778c492a16f5d3f126 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Fri, 20 Dec 2024 20:10:17 +0530 Subject: [PATCH 04/10] Add method that returns an array of URLs to preconnect to based on embed type Signed-off-by: Shyamsundar Gadde --- .../class-embed-optimizer-tag-visitor.php | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index 65f4c25a4..5abe0f396 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -197,16 +197,14 @@ private function reduce_layout_shifts( OD_Tag_Visitor_Context $context ): void { } /** - * Adds preconnect links for embed resources. + * Gets preconnect URLs based on embed type * * @since n.e.x.t * - * @param OD_Tag_Visitor_Context $context Tag visitor context, with the cursor currently at an embed block. + * @param OD_HTML_Tag_Processor $processor Processor. + * @return array Array of URLs to preconnect to. */ - private function add_preconnect_links( OD_Tag_Visitor_Context $context ): void { - $processor = $context->processor; - $embed_wrapper_xpath = self::get_embed_wrapper_xpath( $processor->get_xpath() ); - + private function get_preconnect_urls( OD_HTML_Tag_Processor $processor ): array { /* * The following embeds have been chosen for optimization due to their relative popularity among all embed types. * See . @@ -267,6 +265,21 @@ private function add_preconnect_links( OD_Tag_Visitor_Context $context ): void { $preconnect_hrefs[] = 'https://i.pinimg.com'; } + return $preconnect_hrefs; + } + + /** + * Adds preconnect links for embed resources. + * + * @since n.e.x.t + * + * @param OD_Tag_Visitor_Context $context Tag visitor context, with the cursor currently at an embed block. + */ + private function add_preconnect_links( OD_Tag_Visitor_Context $context ): void { + $processor = $context->processor; + $embed_wrapper_xpath = self::get_embed_wrapper_xpath( $processor->get_xpath() ); + + $preconnect_hrefs = $this->get_preconnect_urls( $processor ); foreach ( $preconnect_hrefs as $preconnect_href ) { foreach ( $context->url_metric_group_collection as $group ) { if ( ! ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) > 0.0 ) ) { From 9f94551cda1b8e5bb0d8708d9cd327c10e5b17f7 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Fri, 20 Dec 2024 21:18:57 +0530 Subject: [PATCH 05/10] Add test case for only mobile metrics present with embed in viewport Signed-off-by: Shyamsundar Gadde --- ...-viewport-with-only-mobile-url-metrics.php | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport-with-only-mobile-url-metrics.php diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport-with-only-mobile-url-metrics.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport-with-only-mobile-url-metrics.php new file mode 100644 index 000000000..b476aca04 --- /dev/null +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport-with-only-mobile-url-metrics.php @@ -0,0 +1,57 @@ + static function ( Test_Embed_Optimizer_Optimization_Detective $test_case ): void { + OD_URL_Metrics_Post_Type::store_url_metric( + od_get_url_metrics_slug( od_get_normalized_query_vars() ), + $test_case->get_sample_url_metric( + array( + 'viewport_width' => 100, + 'elements' => array( + array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]', + 'isLCP' => false, + 'intersectionRatio' => 1, + 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), + ), + ), + ) + ) + ); + }, + 'buffer' => ' + + + + ... + + +
+
+ +
+
+ + + ', + 'expected' => ' + + + + ... + + + + + +
+
+ +
+
+ + + + ', +); From 0b4150c69fb312583e2a33fa307ab180c306a530 Mon Sep 17 00:00:00 2001 From: Shyamsundar Gadde Date: Sat, 21 Dec 2024 02:36:13 +0530 Subject: [PATCH 06/10] Use PHP_FLOAT_EPSILON for safer comparison of max_intersection_ratio Signed-off-by: Shyamsundar Gadde --- .../class-embed-optimizer-tag-visitor.php | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index 5abe0f396..904344c14 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -206,17 +206,17 @@ private function reduce_layout_shifts( OD_Tag_Visitor_Context $context ): void { */ private function get_preconnect_urls( OD_HTML_Tag_Processor $processor ): array { /* - * The following embeds have been chosen for optimization due to their relative popularity among all embed types. - * See . - * The list of hosts being preconnected to was obtained by inserting an embed into a post and then looking - * at the network log on the frontend as the embed renders. Each should include the host of the iframe src - * as well as URLs for assets used by the embed, _if_ the URL looks like it is not geotargeted (e.g. '-us') - * or load-balanced (e.g. 's0.example.com'). For the load balancing case, attempt to load the asset by - * incrementing the number appearing in the subdomain (e.g. s1.example.com). If the asset still loads, then - * it is a likely case of a load balancing domain name which cannot be safely preconnected since it could - * not end up being the load balanced domain used for the embed. Lastly, these domains are only for the URLs - * for GET requests, as POST requests are not likely to be part of the critical rendering path. - */ + * The following embeds have been chosen for optimization due to their relative popularity among all embed types. + * See . + * The list of hosts being preconnected to was obtained by inserting an embed into a post and then looking + * at the network log on the frontend as the embed renders. Each should include the host of the iframe src + * as well as URLs for assets used by the embed, _if_ the URL looks like it is not geotargeted (e.g. '-us') + * or load-balanced (e.g. 's0.example.com'). For the load balancing case, attempt to load the asset by + * incrementing the number appearing in the subdomain (e.g. s1.example.com). If the asset still loads, then + * it is a likely case of a load balancing domain name which cannot be safely preconnected since it could + * not end up being the load balanced domain used for the embed. Lastly, these domains are only for the URLs + * for GET requests, as POST requests are not likely to be part of the critical rendering path. + */ $preconnect_hrefs = array(); $has_class = static function ( string $wanted_class ) use ( $processor ): bool { return true === $processor->has_class( $wanted_class ); @@ -282,7 +282,7 @@ private function add_preconnect_links( OD_Tag_Visitor_Context $context ): void { $preconnect_hrefs = $this->get_preconnect_urls( $processor ); foreach ( $preconnect_hrefs as $preconnect_href ) { foreach ( $context->url_metric_group_collection as $group ) { - if ( ! ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) > 0.0 ) ) { + if ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) < PHP_FLOAT_EPSILON ) { continue; } @@ -320,7 +320,7 @@ private function lazy_load_embeds( OD_Tag_Visitor_Context $context ): void { $embed_wrapper_xpath = self::get_embed_wrapper_xpath( $processor->get_xpath() ); $max_intersection_ratio = $context->url_metric_group_collection->get_element_max_intersection_ratio( $embed_wrapper_xpath ); - if ( ! ( $max_intersection_ratio > 0.0 ) && embed_optimizer_update_markup( $processor, false ) && ! $this->added_lazy_script ) { + if ( $max_intersection_ratio < PHP_FLOAT_EPSILON && embed_optimizer_update_markup( $processor, false ) && ! $this->added_lazy_script ) { $processor->append_body_html( wp_get_inline_script_tag( embed_optimizer_get_lazy_load_script(), array( 'type' => 'module' ) ) ); $this->added_lazy_script = true; } From 399d3b2d140fd192580d6e5de0fcb06270065631 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Jan 2025 11:44:22 -0800 Subject: [PATCH 07/10] Improve comments and remove extraneous variable --- .../class-embed-optimizer-tag-visitor.php | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index 904344c14..6d7dab05a 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -197,26 +197,24 @@ private function reduce_layout_shifts( OD_Tag_Visitor_Context $context ): void { } /** - * Gets preconnect URLs based on embed type + * Gets preconnect URLs based on embed type. + * + * The following embeds have been chosen for optimization due to their relative popularity among all embed types. + * The list of hosts being preconnected to was obtained by inserting an embed into a post and then looking + * at the network log on the frontend as the embed renders. Each should include the host of the iframe src + * as well as URLs for assets used by the embed, _if_ the URL looks like it is not geotargeted (e.g. '-us') + * or load-balanced (e.g. 's0.example.com'). For the load balancing case, attempt to load the asset by + * incrementing the number appearing in the subdomain (e.g. s1.example.com). If the asset still loads, then + * it is a likely case of a load balancing domain name which cannot be safely preconnected since it could + * not end up being the load balanced domain used for the embed. Lastly, these domains are only for the URLs + * for GET requests, as POST requests are not likely to be part of the critical rendering path. * * @since n.e.x.t * - * @param OD_HTML_Tag_Processor $processor Processor. + * @param OD_HTML_Tag_Processor $processor Processor, with the cursor currently at an embed block. * @return array Array of URLs to preconnect to. */ private function get_preconnect_urls( OD_HTML_Tag_Processor $processor ): array { - /* - * The following embeds have been chosen for optimization due to their relative popularity among all embed types. - * See . - * The list of hosts being preconnected to was obtained by inserting an embed into a post and then looking - * at the network log on the frontend as the embed renders. Each should include the host of the iframe src - * as well as URLs for assets used by the embed, _if_ the URL looks like it is not geotargeted (e.g. '-us') - * or load-balanced (e.g. 's0.example.com'). For the load balancing case, attempt to load the asset by - * incrementing the number appearing in the subdomain (e.g. s1.example.com). If the asset still loads, then - * it is a likely case of a load balancing domain name which cannot be safely preconnected since it could - * not end up being the load balanced domain used for the embed. Lastly, these domains are only for the URLs - * for GET requests, as POST requests are not likely to be part of the critical rendering path. - */ $preconnect_hrefs = array(); $has_class = static function ( string $wanted_class ) use ( $processor ): bool { return true === $processor->has_class( $wanted_class ); @@ -279,8 +277,7 @@ private function add_preconnect_links( OD_Tag_Visitor_Context $context ): void { $processor = $context->processor; $embed_wrapper_xpath = self::get_embed_wrapper_xpath( $processor->get_xpath() ); - $preconnect_hrefs = $this->get_preconnect_urls( $processor ); - foreach ( $preconnect_hrefs as $preconnect_href ) { + foreach ( $this->get_preconnect_urls( $processor ) as $preconnect_url ) { foreach ( $context->url_metric_group_collection as $group ) { if ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) < PHP_FLOAT_EPSILON ) { continue; @@ -289,7 +286,7 @@ private function add_preconnect_links( OD_Tag_Visitor_Context $context ): void { $context->link_collection->add_link( array( 'rel' => 'preconnect', - 'href' => $preconnect_href, + 'href' => $preconnect_url, ), $group->get_minimum_viewport_width(), $group->get_maximum_viewport_width() From ca0ca13f145a2cfbef24a4a0ffbd23846efea4e4 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Jan 2025 11:50:59 -0800 Subject: [PATCH 08/10] Exclude plugins/*/tests from coverage See #1653 --- phpunit.xml.dist | 3 +++ 1 file changed, 3 insertions(+) diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 5fffd1773..4b09476b2 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -45,5 +45,8 @@ plugins + + plugins/*/tests + From 6aa98e2fb172dc70466857dbc25e0bbbe3d31567 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Jan 2025 11:58:49 -0800 Subject: [PATCH 09/10] Improve variable naming consistency --- .../class-embed-optimizer-tag-visitor.php | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index 6d7dab05a..2a5a84150 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -215,55 +215,55 @@ private function reduce_layout_shifts( OD_Tag_Visitor_Context $context ): void { * @return array Array of URLs to preconnect to. */ private function get_preconnect_urls( OD_HTML_Tag_Processor $processor ): array { - $preconnect_hrefs = array(); - $has_class = static function ( string $wanted_class ) use ( $processor ): bool { + $urls = array(); + $has_class = static function ( string $wanted_class ) use ( $processor ): bool { return true === $processor->has_class( $wanted_class ); }; if ( $has_class( 'wp-block-embed-youtube' ) ) { - $preconnect_hrefs[] = 'https://www.youtube.com'; - $preconnect_hrefs[] = 'https://i.ytimg.com'; + $urls[] = 'https://www.youtube.com'; + $urls[] = 'https://i.ytimg.com'; } elseif ( $has_class( 'wp-block-embed-twitter' ) ) { - $preconnect_hrefs[] = 'https://syndication.twitter.com'; - $preconnect_hrefs[] = 'https://pbs.twimg.com'; + $urls[] = 'https://syndication.twitter.com'; + $urls[] = 'https://pbs.twimg.com'; } elseif ( $has_class( 'wp-block-embed-vimeo' ) ) { - $preconnect_hrefs[] = 'https://player.vimeo.com'; - $preconnect_hrefs[] = 'https://f.vimeocdn.com'; - $preconnect_hrefs[] = 'https://i.vimeocdn.com'; + $urls[] = 'https://player.vimeo.com'; + $urls[] = 'https://f.vimeocdn.com'; + $urls[] = 'https://i.vimeocdn.com'; } elseif ( $has_class( 'wp-block-embed-spotify' ) ) { - $preconnect_hrefs[] = 'https://apresolve.spotify.com'; - $preconnect_hrefs[] = 'https://embed-cdn.spotifycdn.com'; - $preconnect_hrefs[] = 'https://encore.scdn.co'; - $preconnect_hrefs[] = 'https://i.scdn.co'; + $urls[] = 'https://apresolve.spotify.com'; + $urls[] = 'https://embed-cdn.spotifycdn.com'; + $urls[] = 'https://encore.scdn.co'; + $urls[] = 'https://i.scdn.co'; } elseif ( $has_class( 'wp-block-embed-videopress' ) || $has_class( 'wp-block-embed-wordpress-tv' ) ) { - $preconnect_hrefs[] = 'https://video.wordpress.com'; - $preconnect_hrefs[] = 'https://public-api.wordpress.com'; - $preconnect_hrefs[] = 'https://videos.files.wordpress.com'; - $preconnect_hrefs[] = 'https://v0.wordpress.com'; // This does not appear to be a load-balanced domain since v1.wordpress.com is not valid. + $urls[] = 'https://video.wordpress.com'; + $urls[] = 'https://public-api.wordpress.com'; + $urls[] = 'https://videos.files.wordpress.com'; + $urls[] = 'https://v0.wordpress.com'; // This does not appear to be a load-balanced domain since v1.wordpress.com is not valid. } elseif ( $has_class( 'wp-block-embed-instagram' ) ) { - $preconnect_hrefs[] = 'https://www.instagram.com'; - $preconnect_hrefs[] = 'https://static.cdninstagram.com'; - $preconnect_hrefs[] = 'https://scontent.cdninstagram.com'; + $urls[] = 'https://www.instagram.com'; + $urls[] = 'https://static.cdninstagram.com'; + $urls[] = 'https://scontent.cdninstagram.com'; } elseif ( $has_class( 'wp-block-embed-tiktok' ) ) { - $preconnect_hrefs[] = 'https://www.tiktok.com'; + $urls[] = 'https://www.tiktok.com'; // Note: The other domains used for TikTok embeds include https://lf16-tiktok-web.tiktokcdn-us.com, // https://lf16-cdn-tos.tiktokcdn-us.com, and https://lf16-tiktok-common.tiktokcdn-us.com among others // which either appear to be geo-targeted ('-us') _or_ load-balanced ('lf16'). So these are not added // to the preconnected hosts. } elseif ( $has_class( 'wp-block-embed-amazon' ) ) { - $preconnect_hrefs[] = 'https://read.amazon.com'; - $preconnect_hrefs[] = 'https://m.media-amazon.com'; + $urls[] = 'https://read.amazon.com'; + $urls[] = 'https://m.media-amazon.com'; } elseif ( $has_class( 'wp-block-embed-soundcloud' ) ) { - $preconnect_hrefs[] = 'https://w.soundcloud.com'; - $preconnect_hrefs[] = 'https://widget.sndcdn.com'; + $urls[] = 'https://w.soundcloud.com'; + $urls[] = 'https://widget.sndcdn.com'; // Note: There is also https://i1.sndcdn.com which is for the album art, but the '1' indicates it may be geotargeted/load-balanced. } elseif ( $has_class( 'wp-block-embed-pinterest' ) ) { - $preconnect_hrefs[] = 'https://assets.pinterest.com'; - $preconnect_hrefs[] = 'https://widgets.pinterest.com'; - $preconnect_hrefs[] = 'https://i.pinimg.com'; + $urls[] = 'https://assets.pinterest.com'; + $urls[] = 'https://widgets.pinterest.com'; + $urls[] = 'https://i.pinimg.com'; } - return $preconnect_hrefs; + return $urls; } /** From b789da75cb061386a9935553918fbf5d906cc03c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Jan 2025 12:45:32 -0800 Subject: [PATCH 10/10] Add missing test coverage for preconnect URLs --- .../test-cases/all-embeds-inside-viewport.php | 262 ++++++++++++++++++ .../tests/test-optimization-detective.php | 4 +- 2 files changed, 264 insertions(+), 2 deletions(-) create mode 100644 plugins/embed-optimizer/tests/test-cases/all-embeds-inside-viewport.php diff --git a/plugins/embed-optimizer/tests/test-cases/all-embeds-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/all-embeds-inside-viewport.php new file mode 100644 index 000000000..a78cb962a --- /dev/null +++ b/plugins/embed-optimizer/tests/test-cases/all-embeds-inside-viewport.php @@ -0,0 +1,262 @@ + static function ( Test_Embed_Optimizer_Optimization_Detective $test_case ): void { + + $element_data = array( + 'isLCP' => false, + 'intersectionRatio' => 1, + 'resizedBoundingClientRect' => array_merge( $test_case->get_sample_dom_rect(), array( 'height' => 500 ) ), + ); + + $elements = array(); + for ( $i = 1; $i < 10; $i++ ) { + $elements[] = array_merge( + $element_data, + array( + 'xpath' => "/*[1][self::HTML]/*[2][self::BODY]/*[{$i}][self::FIGURE]/*[1][self::DIV]", + ) + ); + } + + $test_case->populate_url_metrics( $elements ); + }, + 'buffer' => << + + + ... + + + +
+
+ +
+
+ + +
+
+ + +
+
+ + +
+
+ +
+
+ + +
+
+ +
+
+ + +
+
+ +
+
+ + +
+ +
+ + +
+ +
+ + +
+
+ +
+
+ + +
+
+ +
+
+ + +
+
+ +
+
+ + +HTML + , + 'expected' => << + + + ... + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+ +
+
+ + +
+
+ + +
+
+ + +
+
+ +
+
+ + +
+
+ +
+
+ + +
+
+ +
+
+ + +
+ +
+ + +
+ +
+ + +
+
+ +
+
+ + +
+
+ +
+
+ + +
+
+ +
+
+ + +HTML + , +); diff --git a/plugins/embed-optimizer/tests/test-optimization-detective.php b/plugins/embed-optimizer/tests/test-optimization-detective.php index 9994859e1..489c03f8e 100644 --- a/plugins/embed-optimizer/tests/test-optimization-detective.php +++ b/plugins/embed-optimizer/tests/test-optimization-detective.php @@ -118,8 +118,8 @@ static function ( $matches ) { $buffer ); $this->assertEquals( - $this->remove_initial_tabs( $expected ), - $this->remove_initial_tabs( $buffer ), + trim( $this->remove_initial_tabs( $expected ) ), + trim( $this->remove_initial_tabs( $buffer ) ), "Buffer snapshot:\n$buffer" ); }