Reduce impact of third party code

[rsmith4321]

What's the issue?

Most of the time when I test with Pagespeed Insights I don't have any issues. I also use a tool called StatusCake that does automatic testing. Occasionally Pagespeed insights will report a long blocking time for the AMP scripts of over 1 second, and I can see large spikes in the load time on StatusCake as well. If I disable amp, my website always loads in under 400ms and there are no spikes in load time, so it is an amp specific issue.

Screenshot: https://drive.google.com/file/d/1_ZelBbkrfRbL4watjFgUdV6VjBStacix/view?usp=sharing

How do we reproduce the issue?

Test with Pagespeed Insights

What browsers are affected?

All browsers? Some specific browser? What device type?

Which AMP version is affected?

2103060631004

[westonruter]

I believe this is a duplicate of https://github.com/ampproject/amphtml/issues/28638, and it will be mitigated by https://github.com/ampproject/amphtml/issues/28310.

[rsmith4321]

I don't know if it is related. It seems to be an issue of extremely long delays of the initial loading of the amp scripts, not of the size of the script itself. Normally the exact same page will pass pagespeed insights testing with a 97 score. However sometimes I will test and the amp scripts will delay loading by 2-3 seconds. I don't know if the AMP optimizer could somehow delay the scripts if it's not cached? It's possible it's just an issue with Pagespeed Insights.

[rsmith4321]

Sadly I think I'm going to have to disable amp until this is solved. I just got a third party code blocking time of 3,180ms for /v0.js. Over 3 seconds is just too long for javascript blocking that I really don't need loading. I hope this can eventually be solved I like a lot of amp features.

[kristoferbaxter]

Hi there!

The AMP Project focuses its performance efforts around real world devices and traffic instead of simulated ones like those used in this report (only using simulation to understand the difference caused by a PR).

We'll take a look at this report, but highly recommend using https://amp.dev/page-experience/ to find opportunities for improvements based on billions of documents loaded from real users on real devices.

Additionally you might find the Bento effort interesting, which allows for using AMP components outside of AMP documents for your use-case. It's just entered development preview.

Sorry for the delay responding, was a bit underwater today.

[kristoferbaxter]

Also, as a clarification: if you have an approach that works better for your user base please use it!

A faster more efficient web makes users happy if the content displayed is the same.

[rsmith4321]

Great, thanks for the help. I was trying to test with the link you shared. Is that test working? I can't get a response when clicking the Analyze button. There really is something strange going on with the loading of the amp scripts, perhaps it's just a pagespeed insights issue, I've been using amp for a couple of years now and just recently started having issues. I would love to find out that's all it is and could return to using amp.

[kristoferbaxter]

The link I sent should be working. Can you add a URL to test to the issue? I'd be happy to run it thru the tool and include screenshots of the result.

As for pagespeed insights, the indicated time spent is execution time not download or parse... which likely means the document has not been served with an AMP optimizer.

The AMP optimizer reduces the client side JavaScript execution time by processing the execution of the finalized state for many AMP components via server side rendering. This is similar to the transformations done to valid AMP documents by AMP caches and improves user experience significantly.

Edits: typing on mobile devices leads to many spelling and grammar mistakes, sorry!

[westonruter]

@kristoferbaxter The PX guide isn't working for me either. The amp-script appears to not be initializing or it is erroring. It is ending up with 0.7 opacity.

[kristoferbaxter]

@sebastianbenz or @patrickkettner Can you take a look when it's daytime in your timezone?

[patrickkettner]

@kristoferbaxter not sure whats going on, but somethings is broken. I rolled back the image yesterday and it pixi is functional again. Trying to bisect now

[sebastianbenz]

We've switched our deploys from travis to GitHub actions. Seems like this broke the Pixi build.

[westonruter]

@rsmith4321 Can you provide a URL for us to re-check now that Pixi has been fixed?

[rsmith4321]

I went ahead and enabled amp again on my site. You can test here https://www.ryansmithphotography.com/ if you get a chance. With amp disabled I get 96-98 pagespeed score on my homepage with everything green. But even in the amp specific test I'm getting these results. I can't figure out the issue.

[westonruter]

Re-sharing the image since it didn't come through in email:

[westonruter]

The TBT seems erroneous and may indicate a false positive. We'll need to examine this further.

As for the advice for how to improve:

