Leverage URL metrics to reserve space for embeds to reduce CLS #1373

westonruter · 2024-07-17T21:39:12Z

Fixes #1310

TODO: Summarize the changes in this PR. In the mean time, see the videos below.

To do

Collecting the URL metrics needs to wait until after all the embeds have loaded. This is not currently the case for Tweets, for example.
The min-height style should be set on the figure.wp-block-embed but the measurement of the height of the embed should only come from the child .wp-block-embed__wrapper. In this way, an embed can be allowed to shrink its height over time.
Wait to set the min-height until there are URL metrics gathered for the smallest breakpoint group?
Embed Optimizer is sending the min-height of embeds before they are shown in the viewport. We should have a resizedBoundingClientRect instead of overriding boundingClientRect? We can then only consider elements that have the final rect set. This depends on Allow URL metric schema to be extended #1492.
Add tests.

Demo Videos

Before

bad-cls-without-embed-optimizer.webm

After

Once URL metrics have been collected from visitors by Optimization Detective:

good-cls-with-embed-optimizer.webm

westonruter · 2024-07-26T22:07:38Z

plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php

 			return false;
 		}

-		$max_intersection_ratio = $context->url_metrics_group_collection->get_element_max_intersection_ratio( $processor->get_xpath() );
+		$embed_wrapper_xpath = $processor->get_xpath() . '/*[1][self::DIV]';


It's not ideal to be constructing this XPath manually.

I've opened #1407 to explore this, but it's not necessary for this PR to move forward.

westonruter · 2024-07-26T22:38:25Z

I noticed Query Monitor flagging a PHP deprecation error when loading a page with Optimization Detective and Embed Optimizer active. A TikTok embed is present on the page. I'm getting:

PHP Deprecated: Implicit conversion from float 739.9976196289062 to int loses precision in /var/www/html/wp-content/plugins/optimization-detective/class-od-url-metrics-group-collection.php on line 486

Call stack:

Update: Addressed in #1411.

westonruter · 2024-07-26T22:40:30Z

I also saw this for some reason:

PHP Notice: OD_HTML_Tag_Processor::get_updated_html(): Unable to append markup to optimization_detective_end_of_body since the bookmark no longer exists. in /var/www/html/wp-includes/functions.php on line 6085

Update: See fix below in #1373 (comment)

plugins/optimization-detective/class-od-url-metric.php

plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php

westonruter · 2024-08-02T00:07:37Z

plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php

+			$style = $processor->get_attribute( 'style' );
+			if ( is_string( $style ) ) {
+				$style = rtrim( trim( $style ), ';' ) . '; ';
+			} else {
+				$style = '';
+			}
+			$style .= sprintf( 'min-height: %dpx;', $minimum_height );
+			$processor->set_attribute( 'style', $style );


This should be a helper method on OD_HTML_Tag_Processor.

See prior art in the AMP plugin: https://github.com/ampproject/amp-toolbox-php/blob/78531851c59fa5f306315372719f0064b5542cf4/src/Dom/Element.php#L49-L69

westonruter · 2024-08-14T00:47:33Z

