Fetchpriority of LCP is not applied on the LCP markup when the LCP URL is relative

[MathieuLamiot]

Before submitting an issue please check that you’ve completed the following steps:

• Made sure you’re on the latest version: Commit 1b6f1972a78649e317463b5c5eb8b2e3574052b5
• Used the search feature to ensure that the bug hasn’t been reported before ✅

Describe the bug The LCP feature should add the fetchpriority attribute to the LCP markup. When the LCP markup contains a relative URL, this is not done. Note that the preload markup is correctly added with an absolute URL and the fetchpriority.

To Reproduce Steps to reproduce the behavior:

1. Make sure to have LCP feature enabled (by default, it is).
2. Clear the cache and critical images.
3. Manually go to xxx.e2e.rocketlabsqa.ovh/lcp_regular_image_template/ in incognito. Make sure that the LCP/ATF table is filled with information from this page in the DB. The LCP image should be wp-content/rocket-test-data/images/600px-Mapang-test.gif
4. Purge this URL
5. Re-load the page.
6. Check in the dev tools the markup of wp-content/rocket-test-data/images/600px-Mapang-test.gif image. There is no fetchpriority attribute.

If you do the same on http://xxx.e2e.rocketlabsqa.ovh/landing-page/, the attribute is correctly applied.

I think the difference is the absolute/relative paths.

Note that we have similar issues when a CDN is being used: if the URL in the DB contains the CDN hostname, then the fetchpriority is not applied as expected when serving the page.

Expected behavior

• Fetchpriority attribute must be set to the LCP markup when the LCP markup contains an absolute URL ✅
• Fetchpriority attribute must be set to the LCP markup when the LCP markup contains a relative URL 🔴
• Built-in tests should cover those cases. If it make sense to tackle this in the same issue (nice to have, otherwise, create a dedicated GH issue):
• Fetchpriority attribute must be set to the LCP markup when the a CDN is used and the URL stored in the LCP/ATF table as LCP contains the CDN URL.

Additional context The issue seems to be with set_fetchpriority which performs a regex based on the URL we have in the DB. Mismatch between relative/absolute/CDN hostname make the regex fail?

Acceptance Criteria

• Repeat the How to reproduce on the two templates: /lcp_regular_image_template/ & /landing-page/. In both cases, the fetchpriority attribute must be set properly to the LCP markup.
• NRT with Rocket-E2E
• Tests (unit or integration) covers the relative path case and the absolute path case. If dealt with in this PR, the same ACs must be checked with the CDN feature enabled and LCP stored in DB with the CDN hostname.

[Khadreal]

Scope a solution ✅

Engine\Media\AboveTheFold\WarmUp::Controller

• In set_fetchpriority https://github.com/wp-media/wp-rocket/blob/cccbadf1f71561a5621926cfa42ce1d146426ce5/inc/Engine/Media/AboveTheFold/Frontend/Controller.php#L151 the url passed here is absolute url but the image url in html is relative hence why this would always be false in scenario like this.

We need to change the either change the pattern to cover both instance which I'm struggling with. However, I came up with the approach below, DOMDocument() error of invalid entity will fill the debug log except we suppress it which is not ideal.

$dom = new DOMDocument();
$dom->loadHTML( $html );
$images = $dom->getElementsByTagName('img');
$lcp_image_url = $lcp->src;
foreach ($images as $image) {
	$imageUrl = $image->getAttribute('src');
		if (strpos($imageUrl, 'http') === 0 && strpos( $lcp_image_url, 'http' ) === 0) {
			$parsedImageUrl = parse_url( $imageUrl );
			$parsedAbsoluteUrl = parse_url( $lcp_image_url );
			if ( $parsedImageUrl['path'] === $parsedAbsoluteUrl['path'] ) {
				$lcp_image_url =  $imageUrl;
			}
		} else {
			if ( $imageUrl === str_replace( parse_url($lcp->src, PHP_URL_HOST ), '', $lcp_image_url ) ) {
			   $lcp_image_url =  $imageUrl;
			}
		}
	}

$url  = preg_quote( $lcp_image_url, '/' );

• Add test

Estimate the effort ✅

[S]

[wordpressfan]

I really like using DOMDocument but, In the past we used DOMDocument and that was not the best approach and from performance wise it takes more time than regex.

I think converting the url into relative one and adjust the regex a bit to accept any character before the relative url will do the trick.

What do u think @Khadreal ?

[Khadreal]

Regex would be more suitable, just couldn't it to work. I tried turning the URL to relative but the result weren't great. For example image url like this will fail http://1.gravatar.com/avatar/d7a973c7dab26985da5f961be7b74480?s=120&d=mm&r=g.

[Khadreal]

Because of the issue raise in the first solution, I came up with some pointers from @wordpressfan. I created a draft PR. I'll add test foe different cases to make sure it doesn't regress from what we have before now.

$url  = preg_quote( $lcp->src, '/' );
if( ! $this->is_external_file( $lcp->src ) ) {
	$url = preg_quote(
		preg_replace( '#^(://|[^/])+#', '', $lcp->src ),
		'/'
	);
}