1. Markup hero images on your page. This should be resolved by updating to AMP plugin v2.1 Beta 1 since it includes hero image prerendering. This ensure your site logo is rendered before the AMP runtime loads. Nevertheless, I can see that the image is likely going to be deemed “tiny” since it is less than 150 pixels high, so you can force it to be prerendered by adding the data-hero attribute to it.
2. Disable tap delay. This is independent of AMP. You can improve FID by removing the initial-scale=1 from your meta viewport, so you're left with just <meta name="viewport" content="width=device-width">. This is also done automatically in v2.1-beta1 via https://github.com/ampproject/amp-wp/pull/5903.
3. Serve appropriately sized images. This one is perhaps best achieved today via an image CDN service, like Jetpack's photon. This will serve back WebP to browsers that support it automatically. We're also exploring some options for what we can do from the plugin side.
4. Ensure initial Server-response time for the page is short. I can see you have a page caching solution in place (Nginx-Helper), so I'm not sure what else can be done to reduce the TTFB. This is not an AMP-specific issue, although it is more important to ensure you have page caching in place on AMP pages due to the additional processing for page optimization.

[rsmith4321]

Thanks, I'm not too worried about the appropriate image warning, it's just because some images are in responsive columns and it's impossible to size them exactly, especially since wordpress always adds the sizes attribute at 100vw even if they are in a column. The response time might have been because I just cleared caches, and amp does seem to add some processing delay if the page isn't cached. What worries me is the very long delays I see loading the amp scripts that just happens occasionally.

I've tried the 2.1 beta 1 but it was adding data-hero to an image down the page. I know I asked you about this before but I made some changes to my site and now I can't get the logo or hero background image to have data-hero applied. Can data-hero be added to a background, and is there any easy way to select what images have this applied?

On Wed, Apr 7, 2021 at 4:49 PM Weston Ruter @.***> wrote:

The TBT seems erroneous and may indicate a false positive. We'll need to examine this further.

As for the advice for how to improve:

1. Markup hero images on your page. This should be resolved by updating to AMP plugin v2.1 Beta 1 https://github.com/ampproject/amp-wp/releases/tag/2.1.0-beta1 since it includes hero image prerendering. This ensure your site logo is rendered before the AMP runtime loads. Nevertheless, I can see that the image is likely going to be deemed “tiny” since it is less than 150 pixels high, so you can force it to be prerendered by adding the data-hero attribute to it.
2. Disable tap delay. This is independent of AMP. You can improve FID by removing the initial-scale=1 from your meta viewport, so you're left with just . This is also done automatically in v2.1-beta1 via ampproject/amp-wp#5903 https://github.com/ampproject/amp-wp/pull/5903 .
3. Serve appropriately sized images. This one is perhaps best achieved today via an image CDN service, like Jetpack's photon. This will serve back WebP to browsers that support it automatically. We're also exploring some options for what we can do from the plugin side.
4. Ensure initial Server-response time for the page is short. I can see you have a page caching solution in place (Nginx-Helper), so I'm not sure what else can be done to reduce the TTFB. This is not an AMP-specific issue, although it is more important to ensure you have page caching in place on AMP pages due to the additional processing for page optimization.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-815253533, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H5FSZLYKUJ4A2PDAD3THTAPLANCNFSM4ZSHA54A .

[westonruter]

Thanks, I'm not too worried about the appropriate image warning, it's just because some images are in responsive columns and it's impossible to size them exactly, especially since wordpress always adds the sizes attribute at 100vw even if they are in a column.

I see that you are actually serving WebP, so I was mistaken about "size" here referring to byte weight.

I've tried the 2.1 beta 1 but it was adding data-hero to an image down the page.

Yes, this is because it is skipping the logo since it is deemed “tiny”, so it's trying to find the first “large” image to use as the hero. Adding the data-hero attribute to the logo image will force it to prerender that image and not any others.

How are you adding the logo to the page in your template?

Can data-hero be added to a background, and is there any easy way to select what images have this applied?

The data-hero attribute is how you tell the Optimizer which image(s) should get prerendered. This is only applicable to amp-img and not to CSS background images.

SInce the hero image is a background image, and it is defined in CSS, the only way you could improve performance of that is to preload it. Since you aren't serving different background images depending on the viewport size, you could improve performance (regardless of AMP) by adding code like the following to the head:

<link rel="preload" as="image" src="https://www.ryansmithphotography.com/wp-content/uploads/2021/03/Bride-and-groom-sunset-silhouette-North-Myrtle-Beach-2015.jpg">

This will cause the browser to start loading the image even before the CSS is parsed.

Nevertheless, this image is much wider than it needs to be on mobile. Performance could be optimized if you cropped that image and then served the cropped version with a media query like:

@media (max-width: 480px) {
    .page-hero {
        background-image: url("https://www.ryansmithphotography.com/wp-content/uploads/2021/03/Bride-and-groom-sunset-silhouette-North-Myrtle-Beach-2015-mobile-cropped.jpg")
    }
}

Nevertheless, if you do that then you'd no longer be able to use the preload link, because non-Chromium browsers don't yet support the imagesrcset attribute on link[rel=preload]. For more on image preloading, see https://web.dev/preload-responsive-images/

[westonruter]

@kristoferbaxter What should be done about Pixi reporting poor TBT?

[rsmith4321]

Thanks! I implemented your suggestion about the background image responsive size. I think the only other issue, the 2.1 beta adds data-hero to an image down the page which seems to cause this image to be loaded before my background image even though it should be lazyloaded. Is there a way to disable this?

On Wed, Apr 7, 2021 at 5:34 PM Weston Ruter @.***> wrote:

Thanks, I'm not too worried about the appropriate image warning, it's just because some images are in responsive columns and it's impossible to size them exactly, especially since wordpress always adds the sizes attribute at 100vw even if they are in a column.

I see that you are actually serving WebP, so I was mistaken about "size" here referring to byte weight.

I've tried the 2.1 beta 1 but it was adding data-hero to an image down the page.

Yes, this is because it is skipping the logo since it is deemed “tiny”, so it's trying to find the first “large” image to use as the hero. Adding the data-hero attribute to the logo image will force it to prerender that image and not any others.

How are you adding the logo to the page in your template?

Can data-hero be added to a background, and is there any easy way to select what images have this applied?

The data-hero attribute is how you tell the Optimizer which image(s) should get prerendered. This is only applicable to amp-img and not to CSS background images.

SInce the hero image is a background image, and it is defined in CSS, the only way you could improve performance of that is to preload it. Since you aren't serving different background images depending on the viewport size, you could improve performance (regardless of AMP) by adding code like the following to the head:

This will cause the browser to start loading the image even before the CSS is parsed.

Nevertheless, this image is much wider than it needs to be on mobile. Performance could be optimized if you cropped that image and then served the cropped version with a media query like:

@media (max-width: 480px) {

.page-hero {

    background-image: url("https://www.ryansmithphotography.com/wp-content/uploads/2021/03/Bride-and-groom-sunset-silhouette-North-Myrtle-Beach-2015-mobile-cropped.jpg")

}

}

Nevertheless, if you do that then you'd no longer be able to use the preload link, because non-Chromium browsers don't yet support the imagesrcset attribute on link[rel=preload]. For more on image preloading, see https://web.dev/preload-responsive-images/

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-815279768, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H5NZORAUQES6TGDZ4TTHTFW7ANCNFSM4ZSHA54A .

[westonruter]

@rsmith4321 How are you adding the custom logo to the template? What is the PHP code you're using? Ideally the data-hero attribute would be added to that image instead.

[rsmith4321]

I believe this is the code the theme uses for the logo, I'm just adding it in the customizer.

<?php
/**
 * Header elements.
 *
 * @package GeneratePress
 */

if ( ! defined( 'ABSPATH' ) ) {
    exit; // Exit if accessed directly.
}

if ( ! function_exists( 'generate_construct_header' ) ) {
    add_action( 'generate_header', 'generate_construct_header' );
    /**
     * Build the header.
     *
     * @since 1.3.42
     */
    function generate_construct_header() {
        ?>
        <header id="masthead" <?php generate_do_element_classes( 'header' ); ?>>
            <div <?php generate_do_element_classes( 'inside_header' ); ?>>
                <?php
                /**
                 * generate_before_header_content hook.
                 *
                 * @since 0.1
                 */
                do_action( 'generate_before_header_content' );

                if ( ! generate_is_using_flexbox() ) {
                    // Add our main header items.
                    generate_header_items();
                }

                /**
                 * generate_after_header_content hook.
                 *
                 * @since 0.1
                 *
                 * @hooked generate_add_navigation_float_right - 5
                 */
                do_action( 'generate_after_header_content' );
                ?>
            </div>
        </header>
        <?php
    }
}

