Make WordPress Core

Opened 11 months ago

Closed 11 months ago

Last modified 10 months ago

#58680 closed defect (bug) (fixed)

Header image does not add fetch priority attribute

Reported by: spacedmonkey's profile spacedmonkey Owned by: flixos90's profile flixos90
Milestone: 6.3 Priority: normal
Severity: normal Version: 6.3
Component: Media Keywords: has-patch, has-unit-tests, has-dev-note
Focuses: performance Cc:

Description (last modified by spacedmonkey)

Added in #58235 ( [56037] ), fetch priority was added so add fetch priority attribute to large images at the start of the page. However, the custom header functionality built in WordPress support not support adding this attribute.

Custom header images, ( as an image tag ) are used by many of the core teams, including

Twenty Eleven
Twenty Fourteen
Twenty sixteen
Twenty Ten
Twenty Twelve
Twenty Seventeen

It is also used in a number of themes on the repo, using these functions.
the_custom_header_markup
get_custom_header_markup
get_header_image_tag
get_header_image
the_header_image

Attachments (1)

Screenshot 2023-06-30 at 10.40.20.png (2.9 MB) - added by spacedmonkey 11 months ago.

Change History (21)

#1 @spacedmonkey
11 months ago

  • Description modified (diff)

For extra context, the following core themes, do you use either the_custom_header_markup or the_header_image header functions. There are hardcode the image tags in the template.

Twenty Eleven
Twenty Fourteen
Twenty sixteen
Twenty Ten
Twenty Twelve

Retro fitting these themes might be possible. But some of these theme support older version of WP, where the the_header_image function does exist ( Add in WP 4.4 ).

I have already created a ticket for retro fitting Twenty sixteen at #58675.

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


11 months ago
#2

  • Keywords has-patch added

#3 @spacedmonkey
11 months ago

  • Keywords has-patch removed

I put together a quick POC of using the get_header_image_tag and wp_filter_content_tags callback to add the missing attributes. However, in my testing, it added async and lazy loading. Async is fine to add, but adding lazy this large header image does make a lot of sense. See screen.

https://github.com/WordPress/wordpress-develop/pull/4764

#4 @spacedmonkey
11 months ago

  • Keywords has-patch needs-unit-tests added
  • Owner set to flixos90
  • Status changed from new to assigned

#5 @flixos90
11 months ago

  • Milestone changed from Future Release to 6.3
  • Priority changed from high to normal

Adding this to the 6.3 milestone since it's a follow up bug to fix. Probably not necessarily high priority though.

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


11 months ago
#7

  • Keywords has-unit-tests added; needs-unit-tests removed

This PR contains the changes made by @felixarntz in #4772 plus the decoding=async attribute and unit tests.

Trac ticket: https://core.trac.wordpress.org/ticket/58680

@mukesh27 commented on PR #4790:


11 months ago
#8

Per https://github.com/WordPress/wordpress-develop/pull/4790#pullrequestreview-1514206718 feedback do we need to open separate ticket for that?

cc. @felixarntz @spacedmonkey

#9 @mukesh27
11 months ago

  • Keywords changes-requested added

#10 @spacedmonkey
11 months ago

To fix the legacy themes, we have to update the bundled themes. I have created #58675 to handle this.

For now, to fix the current fix, please test with 2017 theme.

@spacedmonkey commented on PR #4790:


11 months ago
#11

Per #4790 (review) feedback do we need to open separate ticket for that?

cc. @felixarntz @spacedmonkey

@mukeshpanchal27 Let's handle this in https://core.trac.wordpress.org/ticket/58675 and https://github.com/WordPress/wordpress-develop/pull/4797

@flixos90 commented on PR #4772:


11 months ago
#12

Closing in favor of #4790.

#13 @flixos90
11 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 56142:

Media: Ensure custom header image tag supports loading optimization attributes.

This changeset is a follow up to [56037] and ensures that the get_header_image_tag() function receives the benefits of wp_get_loading_optimization_attributes() as well. Prior to fetchpriority support, this was not needed since the header image should never be lazy-loaded, but the image certainly is a fetchpriority candidate, so therefore it is crucial to have it supported.

Props felipeelia, spacedmonkey, mukesh27, westonruter, flixos90.
Fixes #58680.

#14 @flixos90
11 months ago

  • Keywords needs-dev-note added; changes-requested removed

#16 @spacedmonkey
11 months ago

@flixos90 It worth noting that there are many existing themes that will not be fixed by this change. See https://www.wpdirectory.net/search/01H4M2MGZ49GQ49BF82EGJCGY9.

Not sure what we can do about this, as it requires opt-in from the theme developer.

#17 @flixos90
11 months ago

@spacedmonkey I think all we can do here is document this and hope that theme developers read it. I'm going to add a paragraph to the dev note draft about this.

The recommendation would be to use get_header_image_tag(), but if the theme developer for some reason wants to use get_header_image(), they will need to manually call wp_get_loading_optimization_attributes().

This ticket was mentioned in Slack in #hosting by javier. View the logs.


11 months ago

#20 @flixos90
10 months ago

  • Keywords needs-dev-note removed
Note: See TracTickets for help on using tickets.