Scratched code to explore using onCLS to detect when an embed has loaded (and causes a layout shift. Problem: In the case of the Twitter embed, the source node being reported is REMOVED from the DOM, so we cannot see the ancestor which has a data-od-xpath attribute.

Code scratch

	const { onLCP, onCLS } = await import( webVitalsLibrarySrc );

	onCLS(
		( metric ) => {
			for ( const entry of metric.entries ) {
				if (
					entry.entryType === 'layout-shift' &&
					! entry.hadRecentInput
				) {
					console.info( entry );
					for ( const source of entry.sources ) {
						console.info( source.node );
					}
				}
			}
		},
		{
			// This is necessary in order to collect all layout shifts, even those which don't cause a bad CLS score.
			reportAllChanges: true,
		}
	);

…d Optimizer JS into one

This is an iteration on top of #1098 Co-authored-by: swissspidy <[email protected]>

westonruter · 2024-08-18T21:43:43Z

I'm seeing this again:

OD_HTML_Tag_Processor::get_updated_html(): Unable to append markup to optimization_detective_end_of_body since the bookmark no longer exists.

wp-includes/functions.php:6095
wp_trigger_error()
wp-content/plugins/optimization-detective/class-od-html-tag-processor.php:612
OD_HTML_Tag_Processor->warn()
wp-content/plugins/optimization-detective/class-od-html-tag-processor.php:579
OD_HTML_Tag_Processor->get_updated_html()
wp-includes/html-api/class-wp-html-tag-processor.php:2500
WP_HTML_Tag_Processor->seek()
wp-content/plugins/optimization-detective/class-od-html-tag-processor.php:428
OD_HTML_Tag_Processor->seek()
wp-content/plugins/optimization-detective/optimization.php:229
od_optimize_template_output_buffer()
wp-content/plugins/performance-lab/includes/server-timing/load.php:138
{closure}()
wp-includes/class-wp-hook.php:324
apply_filters('od_template_output_buffer')
wp-content/plugins/optimization-detective/optimization.php:66
{closure}()
wp-content/plugins/optimization-detective/optimization.php:66
ob_end_flush()
wp-includes/functions.php:5437
wp_ob_end_flush_all()
wp-includes/class-wp-hook.php:324
do_action('shutdown')
wp-includes/load.php:1278
shutdown_action_hook()
wp-includes/load.php:1278

Update: Fixed in edc52fa. The issue is that every time seek() is called, it will also call get_updated_html(). So I introduced a get_final_updated_html() which is intended to be called at the very end of iterating over the document.

…_minimum_heights

…ecially to end-of-body

adamsilverstein · 2024-08-21T14:13:04Z

plugins/optimization-detective/detect.js

+	for ( const extensionModuleUrl of extensionModuleUrls ) {
+		const extension = await import( extensionModuleUrl );
+		extensions.push( 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.


we also have JavaScript hooks you could leverage if the idea is to make this extensible.

These aren't available as script modules though, right?

true, but they could be added separately to filter the an args array here for example

swissspidy · 2024-08-21T14:19:50Z

plugins/optimization-detective/detect.js

-			setStorageLock( getCurrentTime() );
+	for ( const extension of extensions ) {
+		if ( extension.finalize instanceof Function ) {
+			extension.finalize( {


So extensions add their own data by directly modifying the passed urlMetric? Why not have them return the new data instead?

As this PR is currently implemented, Embed Optimizer overrides the boundingClientRect with the final rect reported the ResizeObserver for embeds. But since then I've realized this isn't ideal because we can't know if the boundingClientRect was the initial size or the final size. So in that case I think it would be better if the schema is made extensible (cf. #1490 (comment)) so that Embed Optimizer can add a resizedBoundingClientRect which we then exclusively look for when setting the min-height. So in that case, the extension's finalize function could just return an object that gets merged on top of the urlMetric. But it seems there could be use cases for extensions to still be able modify the entire object?

I think returning an object and using structuredClone here to avoid unintended modifications is cleaner.

OK, sounds good. But using structuredClone() where? We wouldn't need to pass in the urlMetric anymore to the finalize function, so would a clone be needed?

Er wait, we do need to pass the urlMetric because Embed Optimizer needs to amend Element data in the elements key.

plugins/optimization-detective/detect.js

Co-authored-by: swissspidy <[email protected]>

… add/embed-optimizer-min-height-reservation

… is excluded

westonruter · 2024-09-17T21:46:02Z

plugins/embed-optimizer/detect.js

+}
+
+/**
+ * Initialize.


Suggested change

* Initialize.

* Finalize.

This is needed because get_json_params() can return null. Also, no need to force the request body to be JSON. See 1da219f

westonruter added the [Type] Enhancement A suggestion for improvement of an existing feature label Jul 17, 2024

swissspidy added the [Plugin] Embed Optimizer Issues for the Embed Optimizer plugin (formerly Auto Sizes) label Jul 26, 2024

westonruter force-pushed the add/embed-optimizer-min-height-reservation branch from f38247b to 947ca41 Compare July 26, 2024 18:23

westonruter added this to the embed-optimizer n.e.x.t milestone Jul 26, 2024

westonruter added the [Plugin] Optimization Detective Issues for the Optimization Detective plugin label Jul 26, 2024

westonruter force-pushed the add/embed-optimizer-min-height-reservation branch from 3f38eb0 to b3ca4ad Compare July 26, 2024 22:06

westonruter commented Jul 26, 2024

View reviewed changes

westonruter mentioned this pull request Jul 26, 2024

Add helper function to construct XPath from indexed breadcrumbs #1407

Draft

westonruter commented Jul 27, 2024

View reviewed changes

plugins/optimization-detective/class-od-url-metric.php Outdated Show resolved Hide resolved

westonruter force-pushed the add/embed-optimizer-min-height-reservation branch from 02b8fb3 to b17d8ba Compare July 30, 2024 00:40

westonruter changed the base branch from trunk to fix/od-schema July 30, 2024 00:40

westonruter force-pushed the add/embed-optimizer-min-height-reservation branch from b17d8ba to 76369b4 Compare July 30, 2024 00:42

westonruter commented Jul 30, 2024

View reviewed changes

plugins/embed-optimizer/class-embed-optimizer-tag-visitor.php Outdated Show resolved Hide resolved

Base automatically changed from fix/od-schema to trunk July 30, 2024 14:58

westonruter force-pushed the add/embed-optimizer-min-height-reservation branch from 1f5e5f8 to 77bea30 Compare August 2, 2024 00:06

westonruter commented Aug 2, 2024

View reviewed changes

westonruter force-pushed the add/embed-optimizer-min-height-reservation branch 2 times, most recently from 019526c to 36e5620 Compare August 13, 2024 23:03

westonruter modified the milestones: embed-optimizer 0.3.0, embed-optimizer n.e.x.t Aug 15, 2024

westonruter added 7 commits August 16, 2024 17:15

Introduce methods to get minumum height of element

0a376c1

Set the min-height of an embed prior to it loading

8aa7e63

Set min-height on embed-wrapper instead of figure container

11f98f4

Use 500px as a better representation of an element that could be LCP

d7cc7cf

Add test for existing style manipulation

48e57e8

Add helper generator method to get all elements

0d285f2

Try using MutationObserve to watch for embed height changes

4778a3d

westonruter added 2 commits August 17, 2024 16:33

Introduce client-side Optimization Detective extensions and move Embe…

5f0cdbe

…d Optimizer JS into one

Override clientBoundingRect once embed has loaded

0ba2d6e

westonruter force-pushed the add/embed-optimizer-min-height-reservation branch from 363b50d to 0ba2d6e Compare August 17, 2024 23:49

westonruter and others added 2 commits August 17, 2024 18:03

Move jsdoc types to types.d.ts for reuse

5f4189d

Send URL metric when leaving the page

bf2b3c5

This is an iteration on top of #1098 Co-authored-by: swissspidy <[email protected]>

westonruter mentioned this pull request Aug 18, 2024

[POC] Performance Dashboard (Web Vitals) #1098

Draft

westonruter added 4 commits August 18, 2024 15:12

Use boundingClientRect instead of intersectionRect in get_all_element…

b9bad0d

…_minimum_heights

Eliminate timeout for disconneccting ResizeObsever

c6b02ec

Move extension initialization after idle callback

52a2260

Fix warning when prematurely applying buffered text replacements, esp…

edc52fa

…ecially to end-of-body

westonruter force-pushed the add/embed-optimizer-min-height-reservation branch from d10dd3c to edc52fa Compare August 18, 2024 22:12

Prepend min-height to style attribute instead of appending

820d66d

This was referenced Aug 19, 2024

Add fetchpriority=low to occluded initial-viewport images #1482

Draft

Capture userAgent in URL metric when debugging #1490

Closed

adamsilverstein reviewed Aug 21, 2024

View reviewed changes

swissspidy reviewed Aug 21, 2024

View reviewed changes

plugins/optimization-detective/detect.js Outdated Show resolved Hide resolved

westonruter mentioned this pull request Aug 21, 2024

Allow URL metric schema to be extended #1492

Merged

4 tasks

westonruter and others added 2 commits August 22, 2024 10:50

Use object spread

def2aab

Co-authored-by: swissspidy <[email protected]>

Merge branch 'trunk' of https://github.com/WordPress/performance into…

cd9a618

… add/embed-optimizer-min-height-reservation

westonruter added the blocked label Sep 6, 2024

westonruter added 3 commits September 12, 2024 17:14

Merge branch 'trunk' into add/embed-optimizer-min-height-reservation

02c4fd9

Use get_json_params() instead of get_params() so _wpnonce query param…

1da219f

… is excluded

Implement resizedBoundingClientRect extended property in schema

e34d9fe

westonruter commented Sep 17, 2024

View reviewed changes

plugins/embed-optimizer/detect.js

}

/**

* Initialize.

Copy link

Member Author

westonruter Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

* Initialize.

* Finalize.

westonruter added 2 commits September 18, 2024 15:49

Fix testing JSON request

5db6f54

Go back to get_params() by ignoring _wpnonce

0fa263a

This is needed because get_json_params() can return null. Also, no need to force the request body to be JSON. See 1da219f

westonruter mentioned this pull request Sep 19, 2024

Try using WP_HTML_Processor in Optimization Detective #1546

Draft

Merge branch 'trunk' into add/embed-optimizer-min-height-reservation

72b285d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Leverage URL metrics to reserve space for embeds to reduce CLS #1373

Leverage URL metrics to reserve space for embeds to reduce CLS #1373

westonruter commented Jul 17, 2024 •

edited

Loading

westonruter Jul 26, 2024

westonruter Jul 26, 2024

westonruter commented Jul 26, 2024 •

edited

Loading

westonruter commented Jul 26, 2024 •

edited

Loading

westonruter Aug 2, 2024

westonruter Aug 2, 2024

westonruter commented Aug 14, 2024

westonruter commented Aug 18, 2024 •

edited

Loading

adamsilverstein Aug 21, 2024

swissspidy Aug 21, 2024

adamsilverstein Aug 21, 2024

swissspidy Aug 21, 2024

westonruter Aug 21, 2024

swissspidy Aug 22, 2024

westonruter Aug 22, 2024

westonruter Aug 22, 2024

westonruter Sep 17, 2024

Leverage URL metrics to reserve space for embeds to reduce CLS #1373

Are you sure you want to change the base?

Leverage URL metrics to reserve space for embeds to reduce CLS #1373

Conversation

westonruter commented Jul 17, 2024 • edited Loading

To do

Demo Videos

Before

After

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Jul 26, 2024 • edited Loading

westonruter commented Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Aug 14, 2024

westonruter commented Aug 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Jul 17, 2024 •

edited

Loading

westonruter commented Jul 26, 2024 •

edited

Loading

westonruter commented Jul 26, 2024 •

edited

Loading

westonruter commented Aug 18, 2024 •

edited

Loading