if ( ! function_exists( 'generate_header_items' ) ) {
    /**
     * Build the header contents.
     * Wrapping this into a function allows us to customize the order.
     *
     * @since 1.2.9.7
     */
    function generate_header_items() {
        $order = apply_filters(
            'generate_header_items_order',
            array(
                'header-widget',
                'site-branding',
                'logo',
            )
        );

        foreach ( $order as $item ) {
            if ( 'header-widget' === $item ) {
                generate_construct_header_widget();
            }

            if ( 'site-branding' === $item ) {
                generate_construct_site_title();
            }

            if ( 'logo' === $item ) {
                generate_construct_logo();
            }
        }
    }
}

if ( ! function_exists( 'generate_construct_logo' ) ) {
    /**
     * Build the logo
     *
     * @since 1.3.28
     */
    function generate_construct_logo() {
        $logo_url = ( function_exists( 'the_custom_logo' ) && get_theme_mod( 'custom_logo' ) ) ? wp_get_attachment_image_src( get_theme_mod( 'custom_logo' ), 'full' ) : false;
        $logo_url = ( $logo_url ) ? $logo_url[0] : generate_get_option( 'logo' );

        $logo_url = esc_url( apply_filters( 'generate_logo', $logo_url ) );
        $retina_logo_url = esc_url( apply_filters( 'generate_retina_logo', generate_get_option( 'retina_logo' ) ) );

        // If we don't have a logo, bail.
        if ( empty( $logo_url ) ) {
            return;
        }

        /**
         * generate_before_logo hook.
         *
         * @since 0.1
         */
        do_action( 'generate_before_logo' );

        $attr = apply_filters(
            'generate_logo_attributes',
            array(
                'class' => 'header-image is-logo-image',
                'alt'   => esc_attr( apply_filters( 'generate_logo_title', get_bloginfo( 'name', 'display' ) ) ),
                'src'   => $logo_url,
                'title' => esc_attr( apply_filters( 'generate_logo_title', get_bloginfo( 'name', 'display' ) ) ),
            )
        );

        if ( '' !== $retina_logo_url ) {
            $attr['srcset'] = $logo_url . ' 1x, ' . $retina_logo_url . ' 2x';

            // Add dimensions to image if retina is set. This fixes a container width bug in Firefox.
            if ( function_exists( 'the_custom_logo' ) && get_theme_mod( 'custom_logo' ) ) {
                $data = wp_get_attachment_metadata( get_theme_mod( 'custom_logo' ) );

                if ( ! empty( $data ) ) {
                    $attr['width'] = $data['width'];
                    $attr['height'] = $data['height'];
                }
            }
        } elseif ( generate_is_using_flexbox() ) {
            // Add this to flexbox version only until we can verify it won't conflict with existing installs.
            if ( function_exists( 'the_custom_logo' ) && get_theme_mod( 'custom_logo' ) ) {
                $data = wp_get_attachment_metadata( get_theme_mod( 'custom_logo' ) );

                if ( ! empty( $data ) ) {
                    $attr['width'] = $data['width'];
                    $attr['height'] = $data['height'];
                }
            }
        }

        $attr = array_map( 'esc_attr', $attr );

        $html_attr = '';
        foreach ( $attr as $name => $value ) {
            $html_attr .= " $name=" . '"' . $value . '"';
        }

        // Print our HTML.
        echo apply_filters( // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
            'generate_logo_output',
            sprintf(
                '<div class="site-logo">
                    <a href="%1$s" title="%2$s" rel="home">
                        <img %3$s />
                    </a>
                </div>',
                esc_url( apply_filters( 'generate_logo_href', home_url( '/' ) ) ),
                esc_attr( apply_filters( 'generate_logo_title', get_bloginfo( 'name', 'display' ) ) ),
                $html_attr
            ),
            $logo_url,
            $html_attr
        );

        /**
         * generate_after_logo hook.
         *
         * @since 0.1
         */
        do_action( 'generate_after_logo' );
    }
}

