#58894 closed enhancement (fixed)
Enhance `wp_get_loading_optimization_attributes()` to support arbitrary context values
Reported by: | flixos90 | Owned by: | mukesh27 |
---|---|---|---|
Milestone: | 6.4 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Media | Keywords: | has-patch, has-unit-tests, has-dev-note |
Focuses: | performance | Cc: |
Description (last modified by )
The wp_get_loading_optimization_attributes()
function, which was introduced in 6.3 (see #58235) but inherited most of its logic from the now deprecated wp_get_loading_attr_default()
function introduced in 5.5, relies on a $context
parameter based on which it may alter its behavior and the attributes returned. At the moment, it only supports context values used within WordPress core.
While this made sense for a first implementation, it is technically not required and could easily be expanded in several areas to support custom context values. Having to use WordPress core contexts is limiting today when relying on the function in plugins or themes, as at the moment any custom context used would result in incorrect handling of the attributes. This leads to the awkward situation where we currently have to recommend always using one of the core context values, even when the function is actually used in a different context (see e.g. the dev note post which recommends using the wp_get_attachment_image
context even though that function is never called in the example). An actual example for the problem is this theme pull request which, despite looking correct, would lead to incorrect behavior as of today.
We should abstract certain clauses in the function away from specific contexts so that the function can also be correctly used with custom contexts.
This ticket's implementation should be considered blocked by #58891 as making the changes before that would be unnecessarily complicated and error-prone.
Change History (18)
This ticket was mentioned in Slack in #core-media by antpb. View the logs.
14 months ago
This ticket was mentioned in Slack in #core-performance by pereirinha. View the logs.
14 months ago
#6
@
14 months ago
@flixos90, could you please provide more details about the problem you were trying to address with this ticket? I'm having difficulty fully understanding it. Thanks!
#7
@
14 months ago
@mukesh27 Currently, the logic in wp_get_loading_optimization_attributes()
is heavily tied to specific context values. For example, if you want to display a custom image in the header to be enforced without loading="lazy"
, you would need to pass the get_header_image_tag
context, even if the image is not in fact the header image. Or, if I display an image within the main query loop, it only works correctly if I use one of the predefined contexts like the_post_thumbnail
, even if I'm not displaying a featured image. So the limitation of specific contexts is confusing and inflexible, and mostly, unnecessary.
So technically speaking, this ticket is fundamentally about replacing the two big switch
clauses with more generally applicable logic that is mostly decoupled from specific contexts.
More specifically, here is what I would propose:
- Replace the switch clause for the early return if
doing_filter( 'the_content' )
with a more general clause: It should return early if the current context is anything other than 'the_content' whiledoing_filter( 'the_content' )
. - Replace the bigger switch clause with a set of
if elseif
clauses that ignores context, except for the "header enforced contexts":- Define a list with "header enforced contexts", which by default should only have
'template_part_' . WP_TEMPLATE_PART_AREA_HEADER
and'get_header_image_tag'
. If the current context is one of them, run the logic that is currently within that 1stcase
group. That could be the firstif
condition. - Then use the exact logic from the 2nd
case
group (i.e.if ( ! is_admin() && in_the_loop() && is_main_query() ) { ...
) aselseif
. - Then use the exact logic from the 3rd
case
group (i.e.if ( $wp_query->before_loop ...
) aselseif
.
- Define a list with "header enforced contexts", which by default should only have
That way, all the logic, except for the "header enforced contexts" one becomes context unaware, allowing more flexibility for plugins.
Something that needs to be ensured while making these changes is that all existing tests must still pass without modifications.
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
14 months ago
This ticket was mentioned in PR #5197 on WordPress/wordpress-develop by @mukesh27.
14 months ago
#10
- Keywords has-patch added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/58894
I have marked this pull request as a draft due to some test failures that occurred after implementing the changes.
It's crucial to ensure that all the existing tests continue to pass without any modifications while making these changes. As mentioned by @felixarntz earlier here, I am unable to update any unit tests, but the failures seem to be related to certain context values that need adjustment.
Here are lists of tests that going fails.
- test_wp_get_loading_optimization_attributes
- test_content_rendering_with_shortcodes
- test_content_rendering_with_shortcodes_nested
@mukesh27 commented on PR #5197:
14 months ago
#11
cc. @felixarntz @joemcgill
@mukesh27 commented on PR #5197:
14 months ago
#12
Thank you, @felixarntz, for your valuable review feedback. I have taken the necessary steps to address the comments, and the PR is now prepared for further review. cc. @joemcgill.
This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.
14 months ago
@flixos90 commented on PR #5197:
14 months ago
#16
Committed in https://core.trac.wordpress.org/changeset/56612
#17
@
14 months ago
- Keywords needs-dev-note added
The 4 tickets related to wp_get_loading_optimization_attributes()
should receive a single dev note post explaining those changes.
With [56347] committed, this effort is now unblocked. Please refer to the latest codebase for any changes to
wp_get_loading_optimization_attributes()
.