Make WordPress Core

Opened 11 months ago

Closed 9 months ago

#58770 closed defect (bug) (fixed)

Fetch priority and lazy loading attributes not applying correctly in block themes

Reported by: spacedmonkey's profile spacedmonkey Owned by: spacedmonkey's profile spacedmonkey
Milestone: 6.4 Priority: normal
Severity: normal Version: 5.9
Component: Media Keywords: has-patch
Focuses: performance Cc:

Description

Images in block themes are not applying the image attributes correctly. While testing the following behaviours were noticed.

  • Site logos in header of page, having loading="lazy".
  • Image in header, not having fetch priority = high. Also not no lazy loading image.
  • Image block hardcode into patterns not either loading and fetch priority attributes.

Steps to replicate.

  • Activate TT2 theme.

This error has also been spotted in the following block themes.

Attachments (3)

Screenshot 2023-07-10 at 16.03.09.png (1.0 MB) - added by spacedmonkey 11 months ago.
TT2, logo with lazy loading and header image with no attributes.
Screenshot 2023-07-10 at 16.05.57.png (1.3 MB) - added by spacedmonkey 11 months ago.
Frost theme - no lazily loading attributes.
Screenshot 2023-07-10 at 15.57.11.png (2.4 MB) - added by spacedmonkey 11 months ago.
Neve FSE theme- no lazy loading attribute.

Change History (14)

@spacedmonkey
11 months ago

TT2, logo with lazy loading and header image with no attributes.

@spacedmonkey
11 months ago

Frost theme - no lazily loading attributes.

@spacedmonkey
11 months ago

Neve FSE theme- no lazy loading attribute.

#1 @spacedmonkey
11 months ago

For extra context, this ticket was first discussed on slack - https://wordpress.slack.com/archives/C02KGN5K076/p1688723359648689

#2 @flixos90
11 months ago

  • Milestone changed from Awaiting Review to 6.4
  • Owner set to flixos90
  • Status changed from new to assigned
  • Version changed from 5.5 to 5.9

Thanks @spacedmonkey! Can you share a bit more context on the individual problems you saw? For example, for each theme that you tested:

  • How are the images included that have missing or incorrect loading optimization attributes?
  • Are the attributes not applied at all? Or are the images being lazy-loaded when they shouldn't? Or receiving fetchpriority="high" when they shouldn't?

I believe there are multiple underlying issues and different problems, so we need to inspect the individual scenarios more closely to figure those out.

Milestoning this for 6.4 for now, but we can adjust as needed once we know more. Also updating "Version" to 5.9 since only then block themes and the lazy-loading enhancements were introduced.

#3 @spacedmonkey
11 months ago

@flixos90 The screenshots, so the what is happening, so I would look at those.

I believe there are two issues here.

  1. The same image is being processed a number of times, as different filters are applied at different times. I believe the same image might be being processed 2 or 4 times.
  2. Hardcoded images in the templates do not have height and width attributes on the image blocks.

Here is an example of where there are hardcoded images in the TT2 theme. And another example from frost theme.

This ticket was mentioned in PR #4855 on WordPress/wordpress-develop by @spacedmonkey.


10 months ago
#4

  • Keywords has-patch added

@mukesh27 commented on PR #4855:


9 months ago
#6

Code that run only single time:

static $themes_cache = null;

if ( null === $themes_cache ) {
	$stylesheet = get_stylesheet();
	$themes_cache = array(
		$stylesheet => get_stylesheet_directory(),
	);

	$template = get_template();
	if ( $stylesheet !== $template ) {
		$themes_cache[ $template ] = get_template_directory();
	}
}

cc. @felixarntz

#7 @mukesh27
9 months ago

  • Keywords changes-requested added

Thanks @spacedmonkey and @thekt12 for the PRs. I left some feedback on both PRs for consideration.

@thekt12 your PR could potentially impact backward compatibility.

Last edited 9 months ago by mukesh27 (previous) (diff)

@spacedmonkey commented on PR #4855:


9 months ago
#8

Caching that function call is not a good idea. It has a filter in it. That would result in the filter not running.

@mukesh27 commented on PR #4855:


9 months ago
#9

Thank you for the feedback. Is it possible to have distinct templates for various contexts, or will these modifications by the filters in place?

#10 @flixos90
9 months ago

  • Owner changed from flixos90 to spacedmonkey

After another review of this ticket, I am unsure about the next steps:

  • I cannot replicate the issue reported about the Site Logo. Adding a Site Logo in TT2, I see it correctly receive fetchpriority="high" if it's configured to be large enough, and it doesn't receive loading="lazy". Potentially the report was made before WP 6.3 had implemented relevant fixes?
  • Regarding the images hard-coded in the theme, I am unsure how we can fix the issue holistically, other than developer education. For Twenty Twenty-Two specifically, I opened a standalone issue to focus on only that, see #59256. The same thing happens in the other referenced themes, but fixing that in core would involve some sort of extra parsing pipeline of block templates, to then load the image file referenced to extract the dimensions and put them into the template - which would probably be an effort too large to achieve for 6.4 (we have less than 4 weeks at this point).

Let me know what you think @spacedmonkey. Realistically, I think we'll have to either close this issue (if we decide it's nothing to do for core) or punt it to "Future Release" (if we want to implement a central solution for core).

#11 @spacedmonkey
9 months ago

  • Keywords changes-requested removed
  • Resolution set to fixed
  • Status changed from assigned to closed

I agree with @flixos90 . Now that other fixes are in place, I am now seeing fp on site logos. I think we should follow up with #59256 and a dev note to educate developers.

I am closing this ticket as there is nothing to action here.

Note: See TracTickets for help on using tickets.