XPaths for elements in URL Metrics can omit index for children of BODY to avoid needing to vary URL Metrics by whether a user is logged-in

[westonruter]

Discussed initially at #1759 (comment)

URL Metrics currently are varied by whether or not a user is logged-in. This is all of the XPaths used on the page differ based on whether or not the admin bar is present, since the element index is included with each element reference. For example, the following is the XPath for the first IMG in post content when logged-out vs logged-in:

- /*[1][self::HTML]/*[2][self::BODY]/*[1][self::DIV]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::ARTICLE]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::FIGURE]/*[1][self::PICTURE]/*[2][self::IMG]
+ /*[1][self::HTML]/*[2][self::BODY]/*[3][self::DIV]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::ARTICLE]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::FIGURE]/*[1][self::PICTURE]/*[2][self::IMG]

The difference is that when logged-in, two new elements appear at the beginning of the BODY:

1. A SCRIPT coming from wp_customize_support_script() (for admin users).
2. A DIV#wpadminbar for the admin bar.

These result in the page wrapper DIV's XPath going *[1][self::DIV] to *[3][self::DIV].

The purpose of the element index in the XPath is to differentiate one element from another. If there are 3 IMG tags appearing next to each other in a post, and only the first is LCP, then it's important to be able to uniquely reference that IMG. Nevertheless, at the root level of a page (when directly under the BODY) it is highly unlikely for there to be any such ambiguity. This is because themes almost always wrap the content between wp_body_open() and wp_footer(). Classic themes typically wrap the content in a DIV#page element, whereas Block Themes always wrap the content in a DIV.wp-site-blocks element. The only exception in core is that the Twenty Twenty theme which does not have one root wrapper but instead has a HEADER, MAIN, and FOOTER at the root. In this case too there is no possibility for ambiguity since these three are unique.

So I suggest we do the following:

1. Eliminate this logic from od_get_normalized_query_vars():

performance/plugins/optimization-detective/storage/data.php

Lines 80 to 83 in b6e5c5a

	// Vary URL Metrics by whether the user is logged in since additional elements may be present.
	if ( is_user_logged_in() ) {
	$normalized_query_vars['user_logged_in'] = true;
	}

2. Omit the element index from the HTML, BODY, and children directly under the BODY. So the XPaths above would become the following:

/HTML/BODY/DIV/*[2][self::DIV]/*[1][self::DIV]/*[1][self::DIV]/*[1][self::ARTICLE]/*[2][self::DIV]/*[1][self::DIV]/*[1][self::FIGURE]/*[1][self::PICTURE]/*[2][self::IMG]

3. Eliminate the restriction that Optimization Detective be disabled for admins. This was put in place because additional scripts often get added at wp_body_open() (e.g. the wp_customize_support_script()). With the presence/absence of such elements printed at wp_body_open (and wp_footer) eliminated, we can eliminate this code from od_can_optimize_response() which would resolve Optimization Detective should be enabled by default for admins #1425:

performance/plugins/optimization-detective/optimization.php

Lines 121 to 126 in b6e5c5a

	// The aim is to optimize pages for the majority of site visitors, not for those who administer the site, unless
	// in 'plugin' development mode. For admin users, additional elements will be present, like the script from
	// wp_customize_support_script(), which will interfere with the XPath indices. Note that
	// od_get_normalized_query_vars() is varied by is_user_logged_in(), so membership sites and e-commerce sites
	// will still be able to be optimized for their normal visitors.
	( current_user_can( 'customize' ) && ! wp_is_development_mode( 'plugin' ) ) ||

4. During this transitionary period in which old URL Metrics still contain the old XPaths, we can patch OD_Element to convert to this new format so existing optimizations will continue to apply based on old URL Metrics:

--- a/plugins/optimization-detective/class-od-element.php
+++ b/plugins/optimization-detective/class-od-element.php
@@ -52,6 +52,17 @@ class OD_Element implements ArrayAccess, JsonSerializable {
 	 */
 	public function __construct( array $data, OD_URL_Metric $url_metric ) {
 		$this->data       = $data;
+
+		// Convert old XPath scheme.
+		$xpath = preg_replace(
+			'#^/\*\[1\]\[self::HTML\]/\*\[2\]\[self::BODY\]/\*\[\d+\]\[self::(DIV)\]#',
+			'/HTML/BODY/$1',
+			$this->data['xpath']
+		);
+		if ( is_string( $xpath ) && '' !== $xpath ) {
+			$this->data['xpath'] = $xpath;
+		}
+
 		$this->url_metric = $url_metric;
 	}

Then in the next release or two we can eliminate this code since the old URL Metrics should have all been purged by then.

[westonruter]

@felixarntz @adamsilverstein @swissspidy Any concerns with this?

[swissspidy]

Makes sense to me 👍

[felixarntz]

SGTM as well!

[westonruter]

There are four sequential PRs ready for review which implement the changes here:

1. Eliminate varying URL Metrics by logged-in state and discontinue disabling optimization by default for admins #1788
2. Improve snapshot testing for Optimization Detective #1791
3. Wrap contents of BODY in DIV tag in tests related to Optimization Detective #1793
4. Omit element node index in XPaths up to children of BODY #1790

I've broken up the changes into these four PRs because the changesets are massive. Nevertheless, the actual code to review in the PRs is small since the other changes are merely reorganization of test files, regeneration of snapshots, and replacement of XPaths.