if ( ! function_exists( 'generate_construct_site_title' ) ) {
    /**
     * Build the site title and tagline.
     *
     * @since 1.3.28
     */
    function generate_construct_site_title() {
        $generate_settings = wp_parse_args(
            get_option( 'generate_settings', array() ),
            generate_get_defaults()
        );

        // Get the title and tagline.
        $title = get_bloginfo( 'title' );
        $tagline = get_bloginfo( 'description' );

        // If the disable title checkbox is checked, or the title field is empty, return true.
        $disable_title = ( '1' == $generate_settings['hide_title'] || '' == $title ) ? true : false; // phpcs:ignore

        // If the disable tagline checkbox is checked, or the tagline field is empty, return true.
        $disable_tagline = ( '1' == $generate_settings['hide_tagline'] || '' == $tagline ) ? true : false;  // phpcs:ignore

        $schema_type = generate_get_schema_type();

        // Build our site title.
        $site_title = apply_filters(
            'generate_site_title_output',
            sprintf(
                '<%1$s class="main-title"%4$s>
                    <a href="%2$s" rel="home">
                        %3$s
                    </a>
                </%1$s>',
                ( is_front_page() && is_home() ) ? 'h1' : 'p',
                esc_url( apply_filters( 'generate_site_title_href', home_url( '/' ) ) ),
                get_bloginfo( 'name' ),
                'microdata' === generate_get_schema_type() ? ' itemprop="headline"' : ''
            )
        );

        // Build our tagline.
        $site_tagline = apply_filters(
            'generate_site_description_output',
            sprintf(
                '<p class="site-description"%2$s>
                    %1$s
                </p>',
                html_entity_decode( get_bloginfo( 'description', 'display' ) ), // phpcs:ignore
                'microdata' === generate_get_schema_type() ? ' itemprop="description"' : ''
            )
        );

        // Site title and tagline.
        if ( false === $disable_title || false === $disable_tagline ) {
            if ( generate_needs_site_branding_container() ) {
                echo '<div class="site-branding-container">';
                generate_construct_logo();
            }

            // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- outputting site title and tagline. False positive.
            echo apply_filters(
                'generate_site_branding_output',
                sprintf(
                    '<div class="site-branding">
                        %1$s
                        %2$s
                    </div>',
                    ( ! $disable_title ) ? $site_title : '',
                    ( ! $disable_tagline ) ? $site_tagline : ''
                )
            );

            if ( generate_needs_site_branding_container() ) {
                echo '</div>';
            }
        }
    }
}

add_filter( 'generate_header_items_order', 'generate_reorder_inline_site_branding' );
/**
 * Remove the logo from it's usual position.
 *
 * @since 2.3
 * @param array $order Order of the header items.
 */
function generate_reorder_inline_site_branding( $order ) {
    if ( ! generate_get_option( 'inline_logo_site_branding' ) || ! generate_has_logo_site_branding() ) {
        return $order;
    }

    return array(
        'header-widget',
        'site-branding',
    );
}

if ( ! function_exists( 'generate_construct_header_widget' ) ) {
    /**
     * Build the header widget.
     *
     * @since 1.3.28
     */
    function generate_construct_header_widget() {
        if ( is_active_sidebar( 'header' ) ) :
            ?>
            <div class="header-widget">
                <?php dynamic_sidebar( 'header' ); ?>
            </div>
            <?php
        endif;
    }
}

add_action( 'generate_before_header_content', 'generate_do_site_logo', 5 );
/**
 * Add the site logo to our header.
 * Only added if we aren't using floats to preserve backwards compatibility.
 *
 * @since 3.0.0
 */
function generate_do_site_logo() {
    if ( ! generate_is_using_flexbox() || generate_needs_site_branding_container() ) {
        return;
    }

    generate_construct_logo();
}

add_action( 'generate_before_header_content', 'generate_do_site_branding' );
/**
 * Add the site branding to our header.
 * Only added if we aren't using floats to preserve backwards compatibility.
 *
 * @since 3.0.0
 */
function generate_do_site_branding() {
    if ( ! generate_is_using_flexbox() ) {
        return;
    }

    generate_construct_site_title();
}

add_action( 'generate_after_header_content', 'generate_do_header_widget' );
/**
 * Add the header widget to our header.
 * Only used when grid isn't using floats to preserve backwards compatibility.
 *
 * @since 3.0.0
 */
function generate_do_header_widget() {
    if ( ! generate_is_using_flexbox() ) {
        return;
    }

    generate_construct_header_widget();
}

