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.

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

Add helper function to construct XPath from indexed breadcrumbs #1407

Draft
wants to merge 6 commits into
base: add/embed-optimizer-min-height-reservation
Choose a base branch
from
Draft
68 changes: 60 additions & 8 deletions plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,72 @@ final class Embed_Optimizer_Tag_Visitor {
*/
protected $added_lazy_script = false;

/**
* Determines whether the processor is currently at a figure.wp-block-embed tag.
*
* @param OD_HTML_Tag_Processor $processor Processor.
* @return bool Whether at the tag.
*/
private function is_embed_figure( OD_HTML_Tag_Processor $processor ): bool {
return (
'FIGURE' === $processor->get_tag()
&&
true === $processor->has_class( 'wp-block-embed' )
);
}

/**
* Determines whether the processor is currently at a div.wp-block-embed__wrapper tag.
*
* @param OD_HTML_Tag_Processor $processor Processor.
* @return bool Whether at the tag.
*/
private function is_embed_wrapper( OD_HTML_Tag_Processor $processor ): bool {
return (
'DIV' === $processor->get_tag()
&&
true === $processor->has_class( 'wp-block-embed__wrapper' )
);
}

/**
* Visits a tag.
*
* @since 0.2.0
*
* @param OD_Tag_Visitor_Context $context Tag visitor context.
* @return bool Whether the visit or visited the tag.
* @return bool Whether the tag should be measured and stored in URL metrics.
*/
public function __invoke( OD_Tag_Visitor_Context $context ): bool {
$processor = $context->processor;
if ( ! (
'FIGURE' === $processor->get_tag()
&&
true === $processor->has_class( 'wp-block-embed' )
) ) {

/*
* The only thing we need to do if it is a div.wp-block-embed__wrapper tag is return true so that the tag
* will get measured and stored in the URL Metrics.
*/
if ( $this->is_embed_wrapper( $processor ) ) {
return true;
}

// Short-circuit if not a figure.wp-block-embed tag.
if ( ! $this->is_embed_figure( $processor ) ) {
return false;
}

$max_intersection_ratio = $context->url_metrics_group_collection->get_element_max_intersection_ratio( $processor->get_xpath() );
$embed_wrapper_xpath = $processor->get_xpath() . od_construct_xpath( array( array( 'DIV', 0 ) ) );
$minimum_height = $context->url_metrics_group_collection->get_element_minimum_height( $embed_wrapper_xpath );
if ( is_int( $minimum_height ) ) {
$style = $processor->get_attribute( 'style' );
if ( is_string( $style ) ) {
$style = rtrim( trim( $style ), ';' ) . '; ';
} else {
$style = '';
}
$style .= sprintf( 'min-height: %dpx;', $minimum_height );
$processor->set_attribute( 'style', $style );
}

$max_intersection_ratio = $context->url_metrics_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 @@ -119,6 +165,12 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool {
$this->added_lazy_script = true;
}

return true;
/*
* At this point the tag is a figure.wp-block-embed, and we can return false because this does not need to be
* measured and stored in URL Metrics. Only the child div.wp-block-embed__wrapper tag is measured and stored
* so that this visitor can look up the height to set as a min-height on the figure.wp-block-embed. For more
* information on what the return values mean for tag visitors, see <https://github.com/WordPress/performance/issues/1342>.
*/
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 1,
),
Expand All @@ -14,7 +14,7 @@
'intersectionRatio' => 1,
),
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down Expand Up @@ -74,12 +74,12 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool {
<title>...</title>
</head>
<body>
<figure class="wp-block-embed is-type-video">
<figure style="background: black; color:gray" class="wp-block-embed is-type-video">
<div class="wp-block-embed__wrapper">
<video src="https://example.com/video1.mp4" poster="https://example.com/poster1.jpg" width="640" height="480"></video>
</div>
</figure>
<figure class="wp-block-embed is-type-rich is-provider-figurine wp-block-embed-figurine">
<figure style="background: black; color: white;" class="wp-block-embed is-type-rich is-provider-figurine wp-block-embed-figurine">
<div class="wp-block-embed__wrapper">
<figure>
<p>So I heard you like <code>FIGURE</code>?</p>
Expand All @@ -101,13 +101,13 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool {
<link data-od-added-tag rel="preload" as="image" href="https://example.com/poster1.jpg">
</head>
<body>
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" class="wp-block-embed is-type-video">
<div class="wp-block-embed__wrapper">
<figure data-od-replaced-style="background: black; color:gray" style="background: black; color:gray; min-height: 500px;" class="wp-block-embed is-type-video">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<video data-od-added-preload data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]/*[1][self::VIDEO]" preload="auto" src="https://example.com/video1.mp4" poster="https://example.com/poster1.jpg" width="640" height="480"></video>
</div>
</figure>
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]" class="wp-block-embed is-type-rich is-provider-figurine wp-block-embed-figurine">
<div class="wp-block-embed__wrapper">
<figure data-od-replaced-style="background: black; color: white;" style="background: black; color: white; min-height: 500px;" class="wp-block-embed is-type-rich is-provider-figurine wp-block-embed-figurine">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<figure>
<p>So I heard you like <code>FIGURE</code>?</p>
<video data-od-added-preload data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[2][self::FIGURE]/*[1][self::DIV]/*[1][self::FIGURE]/*[2][self::VIDEO]" preload="none" src="https://example.com/video2.mp4" poster="https://example.com/poster2.jpg" width="640" height="480"></video>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down Expand Up @@ -35,8 +35,8 @@
<title>...</title>
</head>
<body>
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" class="wp-block-embed is-type-rich is-provider-spotify wp-block-embed-spotify wp-embed-aspect-21-9 wp-has-aspect-ratio">
<div class="wp-block-embed__wrapper">
<figure data-od-added-style style="min-height: 500px;" class="wp-block-embed is-type-rich is-provider-spotify wp-block-embed-spotify wp-embed-aspect-21-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 data-od-added-loading loading="lazy" title="Spotify Embed: Deep Focus" style="border-radius: 12px" width="100%" height="352" frameborder="0" allowfullscreen allow="autoplay; clipboard-write; encrypted-media; fullscreen; picture-in-picture" src="https://open.spotify.com/embed/playlist/37i9dQZF1DWZeKCadgRdKQ?utm_source=oembed"></iframe>
</div>
</figure>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => true,
'intersectionRatio' => 1,
),
Expand Down Expand Up @@ -36,7 +36,7 @@
<link data-od-added-tag rel="preconnect" href="https://pbs.twimg.com">
</head>
<body>
<figure class="wp-block-embed is-type-rich is-provider-twitter wp-block-embed-twitter">
<figure data-od-added-style style="min-height: 500px;" class="wp-block-embed is-type-rich is-provider-twitter wp-block-embed-twitter">
<div class="wp-block-embed__wrapper">
<blockquote class="twitter-tweet" data-width="550" data-dnt="true"><p lang="en" dir="ltr">We want your feedback for the Privacy Sandbox 📨<br><br>Learn why your feedback is critical through real examples and learn how to provide it ↓ <a href="https://t.co/anGk6gWkbc">https://t.co/anGk6gWkbc</a></p>&mdash; Chrome for Developers (@ChromiumDev) <a href="https://twitter.com/ChromiumDev/status/1636796541368139777?ref_src=twsrc%5Etfw">March 17, 2023</a></blockquote>
<script async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down Expand Up @@ -34,7 +34,7 @@
<title>...</title>
</head>
<body>
<figure class="wp-block-embed is-type-rich is-provider-twitter wp-block-embed-twitter">
<figure data-od-added-style style="min-height: 500px;" class="wp-block-embed is-type-rich is-provider-twitter wp-block-embed-twitter">
<div class="wp-block-embed__wrapper">
<blockquote class="twitter-tweet" data-width="550" data-dnt="true"><p lang="en" dir="ltr">We want your feedback for the Privacy Sandbox 📨<br><br>Learn why your feedback is critical through real examples and learn how to provide it ↓ <a href="https://t.co/anGk6gWkbc">https://t.co/anGk6gWkbc</a></p>&mdash; Chrome for Developers (@ChromiumDev) <a href="https://twitter.com/ChromiumDev/status/1636796541368139777?ref_src=twsrc%5Etfw">March 17, 2023</a></blockquote>
<script data-od-added-type type="application/vnd.embed-optimizer.javascript" async src="https://platform.twitter.com/widgets.js" charset="utf-8"></script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => true,
'intersectionRatio' => 1,
),
Expand Down Expand Up @@ -39,8 +39,8 @@
<link data-od-added-tag rel="preconnect" href="https://v0.wordpress.com">
</head>
<body>
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" 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">
<div class="wp-block-embed__wrapper">
<figure data-od-added-style style="min-height: 500px;" 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">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<iframe title="VideoPress Video Player" aria-label=\'VideoPress Video Player\' width=\'750\' height=\'422\' src=\'https://video.wordpress.com/embed/vaWm9zO6?hd=1&amp;cover=1\' frameborder=\'0\' allowfullscreen allow=\'clipboard-write\'></iframe>
<script src=\'https://v0.wordpress.com/js/next/videopress-iframe.js?m=1674852142\'></script>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down Expand Up @@ -35,8 +35,8 @@
<title>...</title>
</head>
<body>
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" 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">
<div class="wp-block-embed__wrapper">
<figure data-od-added-style style="min-height: 500px;" 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">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<iframe data-od-added-loading loading="lazy" title="VideoPress Video Player" aria-label=\'VideoPress Video Player\' width=\'750\' height=\'422\' src=\'https://video.wordpress.com/embed/vaWm9zO6?hd=1&amp;cover=1\' frameborder=\'0\' allowfullscreen allow=\'clipboard-write\'></iframe>
<script data-od-added-type type="application/vnd.embed-optimizer.javascript" src=\'https://v0.wordpress.com/js/next/videopress-iframe.js?m=1674852142\'></script>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => true,
'intersectionRatio' => 1,
),
Expand Down Expand Up @@ -35,7 +35,7 @@
<link data-od-added-tag rel="preconnect" href="https://i.ytimg.com">
</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">
<figure data-od-added-style style="min-height: 500px;" 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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
$test_case->populate_url_metrics(
array(
array(
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]',
'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]',
'isLCP' => false,
'intersectionRatio' => 0,
),
Expand Down Expand Up @@ -33,7 +33,7 @@
<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">
<figure data-od-added-style style="min-height: 500px;" 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 data-od-added-loading loading="lazy" 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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool {
<title>...</title>
</head>
<body data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]">
<figure data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]" 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">
<div class="wp-block-embed__wrapper">
<figure 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">
<div data-od-xpath="/*[1][self::HTML]/*[2][self::BODY]/*[1][self::FIGURE]/*[1][self::DIV]" class="wp-block-embed__wrapper">
<iframe title="VideoPress Video Player" aria-label=\'VideoPress Video Player\' width=\'750\' height=\'422\' src=\'https://video.wordpress.com/embed/vaWm9zO6?hd=1&amp;cover=1\' frameborder=\'0\' allowfullscreen allow=\'clipboard-write\'></iframe>
<script src=\'https://v0.wordpress.com/js/next/videopress-iframe.js?m=1674852142\'></script>
</div>
Expand Down
11 changes: 4 additions & 7 deletions plugins/optimization-detective/class-od-html-tag-processor.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
* Open stack indices.
*
* @since 0.4.0
* @var int[]
* @var non-negative-int[]
*/
private $open_stack_indices = array();

Expand All @@ -147,7 +147,7 @@ final class OD_HTML_Tag_Processor extends WP_HTML_Tag_Processor {
* populated back into `$this->open_stack_tags` and `$this->open_stack_indices`.
*
* @since 0.4.0
* @var array<string, array{tags: string[], indices: int[]}>
* @var array<string, array{tags: string[], indices: non-negative-int[]}>
*/
private $bookmarked_open_stacks = array();

Expand Down Expand Up @@ -492,7 +492,7 @@ public function release_bookmark( $name ): bool {
*
* @since 0.4.0
*
* @return Generator<array{string, int}> Breadcrumb.
* @return Generator<array{string, non-negative-int}> Breadcrumb.
*/
private function get_breadcrumbs(): Generator {
foreach ( $this->open_stack_tags as $i => $breadcrumb_tag_name ) {
Expand Down Expand Up @@ -528,10 +528,7 @@ private function is_foreign_element(): bool {
*/
public function get_xpath(): string {
if ( null === $this->current_xpath ) {
$this->current_xpath = '';
foreach ( $this->get_breadcrumbs() as list( $tag_name, $index ) ) {
$this->current_xpath .= sprintf( '/*[%d][self::%s]', $index + 1, $tag_name );
}
$this->current_xpath = od_construct_xpath( iterator_to_array( $this->get_breadcrumbs() ) );
}
return $this->current_xpath;
}
Expand Down
Loading