<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[56347] trunk: Media: Simplify logic in `wp_get_loading_optimization_attributes()`.</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { white-space: pre-line; overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta" style="font-size: 105%">
<dt style="float: left; width: 6em; font-weight: bold">Revision</dt> <dd><a style="font-weight: bold" href="https://core.trac.wordpress.org/changeset/56347">56347</a><script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","description":"Review this Commit","action":{"@type":"ViewAction","url":"https://core.trac.wordpress.org/changeset/56347","name":"Review Commit"}}</script></dd>
<dt style="float: left; width: 6em; font-weight: bold">Author</dt> <dd>flixos90</dd>
<dt style="float: left; width: 6em; font-weight: bold">Date</dt> <dd>2023-08-02 17:56:16 +0000 (Wed, 02 Aug 2023)</dd>
</dl>
<pre style='padding-left: 1em; margin: 2em 0; border-left: 2px solid #ccc; line-height: 1.25; font-size: 105%; font-family: sans-serif'>Media: Simplify logic in `wp_get_loading_optimization_attributes()`.
While the `wp_get_loading_optimization_attributes()` function was only recently introduced in 6.3, its code was mostly ported over from the now deprecated `wp_get_loading_attr_default()` function introduced in 5.5.
That function started out in a simple way, but over time was expanded with more and more conditionals on when to avoid lazy-loading, which ended up making the logic extremely complex and hard to follow.
This changeset refactors the logic to simplify it, in a way that allows to follow it more sequentially, and without making any functional changes, ensuring that the extensive existing unit test coverage still passes. This will facilitate future enhancements to the function to be less error-prone and make it more accessible to new contributors.
Props flixos90, joemcgill.
Fixes <a href="https://core.trac.wordpress.org/ticket/58891">#58891</a>.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunksrcwpincludesmediaphp">trunk/src/wp-includes/media.php</a></li>
<li><a href="#trunktestsphpunittestsmediaphp">trunk/tests/phpunit/tests/media.php</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunksrcwpincludesmediaphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/src/wp-includes/media.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/src/wp-includes/media.php 2023-08-02 10:57:10 UTC (rev 56346)
+++ trunk/src/wp-includes/media.php 2023-08-02 17:56:16 UTC (rev 56347)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -5604,33 +5604,6 @@
</span><span class="cx" style="display: block; padding: 0 10px"> function wp_get_loading_optimization_attributes( $tag_name, $attr, $context ) {
</span><span class="cx" style="display: block; padding: 0 10px"> global $wp_query;
</span><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- /*
- * Closure for postprocessing logic.
- * It is here to avoid duplicate logic in many places below, without having
- * to introduce a very specific private global function.
- */
- $postprocess = static function( $loading_attributes, $with_fetchpriority = false ) use ( $tag_name, $attr, $context ) {
- // Potentially add `fetchpriority="high"`.
- if ( $with_fetchpriority ) {
- $loading_attributes = wp_maybe_add_fetchpriority_high_attr( $loading_attributes, $tag_name, $attr );
- }
- // Potentially strip `loading="lazy"` if the feature is disabled.
- if ( isset( $loading_attributes['loading'] ) && ! wp_lazy_loading_enabled( $tag_name, $context ) ) {
- unset( $loading_attributes['loading'] );
- }
- return $loading_attributes;
- };
-
- // Closure to increase media count for images with certain minimum threshold, mostly used for header images.
- $maybe_increase_content_media_count = static function() use ( $attr ) {
- /** This filter is documented in wp-admin/includes/media.php */
- $wp_min_priority_img_pixels = apply_filters( 'wp_min_priority_img_pixels', 50000 );
- // Images with a certain minimum size in the header of the page are also counted towards the threshold.
- if ( $wp_min_priority_img_pixels <= $attr['width'] * $attr['height'] ) {
- wp_increase_content_media_count();
- }
- };
-
</del><span class="cx" style="display: block; padding: 0 10px"> $loading_attrs = array();
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /*
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -5651,104 +5624,163 @@
</span><span class="cx" style="display: block; padding: 0 10px"> return $loading_attrs;
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ /*
+ * Skip programmatically created images within post content as they need to be handled together with the other
+ * images within the post content.
+ * Without this clause, they would already be considered within their own context which skews the image count and
+ * can result in the first post content image being lazy-loaded or an image further down the page being marked as a
+ * high priority.
+ */
+ switch ( $context ) {
+ case 'the_post_thumbnail':
+ case 'wp_get_attachment_image':
+ case 'widget_media_image':
+ if ( doing_filter( 'the_content' ) ) {
+ return $loading_attrs;
+ }
+ }
+
+ /*
+ * The key function logic starts here.
+ */
+ $maybe_in_viewport = null;
+ $increase_count = false;
+ $maybe_increase_count = false;
+
+ // Logic to handle a `loading` attribute that is already provided.
</ins><span class="cx" style="display: block; padding: 0 10px"> if ( isset( $attr['loading'] ) ) {
</span><span class="cx" style="display: block; padding: 0 10px"> /*
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * While any `loading` value could be set in `$loading_attrs`, for
- * consistency we only do it for `loading="lazy"` since that is the
- * only possible value that WordPress core would apply on its own.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * Interpret "lazy" as not in viewport. Any other value can be
+ * interpreted as in viewport (realistically only "eager" or `false`
+ * to force-omit the attribute are other potential values).
</ins><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="cx" style="display: block; padding: 0 10px"> if ( 'lazy' === $attr['loading'] ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- $loading_attrs['loading'] = 'lazy';
- if ( isset( $attr['fetchpriority'] ) && 'high' === $attr['fetchpriority'] ) {
- _doing_it_wrong(
- __FUNCTION__,
- __( 'An image should not be lazy-loaded and marked as high priority at the same time.' ),
- '6.3.0'
- );
- }
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ $maybe_in_viewport = false;
+ } else {
+ $maybe_in_viewport = true;
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">-
- return $postprocess( $loading_attrs, true );
</del><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // An image with `fetchpriority="high"` cannot be assigned `loading="lazy"` at the same time.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ // Logic to handle a `fetchpriority` attribute that is already provided.
</ins><span class="cx" style="display: block; padding: 0 10px"> if ( isset( $attr['fetchpriority'] ) && 'high' === $attr['fetchpriority'] ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- return $postprocess( $loading_attrs, true );
- }
-
- /*
- * Do not lazy-load images in the header block template part, as they are likely above the fold.
- * For classic themes, this is handled in the condition below using the 'get_header' action.
- */
- $header_area = WP_TEMPLATE_PART_AREA_HEADER;
- if ( "template_part_{$header_area}" === $context ) {
- // Increase media count if there are images in header above a certian minimum size threshold.
- $maybe_increase_content_media_count();
- return $postprocess( $loading_attrs, true );
- }
-
- // The custom header image is always expected to be in the header.
- if ( 'get_header_image_tag' === $context ) {
- // Increase media count if there are images in header above a certian minimum size threshold.
- $maybe_increase_content_media_count();
- return $postprocess( $loading_attrs, true );
- }
-
- // Special handling for programmatically created image tags.
- if ( 'the_post_thumbnail' === $context || 'wp_get_attachment_image' === $context || 'widget_media_image' === $context ) {
</del><span class="cx" style="display: block; padding: 0 10px"> /*
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * Skip programmatically created images within post content as they need to be handled together with the other
- * images within the post content.
- * Without this clause, they would already be considered below which skews the image count and can result in
- * the first post content image being lazy-loaded or an image further down the page being marked as a high
- * priority.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * If the image was already determined to not be in the viewport (e.g.
+ * from an already provided `loading` attribute), trigger a warning.
+ * Otherwise, the value can be interpreted as in viewport, since only
+ * the most important in-viewport image should have `fetchpriority` set
+ * to "high".
</ins><span class="cx" style="display: block; padding: 0 10px"> */
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- if ( doing_filter( 'the_content' ) ) {
- return $loading_attrs;
- }
-
- // Conditionally skip lazy-loading on images before the loop.
- if (
- // Only apply for main query but before the loop.
- $wp_query->before_loop && $wp_query->is_main_query()
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( false === $maybe_in_viewport ) {
+ _doing_it_wrong(
+ __FUNCTION__,
+ __( 'An image should not be lazy-loaded and marked as high priority at the same time.' ),
+ '6.3.0'
+ );
</ins><span class="cx" style="display: block; padding: 0 10px"> /*
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * Any image before the loop, but after the header has started should not be lazy-loaded,
- * except when the footer has already started which can happen when the current template
- * does not include any loop.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * Set `fetchpriority` here for backward-compatibility as we should
+ * not override what a developer decided, even though it seems
+ * incorrect.
</ins><span class="cx" style="display: block; padding: 0 10px"> */
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- && did_action( 'get_header' ) && ! did_action( 'get_footer' )
- ) {
- // Increase media count if there are images in header above a certian minimum size threshold.
- $maybe_increase_content_media_count();
- return $postprocess( $loading_attrs, true );
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ $loading_attrs['fetchpriority'] = 'high';
+ } else {
+ $maybe_in_viewport = true;
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( null === $maybe_in_viewport ) {
+ switch ( $context ) {
+ // Consider elements with these header-specific contexts to be in viewport.
+ case 'template_part_' . WP_TEMPLATE_PART_AREA_HEADER:
+ case 'get_header_image_tag':
+ $maybe_in_viewport = true;
+ $maybe_increase_count = true;
+ break;
+ // Count main content elements and detect whether in viewport.
+ case 'the_content':
+ case 'the_post_thumbnail':
+ case 'do_shortcode':
+ // Only elements within the main query loop have special handling.
+ if ( ! is_admin() && in_the_loop() && is_main_query() ) {
+ /*
+ * Get the content media count, since this is a main query
+ * content element. This is accomplished by "increasing"
+ * the count by zero, as the only way to get the count is
+ * to call this function.
+ * The actual count increase happens further below, based
+ * on the `$increase_count` flag set here.
+ */
+ $content_media_count = wp_increase_content_media_count( 0 );
+ $increase_count = true;
+
+ // If the count so far is below the threshold, `loading` attribute is omitted.
+ if ( $content_media_count < wp_omit_loading_attr_threshold() ) {
+ $maybe_in_viewport = true;
+ } else {
+ $maybe_in_viewport = false;
+ }
+ }
+ /*
+ * For the 'the_post_thumbnail' context, the following case
+ * clause needs to be considered as well, therefore skip the
+ * break statement here if the viewport has not been
+ * determined.
+ */
+ if ( 'the_post_thumbnail' !== $context || null !== $maybe_in_viewport ) {
+ break;
+ }
+ // phpcs:ignore Generic.WhiteSpace.ScopeIndent.Incorrect
+ // Consider elements before the loop as being in viewport.
+ case 'wp_get_attachment_image':
+ case 'widget_media_image':
+ if (
+ // Only apply for main query but before the loop.
+ $wp_query->before_loop && $wp_query->is_main_query()
+ /*
+ * Any image before the loop, but after the header has started should not be lazy-loaded,
+ * except when the footer has already started which can happen when the current template
+ * does not include any loop.
+ */
+ && did_action( 'get_header' ) && ! did_action( 'get_footer' )
+ ) {
+ $maybe_in_viewport = true;
+ $maybe_increase_count = true;
+ }
+ break;
+ }
+ }
+
</ins><span class="cx" style="display: block; padding: 0 10px"> /*
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- * The first elements in 'the_content' or 'the_post_thumbnail' should not be lazy-loaded,
- * as they are likely above the fold. Shortcodes are processed after content images, so if
- * thresholds haven't already been met, apply the same logic to those as well.
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * If the element is in the viewport (`true`), potentially add
+ * `fetchpriority` with a value of "high". Otherwise, i.e. if the element
+ * is not not in the viewport (`false`) or it is unknown (`null`), add
+ * `loading` with a value of "lazy".
</ins><span class="cx" style="display: block; padding: 0 10px"> */
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- if ( 'the_content' === $context || 'the_post_thumbnail' === $context || 'do_shortcode' === $context ) {
- // Only elements within the main query loop have special handling.
- if ( is_admin() || ! in_the_loop() || ! is_main_query() ) {
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( $maybe_in_viewport ) {
+ $loading_attrs = wp_maybe_add_fetchpriority_high_attr( $loading_attrs, $tag_name, $attr );
+ } else {
+ // Only add `loading="lazy"` if the feature is enabled.
+ if ( wp_lazy_loading_enabled( $tag_name, $context ) ) {
</ins><span class="cx" style="display: block; padding: 0 10px"> $loading_attrs['loading'] = 'lazy';
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- return $postprocess( $loading_attrs, false );
</del><span class="cx" style="display: block; padding: 0 10px"> }
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ }
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // Increase the counter since this is a main query content element.
- $content_media_count = wp_increase_content_media_count();
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ /*
+ * If flag was set based on contextual logic above, increase the content
+ * media count, either unconditionally, or based on whether the image size
+ * is larger than the threshold.
+ */
+ if ( $increase_count ) {
+ wp_increase_content_media_count();
+ } elseif ( $maybe_increase_count ) {
+ /** This filter is documented in wp-admin/includes/media.php */
+ $wp_min_priority_img_pixels = apply_filters( 'wp_min_priority_img_pixels', 50000 );
</ins><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // If the count so far is below the threshold, `loading` attribute is omitted.
- if ( $content_media_count <= wp_omit_loading_attr_threshold() ) {
- // The first largest image will still get `fetchpriority='high'`.
- return $postprocess( $loading_attrs, true );
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ if ( $wp_min_priority_img_pixels <= $attr['width'] * $attr['height'] ) {
+ wp_increase_content_media_count();
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- // Lazy-load by default for any unknown context.
- $loading_attrs['loading'] = 'lazy';
- return $postprocess( $loading_attrs, false );
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ return $loading_attrs;
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /**
</span></span></pre></div>
<a id="trunktestsphpunittestsmediaphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/tests/phpunit/tests/media.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/tests/phpunit/tests/media.php 2023-08-02 10:57:10 UTC (rev 56346)
+++ trunk/tests/phpunit/tests/media.php 2023-08-02 17:56:16 UTC (rev 56347)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -4956,7 +4956,7 @@
</span><span class="cx" style="display: block; padding: 0 10px"> $attr['loading'] = 'lazy';
</span><span class="cx" style="display: block; padding: 0 10px"> $attr['fetchpriority'] = 'high';
</span><span class="cx" style="display: block; padding: 0 10px">
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- $this->assertSame(
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ $this->assertEqualSetsWithIndex(
</ins><span class="cx" style="display: block; padding: 0 10px"> array(
</span><span class="cx" style="display: block; padding: 0 10px"> 'loading' => 'lazy',
</span><span class="cx" style="display: block; padding: 0 10px"> 'fetchpriority' => 'high',
</span></span></pre>
</div>
</div>
</body>
</html>