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

Lazy Loading Attribute is Not Omitted for First Image on a Page #45224

Closed
nickpagz opened this issue Oct 22, 2022 · 6 comments
Closed

Lazy Loading Attribute is Not Omitted for First Image on a Page #45224

nickpagz opened this issue Oct 22, 2022 · 6 comments
Labels
[Block] Image Affects the Image Block [Type] Bug An existing feature does not function as intended [Type] Performance Related to performance efforts [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.

Comments

@nickpagz
Copy link

nickpagz commented Oct 22, 2022

Description

The loading="lazy" attribute was added to all images in WordPress 5.5. In 5.9 a filter was added that removed the attribute for the first image on a page to help comply with the “Largest Contentful Paint image was lazily loaded” notification in Lighthouse. A full description on the issue here.

However, the filter doesn’t seem to work when an FSE theme is activated. With a classic theme, the first image does not have the attribute when viewed in the developer tools/css inspector, which is expected. When an FSE theme is activated, ALL images contain the attribute. This was tested on Twenty Twenty-Two and Block Base, and several other FSE based themes from the .org themes directory. Non-FSE themes tested included Twenty Twenty One, Twenty Twenty, and Twenty Nineteen.

Further, applying a filter such as mentioned in the 5.9 article on the subject, has no effect when used on FSE themes. The filter does work when a non-FSE theme is activated. Sample filter code:

function skip_lazyloading_on_first_x_images( $omit_threshold ) {
	return 3;
}
add_filter( 'wp_omit_loading_attr_threshold', 'skip_lazyloading_on_first_x_images' );

The filter:
add_filter( 'wp_lazy_loading_enabled', '__return_false' );
...to remove the "loading" attribute from all images continues to work as expected on both FSE and non-FSE themes.

After some digging, this behavior may be related to the core track ticket #55996, specifically comment #5 and #6. There's a potential fix in #44995 as a side effect, however that PR is specifically for resolving #37754 and may or may not resolve this issue.

EDIT: #56588 may be related, however this is specifically related to how the images are counted (ie counting images inside a post excerpt on an archive page).

Step-by-step reproduction instructions

  1. Activate an FSE theme such as Twenty Twenty-Two.
  2. Create a page with several images (at least two or three), set the page template to Blank (optional) and publish the page.
  3. Load the page in the browser, and using the css inspector in the browser developer tools, check the images' attributes for the loading="lazy" attribute. All images should have the tag included.
  4. Activate a non-FSE theme, such as Twenty Twenty-One.
  5. Again check the css inspector for the loading="lazy" attribute on all images. The first image on the page will not have the attribute, which is the expected behavior.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@kathrynwp kathrynwp added [Feature] Full Site Editing [Block] Image Affects the Image Block [Type] Enhancement A suggestion for improvement. labels Oct 24, 2022
@spacedmonkey
Copy link
Member

CC @felixarntz @adamsilverstein

@felixarntz
Copy link
Member

felixarntz commented Nov 2, 2022

@nickpagz @spacedmonkey I'll close this as a duplicate of #41783, as the underlying problem is the same.

My bad, they are different, just the conversation on the other looked as if they were related. Will reply on the other issue.

@felixarntz felixarntz added the [Status] Duplicate Used to indicate that a current issue matches an existing one and can be closed label Nov 2, 2022
@felixarntz felixarntz reopened this Nov 2, 2022
@felixarntz felixarntz added [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked. and removed [Status] Duplicate Used to indicate that a current issue matches an existing one and can be closed labels Nov 2, 2022
@felixarntz
Copy link
Member

@nickpagz @spacedmonkey This issue is effectively caused by https://core.trac.wordpress.org/ticket/55996, and there is a Trac ticket https://core.trac.wordpress.org/ticket/56930 for the same problem too.

There is already a working PR to address this for core: WordPress/wordpress-develop#3549

Not sure if we would need a separate (or the same) fix for Gutenberg.

@felixarntz felixarntz added [Type] Bug An existing feature does not function as intended and removed [Type] Enhancement A suggestion for improvement. labels Nov 2, 2022
@nickpagz
Copy link
Author

nickpagz commented Nov 2, 2022

Not sure if we would need a separate (or the same) fix for Gutenberg.

Agreed. If it's addressed in core then this can probably be closed in favor of https://core.trac.wordpress.org/ticket/56930

@adamsilverstein
Copy link
Member

@nickpagz - core 56930 is now merged: can you please verify this fixes the issue you noted here and if so close this ticket?

@nickpagz
Copy link
Author

Thanks @adamsilverstein
Checked on 6.2-beta2 and working now as expected. Closing the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block [Type] Bug An existing feature does not function as intended [Type] Performance Related to performance efforts [Type] WP Core Ticket Requires an upstream change from WordPress. Core Trac ticket should be linked.
Projects
None yet
Development

No branches or pull requests

5 participants