if ( ! function_exists( 'generate_top_bar' ) ) {
    add_action( 'generate_before_header', 'generate_top_bar', 5 );
    /**
     * Build our top bar.
     *
     * @since 1.3.45
     */
    function generate_top_bar() {
        if ( ! is_active_sidebar( 'top-bar' ) ) {
            return;
        }

        $inside_top_bar_class = '';

        if ( 'contained' === generate_get_option( 'top_bar_inner_width' ) ) {
            $inside_top_bar_class = ' grid-container grid-parent';

            if ( generate_is_using_flexbox() ) {
                $inside_top_bar_class = ' grid-container';
            }
        }
        ?>
        <div <?php generate_do_element_classes( 'top_bar' ); ?>>
            <div class="inside-top-bar<?php echo $inside_top_bar_class; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped -- False positive. ?>">
                <?php dynamic_sidebar( 'top-bar' ); ?>
            </div>
        </div>
        <?php
    }
}

if ( ! function_exists( 'generate_pingback_header' ) ) {
    add_action( 'wp_head', 'generate_pingback_header' );
    /**
     * Add a pingback url auto-discovery header for singularly identifiable articles.
     *
     * @since 1.3.42
     */
    function generate_pingback_header() {
        if ( is_singular() && pings_open() ) {
            printf( '<link rel="pingback" href="%s">' . "\n", esc_url( get_bloginfo( 'pingback_url' ) ) );
        }
    }
}

if ( ! function_exists( 'generate_add_viewport' ) ) {
    add_action( 'wp_head', 'generate_add_viewport' );
    /**
     * Add viewport to wp_head.
     *
     * @since 1.1.0
     */
    function generate_add_viewport() {
        echo apply_filters( 'generate_meta_viewport', '<meta name="viewport" content="width=device-width, initial-scale=1">' );  // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
    }
}

add_action( 'generate_before_header', 'generate_do_skip_to_content_link', 2 );
/**
 * Add skip to content link before the header.
 *
 * @since 2.0
 */
function generate_do_skip_to_content_link() {
    printf(
        '<a class="screen-reader-text skip-link" href="#content" title="%1$s">%2$s</a>',
        esc_attr__( 'Skip to content', 'generatepress' ),
        esc_html__( 'Skip to content', 'generatepress' )
    );
}

[westonruter]

Got it. This plugin code should then do the trick:

add_filter( 'generate_logo_attributes', static function ( $attrs ) {
    $attrs['data-hero'] = '';
    return $attrs;
} );

Ah, I see this is what I shared previously on a support topic.

[rsmith4321]

Thanks yes, what you had given me before seemed to stop working after I made some changes in my theme, but this code worked for the logo. I appreciate it but you don't have to keep troubleshooting my site, but if it's helpful to you in working on the plugin, on my page here https://www.ryansmithphotography.com/myrtle-beach-wedding-photography/ it's also adding data-hero to an image way down the page and not to the standard cover block I'm using. I have the function you gave me before added like the following, but it no longer seems to work:

add_filter( 'render_block', function( $block_content, $block ) {
    static $added = false;
    if ( $added || 'core/image' !== $block['blockName'] ) {
        return $block_content;
    }
    $block_content = preg_replace( '/(?<=<img\s)/', 'data-hero ',
        $block_content, 1, $count );
    if ( 0 !== $count ) {
        $added = true;
    }
    return $block_content;
}, 10, 2 );

[westonruter]

Humm. That's strange. When you manually supply a data-hero attribute then the Optimizer shouldn't be automatically adding any on its own. Maybe comment out that render_block PHP code and look for any other code that is manually adding data-hero other than via the generate_logo_attributes filter? If no data-hero attributes are on the page, then the Optimizer currently adds only one.

[westonruter]

To aid with debugging, you can add this amp-enable-ssr plugin which allows you to try turning off SSR via ?amp_enable_ssr=false. If you still see data-hero in the document, then you know that you're adding it yourself.

[rsmith4321]

Your right getting rid of that additional function, now the data-hero is only added to the logo. But don't you want dtata-hero added to the main image on a page? That would still be the image inside the cover block, or is that still coming later out of beta?

[westonruter]

