Remove requirement for both mobile and desktop URL metrics for preconnect links in Embed Optimizer #1764
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -26,7 +26,7 @@ final class Embed_Optimizer_Tag_Visitor { | |||||
| * | ||||||
| * @var bool | ||||||
| */ | ||||||
| private $added_lazy_script = false; | ||||||
|
|
||||||
| /** | ||||||
| * Determines whether the processor is currently at a figure.wp-block-embed tag. | ||||||
|
|
@@ -103,96 +103,8 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { | |||||
| } | ||||||
|
|
||||||
| $this->reduce_layout_shifts( $context ); | ||||||
| $this->add_preconnect_links( $context ); | ||||||
| $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 | ||||||
|
|
@@ -283,4 +195,134 @@ private function reduce_layout_shifts( OD_Tag_Visitor_Context $context ): void { | |||||
| $processor->append_head_html( sprintf( "<style>\n%s\n</style>\n", join( "\n", $style_rules ) ) ); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Gets preconnect URLs based on embed type | ||||||
| * | ||||||
| * @since n.e.x.t | ||||||
| * | ||||||
| * @param OD_HTML_Tag_Processor $processor Processor. | ||||||
| * @return array<non-empty-string> 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 <https://colab.sandbox.google.com/drive/1nSpg3qoCLY-cBTV2zOUkgUCU7R7X2f_R?resourcekey=0-MgT7Ur0pT__vw-5_AHjgWQ#scrollTo=utZv59sXzXvS>. | ||||||
|
There was a problem hiding this comment. FYI, this link doesn't seem to be publicly accessible. There was a problem hiding this comment. Oh, thanks for that. @adamsilverstein Can this be exported to a Gist? |
||||||
| * 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'; | ||||||
| } | ||||||
|
|
||||||
| 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 ) ) { | ||||||
| continue; | ||||||
| } | ||||||
|
|
||||||
| $context->link_collection->add_link( | ||||||
| array( | ||||||
| 'rel' => 'preconnect', | ||||||
| 'href' => $preconnect_href, | ||||||
| ), | ||||||
| $group->get_minimum_viewport_width(), | ||||||
| $group->get_maximum_viewport_width() | ||||||
| ); | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * 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 ) { | ||||||
|
There was a problem hiding this comment. @westonruter Do we need to use There was a problem hiding this comment. I think
Suggested change
There was a problem hiding this comment. Makes sense, thanks for clarifying. Done in 0b4150c. |
||||||
| $processor->append_body_html( wp_get_inline_script_tag( embed_optimizer_get_lazy_load_script(), array( 'type' => 'module' ) ) ); | ||||||
| $this->added_lazy_script = true; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| <?php | ||
| return array( | ||
| 'set_up' => 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' => ' | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>...</title> | ||
| </head> | ||
| <body> | ||
| <figure class="wp-block-embed is-type-video is-provider-youtube wp-block-embed-youtube wp-embed-aspect-16-9 wp-has-aspect-ratio"> | ||
| <div class="wp-block-embed__wrapper"> | ||
| <iframe title="Matt Mullenweg: State of the Word 2023" width="750" height="422" src="https://www.youtube.com/embed/c7M4mBVgP3Y?feature=oembed" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share" referrerpolicy="strict-origin-when-cross-origin" allowfullscreen></iframe> | ||
| </div> | ||
| </figure> | ||
| </body> | ||
| </html> | ||
| ', | ||
| 'expected' => ' | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>...</title> | ||
| <style> | ||
| @media (max-width: 480px) { #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } } | ||
| </style> | ||
| <link data-od-added-tag rel="preconnect" href="https://i.ytimg.com" media="(max-width: 480px)"> | ||
| <link data-od-added-tag rel="preconnect" href="https://www.youtube.com" media="(max-width: 480px)"> | ||
| </head> | ||
| <body> | ||
| <figure data-od-added-id id="embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8" class="wp-block-embed is-type-video is-provider-youtube wp-block-embed-youtube wp-embed-aspect-16-9 wp-has-aspect-ratio"> | ||
| <div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper"> | ||
| <iframe title="Matt Mullenweg: State of the Word 2023" width="750" height="422" src="https://www.youtube.com/embed/c7M4mBVgP3Y?feature=oembed" frameborder="0" allow="accelerometer; autoplay; clipboard-write; encrypted-media; gyroscope; picture-in-picture; web-share" referrerpolicy="strict-origin-when-cross-origin" allowfullscreen></iframe> | ||
| </div> | ||
| </figure> | ||
| <script type="module">/* import detect ... */</script> | ||
| </body> | ||
| </html> | ||
| ', | ||
| ); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the comment lines here are missing an
*There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. Fixed it in 0b4150c.