3.16: Special character is removed from LCP/ATF causing 404 and not excluding image from LL

[Mai-Saad]

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

• Made sure you’re on the latest version => 3.16-alpha1(commit e901296 (HEAD -> 6519-316-bug-front-end-optimizations-are-not-applied-even-when-lcpatf-data-is-in-the-db)
• Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug
beacon script is removing special char from image URL which will cause 404 for lcp and LL exclusion won't work for ATF

To Reproduce
Steps to reproduce the behavior:

1. install and activate 3.16
2. visit page template with images containing special char i.e https://new.rocketlabsqa.ovh/lcp_relative_url_withspecialchar_template
3. clear cache and visit the page => 404 is there for LCP image
4. enable LL and visit the page => lcp/atf are LL

Expected behavior
LCP/ATF aren't Lazyloaded and no 404 in console

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Add any other context about the problem here.

Acceptance Criteria (for WP Media team use only)
Clear instructions for developers, to be added before the grooming

[MathieuLamiot]

Might be related to #6339?

[Mai-Saad]

We can close 6339 now, As I think it's the same but was behaving differently while this one is in the latest state.

[MathieuLamiot]

@piotrbak about the expected behavior here, I see three possibilities:

• Keep the special characters so that the feature works as expected
  OR
• discard images with special characters from LCP candidates, so picking another one.
  OR
• consider there is no LCP if the one we find has special character.

While I think 1 is ideal, I am not sure it is doable while keeping the safeguarding (TBC by devs when grooming). In which case, can we consider one of the two other options? And which one would be prefered? Thanks

[piotrbak]

I'd say this can be edgy, but we shouldn't cause the current behaviour. I'd go directly with the 3rd option.

I'll ask for confirmation @DahmaniAdame and @benorfaz

[DahmaniAdame]

Definitely not option 2 as it will add the wrong image.

Ideally, option 1. But sounds like too much efforts + risk compromising/over complicating safeguarding data for something that won't be that common. The norm is not to use special characters when naming files.

Option 3 sounds good to me + reporting that the element had a special character as an error.

If we have too many complains, we can reconsider option 1 in the future.

[Khadreal]

I suggest we stick to option 1, option 3 is safer but what do we categorise as special characters ?
-https://atr-law.co.il/wp-content/uploads/2023/01/עדי-רידלמן-ראשי-ארוך-200x300.webp
-wp-content/themes/guardian2021/images/header-instagram@2x.png
We need to decide what's and what not if we would be going with option 3

[MathieuLamiot]

If there is an easy and safe way to go with 1, sure let's go there. From what I got, it is mostly about avoiding sanitization and just doing escaping? Discussion is ongoing here.

If we go with option 3, special characters would be anything removed by the sanitization function ; I don't think we need to precisely define them, they're embedded in WP core.

In any case, to avoid 404 I think we should consider:

• Get the LCP URL from the DB
• Escape it (or sanitize it, TBD)
• Compared the escaped version with the URL from DB
• If they are different, don't add the preload tag and add a debug log. If they are the same continue

[Khadreal]

Created a draft PR here