@rsmith4321 I'm working on https://github.com/ampproject/amp-wp/issues/6061 which will eliminate the need for you to manually identify most hero images, with the exception of your custom logo. Since your theme is not using the standard the_custom_logo() template tag, your logo image is not wrapped in an element with a custom-logo-link class and the logo image itself lacks the custom-logo class. I'll suggest you make a change to what I provided in https://github.com/ampproject/amphtml/issues/33405#issuecomment-816089515. Instead of adding data-hero, add data-hero-candidate instead:

add_filter( 'generate_logo_attributes', static function ( $attrs ) {
    $attrs['data-hero-candidate'] = '';
    return $attrs;
} );

In the PHP AMP Optimizer implementation, this data-hero-candidate doesn't exclude other images from being designated as heros. In contrast, if you supply data-hero yourself then those images and only those images will be marked as hero images.

The only reason why you'd need to explicitly mark the logo as opposed to the hero image is that the logo is “tiny” so the Optimizer skips it from prerendering by default.

We are planning to have v2.1-beta2 available for testing on Monday and it should address most if not all your hero image issues.

[westonruter]

Actually, I'm going to try automatically including any image in the header that has a class that contains logo so you won't have to add data-hero-candidate to your logo image either.

[rsmith4321]

Great, I'll just wait and test it out. I was able to implement the same look on my home page using a cover block instead of a background image, so that should get the data-hero after the update as well.

On Fri, Apr 9, 2021 at 5:15 PM Weston Ruter @.***> wrote:

Actually, I'm going to try automatically including any image in the header that has a class that contains logo so you won't have to add data-hero-candidate to your logo image either.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-816975705, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H777AHDQU2YTARZZJTTH5U73ANCNFSM4ZSHA54A .

[rsmith4321]

I don't know if you made the update you mentioned yet, I downloaded the latest build. I notice on my homepage data-hero is added to my main image, however not to the logo without the extra function you gave me. I also notice data-hero is not added to cover images if they are inside the main page content like the one here https://www.ryansmithphotography.com/myrtle-beach-wedding-photography/

[westonruter]

Sorry, we were going to do the beta2 release yesterday but we're now planning to do tomorrow. The build you tested doesn't include the yet-unmerged https://github.com/ampproject/amp-wp/pull/6062 which is what specifically for improving the hero image determination. I've been using your site specifically to inform how hero images should be determined in the WP context. I'll ping you once it's merged or ready to test.

[westonruter]

@rsmith4321 Plugin builds with the above fix are available for testing: https://github.com/ampproject/amp-wp/pull/6062#issuecomment-819926042. Once that PR is merged we'll release 2.1-beta2, but it will be essentially the same as the linked builds there.

[westonruter]

The 2.1-beta2 prerelease is now available for testing.

[rsmith4321]

Thanks, I'm still not getting data-hero on my home page image for some reason here https://www.ryansmithphotography.com/

On Thu, Apr 15, 2021 at 2:42 AM Weston Ruter @.***> wrote:

The 2.1-beta2 https://github.com/ampproject/amp-wp/releases/tag/2.1.0-beta2 prerelease is now available for testing.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-820159909, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H7XHIUGSA5TP5BEV6LTI2DFDANCNFSM4ZSHA54A .

[westonruter]

Please add this plugin so we can turn off SSR to debug that: https://gist.github.com/westonruter/8d52c0b807e6dfbbdf2219622d0f4a7e

[westonruter]

Actually, I see why it isn't getting prerendered. It's because the original image had loading=lazy. The DetermineHeroImages transformer in the plugin will prerender header images which are not marked with loading=lazy. As noted in wpcore#50425, images in the initial viewport shouldn't have loading=lazy. So that's something you should remove from that image, regardless of AMP.

[rsmith4321]

I don't manually add that to any images, it's just.a standard cover image block, wordpress just seems to add loading=lazy to all images. Is there a way to remove that?

On Thu, Apr 15, 2021 at 2:52 PM Weston Ruter @.***> wrote:

Actually, I see why it isn't getting prerendered. It's because the original image had loading=lazy. The DetermineHeroImages transformer in the plugin will prerender header images which are not marked with loading=lazy. As noted in wpcore#50425 https://core.trac.wordpress.org/ticket/50425, images in the initial viewport shouldn't have loading=lazy. So that's something you should remove from that image, regardless of AMP.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-820657554, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H3QAZX3V2DWN3W4XOLTI4YXVANCNFSM4ZSHA54A .

