Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorporate media queries into preconnect links to account for whether embeds are in viewport #1654

Merged
merged 16 commits into from
Nov 20, 2024
Merged

Incorporate media queries into preconnect links to account for whether embeds are in viewport #1654

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
9a3c838
Fix typo in comment
ShyamGadde Nov 15, 2024
a89a2b4
Fix typo in warning message
ShyamGadde Nov 15, 2024
6fba611
Take breakpoints into account while adding preconnect links
ShyamGadde Nov 15, 2024
622901a
Sort links by `href` before `minimum_viewport_width`
ShyamGadde Nov 15, 2024
c89ba87
Update test cases to account for the updated sorting logic of preconn…
ShyamGadde Nov 15, 2024
85d4fd4
Update test cases to account for updated link sorting logic in Image …
ShyamGadde Nov 15, 2024
192d382
Merge branch 'trunk' into add/breakpoints-in-preconnect-links
ShyamGadde Nov 18, 2024
aa06707
Refactor `OD_URL_Metric_Group_Collection` to leverage `OD_URL_Metric_…
ShyamGadde Nov 18, 2024
c22701f
Add caching for method return values in `OD_URL_Metric_Group` class
ShyamGadde Nov 18, 2024
5ecfcf2
Fix float comparison for intersection ratio
ShyamGadde Nov 18, 2024
9e0da37
Add test cases for when the embed is not visible on mobile
ShyamGadde Nov 19, 2024
096497f
Merge branch 'trunk' into add/breakpoints-in-preconnect-links
ShyamGadde Nov 19, 2024
9169bca
Update YT test case to check different media queries
westonruter Nov 20, 2024
415846d
Explicitly test variable-height embeds
westonruter Nov 20, 2024
dba4073
Bump OD to 0.9.0-alpha and update Embed Optimizer to require at least…
westonruter Nov 20, 2024
2c799ad
Fix test_embed_optimizer_init_optimization_detective
westonruter Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ private function is_embed_wrapper( OD_HTML_Tag_Processor $processor ): bool {
* Otherwise, if the embed is not in any initial viewport, it will add lazy-loading logic.
*
* @since 0.2.0
* @since n.e.x.t Adds preconnect links for each viewport group and skips if the element is not in the viewport for that group.
*
* @param OD_Tag_Visitor_Context $context Tag visitor context.
* @return bool Whether the tag should be tracked in URL Metrics.
Expand Down Expand Up @@ -109,7 +110,8 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool {
&&
$context->url_metric_group_collection->get_last_group()->count() > 0
) {
$max_intersection_ratio = $context->url_metric_group_collection->get_element_max_intersection_ratio( self::get_embed_wrapper_xpath( $processor->get_xpath() ) );
$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.
Expand Down Expand Up @@ -171,12 +173,20 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool {
}

foreach ( $preconnect_hrefs as $preconnect_href ) {
$context->link_collection->add_link(
array(
'rel' => 'preconnect',
'href' => $preconnect_href,
)
);
foreach ( $context->url_metric_group_collection as $group ) {
if ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) <= 0 ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the intersection ratio ever be less than 0? I don't think so, and Gemini confirms. Since we're comparing floats, it's probably not safe to check if the return value is exactly equal to 0. How about:

Suggested change
if ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) <= 0 ) {
if ( ! ( $group->get_element_max_intersection_ratio( $embed_wrapper_xpath ) > 0.0 ) ) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Changed it in 5ecfcf2.

continue;
}

$context->link_collection->add_link(
array(
'rel' => 'preconnect',
'href' => $preconnect_href,
),
$group->get_minimum_viewport_width(),
$group->get_maximum_viewport_width()
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The analogous logic that adds preload links for the LCP image:

performance/plugins/image-prioritizer/class-image-prioritizer-img-tag-visitor.php

Lines 146 to 180 in df29967

foreach ( $context->url_metric_group_collection->get_groups_by_lcp_element( $xpath ) as $group ) {
$link_attributes = array_merge(
array(
'rel' => 'preload',
'fetchpriority' => 'high',
'as' => 'image',
),
array_filter(
array(
'href' => (string) $processor->get_attribute( 'src' ),
'imagesrcset' => (string) $processor->get_attribute( 'srcset' ),
'imagesizes' => (string) $processor->get_attribute( 'sizes' ),
),
static function ( string $value ): bool {
return '' !== $value;
}
)
);
$crossorigin = $this->get_attribute_value( $processor, 'crossorigin' );
if ( null !== $crossorigin ) {
$link_attributes['crossorigin'] = 'use-credentials' === $crossorigin ? 'use-credentials' : 'anonymous';
}
$link_attributes['media'] = 'screen';
$context->link_collection->add_link(
$link_attributes,
$group->get_minimum_viewport_width(),
$group->get_maximum_viewport_width()
);
}
return true;
}

For that we use $context->url_metric_group_collection->get_groups_by_lcp_element() to get the groups, whereas here it is looping over each group in the collection and then checking for each one whether the current embed is visible in it. So this makes sense to me.

}
} elseif ( 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' ) ) );
Expand Down
2 changes: 1 addition & 1 deletion ...-optimizer/tests/test-cases/single-twitter-embed-inside-viewport-without-resized-data.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
<head>
<meta charset="utf-8">
<title>...</title>
<link data-od-added-tag rel="preconnect" href="https://syndication.twitter.com">
<link data-od-added-tag rel="preconnect" href="https://pbs.twimg.com">
<link data-od-added-tag rel="preconnect" href="https://syndication.twitter.com">
</head>
<body>
<figure class="wp-block-embed is-type-rich is-provider-twitter wp-block-embed-twitter">
Expand Down
2 changes: 1 addition & 1 deletion plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@
@media (min-width: 601px) and (max-width: 782px) { #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } }
@media (min-width: 783px) { #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } }
</style>
<link data-od-added-tag rel="preconnect" href="https://syndication.twitter.com">
<link data-od-added-tag rel="preconnect" href="https://pbs.twimg.com">
<link data-od-added-tag rel="preconnect" href="https://syndication.twitter.com">
</head>
<body>
<figure data-od-added-id id="embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8" class="wp-block-embed is-type-rich is-provider-twitter wp-block-embed-twitter">
Expand Down
4 changes: 2 additions & 2 deletions plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@
@media (min-width: 601px) and (max-width: 782px) { #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } }
@media (min-width: 783px) { #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } }
</style>
<link data-od-added-tag rel="preconnect" href="https://video.wordpress.com">
<link data-od-added-tag rel="preconnect" href="https://public-api.wordpress.com">
<link data-od-added-tag rel="preconnect" href="https://videos.files.wordpress.com">
<link data-od-added-tag rel="preconnect" href="https://v0.wordpress.com">
<link data-od-added-tag rel="preconnect" href="https://video.wordpress.com">
<link data-od-added-tag rel="preconnect" href="https://videos.files.wordpress.com">
</head>
<body>
<figure data-od-added-id id="embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8" class="wp-block-embed is-type-video is-provider-wordpress-tv wp-block-embed-wordpress-tv wp-embed-aspect-16-9 wp-has-aspect-ratio">
Expand Down
2 changes: 1 addition & 1 deletion plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
@media (min-width: 601px) and (max-width: 782px) { #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } }
@media (min-width: 783px) { #embed-optimizer-a7659db28ecaa36ddee6ae66857dabd8 { min-height: 500px; } }
</style>
<link data-od-added-tag rel="preconnect" href="https://www.youtube.com">
<link data-od-added-tag rel="preconnect" href="https://i.ytimg.com">
<link data-od-added-tag rel="preconnect" href="https://www.youtube.com">
</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">
Expand Down
2 changes: 1 addition & 1 deletion ...t-lcp-elements-for-non-consecutive-viewport-groups-with-missing-data-for-middle-group.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@
<meta charset="utf-8">
<title>...</title>
<style>/* Never show mobile and desktop logos at the same time. */</style>
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/mobile-logo.png" media="screen and (max-width: 480px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/desktop-logo.png" media="screen and (min-width: 783px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/mobile-logo.png" media="screen and (max-width: 480px)">
</head>
<body>
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]" src="https://example.com/mobile-logo.png" alt="Mobile Logo" width="600" height="600">
Expand Down
2 changes: 1 addition & 1 deletion ...ioritizer/tests/test-cases/different-lcp-elements-for-two-non-consecutive-breakpoints.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ static function () {
<head>
<meta charset="utf-8">
<title>...</title>
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/mobile-logo.png" media="screen and (max-width: 480px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/desktop-logo.png" media="screen and (min-width: 601px) and (max-width: 782px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/mobile-logo.png" media="screen and (max-width: 480px)">
</head>
<body>
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]" src="https://example.com/mobile-logo.png" alt="Mobile Logo" width="600" height="600">
Expand Down
2 changes: 1 addition & 1 deletion plugins/image-prioritizer/tests/test-cases/responsive-background-images.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ static function () use ( $mobile_breakpoint, $tablet_breakpoint ): array {
<meta charset="utf-8">
<title>...</title>
<style>/* responsive styles to show only one div.header at a time... */</style>
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/desktop-bg.jpg" media="screen and (min-width: 601px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/mobile-bg.jpg" media="screen and (max-width: 480px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/tablet-bg.jpg" media="screen and (min-width: 481px) and (max-width: 600px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/desktop-bg.jpg" media="screen and (min-width: 601px)">
</head>
<body>
<div class="header desktop" style="background: red no-repeat center/80% url(\'https://example.com/desktop-bg.jpg\'); width:100%; height: 200px;">This is the desktop background!</div>
Expand Down
2 changes: 1 addition & 1 deletion ...r/tests/test-cases/video-with-poster-lcp-element-on-mobile-and-desktop-but-not-tablet.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ static function () use ( $breakpoint_max_widths ) {
<meta charset="utf-8">
<title>...</title>
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/poster.jpg" media="screen and (max-width: 480px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/tablet-header.jpg" media="screen and (min-width: 481px) and (max-width: 782px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/poster.jpg" media="screen and (min-width: 783px)">
<link data-od-added-tag rel="preload" fetchpriority="high" as="image" href="https://example.com/tablet-header.jpg" media="screen and (min-width: 481px) and (max-width: 782px)">
</head>
<body>
<img data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]" class="tablet" src="https://example.com/tablet-header.jpg" alt="Tablet header" width="800" height="600">
Expand Down
2 changes: 1 addition & 1 deletion plugins/optimization-detective/class-od-html-tag-processor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ public function next_token(): bool {
$this->open_stack_tags = array();
$this->open_stack_indices = array();

// Mark that the end of the document was reached, meaning that get_modified_html() can should now be able to append markup to the HEAD and the BODY.
// Mark that the end of the document was reached, meaning that get_modified_html() should now be able to append markup to the HEAD and the BODY.
$this->reached_end_of_document = true;
return false;
}
Expand Down
15 changes: 13 additions & 2 deletions plugins/optimization-detective/class-od-link-collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,18 +130,29 @@ function ( array $links ): array {
*/
private function merge_consecutive_links( array $links ): array {

// Ensure links are sorted by the minimum_viewport_width.
usort(
$links,
/**
* Comparator.
*
* The links are sorted first by the 'href' attribute to group identical URLs together.
* If the 'href' attributes are the same, the links are then sorted by 'minimum_viewport_width'.
*
* @param Link $a First link.
* @param Link $b Second link.
* @return int Comparison result.
*/
static function ( array $a, array $b ): int {
return $a['minimum_viewport_width'] <=> $b['minimum_viewport_width'];
// Get href values, defaulting to empty string if not present.
$href_a = $a['attributes']['href'] ?? '';
$href_b = $b['attributes']['href'] ?? '';

$href_comparison = strcmp( $href_a, $href_b );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be the same as:

$href_comparison = ( $href_a <=> $href_b );

Right?

Copy link
Contributor Author

@ShyamGadde ShyamGadde Nov 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe in this case it would be the same.

if ( 0 === $href_comparison ) {
return $a['minimum_viewport_width'] <=> $b['minimum_viewport_width'];
}

return $href_comparison;
}
);

Expand Down
56 changes: 56 additions & 0 deletions plugins/optimization-detective/class-od-url-metric-group.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,62 @@ public function get_lcp_element(): ?OD_Element {
return $result;
}

/**
* Gets all elements from all URL Metrics in the viewport group keyed by the elements' XPaths.
*
* @since n.e.x.t
*
* @return array<string, non-empty-array<int, OD_Element>> Keys are XPaths and values are the element instances.
*/
public function get_xpath_elements_map(): array {
$result = ( function () {
westonruter marked this conversation as resolved.
Show resolved Hide resolved
$all_elements = array();
foreach ( $this->url_metrics as $url_metric ) {
foreach ( $url_metric->get_elements() as $element ) {
$all_elements[ $element->get_xpath() ][] = $element;
}
}
return $all_elements;
} )();

return $result;
}

/**
* Gets the max intersection ratios of all elements in the viewport group and its captured URL Metrics.
*
* @since n.e.x.t
*
* @return array<string, float> Keys are XPaths and values are the intersection ratios.
*/
public function get_all_element_max_intersection_ratios(): array {
westonruter marked this conversation as resolved.
Show resolved Hide resolved
$result = ( function () {
$elements_max_intersection_ratios = array();
foreach ( $this->get_xpath_elements_map() as $xpath => $elements ) {
$element_intersection_ratios = array();
foreach ( $elements as $element ) {
$element_intersection_ratios[] = $element->get_intersection_ratio();
}
$elements_max_intersection_ratios[ $xpath ] = (float) max( $element_intersection_ratios );
}
return $elements_max_intersection_ratios;
} )();

return $result;
}

/**
* Gets the max intersection ratio of an element in the viewport group and its captured URL Metrics.
*
* @since n.e.x.t
*
* @param string $xpath XPath for the element.
* @return float|null Max intersection ratio of null if tag is unknown (not captured).
*/
public function get_element_max_intersection_ratio( string $xpath ): ?float {
return $this->get_all_element_max_intersection_ratios()[ $xpath ] ?? null;
}

/**
* Returns an iterator for the URL Metrics in the group.
*
Expand Down
2 changes: 1 addition & 1 deletion plugins/optimization-detective/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function od_initialize_extensions(): void {
*/
function od_generate_media_query( ?int $minimum_viewport_width, ?int $maximum_viewport_width ): ?string {
if ( is_int( $minimum_viewport_width ) && is_int( $maximum_viewport_width ) && $minimum_viewport_width > $maximum_viewport_width ) {
_doing_it_wrong( __FUNCTION__, esc_html__( 'The minimum width must be greater than the maximum width.', 'optimization-detective' ), 'Optimization Detective 0.7.0' );
_doing_it_wrong( __FUNCTION__, esc_html__( 'The minimum width cannot be greater than the maximum width.', 'optimization-detective' ), 'Optimization Detective 0.7.0' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right 😅

return null;
}
$media_attributes = array();
Expand Down
Loading