Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Image URL with special character isnot handled correctly with LCP/ATF #6339

Closed
Mai-Saad opened this issue Dec 18, 2023 · 7 comments
Closed
Labels
lcp type: bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@Mai-Saad
Copy link

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

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

Describe the bug
While having LCP/ATF image with a special character, the image is not excluded from lazyload and is not have fetchpriority="high" beside image markup when it is LCP

To Reproduce
Steps to reproduce the behavior:

  1. Having page template with image containing special char like <img alt="Nature" src='https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/kotł.PNG' >
  2. Visit the page => fetchpriority='high' added in the head but not in the image markup
  3. Enable lazyload and visit the page => image is not excluded from lazyload

Expected behavior
Fetchpriority='high' added in image markup and image excluded from LL

Screenshots
If applicable, add screenshots to help explain your problem.
Screenshot from 2023-12-18 10-28-45
Screenshot from 2023-12-18 10-28-35

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

@Mai-Saad Mai-Saad added type: bug Indicates an unexpected problem or unintended behavior lcp labels Dec 18, 2023
@Mai-Saad Mai-Saad added this to the 3.16 milestone Dec 18, 2023
@Khadreal Khadreal self-assigned this Mar 5, 2024
@piotrbak
Copy link
Contributor

piotrbak commented Mar 7, 2024

We need to make sure this will also happen on the new version. As it's leftover from the previous project, but it looks like it might happen here too.

@MathieuLamiot How you'd handle this? I think it should be checked and fixed at the end, actually.

@MathieuLamiot
Copy link
Contributor

If the issue can be reproduced and is linked to applying the optimization from the DB to the HTML, then it can be done now.
I think there is no other task that can be picked up now, as the ones in the next sprint are all "blocked" wanting for an on-going ticket to be completed before getting started (If I did not miss anything).

@wordpressfan
Copy link
Contributor

Checking this one now...

@MathieuLamiot
Copy link
Contributor

@wordpressfan @Khadreal, is it possible to reproduce and investigate currently or do you prefer to block it for now as suggested yesterday, waiting for a more complete version to be able to test?

@wordpressfan
Copy link
Contributor

OK, let me move my comment from slack here for reference:

I can see that fetchpriority in both locations by doing the following steps:

  • Create a page with a template that has the following image:
<img src="[https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/kotł.PNG](https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/kot%C5%82.PNG)" alt="Nature">
  • Open the database and reach the row for this new page url then manually update the status to be completed and the lcp column to be:
{
                    "type": "img",
                    "src": "[https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/kotł.PNG](https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/kot%C5%82.PNG)",
                    "srcset": "",
                    "sizes": "",
                    "sources": [],
                    "bg_set": [],
                    "current_src": "[https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/kotł.PNG](https://new.rocketlabsqa.ovh/wp-content/rocket-test-data/images/kot%C5%82.PNG)"
                }
  • Clear cache
  • Visit the page
  • You will see fetchpriority as below:

image

So if the data is saved correctly into the database, this fetchpriority will be added in both locations, at this point we can't validate the other part of saving the data into the database so I believe this should be blocked till we have a working version in both directions (saving and retrieving from the DB)

@MathieuLamiot
Copy link
Contributor

Crystal clear, thanks 🙏

@MathieuLamiot
Copy link
Contributor

Based on @wordpressfan description and @Mai-Saad's investigation here, this is closed as a duplicate of #6524

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lcp type: bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

5 participants