Consider whether additional disambiguating attributes should be added as XPath predicates

[westonruter]

First of all, see the plugin's documentation about the XPath format.

A typical XPath generated for a tag looks like the following:

/HTML/BODY/DIV[@class='wp-site-blocks']/*[1][self::HEADER]/*[1][self::DIV]/*[2][self::IMG]

Note that /HTML and /HTML/BODY lack any predicates because there is no possibility for ambiguity in an HTML document at these levels. When it comes to descendants of the BODY, there is the possibility of ambiguity since there can be any number of children of any type. In the case of direct descendants of the BODY, the predicate consists of the id, role, or class attribute since at wp_body_open any number of tags may be output which causes the node index to change, resulting in /HTML/BODY/DIV[@class='wp-site-blocks']. Below this level, however, there is more consistency. So to ensure that an IMG captured during detection remains the same IMG and isn't moved around in the document due to a paragraph being added before, for example, nodes further below are selected specifying the node index with the tag name being another predicate, like *[2][self::IMG]. This XPath will stop matching a given IMG if a preceding tag is added or removed. This helps ensure that optimizations don't get incorrectly applied.

Nevertheless, there are a couple cases I'm aware of where this breaks down.

1. What if someone adds a huge image IMG to a page and it is the LCP element, and then detection occurs. If afterward, this IMG is replaced with a tiny image, the result can be that the IMG may erroneously get fetchpriority=high added to it. (Aside: One option would be to not do any optimization if there are stale URL Metrics according to the ETag, since a post update to change the image would cause the ETag to change.)
2. The Gallery block has a "Randomize Order" option. While I'm not sure how popular this option is, it can cause the wrong optimizations to occur. For example, consider if the portrait image (the white stag) is the LCP element since it is the largest, in the following three page loads the order changes:

1	2	3

This means that an XPath which identified one IMG as LCP for one page load may incorrectly try to optimize the IMG in the same document position as LCP for the next page load.

This issue is something I also encountered as a possibility while working on the Intrinsic Dimensions plugin. If there is a Gallery of images which lack dimensions, for example, and all the images are presented in a random order, then the intrinsic dimensions captured for one IMG may incorrectly be applied to a different IMG which appears in the same document position with the next page load. To address this, I explored introducing a "srcHash" which takes the src and srcset for an image and turns it into an MD5 hash. This hash is then stored in the URL Metric with the element along with the intrinsic width and height. When the tag visitor optimizes an IMG without dimensions, it checks to see if the current srcHash matches the srcHash stored in the URL Metric. If not, it aborts.

One way to deal with all of these issues would be to include the src (and srcset?) of the IMG in its XPath. This would prevent optimizations from erroneously applying to wrong element. (This is also relevant to VIDEO which can also have a src, but which also can have SOURCE elements instead.) For images, an alternative would be to include the class name in the XPath instead.

My suspicion is that the above scenarios are edge cases that most of the time wouldn't cause an issue. I'm not sure they are significant enough to warrant safeguards, but I wanted to raise the scenario for consideration.

[westonruter]

See #1797 for previous work disambiguating children of the BODY.

If we add additional disambiguating attribute predicates, then we should store them but not yet expose them to tag visitors until new URL Metrics have been gathered, at which time they can be exposed à la #1820.

[westonruter]

One way to mitigate this would be to only consider an element to be LCP if it is reported as such by all URL Metrics in a group. Namely, OD_URL_Metric_Group::get_lcp_element() was updated in #1903 to only consider URL Metrics which have a current ETag if there is one. Going beyond this, the logic could be simplified so that instead of checking which element is the LCP most of the time (by counting the number of times an element is seen), it instead could instead return null if any of the URL Metrics have an LCP element with a differing XPath. This would bring the behavior more in line with Image Prioritizer in how it handles prioritizing background images which aren't defined inline: Image_Prioritizer_Background_Image_Styled_Tag_Visitor::get_common_lcp_element_external_background_image().

The only downside with doing this is that there could be two sizes of mobile devices, one small and one large, but both which are in the smallest viewport group of URL Metrics. If on the smaller mobile device the H1 is the LCP element but on the larger mobile device the IMG is the LCP element, then this would mean no preload link would be added for the mobile viewport. So by deciding on the common LCP element based on the majority of URL Metrics, if two of the URL Metrics were collected by larger mobile clients and two were collected by smaller mobile clients, then the IMG would end up getting prioritized. This is good if it so happens that the majority of the visitors are on larger mobile devices, but it also presumes that the sample of URL Metrics just so happened to be collected from larger mobile devices. It could also very well be that be by chance that smaller mobile devices happened to be the visitors from which URL Metrics are collected. So it's not reliable to depend on the majority opinion, given the sample size is so small by default (3).

Therefore, if we find that there is a significant difference in LCP elements between small mobile and large mobile, maybe we should actually introduce a new breakpoint. Here's an example of this for two mobile devices:

Moto G4: LCP H1	Pixel 7: LCP IMG

I cheated a bit with this screenshot by breaking up the initial paragraph into two via local overrides, because in actuality on Pixel 7 and other larger devices like iPhone 12 Pro report the initial P element as LCP. So in this case, should the IMG be prioritized with fetchpriority=high even though the LCP element is text? It seems perhaps so because it would mean this extremely important image would load sooner, but we wouldn't want to somehow cause a delay in loading fonts, for example.