From 04120b9a8d345793425236f8546fa8e1d054f4e5 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 29 Jul 2024 17:35:10 -0700 Subject: [PATCH] Set min-height on embed-wrapper instead of figure container --- .../class-embed-optimizer-tag-visitor.php | 64 ++++++++++++++++--- .../tests/test-cases/nested-figure-embed.php | 12 ++-- ...utside-viewport-with-subsequent-script.php | 6 +- .../single-twitter-embed-inside-viewport.php | 2 +- .../single-twitter-embed-outside-viewport.php | 2 +- ...gle-wordpress-tv-embed-inside-viewport.php | 6 +- ...le-wordpress-tv-embed-outside-viewport.php | 6 +- .../single-youtube-embed-inside-viewport.php | 2 +- .../single-youtube-embed-outside-viewport.php | 2 +- .../tests/test-cases/too-many-bookmarks.php | 4 +- 10 files changed, 75 insertions(+), 31 deletions(-) diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index a1165b2d5f..7faff55504 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -26,25 +26,64 @@ final class Embed_Optimizer_Tag_Visitor { */ protected $added_lazy_script = false; + /** + * Determines whether the processor is currently at a figure.wp-block-embed tag. + * + * @since n.e.x.t + * + * @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. + * + * @since n.e.x.t + * + * @param OD_HTML_Tag_Processor $processor Processor. + * @return bool Whether the tag should be measured and stored in URL metrics + */ + 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; } - $minimum_height = $context->url_metrics_group_collection->get_element_minimum_height( $processor->get_xpath() ); + $embed_wrapper_xpath = $processor->get_xpath() . '/*[1][self::DIV]'; + $minimum_height = $context->url_metrics_group_collection->get_element_minimum_height( $embed_wrapper_xpath ); if ( is_float( $minimum_height ) ) { $style = $processor->get_attribute( 'style' ); if ( is_string( $style ) ) { @@ -56,8 +95,7 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { $processor->set_attribute( 'style', $style ); } - $max_intersection_ratio = $context->url_metrics_group_collection->get_element_max_intersection_ratio( $processor->get_xpath() ); - + $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. @@ -131,6 +169,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 . + */ + return false; } } diff --git a/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php b/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php index 42fe1c7887..30bd54a190 100644 --- a/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php +++ b/plugins/embed-optimizer/tests/test-cases/nested-figure-embed.php @@ -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, ), @@ -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, ), @@ -101,13 +101,13 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool { -
-
+
+
-
-
+
+

So I heard you like FIGURE?

diff --git a/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php b/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php index 06c1172600..b25fb71c53 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php +++ b/plugins/embed-optimizer/tests/test-cases/single-spotify-embed-outside-viewport-with-subsequent-script.php @@ -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, ), @@ -35,8 +35,8 @@ ... -
-
+
+
diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php index 5db811959f..fb5b39df1c 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-inside-viewport.php @@ -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, ), diff --git a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php index c480e5b10c..81bf2c2ae9 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-twitter-embed-outside-viewport.php @@ -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, ), diff --git a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php index 800f77cded..b802cb8632 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-inside-viewport.php @@ -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, ), @@ -39,8 +39,8 @@ -
-
+
+
diff --git a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php index 4ec5a1dbb7..f931034b62 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-wordpress-tv-embed-outside-viewport.php @@ -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, ), @@ -35,8 +35,8 @@ ... -
-
+
+
diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php index ef9b0f0fa3..df31dce205 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-inside-viewport.php @@ -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, ), diff --git a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php index 9641a15b00..89151656d9 100644 --- a/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php +++ b/plugins/embed-optimizer/tests/test-cases/single-youtube-embed-outside-viewport.php @@ -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, ), diff --git a/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php b/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php index 76df5b30e6..696f16c603 100644 --- a/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php +++ b/plugins/embed-optimizer/tests/test-cases/too-many-bookmarks.php @@ -59,8 +59,8 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool { ... -
-
+
+