diff --git a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php index 4791b845cf..42936672f6 100644 --- a/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php +++ b/plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php @@ -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. @@ -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 . + */ + 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 f460fc9030..2878eceacc 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, ), @@ -74,12 +74,12 @@ static function ( OD_Tag_Visitor_Context $context ) use ( $test_case ): bool { ... -
+
-
+

So I heard you like FIGURE?

@@ -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 2867ff1303..523f85e6e7 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 1336dcfcbd..2279286a1a 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, ), @@ -36,7 +36,7 @@ -
+
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 3da5a29fac..62088e25f9 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, ), @@ -34,7 +34,7 @@ ... -
+
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 5992973bf4..9b6234d485 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 a48ed16b11..f9087a8399 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 c4ed8b770c..6c322bc701 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, ), @@ -35,7 +35,7 @@ -
+
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 85c53e81bf..729f25ef2c 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, ), @@ -33,7 +33,7 @@ ... -
+
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 { ... -
-
+
+
diff --git a/plugins/optimization-detective/class-od-html-tag-processor.php b/plugins/optimization-detective/class-od-html-tag-processor.php index 61d39aa107..37e9353d3d 100644 --- a/plugins/optimization-detective/class-od-html-tag-processor.php +++ b/plugins/optimization-detective/class-od-html-tag-processor.php @@ -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(); @@ -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 + * @var array */ private $bookmarked_open_stacks = array(); @@ -492,7 +492,7 @@ public function release_bookmark( $name ): bool { * * @since 0.4.0 * - * @return Generator Breadcrumb. + * @return Generator Breadcrumb. */ private function get_breadcrumbs(): Generator { foreach ( $this->open_stack_tags as $i => $breadcrumb_tag_name ) { @@ -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; } diff --git a/plugins/optimization-detective/class-od-url-metrics-group-collection.php b/plugins/optimization-detective/class-od-url-metrics-group-collection.php index 21bce9a0c5..e27b24fc6d 100644 --- a/plugins/optimization-detective/class-od-url-metrics-group-collection.php +++ b/plugins/optimization-detective/class-od-url-metrics-group-collection.php @@ -80,7 +80,8 @@ final class OD_URL_Metrics_Group_Collection implements Countable, IteratorAggreg * is_every_group_complete?: bool, * get_groups_by_lcp_element?: array, * get_common_lcp_element?: ElementData|null, - * get_all_element_max_intersection_ratios?: array + * get_all_element_max_intersection_ratios?: array, + * get_all_element_minimum_heights?: array * } */ private $result_cache = array(); @@ -430,6 +431,41 @@ public function get_all_element_max_intersection_ratios(): array { return $result; } + /** + * Gets the minimum heights of all elements across all groups and their captured URL metrics. + * + * @return array Keys are XPaths and values are the minimum heights. + */ + public function get_all_element_minimum_heights(): array { + if ( array_key_exists( __FUNCTION__, $this->result_cache ) ) { + return $this->result_cache[ __FUNCTION__ ]; + } + + $result = ( function () { + $element_min_heights = array(); + + /* + * O(n^3) my! Yes. This is why the result is cached. This being said, the number of groups should be 4 (one + * more than the default number of breakpoints) and the number of URL metrics for each group should be 3 + * (the default sample size). Therefore, given the number (n) of visited elements on the page this will only + * end up running n*4*3 times. + */ + foreach ( $this->groups as $group ) { + foreach ( $group as $url_metric ) { + foreach ( $url_metric->get_elements() as $element ) { + $element_min_heights[ $element['xpath'] ] = array_key_exists( $element['xpath'], $element_min_heights ) + ? min( $element_min_heights[ $element['xpath'] ], $element['intersectionRect']['height'] ) + : $element['intersectionRect']['height']; + } + } + } + return $element_min_heights; + } )(); + + $this->result_cache[ __FUNCTION__ ] = $result; + return $result; + } + /** * Gets the max intersection ratio of an element across all groups and their captured URL metrics. * @@ -440,6 +476,16 @@ public function get_element_max_intersection_ratio( string $xpath ): ?float { return $this->get_all_element_max_intersection_ratios()[ $xpath ] ?? null; } + /** + * Gets the minimum height of an element across all groups and their captured URL metrics. + * + * @param string $xpath XPath for the element. + * @return int Minimum height in pixels. + */ + public function get_element_minimum_height( string $xpath ): ?int { + return $this->get_all_element_minimum_heights()[ $xpath ] ?? null; + } + /** * Gets URL metrics from all groups flattened into one list. * diff --git a/plugins/optimization-detective/helper.php b/plugins/optimization-detective/helper.php index 553d55cb11..b6656c5094 100644 --- a/plugins/optimization-detective/helper.php +++ b/plugins/optimization-detective/helper.php @@ -10,6 +10,22 @@ exit; // Exit if accessed directly. } +/** + * Constructs XPath from indexed breadcrumbs. + * + * @since n.e.x.t + * + * @param array $breadcrumbs Indexed breadcrumbs. + * @return string XPath. + */ +function od_construct_xpath( array $breadcrumbs ): string { + $xpath = ''; + foreach ( $breadcrumbs as $breadcrumb ) { + $xpath .= sprintf( '/*[%d][self::%s]', $breadcrumb[1] + 1, $breadcrumb[0] ); + } + return $xpath; +} + /** * Displays the HTML generator meta tag for the Optimization Detective plugin. * diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index a9bc8a0f7d..ad46dca5f9 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -59,12 +59,12 @@ static function ( array $element ): array { 'isLCPCandidate' => $element['isLCP'], 'intersectionRatio' => 1, 'intersectionRect' => array( - 'width' => 100, - 'height' => 100, + 'width' => 500, + 'height' => 500, ), 'boundingClientRect' => array( - 'width' => 100, - 'height' => 100, + 'width' => 500, + 'height' => 500, ), ), $element