Fix first image lazy loading with block themes #3538
Conversation
Disable wp_lazy_loading_enabled by default when context is false
| // See https://html.spec.whatwg.org/multipage/embedded-content.html#attr-img-loading | ||
| // See https://html.spec.whatwg.org/multipage/iframe-embed-object.html#attr-iframe-loading | ||
| $default = ( 'img' === $tag_name || 'iframe' === $tag_name ); | ||
| $default = false !== $context && ( 'img' === $tag_name || 'iframe' === $tag_name ); |
There was a problem hiding this comment.
There are several places we could adjust for block theme behavior, what do you think @felixarntz?
There was a problem hiding this comment.
@adamsilverstein When is the $context false here? Does the post content in a block theme not go through the_content?
There was a problem hiding this comment.
when a block theme is enabled, the_content filter is applied to the content with $context false which appears to happen before the main application of the filter. I'm going to dig a bit further to see why that is happening, maybe we can avoid it in the first place.
There was a problem hiding this comment.
Probably this:
There was a problem hiding this comment.
@adamsilverstein Hmm that code doesn't look odd to me. The $context is never explicitly specified in that filter, it uses current_filter() by default, which means here it should be set to the_content. This would only be a problem if some core code called wp_filter_content_tags() directly without the parameter (i.e. outside of a filter).
There was a problem hiding this comment.
indeed I verified that isn't the problem, I'm going to dig in more to trace back and find where the false context value is coming from.
There was a problem hiding this comment.
@felixarntz I dug in a bit further here...
wp_filter_content_tags is called several times by block themes in two places
: in src/wp-includes/blocks/template-part.php::render_block_core_template_part with no context and current_filter returning false.
Removing this call fixed the issue (but I don't think we can do that):
note: also called in src/wp-includes/block-template.php in get_the_block_template_html:
I considered some other fixes but the check in the current PR seemed the most reliable, what do you think?
There was a problem hiding this comment.
Sample trace:
wp_filter_content_tags (/wp-includes/media.php:1782)
render_block_core_template_part (/wp-includes/blocks/template-part.php:137)
WP_Block->render (/wp-includes/class-wp-block.php:256)
render_block (/wp-includes/blocks.php:1051)
do_blocks (/wp-includes/blocks.php:1089)
get_the_block_template_html (/wp-includes/block-template.php:240)
include (/wp-includes/template-canvas.php:12)
require_once (/wp-includes/template-loader.php:106)
require (/wp-blog-header.php:19)
{main} (/index.php:17)
|
@adamsilverstein I gave this another look, but I think #3560 is a bit of a cleaner solution as it relies on providing new contexts where they're missing today, and also doesn't disable lazy-loading completely for these contexts. The resulting behavior today would be the same, but as we are likely to further enhance the lazy-loading default heuristics later, I think it's better to handle all that in the |
|
Closing in favor of #3560. |
2 participants
wp_lazy_loading_enabledby default when context is false. Prevents counting of the fist block theme content pass which resulted in actual content filtering setting all content images loading attribute tolazyTesting instructions:
twentytwentythreethemeloading="lazy"attributeloading="lazy"attributetwentyninetween) to verify the same expected behaviorTrac ticket: https://core.trac.wordpress.org/ticket/56930
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.