From 8439753830ee0763c7b1e719d826f0c3f9f3c059 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 22 Nov 2024 11:40:27 -0800 Subject: [PATCH 01/27] Move lazy-load script func from hooks.php to helper.php --- plugins/image-prioritizer/helper.php | 19 +++++++++++++++++++ plugins/image-prioritizer/hooks.php | 19 ------------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index 9096fac63b..6b6a0ee41d 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -76,3 +76,22 @@ function image_prioritizer_register_tag_visitors( OD_Tag_Visitor_Registry $regis $video_visitor = new Image_Prioritizer_Video_Tag_Visitor(); $registry->register( 'image-prioritizer/video', $video_visitor ); } + +/** + * Gets the script to lazy-load videos. + * + * Load a video and its poster image when it approaches the viewport using an IntersectionObserver. + * + * Handles 'autoplay' and 'preload' attributes accordingly. + * + * @since 0.2.0 + */ +function image_prioritizer_get_lazy_load_script(): string { + $script = file_get_contents( __DIR__ . sprintf( '/lazy-load%s.js', wp_scripts_get_suffix() ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request. + + if ( false === $script ) { + return ''; + } + + return $script; +} diff --git a/plugins/image-prioritizer/hooks.php b/plugins/image-prioritizer/hooks.php index 9e962ce8f1..62d2fd3158 100644 --- a/plugins/image-prioritizer/hooks.php +++ b/plugins/image-prioritizer/hooks.php @@ -11,22 +11,3 @@ } add_action( 'od_init', 'image_prioritizer_init' ); - -/** - * Gets the script to lazy-load videos. - * - * Load a video and its poster image when it approaches the viewport using an IntersectionObserver. - * - * Handles 'autoplay' and 'preload' attributes accordingly. - * - * @since 0.2.0 - */ -function image_prioritizer_get_lazy_load_script(): string { - $script = file_get_contents( __DIR__ . sprintf( '/lazy-load%s.js', wp_scripts_get_suffix() ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request. - - if ( false === $script ) { - return ''; - } - - return $script; -} From cee5c243d3c65396a5a65b25f5cc19420d5c6aa9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 22 Nov 2024 14:05:44 -0800 Subject: [PATCH 02/27] Export webVitalsLibrarySrc to extensions --- plugins/optimization-detective/detect.js | 3 ++- plugins/optimization-detective/types.ts | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index e5e6cc8ab3..ba5ea4b47d 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -347,7 +347,8 @@ export default async function detect( { extensions.set( extensionModuleUrl, extension ); // TODO: There should to be a way to pass additional args into the module. Perhaps extensionModuleUrls should be a mapping of URLs to args. It's important to pass webVitalsLibrarySrc to the extension so that onLCP, onCLS, or onINP can be obtained. if ( extension.initialize instanceof Function ) { - extension.initialize( { isDebug } ); + // TODO: Should initialize be an async function like finalize is? Probably not because we do not want to wait for it. + extension.initialize( { isDebug, webVitalsLibrarySrc } ); } } catch ( err ) { error( diff --git a/plugins/optimization-detective/types.ts b/plugins/optimization-detective/types.ts index fc4e375b60..ee1f73f8e3 100644 --- a/plugins/optimization-detective/types.ts +++ b/plugins/optimization-detective/types.ts @@ -30,6 +30,7 @@ export interface URLMetricGroupStatus { export type InitializeArgs = { readonly isDebug: boolean; + readonly webVitalsLibrarySrc: string; }; export type InitializeCallback = ( args: InitializeArgs ) => void; From da12ebd9172bb0fb57bcde8955797533b3125d5b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 22 Nov 2024 14:14:13 -0800 Subject: [PATCH 03/27] Add client-side extension to Image Prioritizer to detect LCP external background images --- plugins/image-prioritizer/detect.js | 164 +++++++++++++++++++++++++++ plugins/image-prioritizer/helper.php | 47 ++++++++ plugins/image-prioritizer/hooks.php | 2 + webpack.config.js | 4 + 4 files changed, 217 insertions(+) create mode 100644 plugins/image-prioritizer/detect.js diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js new file mode 100644 index 0000000000..eb591096ae --- /dev/null +++ b/plugins/image-prioritizer/detect.js @@ -0,0 +1,164 @@ +/** + * Image Prioritizer module for Optimization Detective + * + * TODO: Description. + */ + +const consoleLogPrefix = '[Image Prioritizer]'; + +/** + * Detected LCP external background image candidates. + * + * @type {Array<{url: string, tagName: string, parentTagName: string, id: string, className: string}>} + */ +const externalBackgroundImages = []; + +/** + * @typedef {import("web-vitals").LCPMetric} LCPMetric + * @typedef {import("../optimization-detective/types.ts").InitializeCallback} InitializeCallback + * @typedef {import("../optimization-detective/types.ts").InitializeArgs} InitializeArgs + * @typedef {import("../optimization-detective/types.ts").FinalizeArgs} FinalizeArgs + * @typedef {import("../optimization-detective/types.ts").FinalizeCallback} FinalizeCallback + */ + +/** + * Logs a message. + * + * @since n.e.x.t + * + * @param {...*} message + */ +function log( ...message ) { + // eslint-disable-next-line no-console + console.log( consoleLogPrefix, ...message ); +} + +/** + * Initializes extension. + * + * @since n.e.x.t + * + * @type {InitializeCallback} + * @param {InitializeArgs} args Args. + */ +export function initialize( { isDebug, webVitalsLibrarySrc } ) { + import( webVitalsLibrarySrc ).then( ( { onLCP } ) => { + onLCP( + ( /** @type {LCPMetric} */ metric ) => { + handleLCPMetric( metric, isDebug ); + }, + { + // This avoids needing to click to finalize LCP candidate. While this is helpful for testing, it also + // ensures that we always get an LCP candidate reported. Otherwise, the callback may never fire if the + // user never does a click or keydown, per . + reportAllChanges: true, + } + ); + } ); +} + +/** + * Gets the performance resource entry for a given URL. + * + * @since n.e.x.t + * + * @param {string} url - Resource URL. + * @return {PerformanceResourceTiming|null} Resource entry or null. + */ +function getPerformanceResourceByURL( url ) { + const entries = + /** @type PerformanceResourceTiming[] */ performance.getEntriesByType( + 'resource' + ); + for ( const entry of entries ) { + if ( entry.name === url ) { + return entry; + } + } + return null; +} + +/** + * Handles a new LCP metric being reported. + * + * @since n.e.x.t + * + * @param {LCPMetric} metric - LCP Metric. + * @param {boolean} isDebug - Whether in debug mode. + */ +function handleLCPMetric( metric, isDebug ) { + for ( const entry of metric.entries ) { + // Look only for LCP entries that have a URL and a corresponding element which is not an IMG or VIDEO. + if ( + ! entry.url || + ! ( entry.element instanceof HTMLElement ) || + entry.element instanceof HTMLImageElement || + entry.element instanceof HTMLVideoElement + ) { + continue; + } + + // Always ignore data: URLs. + if ( entry.url.startsWith( 'data:' ) ) { + continue; + } + + // Skip elements that have the background image defined inline. + // These are handled by Image_Prioritizer_Background_Image_Styled_Tag_Visitor. + if ( entry.element.style.backgroundImage ) { + continue; + } + + // Now only consider proceeding with the URL if its loading was initiated with CSS. + const resourceEntry = getPerformanceResourceByURL( entry.url ); + if ( ! resourceEntry || resourceEntry.initiatorType !== 'css' ) { + return; + } + + // The id and className allow the tag visitor to detect whether the element is still in the document. + // This is used instead of having a full XPath which is likely not available since the tag visitor would not + // know to return true for this element since it has no awareness of which elements have external backgrounds. + const externalBackgroundImage = { + url: entry.url, + tagName: entry.element.tagName, + parentTagName: entry.element.parentElement.tagName, + id: entry.id, + className: entry.element.className, + }; + + if ( isDebug ) { + log( + 'Detected external LCP background image:', + externalBackgroundImage + ); + } + + externalBackgroundImages.push( externalBackgroundImage ); + } +} + +/** + * Finalizes extension. + * + * @since n.e.x.t + * + * @type {FinalizeCallback} + * @param {FinalizeArgs} args Args. + */ +export async function finalize( { extendRootData, isDebug } ) { + if ( externalBackgroundImages.length === 0 ) { + return; + } + + // Get the last detected external background image which is going to be for the LCP element (or very likely will be). + const lcpElementExternalBackgroundImage = externalBackgroundImages.pop(); + + if ( isDebug ) { + log( + 'Sending external background image for LCP element:', + lcpElementExternalBackgroundImage + ); + } + + extendRootData( { lcpElementExternalBackgroundImage } ); +} diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index 6b6a0ee41d..4c43c24841 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -95,3 +95,50 @@ function image_prioritizer_get_lazy_load_script(): string { return $script; } + +/** + * Filters the list of Optimization Detective extension module URLs to include the extension for Image Prioritizer. + * + * @since n.e.x.t + * + * @param string[]|mixed $extension_module_urls Extension module URLs. + * @return string[] Extension module URLs. + */ +function image_prioritizer_filter_extension_module_urls( $extension_module_urls ): array { + if ( ! is_array( $extension_module_urls ) ) { + $extension_module_urls = array(); + } + $extension_module_urls[] = add_query_arg( 'ver', IMAGE_PRIORITIZER_VERSION, plugin_dir_url( __FILE__ ) . sprintf( 'detect%s.js', wp_scripts_get_suffix() ) ); + return $extension_module_urls; +} + +/** + * Filters additional properties for the element item schema for Optimization Detective. + * + * @since n.e.x.t + * + * @param array $additional_properties Additional properties. + * @return array Additional properties. + */ +function image_prioritizer_add_element_item_schema_properties( array $additional_properties ): array { + // TODO: Validation of the URL. + $additional_properties['lcpElementExternalBackgroundImage'] = array( + 'type' => 'object', + 'properties' => array_fill_keys( + array( + 'url', + 'tagName', + 'parentTagName', + 'id', + 'className', + ), + array( + // TODO: Add constraints on length. + // TODO: Add constraints on formats and patterns. + 'type' => 'string', + 'required' => true, + ) + ), + ); + return $additional_properties; +} diff --git a/plugins/image-prioritizer/hooks.php b/plugins/image-prioritizer/hooks.php index 62d2fd3158..7587e9e67b 100644 --- a/plugins/image-prioritizer/hooks.php +++ b/plugins/image-prioritizer/hooks.php @@ -11,3 +11,5 @@ } add_action( 'od_init', 'image_prioritizer_init' ); +add_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' ); +add_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' ); diff --git a/webpack.config.js b/webpack.config.js index 409f70c37b..38572a5be8 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -97,6 +97,10 @@ const imagePrioritizer = ( env ) => { plugins: [ new CopyWebpackPlugin( { patterns: [ + { + from: `${ pluginDir }/detect.js`, + to: `${ pluginDir }/detect.min.js`, + }, { from: `${ pluginDir }/lazy-load.js`, to: `${ pluginDir }/lazy-load.min.js`, From cf4b6e924b037d5c6bf0704848eb361e2fb9341c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 22 Nov 2024 18:05:52 -0800 Subject: [PATCH 04/27] Add preload links for external background images logged in URL Metrics --- ...er-background-image-styled-tag-visitor.php | 132 ++++++++++++++++-- plugins/image-prioritizer/detect.js | 41 +++++- plugins/image-prioritizer/helper.php | 35 +++-- 3 files changed, 177 insertions(+), 31 deletions(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php index 798d42f1f6..c807ddc80c 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php @@ -14,11 +14,25 @@ /** * Tag visitor that optimizes elements with background-image styles. * + * @phpstan-type LcpElementExternalBackgroundImage array{ + * url: non-empty-string, + * tag: non-empty-string, + * id: string|null, + * class: string|null, + * } + * * @since 0.1.0 * @access private */ final class Image_Prioritizer_Background_Image_Styled_Tag_Visitor extends Image_Prioritizer_Tag_Visitor { + /** + * Tuples of URL Metric group and the common LCP element external background image. + * + * @var array + */ + private $group_common_lcp_element_external_background_images = array(); + /** * Visits a tag. * @@ -49,6 +63,7 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { } if ( is_null( $background_image_url ) ) { + $this->maybe_preload_external_lcp_background_image( $context ); return false; } @@ -56,21 +71,114 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { // If this element is the LCP (for a breakpoint group), add a preload link for it. foreach ( $context->url_metric_group_collection->get_groups_by_lcp_element( $xpath ) as $group ) { - $link_attributes = array( + $this->add_preload_link( $context->link_collection, $group, $background_image_url ); + } + + return true; + } + + /** + * Gets the common LCP element external background image for a URL Metric group. + * + * @since n.e.x.t + * + * @param OD_URL_Metric_Group $group Group. + * @return LcpElementExternalBackgroundImage|null + */ + private function get_common_lcp_element_external_background_image( OD_URL_Metric_Group $group ): ?array { + + // If the group is not fully populated, we don't have enough URL Metrics to reliably know whether the background image is consistent across page loads. + // This is intentionally not using $group->is_complete() because we still will use stale URL Metrics in the calculation. + // TODO: There should be a $group->get_sample_size() method. + if ( $group->count() !== od_get_url_metrics_breakpoint_sample_size() ) { + return null; + } + + $previous_lcp_element_external_background_image = null; + foreach ( $group as $url_metric ) { + /** + * Stored data. + * + * @var LcpElementExternalBackgroundImage|null $lcp_element_external_background_image + */ + $lcp_element_external_background_image = $url_metric->get( 'lcpElementExternalBackgroundImage' ); + if ( ! is_array( $lcp_element_external_background_image ) ) { + return null; + } + if ( null !== $previous_lcp_element_external_background_image && $previous_lcp_element_external_background_image !== $lcp_element_external_background_image ) { + return null; + } + $previous_lcp_element_external_background_image = $lcp_element_external_background_image; + } + + return $previous_lcp_element_external_background_image; + } + + /** + * Maybe preloads external background image. + * + * @since n.e.x.t + * + * @param OD_Tag_Visitor_Context $context Context. + */ + private function maybe_preload_external_lcp_background_image( OD_Tag_Visitor_Context $context ): void { + static $did_collect_data = false; + if ( false === $did_collect_data ) { + foreach ( $context->url_metric_group_collection as $group ) { + $common = $this->get_common_lcp_element_external_background_image( $group ); + if ( is_array( $common ) ) { + $this->group_common_lcp_element_external_background_images[] = array( $group, $common ); + } + } + $did_collect_data = true; + } + + // There are no common LCP background images, so abort. + if ( count( $this->group_common_lcp_element_external_background_images ) === 0 ) { + return; + } + + $processor = $context->processor; + $tag_name = strtoupper( (string) $processor->get_tag() ); + foreach ( $this->group_common_lcp_element_external_background_images as $i => list( $group, $common ) ) { + if ( + // Note that the browser may send a lower-case tag name in the case of XHTML or embedded SVG/MathML, but + // the HTML Tag Processor is currently normalizing to all upper-case. The HTML Processor on the other + // hand may return the expected case. + strtoupper( $common['tag'] ) === $tag_name + && + $processor->get_attribute( 'id' ) === $common['id'] // May be checking equality with null. + && + $processor->get_attribute( 'class' ) === $common['class'] // May be checking equality with null. + ) { + $this->add_preload_link( $context->link_collection, $group, $common['url'] ); + + // Now that the preload link has been added, eliminate the entry to stop looking for it while iterating over the rest of the document. + unset( $this->group_common_lcp_element_external_background_images[ $i ] ); + } + } + } + + /** + * Adds an image preload link for the group. + * + * @since n.e.x.t + * + * @param OD_Link_Collection $link_collection Link collection. + * @param OD_URL_Metric_Group $group URL Metric group. + * @param non-empty-string $url Image URL. + */ + private function add_preload_link( OD_Link_Collection $link_collection, OD_URL_Metric_Group $group, string $url ): void { + $link_collection->add_link( + array( 'rel' => 'preload', 'fetchpriority' => 'high', 'as' => 'image', - 'href' => $background_image_url, + 'href' => $url, 'media' => 'screen', - ); - - $context->link_collection->add_link( - $link_attributes, - $group->get_minimum_viewport_width(), - $group->get_maximum_viewport_width() - ); - } - - return true; + ), + $group->get_minimum_viewport_width(), + $group->get_maximum_viewport_width() + ); } } diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js index eb591096ae..73f61fd69a 100644 --- a/plugins/image-prioritizer/detect.js +++ b/plugins/image-prioritizer/detect.js @@ -9,7 +9,7 @@ const consoleLogPrefix = '[Image Prioritizer]'; /** * Detected LCP external background image candidates. * - * @type {Array<{url: string, tagName: string, parentTagName: string, id: string, className: string}>} + * @type {Array<{url: string, tag: string, id: string, class: string}>} */ const externalBackgroundImages = []; @@ -111,19 +111,48 @@ function handleLCPMetric( metric, isDebug ) { // Now only consider proceeding with the URL if its loading was initiated with CSS. const resourceEntry = getPerformanceResourceByURL( entry.url ); - if ( ! resourceEntry || resourceEntry.initiatorType !== 'css' ) { + if ( + ! resourceEntry || + ! [ 'css', 'link' ].includes( resourceEntry.initiatorType ) // TODO: When is it css and when is it link? + ) { + if ( isDebug ) { + // eslint-disable-next-line no-console + console.warn( + consoleLogPrefix, + 'Skipped considering URL do due to resource initiatorType:', + entry.url, + resourceEntry.initiatorType + ); + } + return; + } + + // Skip URLs that are excessively long. This is the maxLength defined in image_prioritizer_add_element_item_schema_properties(). + if ( entry.url.length > 500 ) { + if ( isDebug ) { + log( `Skipping very long URL: ${ entry.url }` ); + } return; } + // Note that getAttribute() is used instead of properties so that null can be returned in case of an absent attribute. + let id = entry.element.getAttribute( 'id' ); + if ( null !== id ) { + id = id.substring( 0, 100 ); // This is the maxLength defined in image_prioritizer_add_element_item_schema_properties(). + } + let className = entry.element.getAttribute( 'class' ); + if ( null !== className ) { + className = className.substring( 0, 500 ); // This is the maxLength defined in image_prioritizer_add_element_item_schema_properties(). + } + // The id and className allow the tag visitor to detect whether the element is still in the document. // This is used instead of having a full XPath which is likely not available since the tag visitor would not // know to return true for this element since it has no awareness of which elements have external backgrounds. const externalBackgroundImage = { url: entry.url, - tagName: entry.element.tagName, - parentTagName: entry.element.parentElement.tagName, - id: entry.id, - className: entry.element.className, + tag: entry.element.tagName, + id, + class: className, }; if ( isDebug ) { diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index 4c43c24841..74fb9a3b06 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -124,20 +124,29 @@ function image_prioritizer_add_element_item_schema_properties( array $additional // TODO: Validation of the URL. $additional_properties['lcpElementExternalBackgroundImage'] = array( 'type' => 'object', - 'properties' => array_fill_keys( - array( - 'url', - 'tagName', - 'parentTagName', - 'id', - 'className', + 'properties' => array( + 'url' => array( + 'type' => 'string', + 'format' => 'uri', + 'required' => true, + 'maxLength' => 500, // Image URLs can be quite long. + ), + 'tag' => array( + 'type' => 'string', + 'required' => true, + 'minLength' => 1, + 'pattern' => '^[a-zA-Z0-9\-]+$', // Technically emoji can be allowed in a custom element's tag name, but this is not supported here. + ), + 'id' => array( + 'type' => array( 'string', 'null' ), + 'maxLength' => 100, // A reasonable upper-bound length for a long ID. The client will must truncate anything longer. + 'required' => true, + ), + 'class' => array( + 'type' => array( 'string', 'null' ), + 'maxLength' => 500, // There can be a ton of class names on an element. The client will must truncate anything longer. + 'required' => true, ), - array( - // TODO: Add constraints on length. - // TODO: Add constraints on formats and patterns. - 'type' => 'string', - 'required' => true, - ) ), ); return $additional_properties; From a75b94f803ff51b0eacf0122aa011d04b76db1d1 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 22 Nov 2024 18:13:20 -0800 Subject: [PATCH 05/27] Prevent submitting URL Metric if viewport size changed --- plugins/optimization-detective/detect.js | 25 ++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index ba5ea4b47d..f7382a8c7e 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -127,6 +127,12 @@ function recursiveFreeze( obj ) { Object.freeze( obj ); } +// This needs to be captured early in case the user later resizes the window. +const viewport = { + width: win.innerWidth, + height: win.innerHeight, +}; + /** * URL Metric being assembled for submission. * @@ -442,10 +448,7 @@ export default async function detect( { urlMetric = { url: currentUrl, - viewport: { - width: win.innerWidth, - height: win.innerHeight, - }, + viewport, elements: [], }; @@ -506,6 +509,20 @@ export default async function detect( { ); } ); + // Only proceed with submitting the URL Metric if viewport stayed the same size. Changing the viewport size (e.g. due + // to resizing a window or changing the orientation of a device) will result in unexpected metrics being collected. + if ( + window.innerWidth !== urlMetric.viewport.width || + window.innerHeight !== urlMetric.viewport.height + ) { + if ( isDebug ) { + log( + 'Aborting URL Metric collection due to viewport size change.' + ); + } + return; + } + if ( extensions.size > 0 ) { for ( const [ extensionModuleUrl, From f2959cd60d32c9c5bfddd6d5e0575098c27f8af9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 24 Nov 2024 20:29:41 -0800 Subject: [PATCH 06/27] Introduce od_store_url_metric_validity filter so Image Prioritizer can validate background-image URL --- plugins/image-prioritizer/helper.php | 63 ++++++++++++++++++- plugins/image-prioritizer/hooks.php | 1 + .../image-prioritizer/tests/test-helper.php | 2 +- .../storage/rest-api.php | 32 +++++++++- 4 files changed, 94 insertions(+), 4 deletions(-) diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index 74fb9a3b06..dccbf82760 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -18,7 +18,7 @@ * @param string $optimization_detective_version Current version of the optimization detective plugin. */ function image_prioritizer_init( string $optimization_detective_version ): void { - $required_od_version = '0.7.0'; + $required_od_version = '0.9.0'; if ( ! version_compare( (string) strtok( $optimization_detective_version, '-' ), $required_od_version, '>=' ) ) { add_action( 'admin_notices', @@ -121,7 +121,6 @@ function image_prioritizer_filter_extension_module_urls( $extension_module_urls * @return array Additional properties. */ function image_prioritizer_add_element_item_schema_properties( array $additional_properties ): array { - // TODO: Validation of the URL. $additional_properties['lcpElementExternalBackgroundImage'] = array( 'type' => 'object', 'properties' => array( @@ -151,3 +150,63 @@ function image_prioritizer_add_element_item_schema_properties( array $additional ); return $additional_properties; } + +/** + * Validates that the provided background image URL is valid. + * + * @since n.e.x.t + * + * @param bool|WP_Error|mixed $validity Validity. Valid if true or a WP_Error without any errors, or invalid otherwise. + * @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema. + * @return bool|WP_Error Validity. Valid if true or a WP_Error without any errors, or invalid otherwise. + */ +function image_prioritizer_filter_store_url_metric_validity( $validity, OD_Strict_URL_Metric $url_metric ) { + if ( ! is_bool( $validity ) && ! ( $validity instanceof WP_Error ) ) { + $validity = (bool) $validity; + } + + $data = $url_metric->get( 'lcpElementExternalBackgroundImage' ); + if ( ! is_array( $data ) ) { + return $validity; + } + + $r = wp_safe_remote_head( + $data['url'], + array( + 'redirection' => 3, // Allow up to 3 redirects. + ) + ); + if ( $r instanceof WP_Error ) { + return new WP_Error( + WP_DEBUG ? $r->get_error_code() : 'head_request_failure', + __( 'HEAD request for background image URL failed.', 'image-prioritizer' ) . ( WP_DEBUG ? ' ' . $r->get_error_message() : '' ), + array( + 'code' => 500, + ) + ); + } + $response_code = wp_remote_retrieve_response_code( $r ); + if ( $response_code < 200 || $response_code >= 400 ) { + return new WP_Error( + 'background_image_response_not_ok', + __( 'HEAD request for background image URL did not return with a success status code.', 'image-prioritizer' ), + array( + 'code' => WP_DEBUG ? $response_code : 400, + ) + ); + } + + $content_type = wp_remote_retrieve_header( $r, 'Content-Type' ); + if ( ! is_string( $content_type ) || ! str_starts_with( $content_type, 'image/' ) ) { + return new WP_Error( + 'background_image_response_not_image', + __( 'HEAD request for background image URL did not return an image Content-Type.', 'image-prioritizer' ), + array( + 'code' => 400, + ) + ); + } + + // TODO: Check for the Content-Length and return invalid if it is gigantic? + return $validity; +} diff --git a/plugins/image-prioritizer/hooks.php b/plugins/image-prioritizer/hooks.php index 7587e9e67b..4a47f35647 100644 --- a/plugins/image-prioritizer/hooks.php +++ b/plugins/image-prioritizer/hooks.php @@ -13,3 +13,4 @@ add_action( 'od_init', 'image_prioritizer_init' ); add_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' ); add_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' ); +add_filter( 'od_store_url_metric_validity', 'image_prioritizer_filter_store_url_metric_validity', 10, 2 ); diff --git a/plugins/image-prioritizer/tests/test-helper.php b/plugins/image-prioritizer/tests/test-helper.php index ac67ac867e..867c70b0e5 100644 --- a/plugins/image-prioritizer/tests/test-helper.php +++ b/plugins/image-prioritizer/tests/test-helper.php @@ -20,7 +20,7 @@ public function data_provider_to_test_image_prioritizer_init(): array { 'expected' => false, ), 'with_new_version' => array( - 'version' => '0.7.0', + 'version' => '99.0.0', 'expected' => true, ), ); diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index fe622be468..7d8be56df0 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -84,7 +84,7 @@ function od_register_endpoint(): void { return new WP_Error( 'url_metric_storage_locked', __( 'URL Metric storage is presently locked for the current IP.', 'optimization-detective' ), - array( 'status' => 403 ) + array( 'status' => 403 ) // TODO: Consider 423 Locked status code. ); } return true; @@ -152,6 +152,7 @@ function od_handle_rest_request( WP_REST_Request $request ) { $request->get_param( 'viewport' )['width'] ); } catch ( InvalidArgumentException $exception ) { + // Note: This should never happen because an exception only occurs if a viewport width is less than zero, and the JSON Schema enforces that the viewport.width have a minimum of zero. return new WP_Error( 'invalid_viewport_width', $exception->getMessage() ); } if ( $url_metric_group->is_complete() ) { @@ -197,6 +198,35 @@ function od_handle_rest_request( WP_REST_Request $request ) { ); } + /** + * Filters whether a URL Metric is valid for storage. + * + * This allows for custom validation constraints to be applied beyond what can be expressed in JSON Schema. This is + * also necessary because the 'validate_callback' key in a JSON Schema is not respected when gathering the REST API + * endpoint args via the {@see rest_get_endpoint_args_for_schema()} function. Besides this, the REST API doesn't + * support 'validate_callback' for any nested arguments in any case, meaning that custom constraints would be able + * to be applied to multidimensional objects, such as the items inside 'elements'. + * + * This filter only applies when storing a URL Metric via the REST API. It does not run when a stored URL Metric + * loaded from the od_url_metric post type. This means that validation logic enforced via this filter can be more + * expensive, such as doing filesystem checks or HTTP requests. + * + * @since n.e.x.t + * + * @param bool|WP_Error $validity Validity. Valid if true or a WP_Error without any errors, or invalid otherwise. + * @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema. + */ + $validity = apply_filters( 'od_store_url_metric_validity', true, $url_metric ); + if ( false === $validity || ( $validity instanceof WP_Error && $validity->has_errors() ) ) { + if ( false === $validity ) { + $validity = new WP_Error( 'invalid_url_metric', __( 'Validity of URL Metric was rejected by filter.', 'optimization-detective' ) ); + } + if ( ! isset( $validity->error_data['code'] ) ) { + $validity->error_data['code'] = 400; + } + return $validity; + } + // TODO: This should be changed from store_url_metric($slug, $url_metric) instead be update_post( $slug, $group_collection ). As it stands, store_url_metric() is duplicating logic here. $result = OD_URL_Metrics_Post_Type::store_url_metric( $request->get_param( 'slug' ), From f4b766aba66257028e8d0788436909d3116a0b82 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 24 Nov 2024 20:42:50 -0800 Subject: [PATCH 07/27] Implement get_sample_size() and get_freshness_ttl() on OD_URL_Metric_Group --- ...er-background-image-styled-tag-visitor.php | 3 +-- .../class-od-url-metric-group.php | 23 +++++++++++++++++++ .../tests/test-class-od-url-metrics-group.php | 4 ++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php index c807ddc80c..3a901a1f7c 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php @@ -89,8 +89,7 @@ private function get_common_lcp_element_external_background_image( OD_URL_Metric // If the group is not fully populated, we don't have enough URL Metrics to reliably know whether the background image is consistent across page loads. // This is intentionally not using $group->is_complete() because we still will use stale URL Metrics in the calculation. - // TODO: There should be a $group->get_sample_size() method. - if ( $group->count() !== od_get_url_metrics_breakpoint_sample_size() ) { + if ( $group->count() !== $group->get_sample_size() ) { return null; } diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 963f5b8840..94ab910309 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -163,6 +163,29 @@ public function get_maximum_viewport_width(): int { return $this->maximum_viewport_width; } + /** + * Gets the sample size for URL Metrics for a given breakpoint. + * + * @todo Eliminate in favor of readonly public property. + * @phpstan-return positive-int + * @return int Sample size. + */ + public function get_sample_size(): int { + return $this->sample_size; + } + + /** + * Gets the freshness age (TTL) for a given URL Metric. + * + * @todo Eliminate in favor of readonly public property. + * + * @phpstan-return 0|positive-int + * @return int Freshness age. + */ + public function get_freshness_ttl(): int { + return $this->freshness_ttl; + } + /** * Checks whether the provided viewport width is within the minimum/maximum range for * diff --git a/plugins/optimization-detective/tests/test-class-od-url-metrics-group.php b/plugins/optimization-detective/tests/test-class-od-url-metrics-group.php index ef59513515..0afc78b485 100644 --- a/plugins/optimization-detective/tests/test-class-od-url-metrics-group.php +++ b/plugins/optimization-detective/tests/test-class-od-url-metrics-group.php @@ -95,6 +95,8 @@ public function data_provider_test_construction(): array { * @covers ::__construct * @covers ::get_minimum_viewport_width * @covers ::get_maximum_viewport_width + * @covers ::get_sample_size + * @covers ::get_freshness_ttl * @covers ::getIterator * @covers ::count * @@ -116,6 +118,8 @@ public function test_construction( array $url_metrics, int $minimum_viewport_wid $this->assertCount( count( $url_metrics ), $group ); $this->assertSame( $minimum_viewport_width, $group->get_minimum_viewport_width() ); $this->assertSame( $maximum_viewport_width, $group->get_maximum_viewport_width() ); + $this->assertSame( $sample_size, $group->get_sample_size() ); + $this->assertSame( $freshness_ttl, $group->get_freshness_ttl() ); $this->assertCount( count( $url_metrics ), $group ); $this->assertSame( $url_metrics, iterator_to_array( $group ) ); } From dcb7a5cee3712282c81d93ede00980d5241107e6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 24 Nov 2024 20:46:09 -0800 Subject: [PATCH 08/27] Eliminate use of static variable --- ...ge-prioritizer-background-image-styled-tag-visitor.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php index 3a901a1f7c..f59f04f9d1 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php @@ -31,7 +31,7 @@ final class Image_Prioritizer_Background_Image_Styled_Tag_Visitor extends Image_ * * @var array */ - private $group_common_lcp_element_external_background_images = array(); + private $group_common_lcp_element_external_background_images; /** * Visits a tag. @@ -121,15 +121,15 @@ private function get_common_lcp_element_external_background_image( OD_URL_Metric * @param OD_Tag_Visitor_Context $context Context. */ private function maybe_preload_external_lcp_background_image( OD_Tag_Visitor_Context $context ): void { - static $did_collect_data = false; - if ( false === $did_collect_data ) { + // Gather the tuples of URL Metric group and the common LCP element external background image. + if ( ! is_array( $this->group_common_lcp_element_external_background_images ) ) { + $this->group_common_lcp_element_external_background_images = array(); foreach ( $context->url_metric_group_collection as $group ) { $common = $this->get_common_lcp_element_external_background_image( $group ); if ( is_array( $common ) ) { $this->group_common_lcp_element_external_background_images[] = array( $group, $common ); } } - $did_collect_data = true; } // There are no common LCP background images, so abort. From f078723c8d2068f5bb94ed24b98ac14c9db95efc Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 24 Nov 2024 20:54:05 -0800 Subject: [PATCH 09/27] Write up description, add warn helper function, and remove todo --- plugins/image-prioritizer/detect.js | 32 +++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js index 73f61fd69a..7cf60ba829 100644 --- a/plugins/image-prioritizer/detect.js +++ b/plugins/image-prioritizer/detect.js @@ -1,7 +1,12 @@ /** * Image Prioritizer module for Optimization Detective * - * TODO: Description. + * This extension to Optimization Detective captures the LCP element's CSS background image which is not defined with + * an inline style attribute but rather in either an external stylesheet loaded with a LINK tag or by stylesheet in + * a STYLE element. The URL for this LCP background image and the tag's name, ID, and class are all amended to the + * stored URL Metric so that a responsive preload link with fetchpriority=high will be added for that background image + * once a URL Metric group is fully populated with URL Metrics that all agree on that being the LCP image, and if the + * document has a tag with the same name, ID, and class. */ const consoleLogPrefix = '[Image Prioritizer]'; @@ -33,6 +38,18 @@ function log( ...message ) { console.log( consoleLogPrefix, ...message ); } +/** + * Logs a warning. + * + * @since n.e.x.t + * + * @param {...*} message + */ +function warn( ...message ) { + // eslint-disable-next-line no-console + console.warn( consoleLogPrefix, ...message ); +} + /** * Initializes extension. * @@ -109,19 +126,16 @@ function handleLCPMetric( metric, isDebug ) { continue; } - // Now only consider proceeding with the URL if its loading was initiated with CSS. + // Now only consider proceeding with the URL if its loading was initiated with stylesheet or preload link. const resourceEntry = getPerformanceResourceByURL( entry.url ); if ( ! resourceEntry || - ! [ 'css', 'link' ].includes( resourceEntry.initiatorType ) // TODO: When is it css and when is it link? + ! [ 'css', 'link' ].includes( resourceEntry.initiatorType ) ) { if ( isDebug ) { - // eslint-disable-next-line no-console - console.warn( - consoleLogPrefix, - 'Skipped considering URL do due to resource initiatorType:', - entry.url, - resourceEntry.initiatorType + warn( + `Skipped considering URL (${ entry.url }) due to unexpected performance resource timing entry:`, + resourceEntry ); } return; From d3339f0ac58d0e57f0c1ee2988fdae2628c2873b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 24 Nov 2024 21:39:14 -0800 Subject: [PATCH 10/27] Let extension initialize functions always be async --- plugins/embed-optimizer/detect.js | 2 +- plugins/image-prioritizer/detect.js | 27 ++++++++++----------- plugins/optimization-detective/detect.js | 30 +++++++++++++++++++++--- plugins/optimization-detective/types.ts | 2 +- 4 files changed, 42 insertions(+), 19 deletions(-) diff --git a/plugins/embed-optimizer/detect.js b/plugins/embed-optimizer/detect.js index 1aab3d0838..38068387b0 100644 --- a/plugins/embed-optimizer/detect.js +++ b/plugins/embed-optimizer/detect.js @@ -51,7 +51,7 @@ const loadedElementContentRects = new Map(); * @type {InitializeCallback} * @param {InitializeArgs} args Args. */ -export function initialize( { isDebug } ) { +export async function initialize( { isDebug } ) { /** @type NodeListOf */ const embedWrappers = document.querySelectorAll( '.wp-block-embed > .wp-block-embed__wrapper[data-od-xpath]' diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js index 7cf60ba829..8f548fd8e5 100644 --- a/plugins/image-prioritizer/detect.js +++ b/plugins/image-prioritizer/detect.js @@ -58,20 +58,19 @@ function warn( ...message ) { * @type {InitializeCallback} * @param {InitializeArgs} args Args. */ -export function initialize( { isDebug, webVitalsLibrarySrc } ) { - import( webVitalsLibrarySrc ).then( ( { onLCP } ) => { - onLCP( - ( /** @type {LCPMetric} */ metric ) => { - handleLCPMetric( metric, isDebug ); - }, - { - // This avoids needing to click to finalize LCP candidate. While this is helpful for testing, it also - // ensures that we always get an LCP candidate reported. Otherwise, the callback may never fire if the - // user never does a click or keydown, per . - reportAllChanges: true, - } - ); - } ); +export async function initialize( { isDebug, webVitalsLibrarySrc } ) { + const { onLCP } = await import( webVitalsLibrarySrc ); + onLCP( + ( /** @type {LCPMetric} */ metric ) => { + handleLCPMetric( metric, isDebug ); + }, + { + // This avoids needing to click to finalize LCP candidate. While this is helpful for testing, it also + // ensures that we always get an LCP candidate reported. Otherwise, the callback may never fire if the + // user never does a click or keydown, per . + reportAllChanges: true, + } + ); } /** diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index f7382a8c7e..d21af2742a 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -346,15 +346,23 @@ export default async function detect( { /** @type {Map} */ const extensions = new Map(); + + /** @type {Promise[]} */ + const extensionInitializePromises = []; + for ( const extensionModuleUrl of extensionModuleUrls ) { try { /** @type {Extension} */ const extension = await import( extensionModuleUrl ); extensions.set( extensionModuleUrl, extension ); - // TODO: There should to be a way to pass additional args into the module. Perhaps extensionModuleUrls should be a mapping of URLs to args. It's important to pass webVitalsLibrarySrc to the extension so that onLCP, onCLS, or onINP can be obtained. + // TODO: There should to be a way to pass additional args into the module. Perhaps extensionModuleUrls should be a mapping of URLs to args. if ( extension.initialize instanceof Function ) { - // TODO: Should initialize be an async function like finalize is? Probably not because we do not want to wait for it. - extension.initialize( { isDebug, webVitalsLibrarySrc } ); + extensionInitializePromises.push( + extension.initialize( { + isDebug, + webVitalsLibrarySrc, + } ) + ); } } catch ( err ) { error( @@ -364,6 +372,22 @@ export default async function detect( { } } + // Wait for all extensions to finish initializing. + const settledInitializePromises = await Promise.allSettled( + extensionInitializePromises + ); + for ( const [ + i, + settledInitializePromise, + ] of settledInitializePromises.entries() ) { + if ( settledInitializePromise.status === 'rejected' ) { + error( + `Failed to initialize extension '${ extensionModuleUrls[ i ] }':`, + settledInitializePromise.reason + ); + } + } + const breadcrumbedElements = doc.body.querySelectorAll( '[data-od-xpath]' ); /** @type {Map} */ diff --git a/plugins/optimization-detective/types.ts b/plugins/optimization-detective/types.ts index ee1f73f8e3..cafd6f9d3a 100644 --- a/plugins/optimization-detective/types.ts +++ b/plugins/optimization-detective/types.ts @@ -33,7 +33,7 @@ export type InitializeArgs = { readonly webVitalsLibrarySrc: string; }; -export type InitializeCallback = ( args: InitializeArgs ) => void; +export type InitializeCallback = ( args: InitializeArgs ) => Promise< void >; export type FinalizeArgs = { readonly getRootData: () => URLMetric; From ecac93772467e960d6b97ad5b4795d8057e6a771 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 24 Nov 2024 22:06:25 -0800 Subject: [PATCH 11/27] Improve logic for initializing and finalizing extensions --- plugins/optimization-detective/detect.js | 49 +++++++++++++++++++----- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index d21af2742a..53feba11df 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -350,6 +350,9 @@ export default async function detect( { /** @type {Promise[]} */ const extensionInitializePromises = []; + /** @type {string[]} */ + const initializingExtensionModuleUrls = []; + for ( const extensionModuleUrl of extensionModuleUrls ) { try { /** @type {Extension} */ @@ -363,10 +366,11 @@ export default async function detect( { webVitalsLibrarySrc, } ) ); + initializingExtensionModuleUrls.push( extensionModuleUrl ); } } catch ( err ) { error( - `Failed to initialize extension '${ extensionModuleUrl }':`, + `Failed to start initializing extension '${ extensionModuleUrl }':`, err ); } @@ -382,7 +386,7 @@ export default async function detect( { ] of settledInitializePromises.entries() ) { if ( settledInitializePromise.status === 'rejected' ) { error( - `Failed to initialize extension '${ extensionModuleUrls[ i ] }':`, + `Failed to initialize extension '${ initializingExtensionModuleUrls[ i ] }':`, settledInitializePromise.reason ); } @@ -548,27 +552,52 @@ export default async function detect( { } if ( extensions.size > 0 ) { + /** @type {Promise[]} */ + const extensionFinalizePromises = []; + + /** @type {string[]} */ + const finalizingExtensionModuleUrls = []; + for ( const [ extensionModuleUrl, extension, ] of extensions.entries() ) { if ( extension.finalize instanceof Function ) { try { - await extension.finalize( { - isDebug, - getRootData, - getElementData, - extendElementData, - extendRootData, - } ); + extensionFinalizePromises.push( + extension.finalize( { + isDebug, + getRootData, + getElementData, + extendElementData, + extendRootData, + } ) + ); + finalizingExtensionModuleUrls.push( extensionModuleUrl ); } catch ( err ) { error( - `Unable to finalize module '${ extensionModuleUrl }':`, + `Unable to start finalizing extension '${ extensionModuleUrl }':`, err ); } } } + + // Wait for all extensions to finish finalizing. + const settledFinalizePromises = await Promise.allSettled( + extensionFinalizePromises + ); + for ( const [ + i, + settledFinalizePromise, + ] of settledFinalizePromises.entries() ) { + if ( settledFinalizePromise.status === 'rejected' ) { + error( + `Failed to finalize extension '${ finalizingExtensionModuleUrls[ i ] }':`, + settledFinalizePromise.reason + ); + } + } } // Even though the server may reject the REST API request, we still have to set the storage lock From 494e4aa2b2fb9610da587466c5745b834f902fb2 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 24 Nov 2024 22:14:51 -0800 Subject: [PATCH 12/27] Add missing since tags --- .../class-od-url-metric-group.php | 37 ++++++++++++++++++- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/plugins/optimization-detective/class-od-url-metric-group.php b/plugins/optimization-detective/class-od-url-metric-group.php index 94ab910309..eb1f29db3d 100644 --- a/plugins/optimization-detective/class-od-url-metric-group.php +++ b/plugins/optimization-detective/class-od-url-metric-group.php @@ -24,6 +24,8 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer /** * URL Metrics. * + * @since 0.1.0 + * * @var OD_URL_Metric[] */ private $url_metrics; @@ -31,6 +33,8 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer /** * Minimum possible viewport width for the group (inclusive). * + * @since 0.1.0 + * * @var int * @phpstan-var 0|positive-int */ @@ -39,6 +43,8 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer /** * Maximum possible viewport width for the group (inclusive). * + * @since 0.1.0 + * * @var int * @phpstan-var positive-int */ @@ -47,6 +53,8 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer /** * Sample size for URL Metrics for a given breakpoint. * + * @since 0.1.0 + * * @var int * @phpstan-var positive-int */ @@ -55,6 +63,8 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer /** * Freshness age (TTL) for a given URL Metric. * + * @since 0.1.0 + * * @var int * @phpstan-var 0|positive-int */ @@ -63,6 +73,8 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer /** * Collection that this instance belongs to. * + * @since 0.3.0 + * * @var OD_URL_Metric_Group_Collection|null */ private $collection; @@ -70,6 +82,8 @@ final class OD_URL_Metric_Group implements IteratorAggregate, Countable, JsonSer /** * Result cache. * + * @since 0.3.0 + * * @var array{ * get_lcp_element?: OD_Element|null, * is_complete?: bool, @@ -146,6 +160,8 @@ public function __construct( array $url_metrics, int $minimum_viewport_width, in /** * Gets the minimum possible viewport width (inclusive). * + * @since 0.1.0 + * * @todo Eliminate in favor of readonly public property. * @return int<0, max> Minimum viewport width. */ @@ -156,6 +172,8 @@ public function get_minimum_viewport_width(): int { /** * Gets the maximum possible viewport width (inclusive). * + * @since 0.1.0 + * * @todo Eliminate in favor of readonly public property. * @return int<1, max> Minimum viewport width. */ @@ -166,6 +184,8 @@ public function get_maximum_viewport_width(): int { /** * Gets the sample size for URL Metrics for a given breakpoint. * + * @since n.e.x.t + * * @todo Eliminate in favor of readonly public property. * @phpstan-return positive-int * @return int Sample size. @@ -177,8 +197,9 @@ public function get_sample_size(): int { /** * Gets the freshness age (TTL) for a given URL Metric. * - * @todo Eliminate in favor of readonly public property. + * @since n.e.x.t * + * @todo Eliminate in favor of readonly public property. * @phpstan-return 0|positive-int * @return int Freshness age. */ @@ -187,7 +208,9 @@ public function get_freshness_ttl(): int { } /** - * Checks whether the provided viewport width is within the minimum/maximum range for + * Checks whether the provided viewport width is within the minimum/maximum range for. + * + * @since 0.1.0 * * @param int $viewport_width Viewport width. * @return bool Whether the viewport width is in range. @@ -202,6 +225,8 @@ public function is_viewport_width_in_range( int $viewport_width ): bool { /** * Adds a URL Metric to the group. * + * @since 0.1.0 + * * @throws InvalidArgumentException If the viewport width of the URL Metric is not within the min/max bounds of the group. * * @param OD_URL_Metric $url_metric URL Metric. @@ -243,6 +268,8 @@ static function ( OD_URL_Metric $a, OD_URL_Metric $b ): int { * A group is complete if it has the full sample size of URL Metrics * and all of these URL Metrics are fresh. * + * @since 0.1.0 + * * @return bool Whether complete. */ public function is_complete(): bool { @@ -271,6 +298,8 @@ public function is_complete(): bool { /** * Gets the LCP element in the viewport group. * + * @since 0.3.0 + * * @return OD_Element|null LCP element data or null if not available, either because there are no URL Metrics or * the LCP element type is not supported. */ @@ -414,6 +443,8 @@ public function get_element_max_intersection_ratio( string $xpath ): ?float { /** * Returns an iterator for the URL Metrics in the group. * + * @since 0.1.0 + * * @return ArrayIterator ArrayIterator for OD_URL_Metric instances. */ public function getIterator(): ArrayIterator { @@ -423,6 +454,8 @@ public function getIterator(): ArrayIterator { /** * Counts the URL Metrics in the group. * + * @since 0.1.0 + * * @return int<0, max> URL Metric count. */ public function count(): int { From 134814537d43bb617cd8e2cfbb9b3c6b0622eda4 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 24 Nov 2024 22:21:18 -0800 Subject: [PATCH 13/27] Add maxLength constraint for tag name --- plugins/image-prioritizer/detect.js | 10 ++++++++++ plugins/image-prioritizer/helper.php | 1 + 2 files changed, 11 insertions(+) diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js index 8f548fd8e5..1b07e1c5d4 100644 --- a/plugins/image-prioritizer/detect.js +++ b/plugins/image-prioritizer/detect.js @@ -148,6 +148,16 @@ function handleLCPMetric( metric, isDebug ) { return; } + // Also skip Custom Elements which have excessively long tag names. + if ( entry.element.tagName.length > 25 ) { + if ( isDebug ) { + log( + `Skipping very long tag name: ${ entry.element.tagName }` + ); + } + return; + } + // Note that getAttribute() is used instead of properties so that null can be returned in case of an absent attribute. let id = entry.element.getAttribute( 'id' ); if ( null !== id ) { diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index dccbf82760..1b1e7f33a3 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -134,6 +134,7 @@ function image_prioritizer_add_element_item_schema_properties( array $additional 'type' => 'string', 'required' => true, 'minLength' => 1, + 'maxLength' => 25, // The longest HTML tag name is 10 characters (BLOCKQUOTE and FIGCAPTION). This maxLength accounts for possible Custom Elements that are even longer. 'pattern' => '^[a-zA-Z0-9\-]+$', // Technically emoji can be allowed in a custom element's tag name, but this is not supported here. ), 'id' => array( From 94b3feed9f1b83bd66ccf55b1c9cc1b16c16c583 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 25 Nov 2024 09:27:57 -0800 Subject: [PATCH 14/27] Account for initialize or finalize not returning a Promise --- plugins/optimization-detective/detect.js | 38 +++++++++++++----------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index 53feba11df..c8a2faae34 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -360,13 +360,14 @@ export default async function detect( { extensions.set( extensionModuleUrl, extension ); // TODO: There should to be a way to pass additional args into the module. Perhaps extensionModuleUrls should be a mapping of URLs to args. if ( extension.initialize instanceof Function ) { - extensionInitializePromises.push( - extension.initialize( { - isDebug, - webVitalsLibrarySrc, - } ) - ); - initializingExtensionModuleUrls.push( extensionModuleUrl ); + const initializePromise = extension.initialize( { + isDebug, + webVitalsLibrarySrc, + } ); + if ( initializePromise instanceof Promise ) { + extensionInitializePromises.push( initializePromise ); + initializingExtensionModuleUrls.push( extensionModuleUrl ); + } } } catch ( err ) { error( @@ -564,16 +565,19 @@ export default async function detect( { ] of extensions.entries() ) { if ( extension.finalize instanceof Function ) { try { - extensionFinalizePromises.push( - extension.finalize( { - isDebug, - getRootData, - getElementData, - extendElementData, - extendRootData, - } ) - ); - finalizingExtensionModuleUrls.push( extensionModuleUrl ); + const finalizePromise = extension.finalize( { + isDebug, + getRootData, + getElementData, + extendElementData, + extendRootData, + } ); + if ( finalizePromise instanceof Promise ) { + extensionFinalizePromises.push( finalizePromise ); + finalizingExtensionModuleUrls.push( + extensionModuleUrl + ); + } } catch ( err ) { error( `Unable to start finalizing extension '${ extensionModuleUrl }':`, From 74f9241d483b85d04a813c03c10b1b753011f7ac Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 25 Nov 2024 09:32:37 -0800 Subject: [PATCH 15/27] Improve typing for externalBackgroundImages --- plugins/image-prioritizer/detect.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js index 1b07e1c5d4..a3ff1503db 100644 --- a/plugins/image-prioritizer/detect.js +++ b/plugins/image-prioritizer/detect.js @@ -14,7 +14,12 @@ const consoleLogPrefix = '[Image Prioritizer]'; /** * Detected LCP external background image candidates. * - * @type {Array<{url: string, tag: string, id: string, class: string}>} + * @type {Array<{ + * url: string, + * tag: string, + * id: string|null, + * class: string|null, + * }>} */ const externalBackgroundImages = []; From 94e527b0d24ef20195d33d59be604f411964978a Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 25 Nov 2024 09:38:50 -0800 Subject: [PATCH 16/27] Eliminate truncation of ID and className --- plugins/image-prioritizer/detect.js | 18 ++++++++++++------ plugins/image-prioritizer/helper.php | 4 ++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js index a3ff1503db..e04a81d0dd 100644 --- a/plugins/image-prioritizer/detect.js +++ b/plugins/image-prioritizer/detect.js @@ -164,13 +164,19 @@ function handleLCPMetric( metric, isDebug ) { } // Note that getAttribute() is used instead of properties so that null can be returned in case of an absent attribute. - let id = entry.element.getAttribute( 'id' ); - if ( null !== id ) { - id = id.substring( 0, 100 ); // This is the maxLength defined in image_prioritizer_add_element_item_schema_properties(). + const id = entry.element.getAttribute( 'id' ); + if ( typeof id === 'string' && id.length > 100 ) { + if ( isDebug ) { + log( `Skipping very long ID: ${ id }` ); + } + return; } - let className = entry.element.getAttribute( 'class' ); - if ( null !== className ) { - className = className.substring( 0, 500 ); // This is the maxLength defined in image_prioritizer_add_element_item_schema_properties(). + const className = entry.element.getAttribute( 'class' ); + if ( typeof className === 'string' && className.length > 500 ) { + if ( isDebug ) { + log( `Skipping very long className: ${ className }` ); + } + return; } // The id and className allow the tag visitor to detect whether the element is still in the document. diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index 1b1e7f33a3..d01ca64537 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -139,12 +139,12 @@ function image_prioritizer_add_element_item_schema_properties( array $additional ), 'id' => array( 'type' => array( 'string', 'null' ), - 'maxLength' => 100, // A reasonable upper-bound length for a long ID. The client will must truncate anything longer. + 'maxLength' => 100, // A reasonable upper-bound length for a long ID. 'required' => true, ), 'class' => array( 'type' => array( 'string', 'null' ), - 'maxLength' => 500, // There can be a ton of class names on an element. The client will must truncate anything longer. + 'maxLength' => 500, // There can be a ton of class names on an element. 'required' => true, ), ), From d2e90d36e448bdb2ff325fbdcf2af832febf78fa Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Dec 2024 09:53:57 -0800 Subject: [PATCH 17/27] Increase tag max length to 100 --- plugins/image-prioritizer/detect.js | 5 +++-- plugins/image-prioritizer/helper.php | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js index e04a81d0dd..acc72d8a78 100644 --- a/plugins/image-prioritizer/detect.js +++ b/plugins/image-prioritizer/detect.js @@ -153,8 +153,8 @@ function handleLCPMetric( metric, isDebug ) { return; } - // Also skip Custom Elements which have excessively long tag names. - if ( entry.element.tagName.length > 25 ) { + // Also skip Custom Elements which have excessively long tag names. This is the maxLength defined in image_prioritizer_add_element_item_schema_properties(). + if ( entry.element.tagName.length > 100 ) { if ( isDebug ) { log( `Skipping very long tag name: ${ entry.element.tagName }` @@ -164,6 +164,7 @@ function handleLCPMetric( metric, isDebug ) { } // Note that getAttribute() is used instead of properties so that null can be returned in case of an absent attribute. + // The maxLengths are defined in image_prioritizer_add_element_item_schema_properties(). const id = entry.element.getAttribute( 'id' ); if ( typeof id === 'string' && id.length > 100 ) { if ( isDebug ) { diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index 5667a97d2e..6b9a6d4236 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -135,8 +135,11 @@ function image_prioritizer_add_element_item_schema_properties( array $additional 'type' => 'string', 'required' => true, 'minLength' => 1, - 'maxLength' => 25, // The longest HTML tag name is 10 characters (BLOCKQUOTE and FIGCAPTION). This maxLength accounts for possible Custom Elements that are even longer. - 'pattern' => '^[a-zA-Z0-9\-]+$', // Technically emoji can be allowed in a custom element's tag name, but this is not supported here. + // The longest HTML tag name is 10 characters (BLOCKQUOTE and FIGCAPTION), but SVG tag names can be longer + // (e.g. feComponentTransfer). This maxLength accounts for possible Custom Elements that are even longer, + // although the longest known Custom Element from HTTP Archive is 32 characters. See data from . + 'maxLength' => 100, + 'pattern' => '^[a-zA-Z0-9\-]+\z', // Technically emoji can be allowed in a custom element's tag name, but this is not supported here. ), 'id' => array( 'type' => array( 'string', 'null' ), From f54673128558bae000b2d4cfb7781bd462991a70 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Dec 2024 11:35:33 -0800 Subject: [PATCH 18/27] Remove erroneous function from merge conflict in e642c9945 --- plugins/image-prioritizer/helper.php | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index 6b9a6d4236..f569ecbb06 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -77,26 +77,6 @@ function image_prioritizer_register_tag_visitors( OD_Tag_Visitor_Registry $regis $registry->register( 'image-prioritizer/video', $video_visitor ); } -/** - * Gets the script to lazy-load videos. - * - * Load a video and its poster image when it approaches the viewport using an IntersectionObserver. - * - * Handles 'autoplay' and 'preload' attributes accordingly. - * - * @since 0.2.0 - */ -function image_prioritizer_get_lazy_load_script(): string { - $path = image_prioritizer_get_asset_path( 'lazy-load.js' ); - $script = file_get_contents( __DIR__ . '/' . $path ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request. - - if ( false === $script ) { - return ''; - } - - return $script; -} - /** * Filters the list of Optimization Detective extension module URLs to include the extension for Image Prioritizer. * From ec59d85fab7d02d49de49b1b4d9fd899894908eb Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Dec 2024 11:46:57 -0800 Subject: [PATCH 19/27] Reduce concern about unsetting array keys during iteration --- ...s-image-prioritizer-background-image-styled-tag-visitor.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php index beffe5ae48..293593e808 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php @@ -157,7 +157,8 @@ private function maybe_preload_external_lcp_background_image( OD_Tag_Visitor_Con $processor = $context->processor; $tag_name = strtoupper( (string) $processor->get_tag() ); - foreach ( $this->group_common_lcp_element_external_background_images as $i => list( $group, $common ) ) { + foreach ( array_keys( $this->group_common_lcp_element_external_background_images ) as $i ) { + list( $group, $common ) = $this->group_common_lcp_element_external_background_images[ $i ]; if ( // Note that the browser may send a lower-case tag name in the case of XHTML or embedded SVG/MathML, but // the HTML Tag Processor is currently normalizing to all upper-case. The HTML Processor on the other From 514c51344f993c010edff305c27084c65e3da70d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Dec 2024 13:17:02 -0800 Subject: [PATCH 20/27] Remove od_store_url_metric_validity filter to be re-added in follow-up PR --- plugins/image-prioritizer/helper.php | 60 ------------------- plugins/image-prioritizer/hooks.php | 1 - .../storage/rest-api.php | 29 --------- 3 files changed, 90 deletions(-) diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index f569ecbb06..c93153dcfa 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -136,66 +136,6 @@ function image_prioritizer_add_element_item_schema_properties( array $additional return $additional_properties; } -/** - * Validates that the provided background image URL is valid. - * - * @since n.e.x.t - * - * @param bool|WP_Error|mixed $validity Validity. Valid if true or a WP_Error without any errors, or invalid otherwise. - * @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema. - * @return bool|WP_Error Validity. Valid if true or a WP_Error without any errors, or invalid otherwise. - */ -function image_prioritizer_filter_store_url_metric_validity( $validity, OD_Strict_URL_Metric $url_metric ) { - if ( ! is_bool( $validity ) && ! ( $validity instanceof WP_Error ) ) { - $validity = (bool) $validity; - } - - $data = $url_metric->get( 'lcpElementExternalBackgroundImage' ); - if ( ! is_array( $data ) ) { - return $validity; - } - - $r = wp_safe_remote_head( - $data['url'], - array( - 'redirection' => 3, // Allow up to 3 redirects. - ) - ); - if ( $r instanceof WP_Error ) { - return new WP_Error( - WP_DEBUG ? $r->get_error_code() : 'head_request_failure', - __( 'HEAD request for background image URL failed.', 'image-prioritizer' ) . ( WP_DEBUG ? ' ' . $r->get_error_message() : '' ), - array( - 'code' => 500, - ) - ); - } - $response_code = wp_remote_retrieve_response_code( $r ); - if ( $response_code < 200 || $response_code >= 400 ) { - return new WP_Error( - 'background_image_response_not_ok', - __( 'HEAD request for background image URL did not return with a success status code.', 'image-prioritizer' ), - array( - 'code' => WP_DEBUG ? $response_code : 400, - ) - ); - } - - $content_type = wp_remote_retrieve_header( $r, 'Content-Type' ); - if ( ! is_string( $content_type ) || ! str_starts_with( $content_type, 'image/' ) ) { - return new WP_Error( - 'background_image_response_not_image', - __( 'HEAD request for background image URL did not return an image Content-Type.', 'image-prioritizer' ), - array( - 'code' => 400, - ) - ); - } - - // TODO: Check for the Content-Length and return invalid if it is gigantic? - return $validity; -} - /** * Gets the path to a script or stylesheet. * diff --git a/plugins/image-prioritizer/hooks.php b/plugins/image-prioritizer/hooks.php index 4a47f35647..7587e9e67b 100644 --- a/plugins/image-prioritizer/hooks.php +++ b/plugins/image-prioritizer/hooks.php @@ -13,4 +13,3 @@ add_action( 'od_init', 'image_prioritizer_init' ); add_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' ); add_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' ); -add_filter( 'od_store_url_metric_validity', 'image_prioritizer_filter_store_url_metric_validity', 10, 2 ); diff --git a/plugins/optimization-detective/storage/rest-api.php b/plugins/optimization-detective/storage/rest-api.php index 8e4828bec4..38f3c9ddaa 100644 --- a/plugins/optimization-detective/storage/rest-api.php +++ b/plugins/optimization-detective/storage/rest-api.php @@ -210,35 +210,6 @@ function od_handle_rest_request( WP_REST_Request $request ) { ); } - /** - * Filters whether a URL Metric is valid for storage. - * - * This allows for custom validation constraints to be applied beyond what can be expressed in JSON Schema. This is - * also necessary because the 'validate_callback' key in a JSON Schema is not respected when gathering the REST API - * endpoint args via the {@see rest_get_endpoint_args_for_schema()} function. Besides this, the REST API doesn't - * support 'validate_callback' for any nested arguments in any case, meaning that custom constraints would be able - * to be applied to multidimensional objects, such as the items inside 'elements'. - * - * This filter only applies when storing a URL Metric via the REST API. It does not run when a stored URL Metric - * loaded from the od_url_metric post type. This means that validation logic enforced via this filter can be more - * expensive, such as doing filesystem checks or HTTP requests. - * - * @since n.e.x.t - * - * @param bool|WP_Error $validity Validity. Valid if true or a WP_Error without any errors, or invalid otherwise. - * @param OD_Strict_URL_Metric $url_metric URL Metric, already validated against the JSON Schema. - */ - $validity = apply_filters( 'od_store_url_metric_validity', true, $url_metric ); - if ( false === $validity || ( $validity instanceof WP_Error && $validity->has_errors() ) ) { - if ( false === $validity ) { - $validity = new WP_Error( 'invalid_url_metric', __( 'Validity of URL Metric was rejected by filter.', 'optimization-detective' ) ); - } - if ( ! isset( $validity->error_data['code'] ) ) { - $validity->error_data['code'] = 400; - } - return $validity; - } - // TODO: This should be changed from store_url_metric($slug, $url_metric) instead be update_post( $slug, $group_collection ). As it stands, store_url_metric() is duplicating logic here. $result = OD_URL_Metrics_Post_Type::store_url_metric( $request->get_param( 'slug' ), From fbb607664d7198fe70b8f535be42efa4820b0a59 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Dec 2024 13:38:00 -0800 Subject: [PATCH 21/27] Rename add_preload_link to add_image_preload_link Co-authored-by: felixarntz --- ...age-prioritizer-background-image-styled-tag-visitor.php | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php index 293593e808..91c20ec0a1 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php @@ -45,6 +45,7 @@ final class Image_Prioritizer_Background_Image_Styled_Tag_Visitor extends Image_ /** * Tuples of URL Metric group and the common LCP element external background image. * + * @since n.e.x.t * @var array */ private $group_common_lcp_element_external_background_images; @@ -87,7 +88,7 @@ public function __invoke( OD_Tag_Visitor_Context $context ): bool { // If this element is the LCP (for a breakpoint group), add a preload link for it. foreach ( $context->url_metric_group_collection->get_groups_by_lcp_element( $xpath ) as $group ) { - $this->add_preload_link( $context->link_collection, $group, $background_image_url ); + $this->add_image_preload_link( $context->link_collection, $group, $background_image_url ); } $this->lazy_load_bg_images( $context ); @@ -169,7 +170,7 @@ private function maybe_preload_external_lcp_background_image( OD_Tag_Visitor_Con && $processor->get_attribute( 'class' ) === $common['class'] // May be checking equality with null. ) { - $this->add_preload_link( $context->link_collection, $group, $common['url'] ); + $this->add_image_preload_link( $context->link_collection, $group, $common['url'] ); // Now that the preload link has been added, eliminate the entry to stop looking for it while iterating over the rest of the document. unset( $this->group_common_lcp_element_external_background_images[ $i ] ); @@ -186,7 +187,7 @@ private function maybe_preload_external_lcp_background_image( OD_Tag_Visitor_Con * @param OD_URL_Metric_Group $group URL Metric group. * @param non-empty-string $url Image URL. */ - private function add_preload_link( OD_Link_Collection $link_collection, OD_URL_Metric_Group $group, string $url ): void { + private function add_image_preload_link( OD_Link_Collection $link_collection, OD_URL_Metric_Group $group, string $url ): void { $link_collection->add_link( array( 'rel' => 'preload', From b4f117254ff925425b16182f60d921487d43def5 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 3 Dec 2024 15:31:26 -0800 Subject: [PATCH 22/27] Restore viewport capturing to its original location --- plugins/optimization-detective/detect.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index 0fafe3ff77..efbaca7476 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -127,12 +127,6 @@ function recursiveFreeze( obj ) { Object.freeze( obj ); } -// This needs to be captured early in case the user later resizes the window. -const viewport = { - width: win.innerWidth, - height: win.innerHeight, -}; - /** * URL Metric being assembled for submission. * @@ -489,7 +483,10 @@ export default async function detect( { urlMetric = { url: currentUrl, - viewport, + viewport: { + width: win.innerWidth, + height: win.innerHeight, + }, elements: [], }; From b8b79deae6f3ba3ef9d26310a8df2a67badeb40d Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Mon, 9 Dec 2024 08:51:56 -0800 Subject: [PATCH 23/27] Clarify comment about URL metric groups not changing across a single page load. Co-authored-by: Weston Ruter --- ...ass-image-prioritizer-background-image-styled-tag-visitor.php | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php index 91c20ec0a1..c6570d4d2c 100644 --- a/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php +++ b/plugins/image-prioritizer/class-image-prioritizer-background-image-styled-tag-visitor.php @@ -141,6 +141,7 @@ private function get_common_lcp_element_external_background_image( OD_URL_Metric */ private function maybe_preload_external_lcp_background_image( OD_Tag_Visitor_Context $context ): void { // Gather the tuples of URL Metric group and the common LCP element external background image. + // Note the groups of URL Metrics do not change across invocations, we just need to compute this once for all. if ( ! is_array( $this->group_common_lcp_element_external_background_images ) ) { $this->group_common_lcp_element_external_background_images = array(); foreach ( $context->url_metric_group_collection as $group ) { From 47d06dc669b34ef2cffaef01ad8be2d11cbe06ca Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 9 Dec 2024 12:36:42 -0800 Subject: [PATCH 24/27] Export web-vitals callback functions to extensions rather than webVitalsLibrarySrc --- plugins/image-prioritizer/detect.js | 5 ++--- plugins/optimization-detective/detect.js | 21 ++++++++++++++++++--- plugins/optimization-detective/types.ts | 14 +++++++++++++- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/plugins/image-prioritizer/detect.js b/plugins/image-prioritizer/detect.js index acc72d8a78..6a73074c24 100644 --- a/plugins/image-prioritizer/detect.js +++ b/plugins/image-prioritizer/detect.js @@ -63,10 +63,9 @@ function warn( ...message ) { * @type {InitializeCallback} * @param {InitializeArgs} args Args. */ -export async function initialize( { isDebug, webVitalsLibrarySrc } ) { - const { onLCP } = await import( webVitalsLibrarySrc ); +export async function initialize( { isDebug, onLCP } ) { onLCP( - ( /** @type {LCPMetric} */ metric ) => { + ( metric ) => { handleLCPMetric( metric, isDebug ); }, { diff --git a/plugins/optimization-detective/detect.js b/plugins/optimization-detective/detect.js index efbaca7476..fe4518c9ef 100644 --- a/plugins/optimization-detective/detect.js +++ b/plugins/optimization-detective/detect.js @@ -1,6 +1,11 @@ /** * @typedef {import("web-vitals").LCPMetric} LCPMetric * @typedef {import("./types.ts").ElementData} ElementData + * @typedef {import("./types.ts").OnTTFBFunction} OnTTFBFunction + * @typedef {import("./types.ts").OnFCPFunction} OnFCPFunction + * @typedef {import("./types.ts").OnLCPFunction} OnLCPFunction + * @typedef {import("./types.ts").OnINPFunction} OnINPFunction + * @typedef {import("./types.ts").OnCLSFunction} OnCLSFunction * @typedef {import("./types.ts").URLMetric} URLMetric * @typedef {import("./types.ts").URLMetricGroupStatus} URLMetricGroupStatus * @typedef {import("./types.ts").Extension} Extension @@ -335,6 +340,14 @@ export default async function detect( { { once: true } ); + const { + /** @type OnTTFBFunction */ onTTFB, + /** @type OnFCPFunction */ onFCP, + /** @type OnLCPFunction */ onLCP, + /** @type OnINPFunction */ onINP, + /** @type OnCLSFunction */ onCLS, + } = await import( webVitalsLibrarySrc ); + // TODO: Does this make sense here? // Prevent detection when page is not scrolled to the initial viewport. if ( doc.documentElement.scrollTop > 0 ) { @@ -368,7 +381,11 @@ export default async function detect( { if ( extension.initialize instanceof Function ) { const initializePromise = extension.initialize( { isDebug, - webVitalsLibrarySrc, + onTTFB, + onFCP, + onLCP, + onINP, + onCLS, } ); if ( initializePromise instanceof Promise ) { extensionInitializePromises.push( initializePromise ); @@ -454,8 +471,6 @@ export default async function detect( { } ); } - const { onLCP } = await import( webVitalsLibrarySrc ); - /** @type {LCPMetric[]} */ const lcpMetricCandidates = []; diff --git a/plugins/optimization-detective/types.ts b/plugins/optimization-detective/types.ts index cafd6f9d3a..d92c532143 100644 --- a/plugins/optimization-detective/types.ts +++ b/plugins/optimization-detective/types.ts @@ -1,6 +1,8 @@ // h/t https://stackoverflow.com/a/59801602/93579 type ExcludeProps< T > = { [ k: string ]: any } & { [ K in keyof T ]?: never }; +import { onTTFB, onFCP, onLCP, onINP, onCLS } from 'web-vitals'; + export interface ElementData { isLCP: boolean; isLCPCandidate: boolean; @@ -28,9 +30,19 @@ export interface URLMetricGroupStatus { complete: boolean; } +export type OnTTFBFunction = typeof onTTFB; +export type OnFCPFunction = typeof onFCP; +export type OnLCPFunction = typeof onLCP; +export type OnINPFunction = typeof onINP; +export type OnCLSFunction = typeof onCLS; + export type InitializeArgs = { readonly isDebug: boolean; - readonly webVitalsLibrarySrc: string; + readonly onTTFB: OnTTFBFunction; + readonly onFCP: OnFCPFunction; + readonly onLCP: OnLCPFunction; + readonly onINP: OnINPFunction; + readonly onCLS: OnCLSFunction; }; export type InitializeCallback = ( args: InitializeArgs ) => Promise< void >; From 2add13307c54ccbfb9a1d72836b0e135b30b5562 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 11 Dec 2024 09:10:37 -0800 Subject: [PATCH 25/27] Add tests for hooks being added --- plugins/image-prioritizer/tests/test-hooks.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 plugins/image-prioritizer/tests/test-hooks.php diff --git a/plugins/image-prioritizer/tests/test-hooks.php b/plugins/image-prioritizer/tests/test-hooks.php new file mode 100644 index 0000000000..740c71a3d5 --- /dev/null +++ b/plugins/image-prioritizer/tests/test-hooks.php @@ -0,0 +1,18 @@ +assertEquals( 10, has_action( 'od_init', 'image_prioritizer_init' ) ); + $this->assertEquals( 10, has_filter( 'od_extension_module_urls', 'image_prioritizer_filter_extension_module_urls' ) ); + $this->assertEquals( 10, has_filter( 'od_url_metric_schema_root_additional_properties', 'image_prioritizer_add_element_item_schema_properties' ) ); + } +} From 3abf92af715aaf7c3f185b72b147e18a5a8bd3be Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 11 Dec 2024 10:50:22 -0800 Subject: [PATCH 26/27] Add missing unit tests for functions --- plugins/image-prioritizer/helper.php | 3 +- .../image-prioritizer/tests/test-helper.php | 246 +++++++++++++++++- 2 files changed, 242 insertions(+), 7 deletions(-) diff --git a/plugins/image-prioritizer/helper.php b/plugins/image-prioritizer/helper.php index c93153dcfa..1651dcf555 100644 --- a/plugins/image-prioritizer/helper.php +++ b/plugins/image-prioritizer/helper.php @@ -107,7 +107,8 @@ function image_prioritizer_add_element_item_schema_properties( array $additional 'properties' => array( 'url' => array( 'type' => 'string', - 'format' => 'uri', + 'format' => 'uri', // Note: This is excessively lax, as it is used exclusively in rest_sanitize_value_from_schema() and not in rest_validate_value_from_schema(). + 'pattern' => '^https?://', 'required' => true, 'maxLength' => 500, // Image URLs can be quite long. ), diff --git a/plugins/image-prioritizer/tests/test-helper.php b/plugins/image-prioritizer/tests/test-helper.php index e579d54035..180a2d807f 100644 --- a/plugins/image-prioritizer/tests/test-helper.php +++ b/plugins/image-prioritizer/tests/test-helper.php @@ -84,7 +84,7 @@ public function data_provider_test_filter_tag_visitors(): array { } /** - * Test image_prioritizer_register_tag_visitors(). + * Test end-to-end. * * @covers ::image_prioritizer_register_tag_visitors * @covers Image_Prioritizer_Tag_Visitor @@ -97,7 +97,7 @@ public function data_provider_test_filter_tag_visitors(): array { * @param callable|string $buffer Content before. * @param callable|string $expected Expected content after. */ - public function test_image_prioritizer_register_tag_visitors( callable $set_up, $buffer, $expected ): void { + public function test_end_to_end( callable $set_up, $buffer, $expected ): void { $set_up( $this, $this::factory() ); $buffer = is_string( $buffer ) ? $buffer : $buffer(); @@ -219,7 +219,7 @@ public function data_provider_test_auto_sizes(): array { * @dataProvider data_provider_test_auto_sizes * @phpstan-param array{ xpath: string, isLCP: bool, intersectionRatio: int } $element_metrics */ - public function test_auto_sizes( array $element_metrics, string $buffer, string $expected ): void { + public function test_auto_sizes_end_to_end( array $element_metrics, string $buffer, string $expected ): void { $this->populate_url_metrics( array( $element_metrics ) ); $html_start_doc = '...'; @@ -236,30 +236,264 @@ public function test_auto_sizes( array $element_metrics, string $buffer, string ); } + /** + * Test image_prioritizer_register_tag_visitors. + * + * @covers ::image_prioritizer_register_tag_visitors + */ + public function test_image_prioritizer_register_tag_visitors(): void { + $registry = new OD_Tag_Visitor_Registry(); + image_prioritizer_register_tag_visitors( $registry ); + $this->assertTrue( $registry->is_registered( 'image-prioritizer/img' ) ); + $this->assertTrue( $registry->is_registered( 'image-prioritizer/background-image' ) ); + $this->assertTrue( $registry->is_registered( 'image-prioritizer/video' ) ); + } + + /** + * Test image_prioritizer_filter_extension_module_urls. + * + * @covers ::image_prioritizer_filter_extension_module_urls + */ + public function test_image_prioritizer_filter_extension_module_urls(): void { + $initial_modules = array( + home_url( '/module.js' ), + ); + $filtered_modules = image_prioritizer_filter_extension_module_urls( $initial_modules ); + $this->assertCount( 2, $filtered_modules ); + $this->assertSame( $initial_modules[0], $filtered_modules[0] ); + $this->assertStringContainsString( 'detect.', $filtered_modules[1] ); + } + + /** + * Test image_prioritizer_add_element_item_schema_properties. + * + * @covers ::image_prioritizer_add_element_item_schema_properties + */ + public function test_image_prioritizer_add_element_item_schema_properties(): void { + $initial_schema = array( + 'foo' => array( + 'type' => 'string', + ), + ); + $filtered_schema = image_prioritizer_add_element_item_schema_properties( $initial_schema ); + $this->assertCount( 2, $filtered_schema ); + $this->assertArrayHasKey( 'foo', $filtered_schema ); + $this->assertArrayHasKey( 'lcpElementExternalBackgroundImage', $filtered_schema ); + $this->assertSame( 'object', $filtered_schema['lcpElementExternalBackgroundImage']['type'] ); + $this->assertSameSets( array( 'url', 'id', 'tag', 'class' ), array_keys( $filtered_schema['lcpElementExternalBackgroundImage']['properties'] ) ); + } + + /** + * @return array + */ + public function data_provider_for_test_image_prioritizer_add_element_item_schema_properties_inputs(): array { + return array( + 'bad_type' => array( + 'input_value' => 'not_an_object', + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage] is not of type object.', + 'output_value' => null, + ), + 'missing_props' => array( + 'input_value' => array(), + 'expected_exception' => 'url is a required property of OD_URL_Metric[lcpElementExternalBackgroundImage].', + 'output_value' => null, + ), + 'bad_url_protocol' => array( + 'input_value' => array( + 'url' => 'javascript:alert(1)', + 'tag' => 'DIV', + 'id' => null, + 'class' => null, + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][url] does not match pattern ^https?://.', + 'output_value' => null, + ), + 'bad_url_format' => array( + 'input_value' => array( + 'url' => 'https://not a valid URL!!!', + 'tag' => 'DIV', + 'id' => null, + 'class' => null, + ), + 'expected_exception' => null, + 'output_value' => array( + 'url' => 'https://not%20a%20valid%20URL!!!', // This is due to sanitize_url() being used in core. More validation is needed. + 'tag' => 'DIV', + 'id' => null, + 'class' => null, + ), + ), + 'bad_url_length' => array( + 'input_value' => array( + 'url' => 'https://example.com/' . str_repeat( 'a', 501 ), + 'tag' => 'DIV', + 'id' => null, + 'class' => null, + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][url] must be at most 500 characters long.', + 'output_value' => null, + ), + 'bad_null_tag' => array( + 'input_value' => array( + 'url' => 'https://example.com/', + 'tag' => null, + 'id' => null, + 'class' => null, + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][tag] is not of type string.', + 'output_value' => null, + ), + 'bad_format_tag' => array( + 'input_value' => array( + 'url' => 'https://example.com/', + 'tag' => 'bad tag name!!', + 'id' => null, + 'class' => null, + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][tag] does not match pattern ^[a-zA-Z0-9\-]+\z.', + 'output_value' => null, + ), + 'bad_length_tag' => array( + 'input_value' => array( + 'url' => 'https://example.com/', + 'tag' => str_repeat( 'a', 101 ), + 'id' => null, + 'class' => null, + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][tag] must be at most 100 characters long.', + 'output_value' => null, + ), + 'bad_type_id' => array( + 'input_value' => array( + 'url' => 'https://example.com/', + 'tag' => 'DIV', + 'id' => array( 'bad' ), + 'class' => null, + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][id] is not of type string,null.', + 'output_value' => null, + ), + 'bad_length_id' => array( + 'input_value' => array( + 'url' => 'https://example.com/', + 'tag' => 'DIV', + 'id' => str_repeat( 'a', 101 ), + 'class' => null, + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][id] must be at most 100 characters long.', + 'output_value' => null, + ), + 'bad_type_class' => array( + 'input_value' => array( + 'url' => 'https://example.com/', + 'tag' => 'DIV', + 'id' => 'main', + 'class' => array( 'bad' ), + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][class] is not of type string,null.', + 'output_value' => null, + ), + 'bad_length_class' => array( + 'input_value' => array( + 'url' => 'https://example.com/', + 'tag' => 'DIV', + 'id' => 'main', + 'class' => str_repeat( 'a', 501 ), + ), + 'expected_exception' => 'OD_URL_Metric[lcpElementExternalBackgroundImage][class] must be at most 500 characters long.', + 'output_value' => null, + ), + 'ok_minimal' => array( + 'input_value' => array( + 'url' => 'https://example.com/bg.jpg', + 'tag' => 'DIV', + 'id' => null, + 'class' => null, + ), + 'expected_exception' => null, + 'output_value' => array( + 'url' => 'https://example.com/bg.jpg', + 'tag' => 'DIV', + 'id' => null, + 'class' => null, + ), + ), + 'ok_maximal' => array( + 'input_value' => array( + 'url' => 'https://example.com/' . str_repeat( 'a', 476 ) . '.jpg', + 'tag' => str_repeat( 'a', 100 ), + 'id' => str_repeat( 'b', 100 ), + 'class' => str_repeat( 'c', 500 ), + ), + 'expected_exception' => null, + 'output_value' => array( + 'url' => 'https://example.com/' . str_repeat( 'a', 476 ) . '.jpg', + 'tag' => str_repeat( 'a', 100 ), + 'id' => str_repeat( 'b', 100 ), + 'class' => str_repeat( 'c', 500 ), + ), + ), + ); + } + + /** + * Test image_prioritizer_add_element_item_schema_properties for various inputs. + * + * @covers ::image_prioritizer_add_element_item_schema_properties + * + * @dataProvider data_provider_for_test_image_prioritizer_add_element_item_schema_properties_inputs + * + * @param mixed $input_value Input value. + * @param string|null $expected_exception Expected exception message. + * @param array|null $output_value Output value. + */ + public function test_image_prioritizer_add_element_item_schema_properties_inputs( $input_value, ?string $expected_exception, ?array $output_value ): void { + $data = $this->get_sample_url_metric( array() )->jsonSerialize(); + $data['lcpElementExternalBackgroundImage'] = $input_value; + $exception_message = null; + try { + $url_metric = new OD_URL_Metric( $data ); + } catch ( OD_Data_Validation_Exception $e ) { + $exception_message = $e->getMessage(); + } + + $this->assertSame( + $expected_exception, + $exception_message, + isset( $url_metric ) ? 'Data: ' . wp_json_encode( $url_metric->jsonSerialize(), JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES ) : '' + ); + if ( isset( $url_metric ) ) { + $this->assertSame( $output_value, $url_metric->jsonSerialize()['lcpElementExternalBackgroundImage'] ); + } + } + /** * Test image_prioritizer_get_video_lazy_load_script. * * @covers ::image_prioritizer_get_video_lazy_load_script + * @covers ::image_prioritizer_get_asset_path */ public function test_image_prioritizer_get_video_lazy_load_script(): void { - $this->assertGreaterThan( 0, strlen( image_prioritizer_get_video_lazy_load_script() ) ); + $this->assertStringContainsString( 'new IntersectionObserver', image_prioritizer_get_video_lazy_load_script() ); } /** * Test image_prioritizer_get_lazy_load_bg_image_script. * * @covers ::image_prioritizer_get_lazy_load_bg_image_script + * @covers ::image_prioritizer_get_asset_path */ public function test_image_prioritizer_get_lazy_load_bg_image_script(): void { - $this->assertGreaterThan( 0, strlen( image_prioritizer_get_lazy_load_bg_image_script() ) ); + $this->assertStringContainsString( 'new IntersectionObserver', image_prioritizer_get_lazy_load_bg_image_script() ); } /** * Test image_prioritizer_get_lazy_load_bg_image_stylesheet. * * @covers ::image_prioritizer_get_lazy_load_bg_image_stylesheet + * @covers ::image_prioritizer_get_asset_path */ public function test_image_prioritizer_get_lazy_load_bg_image_stylesheet(): void { - $this->assertGreaterThan( 0, strlen( image_prioritizer_get_lazy_load_bg_image_stylesheet() ) ); + $this->assertStringContainsString( '.od-lazy-bg-image', image_prioritizer_get_lazy_load_bg_image_stylesheet() ); } } From 02c6673be016b59b1fca7f0f3cd89a5f552ebc8c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 11 Dec 2024 11:29:15 -0800 Subject: [PATCH 27/27] Add test cases for external background images --- ...ge-complete-samples-but-element-absent.php | 74 +++++++++++++ ...n-document-and-fully-populated-samples.php | 77 +++++++++++++ ...cument-and-partially-populated-samples.php | 101 ++++++++++++++++++ ...ss-optimization-detective-test-helpers.php | 7 +- 4 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-complete-samples-but-element-absent.php create mode 100644 plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-present-in-document-and-fully-populated-samples.php create mode 100644 plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-present-in-document-and-partially-populated-samples.php diff --git a/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-complete-samples-but-element-absent.php b/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-complete-samples-but-element-absent.php new file mode 100644 index 0000000000..f4047e739a --- /dev/null +++ b/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-complete-samples-but-element-absent.php @@ -0,0 +1,74 @@ + static function ( Test_Image_Prioritizer_Helper $test_case ): void { + add_filter( + 'od_breakpoint_max_widths', + static function () { + return array( 480, 600, 782 ); + } + ); + + $slug = od_get_url_metrics_slug( od_get_normalized_query_vars() ); + $sample_size = od_get_url_metrics_breakpoint_sample_size(); + + $bg_images = array( + 'https://example.com/mobile.jpg', + 'https://example.com/tablet.jpg', + 'https://example.com/phablet.jpg', + 'https://example.com/desktop.jpg', + ); + + // Fully populate all viewport groups, but for all except desktop record that the LCP element had a different tag, id, or class. + foreach ( array_merge( od_get_breakpoint_max_widths(), array( 1000 ) ) as $i => $viewport_width ) { + for ( $j = 0; $j < $sample_size; $j++ ) { + OD_URL_Metrics_Post_Type::store_url_metric( + $slug, + $test_case->get_sample_url_metric( + array( + 'viewport_width' => $viewport_width, + 'elements' => array(), + 'extended_root' => array( + 'lcpElementExternalBackgroundImage' => array( + 'url' => $bg_images[ $i ], + 'tag' => 0 === $i ? 'DIV' : 'HEADER', + 'id' => 1 === $i ? 'foo' : 'masthead', + 'class' => 2 === $i ? 'bar' : 'banner', + ), + ), + ) + ) + ); + } + } + }, + 'buffer' => ' + + + + ... + + + + + + + ', + 'expected' => ' + + + + ... + + + + + + + + ', +); diff --git a/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-present-in-document-and-fully-populated-samples.php b/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-present-in-document-and-fully-populated-samples.php new file mode 100644 index 0000000000..d73c475ec3 --- /dev/null +++ b/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-present-in-document-and-fully-populated-samples.php @@ -0,0 +1,77 @@ + static function ( Test_Image_Prioritizer_Helper $test_case ): void { + add_filter( + 'od_breakpoint_max_widths', + static function () { + return array( 480, 600, 782 ); + } + ); + + $slug = od_get_url_metrics_slug( od_get_normalized_query_vars() ); + $sample_size = od_get_url_metrics_breakpoint_sample_size(); + + $bg_images = array( + 'https://example.com/mobile.jpg', + 'https://example.com/tablet.jpg', + 'https://example.com/phablet.jpg', + 'https://example.com/desktop.jpg', + ); + + // Fully populate all viewport groups. + foreach ( array_merge( od_get_breakpoint_max_widths(), array( 1000 ) ) as $i => $viewport_width ) { + for ( $j = 0; $j < $sample_size; $j++ ) { + OD_URL_Metrics_Post_Type::store_url_metric( + $slug, + $test_case->get_sample_url_metric( + array( + 'viewport_width' => $viewport_width, + 'elements' => array(), + 'extended_root' => array( + 'lcpElementExternalBackgroundImage' => array( + 'url' => $bg_images[ $i ], + 'tag' => 'HEADER', + 'id' => 'masthead', + 'class' => 'banner', + ), + ), + ) + ) + ); + } + } + }, + 'buffer' => ' + + + + ... + + + + + + + ', + 'expected' => ' + + + + ... + + + + + + + + + + + ', +); diff --git a/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-present-in-document-and-partially-populated-samples.php b/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-present-in-document-and-partially-populated-samples.php new file mode 100644 index 0000000000..c967d5e9c9 --- /dev/null +++ b/plugins/image-prioritizer/tests/test-cases/lcp-element-external-background-image-present-in-document-and-partially-populated-samples.php @@ -0,0 +1,101 @@ + static function ( Test_Image_Prioritizer_Helper $test_case ): void { + add_filter( + 'od_breakpoint_max_widths', + static function () { + return array( 480, 600, 782 ); + } + ); + + $slug = od_get_url_metrics_slug( od_get_normalized_query_vars() ); + $sample_size = od_get_url_metrics_breakpoint_sample_size(); + + $bg_images = array( + 'https://example.com/mobile.jpg', + 'https://example.com/tablet.jpg', + 'https://example.com/phablet.jpg', + 'https://example.com/desktop.jpg', + ); + + $viewport_sample_sizes = array( + $sample_size, + $sample_size - 1, + 0, + $sample_size, + ); + + // Partially populate all viewport groups. + foreach ( array_merge( od_get_breakpoint_max_widths(), array( 1000 ) ) as $i => $viewport_width ) { + for ( $j = 0; $j < $viewport_sample_sizes[ $i ]; $j++ ) { + OD_URL_Metrics_Post_Type::store_url_metric( + $slug, + $test_case->get_sample_url_metric( + array( + 'viewport_width' => $viewport_width, + 'elements' => array(), + 'extended_root' => array( + 'lcpElementExternalBackgroundImage' => array( + 'url' => $bg_images[ $i ], + 'tag' => 'HEADER', + 'id' => 'masthead', + 'class' => 'banner', + ), + ), + ) + ) + ); + } + } + + // Store one more URL metric for desktop which has a different background image. + OD_URL_Metrics_Post_Type::store_url_metric( + $slug, + $test_case->get_sample_url_metric( + array( + 'viewport_width' => 1000, + 'elements' => array(), + 'extended_root' => array( + 'lcpElementExternalBackgroundImage' => array( + 'url' => 'https://example.com/desktop-alt.jpg', + 'tag' => 'HEADER', + 'id' => 'masthead', + 'class' => 'banner', + ), + ), + ) + ) + ); + }, + 'buffer' => ' + + + + ... + + + + + + + ', + 'expected' => ' + + + + ... + + + + + + + + + ', +); diff --git a/tests/class-optimization-detective-test-helpers.php b/tests/class-optimization-detective-test-helpers.php index c7924480a0..0ddda0adb5 100644 --- a/tests/class-optimization-detective-test-helpers.php +++ b/tests/class-optimization-detective-test-helpers.php @@ -86,6 +86,7 @@ public function get_sample_url_metric( array $params ): OD_URL_Metric { 'viewport_width' => 480, 'elements' => array(), 'timestamp' => microtime( true ), + 'extended_root' => array(), ), $params ); @@ -94,7 +95,7 @@ public function get_sample_url_metric( array $params ): OD_URL_Metric { $params['elements'][] = $params['element']; } - return new OD_URL_Metric( + $data = array_merge( array( 'etag' => $params['etag'], 'url' => $params['url'], @@ -118,8 +119,10 @@ function ( array $element ): array { }, $params['elements'] ), - ) + ), + $params['extended_root'] ); + return new OD_URL_Metric( $data ); } /**