From 831009a0828564138ec4caa7012f24c44428ea2f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 29 May 2021 22:20:20 -0700 Subject: [PATCH 01/12] Introduce amp_bento_enabled filter to enable Bento scripts and styles --- includes/amp-helper-functions.php | 54 ++++++++++++++++++++++++++-- includes/class-amp-theme-support.php | 36 ++++++++++++++++++- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index bae1d58b212..42909c08372 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -873,6 +873,26 @@ function amp_add_generator_metadata() { printf( '', esc_attr( $content ) ); } +/** + * Determine whether Bento is enabled. + * + * @since 2.1.3 + * @internal + * + * @return bool + */ +function amp_is_bento_enabled() { + /** + * Filters whether Bento is enabled. + * + * @since 2.1.3 + * @internal + * + * @param bool $enabled Enabled. + */ + return apply_filters( 'amp_bento_enabled', false ); +} + /** * Register default scripts for AMP components. * @@ -924,17 +944,24 @@ function amp_register_default_scripts( $wp_scripts ) { $extension_specs['amp-carousel']['latest'] = '0.2'; } + $bento_enabled = amp_is_bento_enabled(); foreach ( $extension_specs as $extension_name => $extension_spec ) { + if ( $bento_enabled && ! empty( $extension_spec['bento'] ) ) { + $version = $extension_spec['bento']['version']; + } else { + $version = $extension_spec['latest']; + } + $src = sprintf( 'https://cdn.ampproject.org/v0/%s-%s.js', $extension_name, - $extension_spec['latest'] + $version ); $wp_scripts->add( $extension_name, $src, - [ 'amp-runtime' ], + [ 'amp-runtime' ], // @todo Eventually this will not be present for Bento. null ); } @@ -964,6 +991,26 @@ function amp_register_default_styles( WP_Styles $styles ) { AMP__VERSION ); $styles->add_data( 'amp-icons', 'rtl', 'replace' ); + + if ( amp_is_bento_enabled() ) { + foreach ( AMP_Allowed_Tags_Generated::get_extension_specs() as $extension_name => $extension_spec ) { + if ( empty( $extension_spec['bento']['has_css'] ) ) { + continue; + } + + $src = sprintf( + 'https://cdn.ampproject.org/v0/%s-%s.css', + $extension_name, + $extension_spec['bento']['version'] + ); + $styles->add( + $extension_name, + $src, + [], + null + ); + } + } } /** @@ -1325,6 +1372,9 @@ function amp_is_dev_mode() { ( is_admin_bar_showing() && is_user_logged_in() ) || is_customize_preview() + || + // Force dev mode for Bento since it currently requires the Bento experiment opt-in script. + amp_is_bento_enabled() ) ); } diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 8190f4c0a24..be4c3146706 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -915,6 +915,21 @@ static function() { wp_dequeue_script( 'comment-reply' ); // Handled largely by AMP_Comments_Sanitizer and *reply* methods in this class. } ); + + if ( amp_is_bento_enabled() ) { + add_action( + 'wp_head', + static function () { + ?> + + Attribute::REL_STYLESHEET, + Attribute::TYPE => 'text/css', + Attribute::HREF => wp_styles()->registered[ $amp_script ]->src, + 'data-ampdevmode' => '', + ] + ); + } + } + /* * "4. Use preconnect to speedup the connection to other origin where the full resource URL is not known ahead of time, * for example, when using Google Fonts." * * Note that \AMP_Style_Sanitizer::process_link_element() will ensure preconnect links for Google Fonts are present. */ - $link_relations = [ Attribute::REL_PRECONNECT, Attribute::REL_DNS_PREFETCH, Attribute::REL_PRELOAD, Attribute::REL_PRERENDER, Attribute::REL_PREFETCH ]; + $link_relations = [ Attribute::REL_PRECONNECT, Attribute::REL_DNS_PREFETCH, Attribute::REL_PRELOAD, Attribute::REL_PRERENDER, Attribute::REL_PREFETCH, Attribute::REL_STYLESHEET ]; foreach ( $link_relations as $rel ) { if ( ! isset( $links[ $rel ] ) ) { continue; From b91759b6c140a194fa2b89f2348eb5010468902d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sat, 29 May 2021 22:59:26 -0700 Subject: [PATCH 02/12] Disable SSR when Bento is enabled --- includes/class-amp-theme-support.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index be4c3146706..3566ccae890 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -2164,6 +2164,11 @@ private static function get_optimizer( $args ) { add_filter( 'amp_enable_ssr', static function () use ( $args ) { + // SSR currently does not work with Bento. + if ( amp_is_bento_enabled() ) { + return false; + } + return array_key_exists( ConfigurationArgument::ENABLE_SSR, $args ) ? $args[ ConfigurationArgument::ENABLE_SSR ] : true; From 372c7b01f021c9260fe17faf379aa4edee37b88d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 2 Aug 2021 11:07:58 -0700 Subject: [PATCH 03/12] Add todo --- includes/amp-helper-functions.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 42909c08372..cf451087e58 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -876,6 +876,13 @@ function amp_add_generator_metadata() { /** * Determine whether Bento is enabled. * + * @todo Should this rather be the sandboxing level? Maybe not yet. Bento mode means: + * + * 1. Nothing is sanitized by default. + * 2. Bento versions of components are used. + * 3. is not converted to + * 4. Stylesheets are not processed. + * * @since 2.1.3 * @internal * From 33c7bd534673a1b360fe949baa729f10890c7940 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 3 Aug 2021 18:16:31 -0700 Subject: [PATCH 04/12] Default to keeping all invalid markup when Bento is enabled --- includes/validation/class-amp-validation-manager.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 402ec407099..44c66f68ea3 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -262,7 +262,9 @@ public static function post_supports_validation( $post ) { */ public static function is_sanitization_auto_accepted( $error = null ) { - if ( $error && amp_is_canonical() ) { + if ( amp_is_bento_enabled() ) { + $accepted = false; + } elseif ( $error && amp_is_canonical() ) { // Excessive CSS on AMP-first sites must not be removed by default since removing CSS can severely break a site. $accepted = AMP_Style_Sanitizer::STYLESHEET_TOO_LONG !== $error['code']; } else { From 5ff556544fa501186f0f4ad8fb480376c02e2d78 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 5 Aug 2021 14:30:46 -0700 Subject: [PATCH 05/12] Skip needlessly adding Bento styles to AMP pages --- includes/amp-helper-functions.php | 2 ++ includes/class-amp-theme-support.php | 21 +-------------------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index cf451087e58..22ac6aaa6cb 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -999,6 +999,8 @@ function amp_register_default_styles( WP_Styles $styles ) { ); $styles->add_data( 'amp-icons', 'rtl', 'replace' ); + // These are registered exclusively for used for non-AMP pages that manually enqueue them. They aren't needed on + // AMP pages due to the runtime style being present and because the styles are inlined in the scripts already. if ( amp_is_bento_enabled() ) { foreach ( AMP_Allowed_Tags_Generated::get_extension_specs() as $extension_name => $extension_spec ) { if ( empty( $extension_spec['bento']['has_css'] ) ) { diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 3566ccae890..b943ea5add2 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1566,32 +1566,13 @@ public static function ensure_required_markup( Document $dom, $script_handles = } $links[ Attribute::REL_PRELOAD ] = array_merge( $prioritized_preloads, $links[ Attribute::REL_PRELOAD ] ); - // Add stylesheets for Bento components. - if ( amp_is_bento_enabled() ) { - foreach ( array_keys( $amp_scripts ) as $amp_script ) { - if ( ! wp_style_is( $amp_script, 'registered' ) ) { - continue; - } - $links[ Attribute::REL_STYLESHEET ][] = AMP_DOM_Utils::create_node( - $dom, - Tag::LINK, - [ - Attribute::REL => Attribute::REL_STYLESHEET, - Attribute::TYPE => 'text/css', - Attribute::HREF => wp_styles()->registered[ $amp_script ]->src, - 'data-ampdevmode' => '', - ] - ); - } - } - /* * "4. Use preconnect to speedup the connection to other origin where the full resource URL is not known ahead of time, * for example, when using Google Fonts." * * Note that \AMP_Style_Sanitizer::process_link_element() will ensure preconnect links for Google Fonts are present. */ - $link_relations = [ Attribute::REL_PRECONNECT, Attribute::REL_DNS_PREFETCH, Attribute::REL_PRELOAD, Attribute::REL_PRERENDER, Attribute::REL_PREFETCH, Attribute::REL_STYLESHEET ]; + $link_relations = [ Attribute::REL_PRECONNECT, Attribute::REL_DNS_PREFETCH, Attribute::REL_PRELOAD, Attribute::REL_PRERENDER, Attribute::REL_PREFETCH ]; foreach ( $link_relations as $rel ) { if ( ! isset( $links[ $rel ] ) ) { continue; From a59f8fe0307cfc9e3bfdffd95004cef8ed1c455d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 9 Aug 2021 15:22:36 -0700 Subject: [PATCH 06/12] Remove todo which is out of scope for PR; bump since tags --- includes/amp-helper-functions.php | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 22ac6aaa6cb..f934c2fdbae 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -876,14 +876,7 @@ function amp_add_generator_metadata() { /** * Determine whether Bento is enabled. * - * @todo Should this rather be the sandboxing level? Maybe not yet. Bento mode means: - * - * 1. Nothing is sanitized by default. - * 2. Bento versions of components are used. - * 3. is not converted to - * 4. Stylesheets are not processed. - * - * @since 2.1.3 + * @since 2.2 * @internal * * @return bool @@ -892,7 +885,7 @@ function amp_is_bento_enabled() { /** * Filters whether Bento is enabled. * - * @since 2.1.3 + * @since 2.2 * @internal * * @param bool $enabled Enabled. From 2a5a624649f126bb57d2afe9b7554d2dbd0a8ce0 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 9 Aug 2021 15:35:50 -0700 Subject: [PATCH 07/12] Add tests for scripts and styles in Bento --- tests/php/test-amp-helper-functions.php | 225 +++++++++++++++++++++++- 1 file changed, 219 insertions(+), 6 deletions(-) diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 7125f560420..dd64470b2f5 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -1252,10 +1252,15 @@ static function ( $script ) { $this->assertStringContainsString( '', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript } - /** @covers ::amp_register_default_scripts() */ - public function test_amp_register_default_scripts() { - global $wp_scripts; + /** + * @covers ::amp_register_default_scripts() + * @covers ::amp_register_default_styles() + */ + public function test_amp_register_default_scripts_and_styles() { + global $wp_scripts, $wp_styles; $wp_scripts = null; + $wp_styles = null; + add_filter( 'amp_bento_enabled', '__return_false' ); $registered_script_srcs = []; foreach ( wp_scripts()->registered as $handle => $dependency ) { @@ -1267,7 +1272,7 @@ public function test_amp_register_default_scripts() { ksort( $registered_script_srcs ); // This allows us to ensure that we catch any version changes in scripts. - $expected = [ + $expected_scripts = [ 'amp-3d-gltf' => 'v0/amp-3d-gltf-0.1.js', 'amp-3q-player' => 'v0/amp-3q-player-0.1.js', 'amp-access' => 'v0/amp-access-0.1.js', @@ -1403,10 +1408,218 @@ public function test_amp_register_default_scripts() { 'amp-youtube' => 'v0/amp-youtube-0.1.js', ]; - ksort( $expected ); + ksort( $expected_scripts ); + ksort( $registered_script_srcs ); + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export + $this->assertEquals( $expected_scripts, $registered_script_srcs, "Actual fixture:\n" . var_export( $registered_script_srcs, true ) ); + + $this->assertTrue( wp_style_is( 'amp-default', 'registered' ) ); + $this->assertTrue( wp_style_is( 'amp-icons', 'registered' ) ); + $this->assertFalse( wp_style_is( 'amp-base-carousel', 'registered' ) ); + } + + /** + * @covers ::amp_register_default_scripts() + * @covers ::amp_register_default_styles() + */ + public function test_amp_register_default_scripts_and_styles_with_bento() { + global $wp_scripts, $wp_styles; + $wp_scripts = null; + $wp_styles = null; + add_filter( 'amp_bento_enabled', '__return_true' ); + + $registered_script_srcs = []; + foreach ( wp_scripts()->registered as $handle => $dependency ) { + if ( 'amp-' === substr( $handle, 0, 4 ) ) { + $this->assertStringStartsWith( 'https://cdn.ampproject.org/', $dependency->src ); + $registered_script_srcs[ $handle ] = str_replace( 'https://cdn.ampproject.org/', '', $dependency->src ); + } + } + ksort( $registered_script_srcs ); + + // This allows us to ensure that we catch any version changes in scripts. + $expected_scripts = [ + 'amp-3d-gltf' => 'v0/amp-3d-gltf-0.1.js', + 'amp-3q-player' => 'v0/amp-3q-player-0.1.js', + 'amp-access' => 'v0/amp-access-0.1.js', + 'amp-access-laterpay' => 'v0/amp-access-laterpay-0.2.js', + 'amp-access-poool' => 'v0/amp-access-poool-0.1.js', + 'amp-access-scroll' => 'v0/amp-access-scroll-0.1.js', + 'amp-accordion' => 'v0/amp-accordion-1.0.js', + 'amp-action-macro' => 'v0/amp-action-macro-0.1.js', + 'amp-ad' => 'v0/amp-ad-0.1.js', + 'amp-ad-custom' => 'v0/amp-ad-custom-0.1.js', + 'amp-addthis' => 'v0/amp-addthis-0.1.js', + 'amp-analytics' => 'v0/amp-analytics-0.1.js', + 'amp-anim' => 'v0/amp-anim-0.1.js', + 'amp-animation' => 'v0/amp-animation-0.1.js', + 'amp-apester-media' => 'v0/amp-apester-media-0.1.js', + 'amp-app-banner' => 'v0/amp-app-banner-0.1.js', + 'amp-audio' => 'v0/amp-audio-0.1.js', + 'amp-auto-ads' => 'v0/amp-auto-ads-0.1.js', + 'amp-autocomplete' => 'v0/amp-autocomplete-0.1.js', + 'amp-base-carousel' => 'v0/amp-base-carousel-1.0.js', + 'amp-beopinion' => 'v0/amp-beopinion-0.1.js', + 'amp-bind' => 'v0/amp-bind-0.1.js', + 'amp-bodymovin-animation' => 'v0/amp-bodymovin-animation-0.1.js', + 'amp-brid-player' => 'v0/amp-brid-player-0.1.js', + 'amp-brightcove' => 'v0/amp-brightcove-0.1.js', + 'amp-byside-content' => 'v0/amp-byside-content-0.1.js', + 'amp-cache-url' => 'v0/amp-cache-url-0.1.js', + 'amp-call-tracking' => 'v0/amp-call-tracking-0.1.js', + 'amp-carousel' => 'v0/amp-carousel-0.2.js', + 'amp-connatix-player' => 'v0/amp-connatix-player-0.1.js', + 'amp-consent' => 'v0/amp-consent-0.1.js', + 'amp-dailymotion' => 'v0/amp-dailymotion-0.1.js', + 'amp-date-countdown' => 'v0/amp-date-countdown-1.0.js', + 'amp-date-display' => 'v0/amp-date-display-1.0.js', + 'amp-date-picker' => 'v0/amp-date-picker-0.1.js', + 'amp-delight-player' => 'v0/amp-delight-player-0.1.js', + 'amp-dynamic-css-classes' => 'v0/amp-dynamic-css-classes-0.1.js', + 'amp-embedly-card' => 'v0/amp-embedly-card-0.1.js', + 'amp-experiment' => 'v0/amp-experiment-0.1.js', + 'amp-facebook' => 'v0/amp-facebook-0.1.js', + 'amp-facebook-comments' => 'v0/amp-facebook-comments-0.1.js', + 'amp-facebook-like' => 'v0/amp-facebook-like-0.1.js', + 'amp-facebook-page' => 'v0/amp-facebook-page-0.1.js', + 'amp-fit-text' => 'v0/amp-fit-text-1.0.js', + 'amp-font' => 'v0/amp-font-0.1.js', + 'amp-form' => 'v0/amp-form-0.1.js', + 'amp-fx-collection' => 'v0/amp-fx-collection-0.1.js', + 'amp-fx-flying-carpet' => 'v0/amp-fx-flying-carpet-0.1.js', + 'amp-geo' => 'v0/amp-geo-0.1.js', + 'amp-gfycat' => 'v0/amp-gfycat-0.1.js', + 'amp-gist' => 'v0/amp-gist-0.1.js', + 'amp-google-document-embed' => 'v0/amp-google-document-embed-0.1.js', + 'amp-hulu' => 'v0/amp-hulu-0.1.js', + 'amp-iframe' => 'v0/amp-iframe-0.1.js', + 'amp-iframely' => 'v0/amp-iframely-0.1.js', + 'amp-ima-video' => 'v0/amp-ima-video-0.1.js', + 'amp-image-lightbox' => 'v0/amp-image-lightbox-0.1.js', + 'amp-image-slider' => 'v0/amp-image-slider-0.1.js', + 'amp-imgur' => 'v0/amp-imgur-0.1.js', + 'amp-inline-gallery' => 'v0/amp-inline-gallery-1.0.js', + 'amp-inputmask' => 'v0/amp-inputmask-0.1.js', + 'amp-instagram' => 'v0/amp-instagram-1.0.js', + 'amp-install-serviceworker' => 'v0/amp-install-serviceworker-0.1.js', + 'amp-izlesene' => 'v0/amp-izlesene-0.1.js', + 'amp-jwplayer' => 'v0/amp-jwplayer-0.1.js', + 'amp-kaltura-player' => 'v0/amp-kaltura-player-0.1.js', + 'amp-lightbox' => 'v0/amp-lightbox-1.0.js', + 'amp-lightbox-gallery' => 'v0/amp-lightbox-gallery-1.0.js', + 'amp-link-rewriter' => 'v0/amp-link-rewriter-0.1.js', + 'amp-list' => 'v0/amp-list-0.1.js', + 'amp-live-list' => 'v0/amp-live-list-0.1.js', + 'amp-mathml' => 'v0/amp-mathml-0.1.js', + 'amp-mega-menu' => 'v0/amp-mega-menu-0.1.js', + 'amp-megaphone' => 'v0/amp-megaphone-0.1.js', + 'amp-minute-media-player' => 'v0/amp-minute-media-player-0.1.js', + 'amp-mowplayer' => 'v0/amp-mowplayer-0.1.js', + 'amp-mustache' => 'v0/amp-mustache-0.2.js', + 'amp-nested-menu' => 'v0/amp-nested-menu-0.1.js', + 'amp-next-page' => 'v0/amp-next-page-1.0.js', + 'amp-nexxtv-player' => 'v0/amp-nexxtv-player-0.1.js', + 'amp-o2-player' => 'v0/amp-o2-player-0.1.js', + 'amp-onetap-google' => 'v0/amp-onetap-google-0.1.js', + 'amp-ooyala-player' => 'v0/amp-ooyala-player-0.1.js', + 'amp-orientation-observer' => 'v0/amp-orientation-observer-0.1.js', + 'amp-pan-zoom' => 'v0/amp-pan-zoom-0.1.js', + 'amp-pinterest' => 'v0/amp-pinterest-0.1.js', + 'amp-playbuzz' => 'v0/amp-playbuzz-0.1.js', + 'amp-position-observer' => 'v0/amp-position-observer-0.1.js', + 'amp-powr-player' => 'v0/amp-powr-player-0.1.js', + 'amp-reach-player' => 'v0/amp-reach-player-0.1.js', + 'amp-recaptcha-input' => 'v0/amp-recaptcha-input-0.1.js', + 'amp-redbull-player' => 'v0/amp-redbull-player-0.1.js', + 'amp-reddit' => 'v0/amp-reddit-0.1.js', + 'amp-render' => 'v0/amp-render-1.0.js', + 'amp-riddle-quiz' => 'v0/amp-riddle-quiz-0.1.js', + 'amp-runtime' => 'v0.js', + 'amp-script' => 'v0/amp-script-0.1.js', + 'amp-selector' => 'v0/amp-selector-1.0.js', + 'amp-shadow' => 'shadow-v0.js', + 'amp-sidebar' => 'v0/amp-sidebar-0.1.js', + 'amp-skimlinks' => 'v0/amp-skimlinks-0.1.js', + 'amp-smartlinks' => 'v0/amp-smartlinks-0.1.js', + 'amp-social-share' => 'v0/amp-social-share-1.0.js', + 'amp-soundcloud' => 'v0/amp-soundcloud-0.1.js', + 'amp-springboard-player' => 'v0/amp-springboard-player-0.1.js', + 'amp-sticky-ad' => 'v0/amp-sticky-ad-1.0.js', + 'amp-story' => 'v0/amp-story-1.0.js', + 'amp-story-360' => 'v0/amp-story-360-0.1.js', + 'amp-story-auto-ads' => 'v0/amp-story-auto-ads-0.1.js', + 'amp-story-auto-analytics' => 'v0/amp-story-auto-analytics-0.1.js', + 'amp-story-interactive' => 'v0/amp-story-interactive-0.1.js', + 'amp-story-panning-media' => 'v0/amp-story-panning-media-0.1.js', + 'amp-story-player' => 'v0/amp-story-player-0.1.js', + 'amp-stream-gallery' => 'v0/amp-stream-gallery-1.0.js', + 'amp-subscriptions' => 'v0/amp-subscriptions-0.1.js', + 'amp-subscriptions-google' => 'v0/amp-subscriptions-google-0.1.js', + 'amp-tiktok' => 'v0/amp-tiktok-0.1.js', + 'amp-timeago' => 'v0/amp-timeago-1.0.js', + 'amp-truncate-text' => 'v0/amp-truncate-text-0.1.js', + 'amp-twitter' => 'v0/amp-twitter-1.0.js', + 'amp-user-notification' => 'v0/amp-user-notification-0.1.js', + 'amp-video' => 'v0/amp-video-1.0.js', + 'amp-video-docking' => 'v0/amp-video-docking-0.1.js', + 'amp-video-iframe' => 'v0/amp-video-iframe-1.0.js', + 'amp-vimeo' => 'v0/amp-vimeo-1.0.js', + 'amp-vine' => 'v0/amp-vine-0.1.js', + 'amp-viqeo-player' => 'v0/amp-viqeo-player-0.1.js', + 'amp-vk' => 'v0/amp-vk-0.1.js', + 'amp-web-push' => 'v0/amp-web-push-0.1.js', + 'amp-wistia-player' => 'v0/amp-wistia-player-0.1.js', + 'amp-wordpress-embed' => 'v0/amp-wordpress-embed-1.0.js', + 'amp-yotpo' => 'v0/amp-yotpo-0.1.js', + 'amp-youtube' => 'v0/amp-youtube-1.0.js', + ]; + + ksort( $expected_scripts ); ksort( $registered_script_srcs ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export - $this->assertEquals( $expected, $registered_script_srcs, "Actual fixture:\n" . var_export( $registered_script_srcs, true ) ); + $this->assertEquals( $expected_scripts, $registered_script_srcs, "Actual fixture:\n" . var_export( $registered_script_srcs, true ) ); + + $bundled_styles = [ 'amp-default', 'amp-icons' ]; + foreach ( $bundled_styles as $bundled_style ) { + $this->assertTrue( wp_style_is( $bundled_style, 'registered' ) ); + } + $registered_style_srcs = []; + foreach ( wp_styles()->registered as $handle => $dependency ) { + if ( in_array( $handle, $bundled_styles, true ) ) { + continue; + } + + if ( 'amp-' === substr( $handle, 0, 4 ) ) { + $this->assertStringStartsWith( 'https://cdn.ampproject.org/', $dependency->src ); + $registered_style_srcs[ $handle ] = str_replace( 'https://cdn.ampproject.org/', '', $dependency->src ); + } + } + ksort( $registered_style_srcs ); + + // This allows us to ensure that we catch any version changes in styles. + $expected_styles = [ + 'amp-accordion' => 'v0/amp-accordion-1.0.css', + 'amp-base-carousel' => 'v0/amp-base-carousel-1.0.css', + 'amp-fit-text' => 'v0/amp-fit-text-1.0.css', + 'amp-inline-gallery' => 'v0/amp-inline-gallery-1.0.css', + 'amp-instagram' => 'v0/amp-instagram-1.0.css', + 'amp-lightbox' => 'v0/amp-lightbox-1.0.css', + 'amp-lightbox-gallery' => 'v0/amp-lightbox-gallery-1.0.css', + 'amp-selector' => 'v0/amp-selector-1.0.css', + 'amp-social-share' => 'v0/amp-social-share-1.0.css', + 'amp-stream-gallery' => 'v0/amp-stream-gallery-1.0.css', + 'amp-timeago' => 'v0/amp-timeago-1.0.css', + 'amp-twitter' => 'v0/amp-twitter-1.0.css', + 'amp-video' => 'v0/amp-video-1.0.css', + 'amp-video-iframe' => 'v0/amp-video-iframe-1.0.css', + 'amp-vimeo' => 'v0/amp-vimeo-1.0.css', + 'amp-youtube' => 'v0/amp-youtube-1.0.css', + ]; + + ksort( $expected_styles ); + ksort( $registered_style_srcs ); + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_var_export + $this->assertEquals( $expected_styles, $registered_style_srcs, "Actual fixture:\n" . var_export( $registered_style_srcs, true ) ); } /** From d205f75c8436e7b13c2de61e3dcff9488b34ffb6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 9 Aug 2021 15:43:15 -0700 Subject: [PATCH 08/12] Revert "Default to keeping all invalid markup when Bento is enabled" This reverts commit 0b7062729f737de3e8584126c169e9129e9d3e21. --- includes/validation/class-amp-validation-manager.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index 44c66f68ea3..402ec407099 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -262,9 +262,7 @@ public static function post_supports_validation( $post ) { */ public static function is_sanitization_auto_accepted( $error = null ) { - if ( amp_is_bento_enabled() ) { - $accepted = false; - } elseif ( $error && amp_is_canonical() ) { + if ( $error && amp_is_canonical() ) { // Excessive CSS on AMP-first sites must not be removed by default since removing CSS can severely break a site. $accepted = AMP_Style_Sanitizer::STYLESHEET_TOO_LONG !== $error['code']; } else { From 41229219f7759ea363b1ed1aa4f6c2b48de46072 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 9 Aug 2021 15:55:03 -0700 Subject: [PATCH 09/12] Improve php comments --- includes/amp-helper-functions.php | 17 ++++++++++++----- includes/class-amp-theme-support.php | 4 +++- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index f934c2fdbae..e7150c8b58d 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -874,19 +874,25 @@ function amp_add_generator_metadata() { } /** - * Determine whether Bento is enabled. + * Determine whether the use of Bento components is enabled. + * + * When Bento is enabled, newer experimental versions of AMP components are used which incorporate the next generation + * of the component framework. * * @since 2.2 - * @internal + * @link https://blog.amp.dev/2021/01/28/bento/ * - * @return bool + * @return bool Whether Bento components are enabled. */ function amp_is_bento_enabled() { /** - * Filters whether Bento is enabled. + * Filters whether the use of Bento components is enabled. + * + * When Bento is enabled, newer experimental versions of AMP components are used which incorporate the next generation + * of the component framework. * * @since 2.2 - * @internal + * @link https://blog.amp.dev/2021/01/28/bento/ * * @param bool $enabled Enabled. */ @@ -1376,6 +1382,7 @@ function amp_is_dev_mode() { is_customize_preview() || // Force dev mode for Bento since it currently requires the Bento experiment opt-in script. + // @todo Remove this once Bento no longer requires an experiment to opt-in. See . amp_is_bento_enabled() ) ); diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index b943ea5add2..b31088e82ea 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -916,6 +916,8 @@ static function() { } ); + // Enable Bento experiment per . + // @todo Remove this once Bento no longer requires an experiment to opt-in. if ( amp_is_bento_enabled() ) { add_action( 'wp_head', @@ -2145,7 +2147,7 @@ private static function get_optimizer( $args ) { add_filter( 'amp_enable_ssr', static function () use ( $args ) { - // SSR currently does not work with Bento. + // SSR currently does not work reliably with Bento. See . if ( amp_is_bento_enabled() ) { return false; } From 91da7a39c6d29e32d440f6b705ff427c3d259554 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 11 Aug 2021 21:43:02 -0700 Subject: [PATCH 10/12] Update test_amp_register_default_scripts_and_styles_with_bento --- tests/php/test-amp-helper-functions.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index dd64470b2f5..a09528ecfe5 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -1463,7 +1463,7 @@ public function test_amp_register_default_scripts_and_styles_with_bento() { 'amp-bind' => 'v0/amp-bind-0.1.js', 'amp-bodymovin-animation' => 'v0/amp-bodymovin-animation-0.1.js', 'amp-brid-player' => 'v0/amp-brid-player-0.1.js', - 'amp-brightcove' => 'v0/amp-brightcove-0.1.js', + 'amp-brightcove' => 'v0/amp-brightcove-1.0.js', 'amp-byside-content' => 'v0/amp-byside-content-0.1.js', 'amp-cache-url' => 'v0/amp-cache-url-0.1.js', 'amp-call-tracking' => 'v0/amp-call-tracking-0.1.js', @@ -1478,7 +1478,7 @@ public function test_amp_register_default_scripts_and_styles_with_bento() { 'amp-dynamic-css-classes' => 'v0/amp-dynamic-css-classes-0.1.js', 'amp-embedly-card' => 'v0/amp-embedly-card-0.1.js', 'amp-experiment' => 'v0/amp-experiment-0.1.js', - 'amp-facebook' => 'v0/amp-facebook-0.1.js', + 'amp-facebook' => 'v0/amp-facebook-1.0.js', 'amp-facebook-comments' => 'v0/amp-facebook-comments-0.1.js', 'amp-facebook-like' => 'v0/amp-facebook-like-0.1.js', 'amp-facebook-page' => 'v0/amp-facebook-page-0.1.js', @@ -1600,6 +1600,8 @@ public function test_amp_register_default_scripts_and_styles_with_bento() { $expected_styles = [ 'amp-accordion' => 'v0/amp-accordion-1.0.css', 'amp-base-carousel' => 'v0/amp-base-carousel-1.0.css', + 'amp-brightcove' => 'v0/amp-brightcove-1.0.css', + 'amp-facebook' => 'v0/amp-facebook-1.0.css', 'amp-fit-text' => 'v0/amp-fit-text-1.0.css', 'amp-inline-gallery' => 'v0/amp-inline-gallery-1.0.css', 'amp-instagram' => 'v0/amp-instagram-1.0.css', From 16cbafb4d0180405aea232840f0c1d21887ed12e Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 11 Aug 2021 23:07:54 -0700 Subject: [PATCH 11/12] Add test for bento experiment opt-in being present; improve code coverage --- includes/class-amp-theme-support.php | 2 ++ tests/php/test-class-amp-theme-support.php | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index b31088e82ea..9a4760b2491 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -2147,10 +2147,12 @@ private static function get_optimizer( $args ) { add_filter( 'amp_enable_ssr', static function () use ( $args ) { + // @codeCoverageIgnoreStart // SSR currently does not work reliably with Bento. See . if ( amp_is_bento_enabled() ) { return false; } + // @codeCoverageIgnoreEnd return array_key_exists( ConfigurationArgument::ENABLE_SSR, $args ) ? $args[ ConfigurationArgument::ENABLE_SSR ] diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 6948ca4396e..43b357f7b6a 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -1758,11 +1758,7 @@ static function ( $url ) { /** * Test prepare_response when dev mode is forced. * - * @global WP_Widget_Factory $wp_widget_factory - * @global WP_Scripts $wp_scripts * @covers AMP_Theme_Support::prepare_response() - * @covers AMP_Theme_Support::ensure_required_markup() - * @covers ::amp_render_scripts() */ public function test_prepare_response_in_forced_dev_mode() { $this->set_template_mode( AMP_Theme_Support::STANDARD_MODE_SLUG ); @@ -1779,6 +1775,21 @@ public function test_prepare_response_in_forced_dev_mode() { $this->assertStringNotContainsString( 'set_template_mode( AMP_Theme_Support::STANDARD_MODE_SLUG ); + + add_filter( 'amp_bento_enabled', '__return_true' ); + wp(); + + $html = AMP_Theme_Support::prepare_response( $this->get_original_html() ); + $this->assertStringContainsString( 'AMP.toggleExperiment(\'bento\', true);', $html ); + } + /** * Test prepare_response for standard mode when some validation errors aren't auto-sanitized. * From 76e9c71242732b1a94934ca0af6adfc290492fc9 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 11 Aug 2021 23:11:16 -0700 Subject: [PATCH 12/12] Fix typo in comment Co-authored-by: Pierre Gordon <16200219+pierlon@users.noreply.github.com> --- includes/amp-helper-functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index e7150c8b58d..478e970c926 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -998,7 +998,7 @@ function amp_register_default_styles( WP_Styles $styles ) { ); $styles->add_data( 'amp-icons', 'rtl', 'replace' ); - // These are registered exclusively for used for non-AMP pages that manually enqueue them. They aren't needed on + // These are registered exclusively for non-AMP pages that manually enqueue them. They aren't needed on // AMP pages due to the runtime style being present and because the styles are inlined in the scripts already. if ( amp_is_bento_enabled() ) { foreach ( AMP_Allowed_Tags_Generated::get_extension_specs() as $extension_name => $extension_spec ) {