diff --git a/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-on-lcp-image-common-on-mobile-and-desktop-with-url-metrics-missing-in-other-groups.php b/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-on-lcp-image-common-on-mobile-and-desktop-with-url-metrics-missing-in-other-groups.php new file mode 100644 index 0000000000..251a550fad --- /dev/null +++ b/plugins/image-prioritizer/tests/test-cases/fetch-priority-high-on-lcp-image-common-on-mobile-and-desktop-with-url-metrics-missing-in-other-groups.php @@ -0,0 +1,68 @@ + static function ( Test_Image_Prioritizer_Helper $test_case ): void { + $breakpoint_max_widths = array( 480, 600, 782 ); + + add_filter( + 'od_breakpoint_max_widths', + static function () use ( $breakpoint_max_widths ) { + return $breakpoint_max_widths; + } + ); + + 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' => 375, + 'elements' => array( + array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => true, + ), + ), + ) + ) + ); + + 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' => 1000, + 'elements' => array( + array( + 'xpath' => '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]', + 'isLCP' => true, + ), + ), + ) + ) + ); + }, + 'buffer' => ' + + + + ... + + + Foo + + + ', + 'expected' => ' + + + + ... + + + + + Foo + + + + ', +); diff --git a/plugins/optimization-detective/class-od-url-metric-group-collection.php b/plugins/optimization-detective/class-od-url-metric-group-collection.php index 9b50846eb6..ed7fe6928c 100644 --- a/plugins/optimization-detective/class-od-url-metric-group-collection.php +++ b/plugins/optimization-detective/class-od-url-metric-group-collection.php @@ -427,6 +427,7 @@ public function get_groups_by_lcp_element( string $xpath ): array { * Gets common LCP element. * * @since 0.3.0 + * @since n.e.x.t An LCP element is also considered common if it is the same in the narrowest and widest viewport groups, and all intermediate groups are empty. * * @return OD_Element|null Common LCP element if it exists. */ @@ -437,38 +438,40 @@ public function get_common_lcp_element(): ?OD_Element { $result = ( function () { - // If every group isn't populated, then we can't say whether there is a common LCP element across every viewport group. - if ( ! $this->is_every_group_populated() ) { + // Ensure both the narrowest (first) and widest (last) viewport groups are populated. + $first_group = $this->get_first_group(); + $last_group = $this->get_last_group(); + if ( $first_group->count() === 0 || $last_group->count() === 0 ) { return null; } - // Look at the LCP elements across all the viewport groups. - $groups_by_lcp_element_xpath = array(); - $lcp_elements_by_xpath = array(); - $group_has_unknown_lcp_element = false; - foreach ( $this->groups as $group ) { - $lcp_element = $group->get_lcp_element(); - if ( $lcp_element instanceof OD_Element ) { - $groups_by_lcp_element_xpath[ $lcp_element->get_xpath() ][] = $group; - $lcp_elements_by_xpath[ $lcp_element->get_xpath() ][] = $lcp_element; - } else { - $group_has_unknown_lcp_element = true; - } - } + $first_group_lcp_element = $first_group->get_lcp_element(); + $last_group_lcp_element = $last_group->get_lcp_element(); + // Validate LCP elements exist and have matching XPaths in the extreme viewport groups. if ( - // All breakpoints share the same LCP element. - 1 === count( $groups_by_lcp_element_xpath ) - && - // The breakpoints don't share a common lack of a detected LCP element. - ! $group_has_unknown_lcp_element + ! $first_group_lcp_element instanceof OD_Element + || + ! $last_group_lcp_element instanceof OD_Element + || + $first_group_lcp_element->get_xpath() !== $last_group_lcp_element->get_xpath() ) { - $xpath = key( $lcp_elements_by_xpath ); + return null; // No common LCP element across the narrowest and widest viewports. + } - return $lcp_elements_by_xpath[ $xpath ][0]; + // Check intermediate viewport groups for conflicting LCP elements. + foreach ( array_slice( $this->groups, 1, -1 ) as $group ) { + $group_lcp_element = $group->get_lcp_element(); + if ( + $group_lcp_element instanceof OD_Element + && + $group_lcp_element->get_xpath() !== $first_group_lcp_element->get_xpath() + ) { + return null; // Conflicting LCP element found in an intermediate group. + } } - return null; + return $first_group_lcp_element; } )(); $this->result_cache[ __FUNCTION__ ] = $result; diff --git a/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php index 066f34f8b2..179d282c43 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metrics-group-collection.php @@ -721,45 +721,119 @@ public function test_get_groups_by_lcp_element(): void { $this->assertNull( $group_collection->get_common_lcp_element() ); } + /** + * Data provider. + * + * @return array + */ + public function data_provider_test_get_common_lcp_element(): array { + $xpath1 = '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]/*[1]'; + $xpath2 = '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]/*[2]'; + + $get_sample_url_metric = function ( int $viewport_width, string $lcp_element_xpath, bool $is_lcp = true ): OD_URL_Metric { + return $this->get_sample_url_metric( + array( + 'viewport_width' => $viewport_width, + 'element' => array( + 'isLCP' => $is_lcp, + 'xpath' => $lcp_element_xpath, + ), + ) + ); + }; + + return array( + 'all_groups_have_common_lcp' => array( + 'url_metrics' => array( + $get_sample_url_metric( 400, $xpath1 ), + $get_sample_url_metric( 600, $xpath1 ), + $get_sample_url_metric( 1000, $xpath1 ), + ), + 'expected' => $xpath1, + ), + 'no_url_metrics' => array( + 'url_metrics' => array(), + 'expected' => null, + ), + 'empty_first_group' => array( + 'url_metrics' => array( + $get_sample_url_metric( 600, $xpath1 ), + $get_sample_url_metric( 1000, $xpath1 ), + ), + 'expected' => null, + ), + 'empty_last_group' => array( + 'url_metrics' => array( + $get_sample_url_metric( 400, $xpath1 ), + $get_sample_url_metric( 600, $xpath1 ), + ), + 'expected' => null, + ), + 'first_and_last_common_lcp_others_empty' => array( + 'url_metrics' => array( + $get_sample_url_metric( 400, $xpath1 ), + $get_sample_url_metric( 1000, $xpath1 ), + ), + 'expected' => $xpath1, + ), + 'intermediate_groups_conflict' => array( + 'url_metrics' => array( + $get_sample_url_metric( 400, $xpath1 ), + $get_sample_url_metric( 600, $xpath2 ), + $get_sample_url_metric( 1000, $xpath1 ), + ), + 'expected' => null, + ), + 'first_and_last_lcp_mismatch' => array( + 'url_metrics' => array( + $get_sample_url_metric( 400, $xpath1 ), + $get_sample_url_metric( 600, $xpath1 ), + $get_sample_url_metric( 1000, $xpath2 ), + ), + 'expected' => null, + ), + 'no_lcp_metrics' => array( + 'url_metrics' => array( + $get_sample_url_metric( 400, $xpath1, false ), + $get_sample_url_metric( 600, $xpath1, false ), + $get_sample_url_metric( 1000, $xpath1, false ), + ), + 'expected' => null, + ), + ); + } + /** * Test get_common_lcp_element(). * * @covers ::get_common_lcp_element + * + * @dataProvider data_provider_test_get_common_lcp_element + * + * @param OD_URL_Metric[] $url_metrics URL Metrics. + * @param string|null $expected Expected. */ - public function test_get_common_lcp_element(): void { + public function test_get_common_lcp_element( array $url_metrics, ?string $expected ): void { $breakpoints = array( 480, 800 ); $sample_size = 3; $current_etag = md5( '' ); $group_collection = new OD_URL_Metric_Group_Collection( - array(), + $url_metrics, $current_etag, $breakpoints, $sample_size, HOUR_IN_SECONDS ); - $lcp_element_xpath = '/*[1][self::HTML]/*[2][self::BODY]/*[1][self::IMG]/*[1]'; - - foreach ( array_merge( $breakpoints, array( 1000 ) ) as $viewport_width ) { - for ( $i = 0; $i < $sample_size; $i++ ) { - $group_collection->add_url_metric( - $this->get_sample_url_metric( - array( - 'viewport_width' => $viewport_width, - 'element' => array( - 'isLCP' => true, - 'xpath' => $lcp_element_xpath, - ), - ) - ) - ); - } - } - $this->assertCount( 3, $group_collection ); + $common_lcp_element = $group_collection->get_common_lcp_element(); - $this->assertInstanceOf( OD_Element::class, $common_lcp_element ); - $this->assertSame( $lcp_element_xpath, $common_lcp_element['xpath'] ); + if ( is_string( $expected ) ) { + $this->assertInstanceOf( OD_Element::class, $common_lcp_element ); + $this->assertSame( $expected, $common_lcp_element->get_xpath() ); + } else { + $this->assertNull( $common_lcp_element ); + } } /**