[westonruter]

Yes. Something like this should do the trick, to prevent lazy-loading the first cover image on the front page:

add_filter( 'wp_img_tag_add_loading_attr', function( $value, $image, $context ) {
    static $skipped = false;

    if (
        ! $skipped
        &&
        is_front_page()
        &&
        'the_content' === $context
        &&
        false !== strpos( $image, 'wp-block-cover__image-background' )
    ) {
        $value   = false;
        $skipped = true;
    }

    return $value;
}, 10, 3 );

[rsmith4321]

Hmm I tried that and it still doesn't seem to be working. I also notice on other pages like here https://www.ryansmithphotography.com/myrtle-beach-wedding-photography/ it adds data-hero-candidiate to the hero image but not data-hero. However data-hero does work on just standard blog posts with a featured image. I think the problem is on a lot of my custom pages I'm not using a standard featured image but instead a cover block. I wonder if it would be possible to add a selection to the cover block to select that this block is a hero image.

On Thu, Apr 15, 2021 at 3:52 PM Weston Ruter @.***> wrote:

Yes. Something like this should do the trick, to prevent lazy-loading the first cover image on the front page:

add_filter( 'wp_img_tag_add_loading_attr', function( $value, $image, $context ) { static $skipped = false;

if ( ! $skipped && is_front_page() && 'the_content' === $context && false !== strpos( $image, 'wp-block-cover__image-background' ) ) { $value = false; $skipped = true; }

return $value; }, 10, 3 );

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-820691258, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H734WZTIIAOQZ3ST43TI47YDANCNFSM4ZSHA54A .

[westonruter]

My filter code removed loading="lazy from the Cover image block in content for me locally in a core theme. You'll have to adjust the conditions in the if statement based on how your theme templates are set up.

[rsmith4321]

I was able to move some things around on my home page to get the hero image inside of the_content, I couldn't figure out how to do it with the hook it was added in. Now it's getting data-hero-candidate added to the image like with my other pages with cover blocks, however I'm still getting the recommendation to add data-hero to my images in the page experience test, so I'm thinking it's still not working correctly.

On Thu, Apr 15, 2021 at 4:34 PM Weston Ruter @.***> wrote:

My filter code removed loading="lazy from the Cover image block in content for me locally in a core theme. You'll have to adjust the conditions in the if statement based on how your theme templates are set up.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-820713153, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H44NEIYLU3GDMEPQZ3TI5ETXANCNFSM4ZSHA54A .

[westonruter]

You have your homepage's featured image set to be hidden entirely:

.page-header-image {
    display: none;
}

Nevertheless, it is the second image on the page. So it is getting data-hero in addition to the logo.

If you eliminate the featured image from being output on the page at all instead of just hiding it, then the cover image should get data-hero as well.

The Optimizer is not able to evaluate CSS to know whether or not a given element will be hidden on the page when rendered.

[rsmith4321]

Great that is working now. I just liked having a featured image for other reasons like sharing but it doesn't seem possible to have the featured image set in Wordpress without it including the image in the html, at least not with my theme it just hides it with css. I'll just remove the featured image from pages that don't use it.

On Thu, Apr 15, 2021 at 6:56 PM Weston Ruter @.***> wrote:

You have your homepage's featured image set to be hidden entirely:

.page-header-image { display: none; }

Nevertheless, it is the second image on the page. So it is getting data-hero in addition to the logo.

If you eliminate the featured image from being output on the page at all instead of just hiding it, then the cover image should get data-hero as well.

The Optimizer is not able to evaluate CSS to know whether or not a given element will be hidden on the page when rendered.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-820782171, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H2BZBK267QTXKRAMIDTI5VJZANCNFSM4ZSHA54A .

[westonruter]

You can use the Yoast SEO plugin to set the image that will show up when sharing on social media.

[rsmith4321]

Ok thanks, I'm contacting the theme developer as well to see if there is a way to filter the featured image output as well.

On Thu, Apr 15, 2021 at 8:05 PM Weston Ruter @.***> wrote:

You can use the Yoast SEO plugin to set the image that will show up when sharing on social media.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ampproject/amphtml/issues/33405#issuecomment-820815337, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Y5H63NKRG5P2KD3NK2LTTI55M5ANCNFSM4ZSHA54A .