From 2a69d65a57f2e2262ee070588b0373b0d617e3fd Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 7 Dec 2021 21:06:07 -0800 Subject: [PATCH 01/36] Check dependency_support in is_needed for AmpPlugins, AmpThemes, and PluginActivationSiteScan --- src/Admin/AmpPlugins.php | 5 +++++ src/Admin/AmpThemes.php | 5 +++++ src/Admin/PluginActivationSiteScan.php | 4 +++- src/Admin/SupportScreen.php | 1 + 4 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Admin/AmpPlugins.php b/src/Admin/AmpPlugins.php index 7162803c73e..5da16c1e73b 100644 --- a/src/Admin/AmpPlugins.php +++ b/src/Admin/AmpPlugins.php @@ -11,6 +11,7 @@ use AmpProject\AmpWP\Infrastructure\Delayed; use AmpProject\AmpWP\Infrastructure\Registerable; use AmpProject\AmpWP\Infrastructure\Service; +use AmpProject\AmpWP\Services; use WP_Screen; use function get_current_screen; use stdClass; @@ -61,6 +62,10 @@ public static function get_registration_action() { */ public static function is_needed() { + if ( ! Services::get( 'dependency_support' )->has_support() ) { + return false; + } + /** This filter is documented in src/Admin/AmpThemes.php */ return is_admin() && apply_filters( 'amp_compatible_ecosystem_shown', true, 'plugins' ); } diff --git a/src/Admin/AmpThemes.php b/src/Admin/AmpThemes.php index 7f4a25ecf2c..834e7ca308d 100644 --- a/src/Admin/AmpThemes.php +++ b/src/Admin/AmpThemes.php @@ -11,6 +11,7 @@ use AmpProject\AmpWP\Infrastructure\Delayed; use AmpProject\AmpWP\Infrastructure\Registerable; use AmpProject\AmpWP\Infrastructure\Service; +use AmpProject\AmpWP\Services; use WP_Screen; use stdClass; @@ -60,6 +61,10 @@ public static function get_registration_action() { */ public static function is_needed() { + if ( ! Services::get( 'dependency_support' )->has_support() ) { + return false; + } + /** * Filters whether to show AMP compatible ecosystem in the admin. * diff --git a/src/Admin/PluginActivationSiteScan.php b/src/Admin/PluginActivationSiteScan.php index 1216ec5886f..40ab8bfbcaa 100644 --- a/src/Admin/PluginActivationSiteScan.php +++ b/src/Admin/PluginActivationSiteScan.php @@ -13,11 +13,11 @@ use AMP_Options_Manager; use AMP_Validation_Manager; -use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Infrastructure\Conditional; use AmpProject\AmpWP\Infrastructure\Delayed; use AmpProject\AmpWP\Infrastructure\Registerable; use AmpProject\AmpWP\Infrastructure\Service; +use AmpProject\AmpWP\Services; /** * Class PluginActivationSiteScan @@ -67,6 +67,8 @@ public static function is_needed() { return ( is_admin() && + Services::get( 'dependency_support' )->has_support() + && ! is_network_admin() && 'plugins.php' === $pagenow diff --git a/src/Admin/SupportScreen.php b/src/Admin/SupportScreen.php index 66cbe556247..3f8ef26b193 100644 --- a/src/Admin/SupportScreen.php +++ b/src/Admin/SupportScreen.php @@ -106,6 +106,7 @@ public static function is_needed() { return ( is_admin() && + // Note: The view_site_health_checks capability was introduced in WordPress 5.2, so this will return false in older versions. self::has_cap() ); } From 949eeb34e14973d126e2a5fd9cb315efd2bfd154 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 7 Dec 2021 21:39:15 -0800 Subject: [PATCH 02/36] Add status=active parameter to /wp/v2/themes REST API call for WP<5.7 compat --- assets/src/components/themes-context-provider/index.js | 2 +- src/Admin/OnboardingWizardSubmenuPage.php | 2 +- src/Admin/OptionsMenu.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/src/components/themes-context-provider/index.js b/assets/src/components/themes-context-provider/index.js index ffd57c2243b..5efcecb127c 100644 --- a/assets/src/components/themes-context-provider/index.js +++ b/assets/src/components/themes-context-provider/index.js @@ -62,7 +62,7 @@ export function ThemesContextProvider( { try { const fetchedThemes = await apiFetch( { - path: '/wp/v2/themes', + path: '/wp/v2/themes?status=active', } ); if ( hasUnmounted.current === true ) { diff --git a/src/Admin/OnboardingWizardSubmenuPage.php b/src/Admin/OnboardingWizardSubmenuPage.php index ba9345ac55c..0b43d18f2b9 100644 --- a/src/Admin/OnboardingWizardSubmenuPage.php +++ b/src/Admin/OnboardingWizardSubmenuPage.php @@ -303,7 +303,7 @@ protected function add_preload_rest_paths() { ), '/wp/v2/plugins', '/wp/v2/settings', - '/wp/v2/themes', + '/wp/v2/themes?status=active', '/wp/v2/users/me', ]; diff --git a/src/Admin/OptionsMenu.php b/src/Admin/OptionsMenu.php index b712fed936c..95bcfaa7f12 100644 --- a/src/Admin/OptionsMenu.php +++ b/src/Admin/OptionsMenu.php @@ -322,7 +322,7 @@ protected function add_preload_rest_paths() { ), '/wp/v2/plugins', '/wp/v2/settings', - '/wp/v2/themes', + '/wp/v2/themes?status=active', '/wp/v2/users/me', ]; From 7fa097edc886f2b97272435c0c9a9b9e32b4faa7 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 8 Dec 2021 10:13:44 +0100 Subject: [PATCH 03/36] Do not search for blocks when on WordPress 4.9 and older --- includes/validation/class-amp-validation-manager.php | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index af072d1b800..e606d10bb6d 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -666,7 +666,16 @@ public static function add_validation_error_sourcing() { add_filter( 'do_shortcode_tag', [ __CLASS__, 'decorate_shortcode_source' ], PHP_INT_MAX, 2 ); add_filter( 'embed_oembed_html', [ __CLASS__, 'decorate_embed_source' ], PHP_INT_MAX, 3 ); - add_filter( 'the_content', [ __CLASS__, 'add_block_source_comments' ], 8 ); // The do_blocks() function runs at priority 9. + + // The `WP_Block_Type_Registry` class was added in WordPress 5.0.0. Because of that it sometimes caused issues + // on the AMP Validated URL screen when on WordPress 4.9. + if ( class_exists( 'WP_Block_Type_Registry' ) ) { + add_filter( 'the_content', [ + __CLASS__, + 'add_block_source_comments' + ], 8 ); // The do_blocks() function runs at priority 9. + } + add_filter( 'the_editor', [ __CLASS__, 'filter_the_editor_to_detect_sources' ] ); } From f57f25ce378fb7301430b285c7c965cf57a326db Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 8 Dec 2021 10:32:22 +0100 Subject: [PATCH 04/36] Fail silently if plugins/themes endpoints are not available On WordPress 5.4 and older, the themes and plugins REST endpoints are not available. We use them in Site Scan for providing a source name, version, and author instead of a bare slug. Since this information is supplementary, we can fail silently if the endpoints are not available. --- assets/src/admin/site-scan-notice/index.js | 2 +- .../plugins-context-provider/index.js | 37 +++---------------- .../themes-context-provider/index.js | 37 +++---------------- assets/src/onboarding-wizard/index.js | 4 +- assets/src/settings-page/index.js | 4 +- 5 files changed, 17 insertions(+), 67 deletions(-) diff --git a/assets/src/admin/site-scan-notice/index.js b/assets/src/admin/site-scan-notice/index.js index 74c5b6d44c3..ff73fd6d559 100644 --- a/assets/src/admin/site-scan-notice/index.js +++ b/assets/src/admin/site-scan-notice/index.js @@ -49,7 +49,7 @@ function Providers( { children } ) { optionsRestPath={ OPTIONS_REST_PATH } populateDefaultValues={ false } > - + - - + + - - + + Date: Wed, 8 Dec 2021 10:44:24 +0100 Subject: [PATCH 05/36] Ensure Onboarding Wizard works on WordPress 4.9 --- webpack.config.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/webpack.config.js b/webpack.config.js index 1aaedc8327e..97c8929a567 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -256,9 +256,6 @@ const onboardingWizard = { switch ( handle ) { case 'lodash': case '@wordpress/api-fetch': - case '@wordpress/dom-ready': - case '@wordpress/html-entities': - case '@wordpress/url': case '@wordpress/i18n': return defaultRequestToHandle( handle ); @@ -270,9 +267,6 @@ const onboardingWizard = { switch ( external ) { case 'lodash': case '@wordpress/api-fetch': - case '@wordpress/dom-ready': - case '@wordpress/html-entities': - case '@wordpress/url': case '@wordpress/i18n': return defaultRequestToExternal( external ); From 148d3977addf45d71487b187234833e94dce5718 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 8 Dec 2021 11:01:37 +0100 Subject: [PATCH 06/36] Fetch used plugin and theme fields only --- .../src/components/plugins-context-provider/index.js | 5 ++++- .../src/components/themes-context-provider/index.js | 5 ++++- src/Admin/OnboardingWizardSubmenuPage.php | 12 ++++++++++-- src/Admin/OptionsMenu.php | 12 ++++++++++-- src/Admin/PluginActivationSiteScan.php | 6 +++++- .../src/Admin/OnboardingWizardSubmenuPageTest.php | 4 ++-- tests/php/src/Admin/OptionsMenuTest.php | 4 ++-- tests/php/src/Admin/PluginActivationSiteScanTest.php | 2 +- 8 files changed, 38 insertions(+), 12 deletions(-) diff --git a/assets/src/components/plugins-context-provider/index.js b/assets/src/components/plugins-context-provider/index.js index 7b26fa41855..35699444533 100644 --- a/assets/src/components/plugins-context-provider/index.js +++ b/assets/src/components/plugins-context-provider/index.js @@ -8,6 +8,7 @@ import PropTypes from 'prop-types'; */ import { createContext, useEffect, useRef, useState } from '@wordpress/element'; import apiFetch from '@wordpress/api-fetch'; +import { addQueryArgs } from '@wordpress/url'; export const Plugins = createContext(); @@ -44,7 +45,9 @@ export function PluginsContextProvider( { children } ) { try { const fetchedPlugins = await apiFetch( { - path: '/wp/v2/plugins', + path: addQueryArgs( '/wp/v2/plugins', { + _fields: [ 'author', 'name', 'plugin', 'status', 'version' ], + } ), } ); if ( hasUnmounted.current === true ) { diff --git a/assets/src/components/themes-context-provider/index.js b/assets/src/components/themes-context-provider/index.js index b124c9e7e54..24bb6d8d6cc 100644 --- a/assets/src/components/themes-context-provider/index.js +++ b/assets/src/components/themes-context-provider/index.js @@ -8,6 +8,7 @@ import PropTypes from 'prop-types'; */ import { createContext, useEffect, useRef, useState } from '@wordpress/element'; import apiFetch from '@wordpress/api-fetch'; +import { addQueryArgs } from '@wordpress/url'; export const Themes = createContext(); @@ -44,7 +45,9 @@ export function ThemesContextProvider( { children } ) { try { const fetchedThemes = await apiFetch( { - path: '/wp/v2/themes?status=active', + path: addQueryArgs( '/wp/v2/themes', { + _fields: [ 'author', 'name', 'status', 'stylesheet', 'version' ], + } ), } ); if ( hasUnmounted.current === true ) { diff --git a/src/Admin/OnboardingWizardSubmenuPage.php b/src/Admin/OnboardingWizardSubmenuPage.php index 0b43d18f2b9..dfc038c21fe 100644 --- a/src/Admin/OnboardingWizardSubmenuPage.php +++ b/src/Admin/OnboardingWizardSubmenuPage.php @@ -301,9 +301,17 @@ protected function add_preload_rest_paths() { ], '/amp/v1/scannable-urls' ), - '/wp/v2/plugins', + add_query_arg( + '_fields', + [ 'author', 'name', 'plugin', 'status', 'version' ], + '/wp/v2/plugins' + ), '/wp/v2/settings', - '/wp/v2/themes?status=active', + add_query_arg( + '_fields', + [ 'author', 'name', 'status', 'stylesheet', 'version' ], + '/wp/v2/themes' + ), '/wp/v2/users/me', ]; diff --git a/src/Admin/OptionsMenu.php b/src/Admin/OptionsMenu.php index 95bcfaa7f12..0cfd9f8de09 100644 --- a/src/Admin/OptionsMenu.php +++ b/src/Admin/OptionsMenu.php @@ -320,9 +320,17 @@ protected function add_preload_rest_paths() { [ 'url', 'amp_url', 'type', 'label', 'validation_errors', 'stale' ], '/amp/v1/scannable-urls' ), - '/wp/v2/plugins', + add_query_arg( + '_fields', + [ 'author', 'name', 'plugin', 'status', 'version' ], + '/wp/v2/plugins' + ), '/wp/v2/settings', - '/wp/v2/themes?status=active', + add_query_arg( + '_fields', + [ 'author', 'name', 'status', 'stylesheet', 'version' ], + '/wp/v2/themes' + ), '/wp/v2/users/me', ]; diff --git a/src/Admin/PluginActivationSiteScan.php b/src/Admin/PluginActivationSiteScan.php index 40ab8bfbcaa..c3fa3f9576d 100644 --- a/src/Admin/PluginActivationSiteScan.php +++ b/src/Admin/PluginActivationSiteScan.php @@ -182,7 +182,11 @@ protected function add_preload_rest_paths() { ], '/amp/v1/scannable-urls' ), - '/wp/v2/plugins', + add_query_arg( + '_fields', + [ 'author', 'name', 'plugin', 'status', 'version' ], + '/wp/v2/plugins' + ), '/wp/v2/users/me', ]; diff --git a/tests/php/src/Admin/OnboardingWizardSubmenuPageTest.php b/tests/php/src/Admin/OnboardingWizardSubmenuPageTest.php index 9ea51a0cef2..7d226be32e8 100644 --- a/tests/php/src/Admin/OnboardingWizardSubmenuPageTest.php +++ b/tests/php/src/Admin/OnboardingWizardSubmenuPageTest.php @@ -189,9 +189,9 @@ function ( $caps, $cap ) { '/amp/v1/options', '/amp/v1/reader-themes', '/amp/v1/scannable-urls?_fields%5B0%5D=url&_fields%5B1%5D=amp_url&_fields%5B2%5D=type&_fields%5B3%5D=label&force_standard_mode=1', - '/wp/v2/plugins', + '/wp/v2/plugins?_fields%5B0%5D=author&_fields%5B1%5D=name&_fields%5B2%5D=plugin&_fields%5B3%5D=status&_fields%5B4%5D=version', '/wp/v2/settings', - '/wp/v2/themes', + '/wp/v2/themes?_fields%5B0%5D=author&_fields%5B1%5D=name&_fields%5B2%5D=status&_fields%5B3%5D=stylesheet&_fields%5B4%5D=version', '/wp/v2/users/me', ], $this->get_private_property( $rest_preloader, 'paths' ) diff --git a/tests/php/src/Admin/OptionsMenuTest.php b/tests/php/src/Admin/OptionsMenuTest.php index 5bc891f1a53..2c55f3eea95 100644 --- a/tests/php/src/Admin/OptionsMenuTest.php +++ b/tests/php/src/Admin/OptionsMenuTest.php @@ -244,9 +244,9 @@ function ( $caps, $cap ) { '/amp/v1/options', '/amp/v1/reader-themes', '/amp/v1/scannable-urls?_fields%5B0%5D=url&_fields%5B1%5D=amp_url&_fields%5B2%5D=type&_fields%5B3%5D=label&_fields%5B4%5D=validation_errors&_fields%5B5%5D=stale', - '/wp/v2/plugins', + '/wp/v2/plugins?_fields%5B0%5D=author&_fields%5B1%5D=name&_fields%5B2%5D=plugin&_fields%5B3%5D=status&_fields%5B4%5D=version', '/wp/v2/settings', - '/wp/v2/themes', + '/wp/v2/themes?_fields%5B0%5D=author&_fields%5B1%5D=name&_fields%5B2%5D=status&_fields%5B3%5D=stylesheet&_fields%5B4%5D=version', '/wp/v2/users/me', ], $this->get_private_property( $rest_preloader, 'paths' ) diff --git a/tests/php/src/Admin/PluginActivationSiteScanTest.php b/tests/php/src/Admin/PluginActivationSiteScanTest.php index 2b14ae6d840..907ac93102c 100644 --- a/tests/php/src/Admin/PluginActivationSiteScanTest.php +++ b/tests/php/src/Admin/PluginActivationSiteScanTest.php @@ -226,7 +226,7 @@ function ( $caps, $cap ) { [ '/amp/v1/options', '/amp/v1/scannable-urls?_fields%5B0%5D=url&_fields%5B1%5D=amp_url&_fields%5B2%5D=type&_fields%5B3%5D=label', - '/wp/v2/plugins', + '/wp/v2/plugins?_fields%5B0%5D=author&_fields%5B1%5D=name&_fields%5B2%5D=plugin&_fields%5B3%5D=status&_fields%5B4%5D=version', '/wp/v2/users/me', ], $this->get_private_property( $rest_preloader, 'paths' ) From 6b553c43e5b79ad7aaf14eefb5e0a2abdcc00eca Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Wed, 8 Dec 2021 11:18:05 +0100 Subject: [PATCH 07/36] Update message copy --- .../site-scan-results/plugins-with-amp-incompatibility.js | 2 +- .../site-scan-results/themes-with-amp-incompatibility.js | 2 +- tests/e2e/specs/admin/site-scan-panel.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/src/components/site-scan-results/plugins-with-amp-incompatibility.js b/assets/src/components/site-scan-results/plugins-with-amp-incompatibility.js index b3afb2b6381..489689eefa1 100644 --- a/assets/src/components/site-scan-results/plugins-with-amp-incompatibility.js +++ b/assets/src/components/site-scan-results/plugins-with-amp-incompatibility.js @@ -72,7 +72,7 @@ export function PluginsWithAmpIncompatibility( { ); diff --git a/assets/src/components/site-scan-results/themes-with-amp-incompatibility.js b/assets/src/components/site-scan-results/themes-with-amp-incompatibility.js index c12068ad263..ecb7380ec1a 100644 --- a/assets/src/components/site-scan-results/themes-with-amp-incompatibility.js +++ b/assets/src/components/site-scan-results/themes-with-amp-incompatibility.js @@ -72,7 +72,7 @@ export function ThemesWithAmpIncompatibility( { ); diff --git a/tests/e2e/specs/admin/site-scan-panel.js b/tests/e2e/specs/admin/site-scan-panel.js index 66ec2403e5c..021e17cc3d2 100644 --- a/tests/e2e/specs/admin/site-scan-panel.js +++ b/tests/e2e/specs/admin/site-scan-panel.js @@ -163,7 +163,7 @@ describe( 'AMP settings screen Site Scan panel', () => { await visitAdminPage( 'admin.php', 'page=amp-options' ); await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-slug', { text: /e2e-tests-demo-plugin/ } ); - await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-notice', { text: /This plugin has been uninstalled since last site scan./ } ); + await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-notice', { text: /This plugin has been uninstalled or the plugins data is not available../ } ); // Clean up. await installLocalPlugin( 'e2e-tests-demo-plugin' ); From c8a2d418f05ef0cd11d264ea6392cb1015f4f7c9 Mon Sep 17 00:00:00 2001 From: Dhaval Parekh Date: Wed, 8 Dec 2021 19:30:35 +0530 Subject: [PATCH 08/36] Add minimum WordPress version check for Support page --- src/Admin/SupportLink.php | 6 +++++- src/Admin/SupportScreen.php | 18 ++++++++++++++++++ tests/php/src/Admin/SupportScreenTest.php | 19 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/Admin/SupportLink.php b/src/Admin/SupportLink.php index 2e3d511ce45..5484fb24e41 100644 --- a/src/Admin/SupportLink.php +++ b/src/Admin/SupportLink.php @@ -38,7 +38,11 @@ public static function get_registration_action() { * @return bool Whether the conditional object is needed. */ public static function is_needed() { - return SupportScreen::has_cap(); + return ( + SupportScreen::check_core_version() + && + SupportScreen::has_cap() + ); } /** diff --git a/src/Admin/SupportScreen.php b/src/Admin/SupportScreen.php index 3f8ef26b193..c138d9b6939 100644 --- a/src/Admin/SupportScreen.php +++ b/src/Admin/SupportScreen.php @@ -32,6 +32,13 @@ class SupportScreen implements Conditional, Delayed, Service, Registerable { */ const ASSET_HANDLE = 'amp-support'; + /** + * The minimum version of WordPress support for the "Support page". + * + * @var string + */ + const WP_MIN_VERSION = '5.2'; + /** * Injector. * @@ -97,6 +104,15 @@ public static function has_cap() { ); } + /** + * Returns whether minimum WordPress version is available for support page or not. + * + * @return bool True if current WordPress's version is greater than or equal to minimum version. + */ + public static function check_core_version() { + return version_compare( get_bloginfo( 'version' ), self::WP_MIN_VERSION, '>=' ); + } + /** * Check whether the conditional object is currently needed. * @@ -104,6 +120,8 @@ public static function has_cap() { */ public static function is_needed() { return ( + self::check_core_version() + && is_admin() && // Note: The view_site_health_checks capability was introduced in WordPress 5.2, so this will return false in older versions. diff --git a/tests/php/src/Admin/SupportScreenTest.php b/tests/php/src/Admin/SupportScreenTest.php index bc21254094a..24ebfb556f4 100644 --- a/tests/php/src/Admin/SupportScreenTest.php +++ b/tests/php/src/Admin/SupportScreenTest.php @@ -69,6 +69,25 @@ public function test_get_registration_action() { $this->assertEquals( 'init', SupportScreen::get_registration_action() ); } + /** + * @covers ::check_core_version() + */ + public function test_check_core_version() { + + global $wp_version; + + $original_wp_version = $wp_version; + + $wp_version = '4.9'; + $this->assertFalse( SupportScreen::check_core_version() ); + + $wp_version = '5.2'; + $this->assertTrue( SupportScreen::check_core_version() ); + + $wp_version = $original_wp_version; + $this->assertTrue( SupportScreen::check_core_version() ); + } + /** * @covers ::is_needed() * @covers ::has_cap() From da361aee47cc075cedfa3becc4e9c38bf5660ae0 Mon Sep 17 00:00:00 2001 From: Dhaval Parekh Date: Wed, 8 Dec 2021 19:49:09 +0530 Subject: [PATCH 09/36] Update unit test cases --- src/Admin/SupportScreen.php | 1 - tests/php/src/Admin/AmpPluginsTest.php | 12 ++++++++++-- tests/php/src/Admin/AmpThemesTest.php | 13 +++++++++++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/Admin/SupportScreen.php b/src/Admin/SupportScreen.php index c138d9b6939..5954d597df1 100644 --- a/src/Admin/SupportScreen.php +++ b/src/Admin/SupportScreen.php @@ -124,7 +124,6 @@ public static function is_needed() { && is_admin() && - // Note: The view_site_health_checks capability was introduced in WordPress 5.2, so this will return false in older versions. self::has_cap() ); } diff --git a/tests/php/src/Admin/AmpPluginsTest.php b/tests/php/src/Admin/AmpPluginsTest.php index 057f33a9abf..c2f01ec3eee 100644 --- a/tests/php/src/Admin/AmpPluginsTest.php +++ b/tests/php/src/Admin/AmpPluginsTest.php @@ -134,15 +134,22 @@ public function test_get_registration_action() { * @covers ::is_needed() */ public function test_is_needed() { + global $wp_version; + $original_wp_version = $wp_version; // Test 1: Not admin request. $this->assertFalse( AmpPlugins::is_needed() ); - // Test 2: Admin request. + // Test 2: Check with older version of WordPress. + $wp_version = '4.9'; + $this->assertFalse( AmpPlugins::is_needed() ); + + // Test 3: Admin request in supported WordPress . + $wp_version = '5.6'; set_current_screen( 'index.php' ); $this->assertTrue( AmpPlugins::is_needed() ); - // Test 3: Filter disables. + // Test 4: Filter disables. add_filter( 'amp_compatible_ecosystem_shown', static function ( $shown, $type ) { @@ -158,6 +165,7 @@ static function ( $shown, $type ) { $this->assertFalse( AmpPlugins::is_needed() ); set_current_screen( 'front' ); + $wp_version = $original_wp_version; } /** diff --git a/tests/php/src/Admin/AmpThemesTest.php b/tests/php/src/Admin/AmpThemesTest.php index 1d88eb8c1c5..a2f914e5848 100644 --- a/tests/php/src/Admin/AmpThemesTest.php +++ b/tests/php/src/Admin/AmpThemesTest.php @@ -52,18 +52,26 @@ public function test_get_registration_action() { * @covers ::is_needed() */ public function test_is_needed() { + global $wp_version; + $original_wp_version = $wp_version; + set_current_screen( 'front' ); // Test 1: Not admin request. $this->assertFalse( is_admin() ); $this->assertFalse( AmpThemes::is_needed() ); - // Test 2: Admin request. + // Test 2: Check with older version of WordPress. + $wp_version = '4.9'; + $this->assertFalse( AmpThemes::is_needed() ); + + // Test 3: Admin request. + $wp_version = '5.6'; set_current_screen( 'index.php' ); $this->assertTrue( is_admin() ); $this->assertTrue( AmpThemes::is_needed() ); - // Test 3: Filter disables. + // Test 4: Filter disables. add_filter( 'amp_compatible_ecosystem_shown', static function ( $shown, $type ) { @@ -78,6 +86,7 @@ static function ( $shown, $type ) { $this->assertFalse( AmpThemes::is_needed() ); set_current_screen( 'front' ); + $wp_version = $original_wp_version; } /** From bec8cbe1c7401e8b066002f190488a2b3488acd0 Mon Sep 17 00:00:00 2001 From: Dhaval Parekh Date: Wed, 8 Dec 2021 19:59:23 +0530 Subject: [PATCH 10/36] Fix phpcs issues --- includes/validation/class-amp-validation-manager.php | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php index e606d10bb6d..df7ac1e86ea 100644 --- a/includes/validation/class-amp-validation-manager.php +++ b/includes/validation/class-amp-validation-manager.php @@ -670,10 +670,14 @@ public static function add_validation_error_sourcing() { // The `WP_Block_Type_Registry` class was added in WordPress 5.0.0. Because of that it sometimes caused issues // on the AMP Validated URL screen when on WordPress 4.9. if ( class_exists( 'WP_Block_Type_Registry' ) ) { - add_filter( 'the_content', [ - __CLASS__, - 'add_block_source_comments' - ], 8 ); // The do_blocks() function runs at priority 9. + add_filter( + 'the_content', + [ + __CLASS__, + 'add_block_source_comments', + ], + 8 + ); // The do_blocks() function runs at priority 9. } add_filter( 'the_editor', [ __CLASS__, 'filter_the_editor_to_detect_sources' ] ); From 15a2a0f3f6cd509b035382dcd4ce3e5f9ed3de91 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Dec 2021 10:17:56 -0800 Subject: [PATCH 11/36] Improve theme/plugin unavailability messaging --- .../site-scan-results/plugins-with-amp-incompatibility.js | 2 +- .../site-scan-results/themes-with-amp-incompatibility.js | 2 +- tests/e2e/specs/admin/site-scan-panel.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/src/components/site-scan-results/plugins-with-amp-incompatibility.js b/assets/src/components/site-scan-results/plugins-with-amp-incompatibility.js index 489689eefa1..2ecb492fa54 100644 --- a/assets/src/components/site-scan-results/plugins-with-amp-incompatibility.js +++ b/assets/src/components/site-scan-results/plugins-with-amp-incompatibility.js @@ -72,7 +72,7 @@ export function PluginsWithAmpIncompatibility( { ); diff --git a/assets/src/components/site-scan-results/themes-with-amp-incompatibility.js b/assets/src/components/site-scan-results/themes-with-amp-incompatibility.js index ecb7380ec1a..4b0a11b1051 100644 --- a/assets/src/components/site-scan-results/themes-with-amp-incompatibility.js +++ b/assets/src/components/site-scan-results/themes-with-amp-incompatibility.js @@ -72,7 +72,7 @@ export function ThemesWithAmpIncompatibility( { ); diff --git a/tests/e2e/specs/admin/site-scan-panel.js b/tests/e2e/specs/admin/site-scan-panel.js index 021e17cc3d2..6307186f424 100644 --- a/tests/e2e/specs/admin/site-scan-panel.js +++ b/tests/e2e/specs/admin/site-scan-panel.js @@ -163,7 +163,7 @@ describe( 'AMP settings screen Site Scan panel', () => { await visitAdminPage( 'admin.php', 'page=amp-options' ); await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-slug', { text: /e2e-tests-demo-plugin/ } ); - await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-notice', { text: /This plugin has been uninstalled or the plugins data is not available../ } ); + await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-notice', { text: /This plugin has been uninstalled or its metadata is unavailable../ } ); // Clean up. await installLocalPlugin( 'e2e-tests-demo-plugin' ); From cff0d8fcc508ccd24c63456979b45f4089d45b1f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Dec 2021 10:30:18 -0800 Subject: [PATCH 12/36] Ensure ID attribute is added to scripts in WP<5.5 --- includes/amp-helper-functions.php | 30 +++++++++++++++++++ .../class-amp-validation-manager.php | 13 ++++++-- tests/php/test-amp-helper-functions.php | 18 +++++++++++ 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 060ed4e20f1..50f10519a8d 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -92,6 +92,11 @@ function amp_bootstrap_plugin() { // Ensure async and custom-element/custom-template attributes are present on script tags. add_filter( 'script_loader_tag', 'amp_filter_script_loader_tag', PHP_INT_MAX, 2 ); + // Ensure ID attribute is present in WP<5.5. + if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { + add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_script_loader_tag', 1000, 2 ); + } + // Ensure crossorigin=anonymous is added to font links. add_filter( 'style_loader_tag', 'amp_filter_font_style_loader_tag_with_crossorigin_anonymous', 10, 4 ); @@ -1157,6 +1162,31 @@ function amp_filter_script_loader_tag( $tag, $handle ) { return $tag; } +/** + * Ensure ID attribute is added to printed scripts. + * + * Core started adding in WP 5.5. This is used both by validation logic for sourcing attribution as well as in the + * script and comments sanitizers. + * + * @link https://core.trac.wordpress.org/changeset/48295 + * @since 2.2 + * @internal + * + * @param string $tag The ', + amp_ensure_id_attribute_script_loader_tag( '', 'foo' ) + ); + + $this->assertEquals( + '', + amp_ensure_id_attribute_script_loader_tag( '', 'foo' ) + ); + } + /** @covers ::amp_init() */ public function test_amp_init_migration() { global $wp_actions; From 734507097d5985830a04e48e8f9dac75e873ac9d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Dec 2021 11:04:03 -0800 Subject: [PATCH 13/36] Harden injection of ID attribute into script loader tag --- includes/amp-helper-functions.php | 18 ++++++++++----- tests/php/test-amp-helper-functions.php | 29 ++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 50f10519a8d..f20e125667c 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1172,16 +1172,22 @@ function amp_filter_script_loader_tag( $tag, $handle ) { * @since 2.2 * @internal * - * @param string $tag The ', + '', amp_ensure_id_attribute_script_loader_tag( '', 'foo' ) ); + $inline_script_before = ''; + $inline_script_after = ''; $this->assertEquals( - '', - amp_ensure_id_attribute_script_loader_tag( '', 'foo' ) + $inline_script_before . '' . $inline_script_after, + amp_ensure_id_attribute_script_loader_tag( + $inline_script_before . '' . $inline_script_after, + 'foo' + ) + ); + + $foo_script = ''; + $this->assertEquals( + $foo_script, + amp_ensure_id_attribute_script_loader_tag( $foo_script, 'foo' ) + ); + + $foo_script = ''; + $this->assertEquals( + $foo_script, + amp_ensure_id_attribute_script_loader_tag( $foo_script, 'foo' ) + ); + + $amp_runtime_script = ''; + $this->assertEquals( + $amp_runtime_script, + amp_ensure_id_attribute_script_loader_tag( $amp_runtime_script, 'amp-runtime' ) ); } From 9360c3b12e50d8c5b7b930c0494d9dfa64fdca92 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Dec 2021 11:22:22 -0800 Subject: [PATCH 14/36] Fix regex pattern inversion --- includes/amp-helper-functions.php | 2 +- tests/php/test-amp-helper-functions.php | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index f20e125667c..5f4d5aa5593 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1179,7 +1179,7 @@ function amp_filter_script_loader_tag( $tag, $handle ) { function amp_ensure_id_attribute_script_loader_tag( $tag, $handle ) { if ( 0 !== strpos( $handle, 'amp-' ) ) { $tag = preg_replace_callback( - '/(]*?\ssrc=["\'].*?["\'])([>]*?>)/', + '/(]*?\ssrc=["\'].*?["\'])([^>]*?>)/', static function ( $matches ) use ( $handle ) { if ( false === strpos( $matches[0], 'id=' ) ) { return $matches[1] . sprintf( ' id="%s"', esc_attr( "$handle-js" ) ) . $matches[2]; diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index 58a7197ad58..aeded3fcb1c 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -157,6 +157,16 @@ public function test_amp_ensure_id_attribute_script_loader_tag() { amp_ensure_id_attribute_script_loader_tag( '', 'foo' ) ); + $this->assertEquals( + '', + amp_ensure_id_attribute_script_loader_tag( '', 'foo' ) + ); + + $this->assertEquals( + '', + amp_ensure_id_attribute_script_loader_tag( '', 'comment-reply' ) + ); + $inline_script_before = ''; $inline_script_after = ''; $this->assertEquals( From b58284da6138115aa8dcfbe9a983bfee23b6d801 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Dec 2021 15:49:47 -0800 Subject: [PATCH 15/36] Prevent access to validation UI when dependency_support is absent Fixes #6775 --- src/DevTools/UserAccess.php | 21 +++++++++++++++++++++ tests/php/src/DevTools/UserAccessTest.php | 17 +++++++++++------ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/src/DevTools/UserAccess.php b/src/DevTools/UserAccess.php index f22b3d2f45d..852ce74f140 100644 --- a/src/DevTools/UserAccess.php +++ b/src/DevTools/UserAccess.php @@ -12,6 +12,7 @@ use AMP_Options_Manager; use AMP_Theme_Support; use AMP_Validation_Manager; +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\Infrastructure\Registerable; use AmpProject\AmpWP\Infrastructure\Service; use AmpProject\AmpWP\Option; @@ -26,6 +27,18 @@ */ final class UserAccess implements Service, Registerable { + /** @var DependencySupport */ + private $dependency_support; + + /** + * Constructor. + * + * @param DependencySupport $dependency_support DependencySupport instance. + */ + public function __construct( DependencySupport $dependency_support ) { + $this->dependency_support = $dependency_support; + } + /** * User meta key enabling or disabling developer tools. * @@ -50,6 +63,14 @@ public function register() { * @return bool Whether developer tools are enabled for the user. */ public function is_user_enabled( $user = null ) { + // Note: This is somewhat of a shortcut for blocking access to the validation UI on unsupported versions of WP. + // It works because we're already using this method in many places to check if the user interface should be + // shown, and its the user interface which is particularly problematic on older versions of WP due to the + // JavaScript dependencies. + if ( ! $this->dependency_support->has_support() ) { + return false; + } + if ( null === $user ) { $user = wp_get_current_user(); } elseif ( ! $user instanceof WP_User ) { diff --git a/tests/php/src/DevTools/UserAccessTest.php b/tests/php/src/DevTools/UserAccessTest.php index 65dc8801329..23c3292542f 100644 --- a/tests/php/src/DevTools/UserAccessTest.php +++ b/tests/php/src/DevTools/UserAccessTest.php @@ -11,7 +11,8 @@ use AMP_Theme_Support; use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Option; -use AmpProject\AmpWP\Tests\TestCase; +use AmpProject\AmpWP\Tests\DependencyInjectedTestCase; +use AmpProject\AmpWP\Tests\Helpers\PrivateAccess; use WP_Error; /** @@ -23,7 +24,9 @@ * * @coversDefaultClass \AmpProject\AmpWP\DevTools\UserAccess */ -class UserAccessTest extends TestCase { +class UserAccessTest extends DependencyInjectedTestCase { + + use PrivateAccess; /** * Test instance. @@ -35,7 +38,7 @@ class UserAccessTest extends TestCase { public function setUp() { parent::setUp(); - $this->dev_tools_user_access = new UserAccess(); + $this->dev_tools_user_access = $this->injector->make( UserAccess::class ); } /** @@ -61,12 +64,14 @@ public function test_is_user_enabled() { $admin_user = self::factory()->user->create_and_get( [ 'role' => 'administrator' ] ); $editor_user = self::factory()->user->create_and_get( [ 'role' => 'editor' ] ); - $this->assertTrue( $this->dev_tools_user_access->is_user_enabled( $admin_user ) ); - $this->assertTrue( $this->dev_tools_user_access->is_user_enabled( $admin_user->ID ) ); + $has_dependency_support = $this->get_private_property( $this->dev_tools_user_access, 'dependency_support' )->has_support(); + + $this->assertEquals( $has_dependency_support, $this->dev_tools_user_access->is_user_enabled( $admin_user ) ); + $this->assertEquals( $has_dependency_support, $this->dev_tools_user_access->is_user_enabled( $admin_user->ID ) ); $this->assertFalse( $this->dev_tools_user_access->is_user_enabled( $editor_user ) ); $this->assertFalse( $this->dev_tools_user_access->is_user_enabled( $editor_user->ID ) ); wp_set_current_user( $admin_user->ID ); - $this->assertTrue( $this->dev_tools_user_access->is_user_enabled() ); + $this->assertEquals( $has_dependency_support, $this->dev_tools_user_access->is_user_enabled() ); $this->dev_tools_user_access->set_user_enabled( $admin_user, false ); $this->assertFalse( $this->dev_tools_user_access->is_user_enabled() ); wp_set_current_user( $editor_user->ID ); From 2913e59cd337971745135c0435dc2342fca410da Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Dec 2021 16:08:13 -0800 Subject: [PATCH 16/36] Fix URLValidationRESTControllerTest after new DependencySupport dependency added to UserAccess --- .../src/Validation/URLValidationRESTControllerTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/php/src/Validation/URLValidationRESTControllerTest.php b/tests/php/src/Validation/URLValidationRESTControllerTest.php index 08f47877a9f..aa01b72046a 100644 --- a/tests/php/src/Validation/URLValidationRESTControllerTest.php +++ b/tests/php/src/Validation/URLValidationRESTControllerTest.php @@ -8,8 +8,8 @@ namespace AmpProject\AmpWP\Tests\Validation; use AmpProject\AmpWP\DevTools\UserAccess; +use AmpProject\AmpWP\Tests\DependencyInjectedTestCase; use AmpProject\AmpWP\Tests\Helpers\ValidationRequestMocking; -use AmpProject\AmpWP\Tests\TestCase; use AmpProject\AmpWP\Validation\URLValidationProvider; use AmpProject\AmpWP\Validation\URLValidationRESTController; use WP_REST_Controller; @@ -22,7 +22,7 @@ * * @coversDefaultClass \AmpProject\AmpWP\Validation\URLValidationRESTController */ -class URLValidationRESTControllerTest extends TestCase { +class URLValidationRESTControllerTest extends DependencyInjectedTestCase { use ValidationRequestMocking; /** @@ -46,8 +46,8 @@ public function setUp() { parent::setUp(); do_action( 'rest_api_init' ); - $this->user_access = new UserAccess(); - $this->controller = new URLValidationRESTController( new URLValidationProvider(), $this->user_access ); + $this->user_access = $this->injector->make( UserAccess::class ); + $this->controller = $this->injector->make( URLValidationRESTController::class ); $this->add_validate_response_mocking_filter(); } From d830cb6420f428acba8da44daf90ed62b3201436 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 7 Dec 2021 14:56:25 -0800 Subject: [PATCH 17/36] Remove obsolete SavePostValidationEvent Closes #6771 --- .phpstorm.meta.php | 1 - src/AmpWpPlugin.php | 2 - src/Validation/SavePostValidationEvent.php | 171 ----------------- .../SavePostValidationEventTest.php | 177 ------------------ 4 files changed, 351 deletions(-) delete mode 100644 src/Validation/SavePostValidationEvent.php delete mode 100644 tests/php/src/Validation/SavePostValidationEventTest.php diff --git a/.phpstorm.meta.php b/.phpstorm.meta.php index 0065d6e2990..c65e3ae2f92 100644 --- a/.phpstorm.meta.php +++ b/.phpstorm.meta.php @@ -54,7 +54,6 @@ 'rest.scannable_urls_controller' => \AmpProject\AmpWP\Validation\ScannableURLsRestController::class, 'rest.validation_counts_controller' => \AmpProject\AmpWP\Validation\ValidationCountsRestController::class, 'sandboxing' => \AmpProject\AmpWP\Sandboxing::class, - 'save_post_validation_event' => \AmpProject\AmpWP\Validation\SavePostValidationEvent::class, 'server_timing' => \AmpProject\AmpWP\Instrumentation\ServerTiming::class, 'site_health_integration' => \AmpProject\AmpWP\Admin\SiteHealth::class, 'support' => \AmpProject\AmpWP\Support\SupportCliCommand::class, diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php index f9f28ab6cbe..01575d06ecd 100644 --- a/src/AmpWpPlugin.php +++ b/src/AmpWpPlugin.php @@ -20,7 +20,6 @@ use AmpProject\AmpWP\RemoteRequest\WpHttpRemoteGetRequest; use AmpProject\AmpWP\Support\SupportCliCommand; use AmpProject\AmpWP\Support\SupportRESTController; -use AmpProject\AmpWP\Validation\SavePostValidationEvent; use AmpProject\AmpWP\Validation\ScannableURLProvider; use AmpProject\AmpWP\Validation\URLValidationCron; use AmpProject\AmpWP\Validation\URLValidationProvider; @@ -118,7 +117,6 @@ final class AmpWpPlugin extends ServiceBasedPlugin { 'rest.scannable_urls_controller' => Validation\ScannableURLsRestController::class, 'rest.validation_counts_controller' => Validation\ValidationCountsRestController::class, 'sandboxing' => Sandboxing::class, - 'save_post_validation_event' => SavePostValidationEvent::class, 'server_timing' => Instrumentation\ServerTiming::class, 'site_health_integration' => Admin\SiteHealth::class, 'support' => SupportCliCommand::class, diff --git a/src/Validation/SavePostValidationEvent.php b/src/Validation/SavePostValidationEvent.php deleted file mode 100644 index 3aaa070253a..00000000000 --- a/src/Validation/SavePostValidationEvent.php +++ /dev/null @@ -1,171 +0,0 @@ -dev_tools_user_access = $dev_tools_user_access; - $this->url_validation_provider = $url_validation_provider; - } - - /** - * Callback for the cron action. - * - * @param mixed ...$args The args received with the action hook where the event was scheduled. - */ - public function process( ...$args ) { - $post_id = reset( $args ); - - if ( empty( $post_id ) || empty( get_post( $post_id ) ) ) { - return; - } - - $this->url_validation_provider->get_url_validation( - get_the_permalink( $post_id ), - get_post_type( $post_id ) - ); - } - - /** - * Get the event name. - * - * This is the "slug" of the event, not the display name. - * - * Note: the event name should be prefixed to prevent naming collisions. - * - * @return string Name of the event. - */ - protected function get_event_name() { - return self::BACKGROUND_TASK_NAME; - } - - /** - * Returns whether the event should be scheduled. - * - * @param array $args Args passed from the action hook where the event is scheduled. - * @return boolean - */ - protected function should_schedule_event( $args ) { - if ( ! is_array( $args ) || count( $args ) !== 1 ) { - return false; - } - - // Validation is performed on post save if user has dev tools on. - if ( $this->dev_tools_user_access->is_user_enabled( wp_get_current_user() ) ) { - return false; - } - - $id = reset( $args ); - if ( empty( $id ) ) { - return false; - } - - $post = get_post( $id ); - - // @todo This needs to be limited to when the status is publish because otherwise the validation request will fail to be able to access the post, as the request is not authenticated. - if ( ! $post - || - wp_is_post_revision( $post ) - || - wp_is_post_autosave( $post ) - || - 'auto-draft' === $post->post_status - || - 'trash' === $post->post_status - ) { - return false; - } - - if ( ! amp_is_post_supported( $id ) ) { - return false; - } - - return true; - } - - /** - * Gets the hook on which to schedule the event. - * - * @return string The action hook name. - */ - protected function get_action_hook() { - return 'save_post'; - } -} diff --git a/tests/php/src/Validation/SavePostValidationEventTest.php b/tests/php/src/Validation/SavePostValidationEventTest.php deleted file mode 100644 index 10f122fbc86..00000000000 --- a/tests/php/src/Validation/SavePostValidationEventTest.php +++ /dev/null @@ -1,177 +0,0 @@ -test_instance = new SavePostValidationEvent( new BackgroundTaskDeactivator(), new UserAccess(), new URLValidationProvider() ); - $this->dev_tools_user_access = new UserAccess(); - $this->add_validate_response_mocking_filter(); - } - - /** @covers ::__construct() */ - public function test__construct() { - $this->assertInstanceof( SingleScheduledBackgroundTask::class, $this->test_instance ); - $this->assertInstanceof( SavePostValidationEvent::class, $this->test_instance ); - $this->assertInstanceof( Service::class, $this->test_instance ); - $this->assertInstanceof( Registerable::class, $this->test_instance ); - $this->assertInstanceof( Conditional::class, $this->test_instance ); - } - - /** @covers ::is_needed() */ - public function test_is_needed() { - $this->assertFalse( SavePostValidationEvent::is_needed() ); - - add_filter( 'amp_temp_validation_cron_tasks_enabled', '__return_true' ); - $this->assertTrue( SavePostValidationEvent::is_needed() ); - - remove_filter( 'amp_temp_validation_cron_tasks_enabled', '__return_true' ); - } - - /** - * @covers ::register() - * @covers ::get_event_name() - * @covers ::get_action_hook_arg_count() - */ - public function test_register() { - $this->test_instance->register(); - - $this->assertEquals( 10, has_action( 'save_post', [ $this->test_instance, 'schedule_event' ] ) ); - $this->assertEquals( 10, has_action( 'amp_single_post_validate', [ $this->test_instance, 'process' ] ) ); - } - - /** - * @covers ::process() - */ - public function test_process() { - $this->test_instance->process(); - $this->assertCount( 0, $this->get_validated_urls() ); - - $post = $this->factory()->post->create_and_get( - [ - 'post_content' => '
', - ] - ); - - $this->test_instance->process( $post->ID ); - - $this->assertCount( 1, $this->get_validated_urls() ); - - $this->assertInstanceof( - URLValidationProvider::class, - $this->get_private_property( $this->test_instance, 'url_validation_provider' ) - ); - } - - /** @covers ::schedule_event() */ - public function test_schedule_event_with_no_post() { - wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); - - $event_was_scheduled = false; - $filter_cb = static function ( $event ) use ( &$event_was_scheduled ) { - $event_was_scheduled = true; - return $event; - }; - add_filter( 'schedule_event', $filter_cb ); - - $this->test_instance->schedule_event( [] ); - - $this->assertFalse( $event_was_scheduled ); - - remove_filter( 'schedule_event', $filter_cb ); - } - - /** @covers ::schedule_event() */ - public function test_schedule_event_with_post() { - wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); - $this->dev_tools_user_access->set_user_enabled( wp_get_current_user(), true ); - - $event_was_scheduled = false; - $filter_cb = static function ( $event ) use ( &$event_was_scheduled ) { - $event_was_scheduled = true; - return $event; - }; - add_filter( 'schedule_event', $filter_cb ); - - $post = $this->factory()->post->create(); - - $this->test_instance->schedule_event( $post ); - - $this->assertFalse( $event_was_scheduled ); - - wp_set_current_user( $this->factory()->user->create( [ 'role' => 'author' ] ) ); - - $this->test_instance->schedule_event( $post ); - - $this->assertTrue( $event_was_scheduled ); - - remove_filter( 'schedule_event', $filter_cb ); - } - - /** @covers ::should_schedule_event() */ - public function test_should_schedule_event() { - // No user set. - $this->assertFalse( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [] ] ) ); - - // Array not passed. - $this->assertFalse( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ null ] ) ); - - // Too many args passed. - $this->assertFalse( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [ 'arg1', 'arg2' ] ] ) ); - - // User with insufficient permissions. - wp_set_current_user( $this->factory()->user->create( [ 'role' => 'subscriber' ] ) ); - $post = $this->factory()->post->create(); - $this->assertTrue( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [ $post ] ] ) ); - - // User with dev tools off. - wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); - $this->dev_tools_user_access->set_user_enabled( wp_get_current_user(), false ); - $post = $this->factory()->post->create(); - $this->assertTrue( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [ $post ] ] ) ); - - wp_set_current_user( $this->factory()->user->create( [ 'role' => 'administrator' ] ) ); - $this->dev_tools_user_access->set_user_enabled( wp_get_current_user(), true ); - $post = $this->factory()->post->create(); - $this->assertFalse( $this->call_private_method( $this->test_instance, 'should_schedule_event', [ [ $post ] ] ) ); - } - - /** @covers ::get_action_hook() */ - public function test_get_action_hook() { - $this->assertEquals( 'save_post', $this->call_private_method( $this->test_instance, 'get_action_hook' ) ); - } -} From 75d902cfc40fa3856df7d13778528ada03ca8570 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 8 Dec 2021 22:03:59 -0800 Subject: [PATCH 18/36] Block access to AMP Customizer if site lacks dependency_support --- includes/admin/functions.php | 33 +++++++++++++++++++ .../class-amp-customizer-design-settings.php | 4 +++ 2 files changed, 37 insertions(+) diff --git a/includes/admin/functions.php b/includes/admin/functions.php index e2d53bfac64..2b147f10213 100644 --- a/includes/admin/functions.php +++ b/includes/admin/functions.php @@ -5,8 +5,10 @@ * @package AMP */ +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\Option; use AmpProject\AmpWP\QueryVar; +use AmpProject\AmpWP\Services; /** * Sets up the AMP template editor for the Customizer. @@ -15,6 +17,37 @@ */ function amp_init_customizer() { + if ( ! Services::get( 'dependency_support' )->has_support() ) { + add_action( + 'customize_controls_init', + static function () { + global $wp_customize; + if ( + Services::get( 'reader_theme_loader' )->is_theme_overridden() + || + array_intersect( $wp_customize->get_autofocus(), [ 'panel' => AMP_Template_Customizer::PANEL_ID ] ) + || + isset( $_GET[ QueryVar::AMP_PREVIEW ] ) // phpcs:ignore WordPress.Security.NonceVerification.Recommended + ) { + wp_die( + esc_html( + sprintf( + /* translators: %s is minimum WordPress version */ + __( 'Customizer for AMP is unavailable due to WordPress being out of date. Please upgrade to WordPress %s or greater.', 'amp' ), + DependencySupport::WP_MIN_VERSION + ) + ), + esc_html__( 'AMP Customizer Unavailable', 'amp' ), + [ + 'response' => 503, + 'back_link' => true, + ] + ); + } + } + ); + } + // Fire up the AMP Customizer. add_action( 'customize_register', [ AMP_Template_Customizer::class, 'init' ], 500 ); diff --git a/includes/settings/class-amp-customizer-design-settings.php b/includes/settings/class-amp-customizer-design-settings.php index 28e91e3802e..205315b9c3e 100644 --- a/includes/settings/class-amp-customizer-design-settings.php +++ b/includes/settings/class-amp-customizer-design-settings.php @@ -6,6 +6,7 @@ */ use AmpProject\AmpWP\Option; +use AmpProject\AmpWP\Services; /** * Class AMP_Customizer_Design_Settings @@ -72,6 +73,9 @@ public static function init() { * Init customizer. */ public static function init_customizer() { + if ( ! Services::get( 'dependency_support' )->has_support() ) { + return; + } add_action( 'amp_customizer_register_settings', [ __CLASS__, 'register_customizer_settings' ] ); add_action( 'amp_customizer_register_ui', [ __CLASS__, 'register_customizer_ui' ] ); add_action( 'amp_customizer_enqueue_preview_scripts', [ __CLASS__, 'enqueue_customizer_preview_scripts' ] ); From ae04461d68b738703712d0fad91fd2aa343e10bd Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Thu, 9 Dec 2021 17:45:19 +0100 Subject: [PATCH 19/36] Fix typo --- tests/e2e/specs/admin/site-scan-panel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/specs/admin/site-scan-panel.js b/tests/e2e/specs/admin/site-scan-panel.js index 6307186f424..a6de36583ca 100644 --- a/tests/e2e/specs/admin/site-scan-panel.js +++ b/tests/e2e/specs/admin/site-scan-panel.js @@ -163,7 +163,7 @@ describe( 'AMP settings screen Site Scan panel', () => { await visitAdminPage( 'admin.php', 'page=amp-options' ); await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-slug', { text: /e2e-tests-demo-plugin/ } ); - await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-notice', { text: /This plugin has been uninstalled or its metadata is unavailable../ } ); + await expect( page ).toMatchElement( '.site-scan-results--plugins .site-scan-results__source-notice', { text: /This plugin has been uninstalled or its metadata is unavailable./ } ); // Clean up. await installLocalPlugin( 'e2e-tests-demo-plugin' ); From c5ef2ec0f0f8ab3887832bd33929b195ae6c6163 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 9 Dec 2021 09:48:07 -0800 Subject: [PATCH 20/36] Fix tests for restricting DevTools to whether DependencySupport present --- tests/php/src/Admin/PairedBrowsingTest.php | 11 +++-- tests/php/src/DevTools/UserAccessTest.php | 11 ++++- .../URLValidationRESTControllerTest.php | 17 +++++-- .../ValidationCountsRestControllerTest.php | 14 +++++- ...test-class-amp-validated-url-post-type.php | 9 +++- ...st-class-amp-validation-error-taxonomy.php | 8 +++- .../test-class-amp-validation-manager.php | 48 +++++++++++++++---- 7 files changed, 95 insertions(+), 23 deletions(-) diff --git a/tests/php/src/Admin/PairedBrowsingTest.php b/tests/php/src/Admin/PairedBrowsingTest.php index 5a71a04c663..94518d481ca 100644 --- a/tests/php/src/Admin/PairedBrowsingTest.php +++ b/tests/php/src/Admin/PairedBrowsingTest.php @@ -10,6 +10,7 @@ use AMP_Options_Manager; use AMP_Theme_Support; use AmpProject\AmpWP\Admin\PairedBrowsing; +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\Infrastructure\Conditional; use AmpProject\AmpWP\Infrastructure\Delayed; use AmpProject\AmpWP\Infrastructure\HasRequirements; @@ -186,9 +187,13 @@ public function test_add_admin_bar_menu_item() { // Test when DevTools enabled. wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); - $this->assertTrue( $this->instance->dev_tools_user_access->is_user_enabled() ); - $this->instance->add_admin_bar_menu_item( $wp_admin_bar ); - $this->assertNotEmpty( $wp_admin_bar->get_node( 'amp-paired-browsing' ) ); + if ( ( new DependencySupport() )->has_support() ) { + $this->assertTrue( $this->instance->dev_tools_user_access->is_user_enabled() ); + $this->instance->add_admin_bar_menu_item( $wp_admin_bar ); + $this->assertNotEmpty( $wp_admin_bar->get_node( 'amp-paired-browsing' ) ); + } else { + $this->assertFalse( $this->instance->dev_tools_user_access->is_user_enabled() ); + } } /** @covers ::get_paired_browsing_url() */ diff --git a/tests/php/src/DevTools/UserAccessTest.php b/tests/php/src/DevTools/UserAccessTest.php index 23c3292542f..f74eae473ea 100644 --- a/tests/php/src/DevTools/UserAccessTest.php +++ b/tests/php/src/DevTools/UserAccessTest.php @@ -9,6 +9,7 @@ use AMP_Options_Manager; use AMP_Theme_Support; +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Option; use AmpProject\AmpWP\Tests\DependencyInjectedTestCase; @@ -219,13 +220,19 @@ public function test_rest_get_dev_tools_enabled() { $this->assertFalse( $this->dev_tools_user_access->rest_get_dev_tools_enabled( [ 'id' => $user->ID ] ) ); $user->set_role( 'administrator' ); - $this->assertTrue( $this->dev_tools_user_access->rest_get_dev_tools_enabled( [ 'id' => $user->ID ] ) ); + $this->assertEquals( + ( new DependencySupport() )->has_support(), + $this->dev_tools_user_access->rest_get_dev_tools_enabled( [ 'id' => $user->ID ] ) + ); update_user_meta( $user->ID, 'amp_dev_tools_enabled', 'false' ); $this->assertFalse( $this->dev_tools_user_access->rest_get_dev_tools_enabled( [ 'id' => $user->ID ] ) ); update_user_meta( $user->ID, 'amp_dev_tools_enabled', 'true' ); - $this->assertTrue( $this->dev_tools_user_access->rest_get_dev_tools_enabled( [ 'id' => $user->ID ] ) ); + $this->assertEquals( + ( new DependencySupport() )->has_support(), + $this->dev_tools_user_access->rest_get_dev_tools_enabled( [ 'id' => $user->ID ] ) + ); } /** diff --git a/tests/php/src/Validation/URLValidationRESTControllerTest.php b/tests/php/src/Validation/URLValidationRESTControllerTest.php index aa01b72046a..45ed31fa9df 100644 --- a/tests/php/src/Validation/URLValidationRESTControllerTest.php +++ b/tests/php/src/Validation/URLValidationRESTControllerTest.php @@ -7,10 +7,10 @@ namespace AmpProject\AmpWP\Tests\Validation; +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Tests\DependencyInjectedTestCase; use AmpProject\AmpWP\Tests\Helpers\ValidationRequestMocking; -use AmpProject\AmpWP\Validation\URLValidationProvider; use AmpProject\AmpWP\Validation\URLValidationRESTController; use WP_REST_Controller; use WP_REST_Request; @@ -78,7 +78,12 @@ public function test_create_item_permissions_check() { wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); $this->user_access->set_user_enabled( wp_get_current_user(), true ); - $this->assertTrue( $this->controller->create_item_permissions_check( new WP_REST_Request( 'POST', '/amp/v1/validate-post-url/' ) ) ); + $value = $this->controller->create_item_permissions_check( new WP_REST_Request( 'POST', '/amp/v1/validate-post-url/' ) ); + if ( ( new DependencySupport() )->has_support() ) { + $this->assertTrue( $value ); + } else { + $this->assertWPError( $value ); + } } /** @covers ::is_valid_preview_nonce() */ @@ -148,7 +153,7 @@ public function get_data_for_test_validate_post_url() { ], 'post', 'administrator', - 'amp_post_preview_denied', + ( new DependencySupport() )->has_support() ? 'amp_post_preview_denied' : 'amp_rest_no_dev_tools', ], 'post_id' => [ @@ -198,7 +203,11 @@ public function test_validate_post_url( $params, $post_type, $user_role, $expect $request->set_body_params( $params ); $response = rest_get_server()->dispatch( $request ); - if ( true === $expected_validity ) { + if ( ! ( new DependencySupport() )->has_support() && true === $expected_validity ) { + $this->assertTrue( $response->is_error() ); + $error = $response->as_error(); + $this->assertEquals( 'amp_rest_no_dev_tools', $error->get_error_code() ); + } elseif ( true === $expected_validity ) { $this->assertFalse( $response->is_error() ); $data = $response->get_data(); diff --git a/tests/php/src/Validation/ValidationCountsRestControllerTest.php b/tests/php/src/Validation/ValidationCountsRestControllerTest.php index 31f62cc89e1..0ba76d3ff22 100644 --- a/tests/php/src/Validation/ValidationCountsRestControllerTest.php +++ b/tests/php/src/Validation/ValidationCountsRestControllerTest.php @@ -8,6 +8,7 @@ namespace AmpProject\AmpWP\Tests\Validation; use AMP_Validated_URL_Post_Type; +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Tests\DependencyInjectedTestCase; use AmpProject\AmpWP\Validation\ValidationCountsRestController; @@ -59,7 +60,12 @@ public function test_get_items_permissions_check() { $this->assertWPError( $this->controller->get_items_permissions_check( new WP_REST_Request( 'GET', '/amp/v1/unreviewed-validation-counts' ) ) ); update_user_meta( $admin_user->ID, UserAccess::USER_FIELD_DEVELOPER_TOOLS_ENABLED, wp_json_encode( true ) ); - $this->assertTrue( $this->controller->get_items_permissions_check( new WP_REST_Request( 'GET', '/amp/v1/unreviewed-validation-counts' ) ) ); + $value = $this->controller->get_items_permissions_check( new WP_REST_Request( 'GET', '/amp/v1/unreviewed-validation-counts' ) ); + if ( ( new DependencySupport() )->has_support() ) { + $this->assertTrue( $value ); + } else { + $this->assertWPError( $value ); + } } /** @@ -76,6 +82,12 @@ public function test_get_items() { $request = new WP_REST_Request( 'GET', '/amp/v1/unreviewed-validation-counts' ); $response = rest_get_server()->dispatch( $request ); + + if ( ! ( new DependencySupport() )->has_support() ) { + $this->assertWPError( $response->as_error() ); + return; + } + $this->assertEquals( [ 'validated_urls' => 0, diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 7febc965775..3399e036b4b 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -6,6 +6,7 @@ */ use AmpProject\AmpWP\Admin\ReaderThemes; +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Option; use AmpProject\AmpWP\Services; @@ -58,13 +59,17 @@ public function test_register() { AMP_Validated_URL_Post_Type::register(); $amp_post_type = get_post_type_object( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ); - $this->assertStringContainsString( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, get_post_types() ); + $this->assertContains( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, get_post_types() ); $this->assertEquals( [], get_all_post_type_supports( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG ) ); $this->assertEquals( AMP_Validated_URL_Post_Type::POST_TYPE_SLUG, $amp_post_type->name ); $this->assertEquals( 'AMP Validated URLs', $amp_post_type->label ); $this->assertEquals( false, $amp_post_type->public ); $this->assertTrue( $amp_post_type->show_ui ); - $this->assertEquals( AMP_Options_Manager::OPTION_NAME, $amp_post_type->show_in_menu ); + if ( ( new DependencySupport() )->has_support() ) { + $this->assertEquals( AMP_Options_Manager::OPTION_NAME, $amp_post_type->show_in_menu ); + } else { + $this->assertFalse( $amp_post_type->show_in_menu ); + } $this->assertTrue( $amp_post_type->show_in_admin_bar ); $this->assertStringNotContainsString( AMP_Validated_URL_Post_Type::REMAINING_ERRORS, wp_removable_query_args() ); $this->assertEquals( 10, has_action( 'admin_menu', [ self::TESTED_CLASS, 'update_validated_url_menu_item' ] ) ); diff --git a/tests/php/validation/test-class-amp-validation-error-taxonomy.php b/tests/php/validation/test-class-amp-validation-error-taxonomy.php index 5d09a57420f..2e4c5d6e2b5 100644 --- a/tests/php/validation/test-class-amp-validation-error-taxonomy.php +++ b/tests/php/validation/test-class-amp-validation-error-taxonomy.php @@ -6,6 +6,7 @@ */ use AmpProject\AmpWP\DevTools\UserAccess; +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\Option; use AmpProject\AmpWP\Services; use AmpProject\AmpWP\Tests\Helpers\HandleValidation; @@ -58,7 +59,7 @@ public function test_register() { $this->assertFalse( $taxonomy_object->show_tagcloud ); $this->assertFalse( $taxonomy_object->show_in_quick_edit ); $this->assertFalse( $taxonomy_object->hierarchical ); - $this->assertTrue( $taxonomy_object->show_in_menu ); + $this->assertEquals( ( new DependencySupport() )->has_support(), $taxonomy_object->show_in_menu ); $this->assertFalse( $taxonomy_object->meta_box_cb ); $this->assertEquals( 'AMP Validation Error Index', $taxonomy_object->label ); $this->assertEquals( 'do_not_allow', $taxonomy_object->cap->assign_terms ); @@ -565,7 +566,10 @@ public function test_add_admin_hooks() { $this->assertEquals( 10, has_filter( 'terms_clauses', [ self::TESTED_CLASS, 'filter_terms_clauses_for_description_search' ] ) ); $this->assertEquals( 10, has_action( 'admin_notices', [ self::TESTED_CLASS, 'add_admin_notices' ] ) ); $this->assertEquals( PHP_INT_MAX, has_filter( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_row_actions', [ self::TESTED_CLASS, 'filter_tag_row_actions' ] ) ); - $this->assertEquals( 10, has_action( 'admin_menu', [ self::TESTED_CLASS, 'add_admin_menu_validation_error_item' ] ) ); + $this->assertEquals( + ( new DependencySupport() )->has_support() ? 10 : false, + has_action( 'admin_menu', [ self::TESTED_CLASS, 'add_admin_menu_validation_error_item' ] ) + ); $this->assertEquals( 10, has_filter( 'manage_' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_custom_column', [ self::TESTED_CLASS, 'filter_manage_custom_columns' ] ) ); $this->assertEquals( 10, has_filter( 'manage_' . AMP_Validated_URL_Post_Type::POST_TYPE_SLUG . '_sortable_columns', [ self::TESTED_CLASS, 'add_single_post_sortable_columns' ] ) ); $this->assertEquals( 10, has_filter( 'posts_where', [ self::TESTED_CLASS, 'filter_posts_where_for_validation_error_status' ] ) ); diff --git a/tests/php/validation/test-class-amp-validation-manager.php b/tests/php/validation/test-class-amp-validation-manager.php index bf8edb5ff34..6178dc22e24 100644 --- a/tests/php/validation/test-class-amp-validation-manager.php +++ b/tests/php/validation/test-class-amp-validation-manager.php @@ -7,6 +7,7 @@ // phpcs:disable Generic.Formatting.MultipleStatementAlignment.NotSameWarning +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\DevTools\UserAccess; use AmpProject\AmpWP\Dom\Options; use AmpProject\AmpWP\Option; @@ -438,11 +439,22 @@ public function test_is_sanitization_auto_accepted() { * @covers AMP_Validation_Manager::add_admin_bar_menu_items() */ public function test_add_admin_bar_menu_items() { + require_once ABSPATH . WPINC . '/class-wp-admin-bar.php'; + + if ( ! ( new DependencySupport() )->has_support() ) { + wp_set_current_user( self::factory()->user->create( [ 'role' => 'administrator' ] ) ); + AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::STANDARD_MODE_SLUG ); + $admin_bar = new WP_Admin_Bar(); + AMP_Validation_Manager::add_admin_bar_menu_items( $admin_bar ); + $this->assertNull( $admin_bar->get_node( 'amp' ) ); + $this->assertNull( $admin_bar->get_node( 'amp-view' ) ); + return; + } + $this->accept_sanitization_by_default( false ); // No admin bar item when user lacks capability. $this->go_to( home_url( '/' ) ); - require_once ABSPATH . WPINC . '/class-wp-admin-bar.php'; $this->assertFalse( is_admin() ); $this->assertFalse( AMP_Validation_Manager::has_cap() ); $admin_bar = new WP_Admin_Bar(); @@ -490,20 +502,28 @@ public function test_add_admin_bar_menu_items() { $admin_bar = new WP_Admin_Bar(); AMP_Validation_Manager::add_admin_bar_menu_items( $admin_bar ); $node = $admin_bar->get_node( 'amp' ); - $this->assertIsObject( $node ); + if ( ( new DependencySupport() )->has_support() ) { + $this->assertIsObject( $node ); + } else { + $this->assertNull( $node ); + } // Admin bar item available in paired mode. AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); $admin_bar = new WP_Admin_Bar(); AMP_Validation_Manager::add_admin_bar_menu_items( $admin_bar ); $root_node = $admin_bar->get_node( 'amp' ); - $this->assertIsObject( $root_node ); - $this->assertEqualSets( [ QueryVar::AMP ], array_keys( $this->get_url_query_vars( $root_node->href ) ) ); - - $view_item = $admin_bar->get_node( 'amp-view' ); - $this->assertIsObject( $view_item ); - $this->assertEqualSets( [ QueryVar::AMP ], array_keys( $this->get_url_query_vars( $view_item->href ) ) ); - $this->assertIsObject( $admin_bar->get_node( 'amp-validity' ) ); + if ( ( new DependencySupport() )->has_support() ) { + $this->assertIsObject( $root_node ); + $this->assertEqualSets( [ QueryVar::AMP ], array_keys( $this->get_url_query_vars( $root_node->href ) ) ); + + $view_item = $admin_bar->get_node( 'amp-view' ); + $this->assertIsObject( $view_item ); + $this->assertEqualSets( [ QueryVar::AMP ], array_keys( $this->get_url_query_vars( $view_item->href ) ) ); + $this->assertIsObject( $admin_bar->get_node( 'amp-validity' ) ); + } else { + $this->assertNull( $root_node ); + } // Lastly, confirm that the settings item is added if the user is an admin. wp_set_current_user( 0 ); @@ -2393,6 +2413,10 @@ function () { * @covers \AMP_Validation_Manager::add_admin_bar_menu_items() */ public function test_finalize_validation() { + if ( ! ( new DependencySupport() )->has_support() ) { + $this->markTestSkipped( 'Test requires newer version of WP.' ); + } + self::set_capability(); require_once ABSPATH . WPINC . '/class-wp-admin-bar.php'; show_admin_bar( true ); @@ -2838,6 +2862,12 @@ public function test_enqueue_block_validation() { $this->setup_environment( true, true ); $service->set_user_enabled( wp_get_current_user()->ID, true ); AMP_Validation_Manager::enqueue_block_validation(); + + if ( ! ( new DependencySupport() )->has_support() ) { + $this->assertFalse( wp_script_is( $slug, 'enqueued' ) ); + return; + } + $this->assertTrue( wp_script_is( $slug, 'enqueued' ) ); $script = wp_scripts()->registered[ $slug ]; From 0f2430b4bdabef4485ed55876187f038a26a6074 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 9 Dec 2021 10:37:43 -0800 Subject: [PATCH 21/36] Further fix PHPUnit tests after c5ef2ec0f --- .../php/src/Validation/URLValidationRESTControllerTest.php | 4 +++- .../validation/test-class-amp-validated-url-post-type.php | 7 +++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/tests/php/src/Validation/URLValidationRESTControllerTest.php b/tests/php/src/Validation/URLValidationRESTControllerTest.php index 45ed31fa9df..153c555dfca 100644 --- a/tests/php/src/Validation/URLValidationRESTControllerTest.php +++ b/tests/php/src/Validation/URLValidationRESTControllerTest.php @@ -143,7 +143,9 @@ public function get_data_for_test_validate_post_url() { ], 'post', 'administrator', - version_compare( get_bloginfo( 'version' ), '5.2', '<' ) ? 'amp_post_preview_denied' : 'rest_invalid_param', + ( new DependencySupport() )->has_support() + ? ( version_compare( get_bloginfo( 'version' ), '5.2', '<' ) ? 'amp_post_preview_denied' : 'rest_invalid_param' ) + : 'amp_rest_no_dev_tools', ], 'bad_preview2' => [ diff --git a/tests/php/validation/test-class-amp-validated-url-post-type.php b/tests/php/validation/test-class-amp-validated-url-post-type.php index 3399e036b4b..9ec73d35a78 100644 --- a/tests/php/validation/test-class-amp-validated-url-post-type.php +++ b/tests/php/validation/test-class-amp-validated-url-post-type.php @@ -70,9 +70,12 @@ public function test_register() { } else { $this->assertFalse( $amp_post_type->show_in_menu ); } - $this->assertTrue( $amp_post_type->show_in_admin_bar ); + $this->assertEquals( ( new DependencySupport() )->has_support(), $amp_post_type->show_in_admin_bar ); $this->assertStringNotContainsString( AMP_Validated_URL_Post_Type::REMAINING_ERRORS, wp_removable_query_args() ); - $this->assertEquals( 10, has_action( 'admin_menu', [ self::TESTED_CLASS, 'update_validated_url_menu_item' ] ) ); + $this->assertEquals( + ( new DependencySupport() )->has_support() ? 10 : false, + has_action( 'admin_menu', [ self::TESTED_CLASS, 'update_validated_url_menu_item' ] ) + ); // Make sure that add_admin_hooks() gets called. set_current_screen( 'index.php' ); From c1e18f8a5f5f4d6aa1d837a9328209b6b0d71097 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 10 Dec 2021 15:04:18 -0800 Subject: [PATCH 22/36] Temporarily register dependency_support as first service --- src/AmpWpPlugin.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php index 01575d06ecd..6053cc35cc0 100644 --- a/src/AmpWpPlugin.php +++ b/src/AmpWpPlugin.php @@ -69,6 +69,9 @@ final class AmpWpPlugin extends ServiceBasedPlugin { * @var string[] */ const SERVICES = [ + // @todo This currently has to appear first to prevent a fatal error during activation. See . + 'dependency_support' => DependencySupport::class, + 'admin.analytics_menu' => Admin\AnalyticsOptionsSubmenu::class, 'admin.google_fonts' => Admin\GoogleFonts::class, 'admin.onboarding_menu' => Admin\OnboardingWizardSubmenu::class, @@ -91,7 +94,6 @@ final class AmpWpPlugin extends ServiceBasedPlugin { 'cli.validation_command' => Cli\ValidationCommand::class, 'css_transient_cache.ajax_handler' => Admin\ReenableCssTransientCachingAjaxAction::class, 'css_transient_cache.monitor' => BackgroundTask\MonitorCssTransientCaching::class, - 'dependency_support' => DependencySupport::class, 'dev_tools.block_sources' => DevTools\BlockSources::class, 'dev_tools.callback_reflection' => DevTools\CallbackReflection::class, 'dev_tools.error_page' => DevTools\ErrorPage::class, From 721916a81b69e337c60c81755ac0b6c112f6275b Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 10 Dec 2021 15:39:01 -0800 Subject: [PATCH 23/36] Revert "Ensure Onboarding Wizard works on WordPress 4.9" This reverts commit 3f3cb575b83c9bb0d72feb26aab590894a68bc31. --- webpack.config.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/webpack.config.js b/webpack.config.js index c852bf5b286..5478041b92e 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -251,6 +251,9 @@ const onboardingWizard = { switch ( handle ) { case 'lodash': case '@wordpress/api-fetch': + case '@wordpress/dom-ready': + case '@wordpress/html-entities': + case '@wordpress/url': case '@wordpress/i18n': return defaultRequestToHandle( handle ); @@ -262,6 +265,9 @@ const onboardingWizard = { switch ( external ) { case 'lodash': case '@wordpress/api-fetch': + case '@wordpress/dom-ready': + case '@wordpress/html-entities': + case '@wordpress/url': case '@wordpress/i18n': return defaultRequestToExternal( external ); From 30b78fcd1a6df50e789ab900d4f16e7df14e1e52 Mon Sep 17 00:00:00 2001 From: Alain Schlesser Date: Sun, 12 Dec 2021 17:03:36 -0500 Subject: [PATCH 24/36] Add dependency_support as a requirement for AmpPlugins & AmpThemes --- src/Admin/AmpPlugins.php | 12 +++++++++++- src/Admin/AmpThemes.php | 12 +++++++++++- src/AmpWpPlugin.php | 4 +--- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/Admin/AmpPlugins.php b/src/Admin/AmpPlugins.php index 5da16c1e73b..ab71d942a39 100644 --- a/src/Admin/AmpPlugins.php +++ b/src/Admin/AmpPlugins.php @@ -9,6 +9,7 @@ use AmpProject\AmpWP\Infrastructure\Conditional; use AmpProject\AmpWP\Infrastructure\Delayed; +use AmpProject\AmpWP\Infrastructure\HasRequirements; use AmpProject\AmpWP\Infrastructure\Registerable; use AmpProject\AmpWP\Infrastructure\Service; use AmpProject\AmpWP\Services; @@ -22,7 +23,7 @@ * @since 2.2 * @internal */ -class AmpPlugins implements Conditional, Delayed, Service, Registerable { +class AmpPlugins implements Conditional, Delayed, HasRequirements, Service, Registerable { /** * Slug for amp-compatible. @@ -55,6 +56,15 @@ public static function get_registration_action() { return 'current_screen'; } + /** + * Get the list of service IDs required for this service to be registered. + * + * @return string[] List of required services. + */ + public static function get_requirements() { + return [ 'dependency_support' ]; + } + /** * Check whether the conditional object is currently needed. * diff --git a/src/Admin/AmpThemes.php b/src/Admin/AmpThemes.php index 834e7ca308d..8a5a9582dc9 100644 --- a/src/Admin/AmpThemes.php +++ b/src/Admin/AmpThemes.php @@ -9,6 +9,7 @@ use AmpProject\AmpWP\Infrastructure\Conditional; use AmpProject\AmpWP\Infrastructure\Delayed; +use AmpProject\AmpWP\Infrastructure\HasRequirements; use AmpProject\AmpWP\Infrastructure\Registerable; use AmpProject\AmpWP\Infrastructure\Service; use AmpProject\AmpWP\Services; @@ -21,7 +22,7 @@ * @since 2.2 * @internal */ -class AmpThemes implements Service, Registerable, Conditional, Delayed { +class AmpThemes implements Service, HasRequirements, Registerable, Conditional, Delayed { /** * Slug for amp-compatible. @@ -54,6 +55,15 @@ public static function get_registration_action() { return 'admin_init'; } + /** + * Get the list of service IDs required for this service to be registered. + * + * @return string[] List of required services. + */ + public static function get_requirements() { + return [ 'dependency_support' ]; + } + /** * Check whether the conditional object is currently needed. * diff --git a/src/AmpWpPlugin.php b/src/AmpWpPlugin.php index 6053cc35cc0..01575d06ecd 100644 --- a/src/AmpWpPlugin.php +++ b/src/AmpWpPlugin.php @@ -69,9 +69,6 @@ final class AmpWpPlugin extends ServiceBasedPlugin { * @var string[] */ const SERVICES = [ - // @todo This currently has to appear first to prevent a fatal error during activation. See . - 'dependency_support' => DependencySupport::class, - 'admin.analytics_menu' => Admin\AnalyticsOptionsSubmenu::class, 'admin.google_fonts' => Admin\GoogleFonts::class, 'admin.onboarding_menu' => Admin\OnboardingWizardSubmenu::class, @@ -94,6 +91,7 @@ final class AmpWpPlugin extends ServiceBasedPlugin { 'cli.validation_command' => Cli\ValidationCommand::class, 'css_transient_cache.ajax_handler' => Admin\ReenableCssTransientCachingAjaxAction::class, 'css_transient_cache.monitor' => BackgroundTask\MonitorCssTransientCaching::class, + 'dependency_support' => DependencySupport::class, 'dev_tools.block_sources' => DevTools\BlockSources::class, 'dev_tools.callback_reflection' => DevTools\CallbackReflection::class, 'dev_tools.error_page' => DevTools\ErrorPage::class, From 3214f92c30617ff6125492f22d9d10dde7cbaeba Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 12 Dec 2021 16:01:45 -0800 Subject: [PATCH 25/36] Add missing HasRequirements interface to PluginActivationSiteScan --- src/Admin/PluginActivationSiteScan.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Admin/PluginActivationSiteScan.php b/src/Admin/PluginActivationSiteScan.php index c3fa3f9576d..936e30fed32 100644 --- a/src/Admin/PluginActivationSiteScan.php +++ b/src/Admin/PluginActivationSiteScan.php @@ -15,6 +15,7 @@ use AMP_Validation_Manager; use AmpProject\AmpWP\Infrastructure\Conditional; use AmpProject\AmpWP\Infrastructure\Delayed; +use AmpProject\AmpWP\Infrastructure\HasRequirements; use AmpProject\AmpWP\Infrastructure\Registerable; use AmpProject\AmpWP\Infrastructure\Service; use AmpProject\AmpWP\Services; @@ -25,7 +26,7 @@ * @since 2.2 * @internal */ -final class PluginActivationSiteScan implements Conditional, Delayed, Service, Registerable { +final class PluginActivationSiteScan implements Conditional, Delayed, HasRequirements, Service, Registerable { /** * Handle for JS file. * @@ -47,6 +48,15 @@ final class PluginActivationSiteScan implements Conditional, Delayed, Service, R */ private $rest_preloader; + /** + * Get the list of service IDs required for this service to be registered. + * + * @return string[] List of required services. + */ + public static function get_requirements() { + return [ 'dependency_support' ]; + } + /** * OnboardingWizardSubmenuPage constructor. * From 942f91e1136b33cff0cd65a99b41c54c4ff17943 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Sun, 12 Dec 2021 16:30:48 -0800 Subject: [PATCH 26/36] Add codeCoverageIgnore comments --- includes/admin/functions.php | 2 ++ includes/amp-helper-functions.php | 2 +- includes/settings/class-amp-customizer-design-settings.php | 2 +- src/Admin/AmpPlugins.php | 2 +- src/Admin/AmpThemes.php | 3 +-- src/DevTools/UserAccess.php | 2 +- 6 files changed, 7 insertions(+), 6 deletions(-) diff --git a/includes/admin/functions.php b/includes/admin/functions.php index 2b147f10213..3e0a29fa962 100644 --- a/includes/admin/functions.php +++ b/includes/admin/functions.php @@ -18,6 +18,7 @@ function amp_init_customizer() { if ( ! Services::get( 'dependency_support' )->has_support() ) { + // @codeCoverageIgnoreStart add_action( 'customize_controls_init', static function () { @@ -46,6 +47,7 @@ static function () { } } ); + // @codeCoverageIgnoreEnd } // Fire up the AMP Customizer. diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 5f4d5aa5593..036fc4bdcda 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -94,7 +94,7 @@ function amp_bootstrap_plugin() { // Ensure ID attribute is present in WP<5.5. if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { - add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_script_loader_tag', 1000, 2 ); + add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_script_loader_tag', 1000, 2 ); // @codeCoverageIgnore } // Ensure crossorigin=anonymous is added to font links. diff --git a/includes/settings/class-amp-customizer-design-settings.php b/includes/settings/class-amp-customizer-design-settings.php index 205315b9c3e..f1fbc80491f 100644 --- a/includes/settings/class-amp-customizer-design-settings.php +++ b/includes/settings/class-amp-customizer-design-settings.php @@ -74,7 +74,7 @@ public static function init() { */ public static function init_customizer() { if ( ! Services::get( 'dependency_support' )->has_support() ) { - return; + return; // @codeCoverageIgnore } add_action( 'amp_customizer_register_settings', [ __CLASS__, 'register_customizer_settings' ] ); add_action( 'amp_customizer_register_ui', [ __CLASS__, 'register_customizer_ui' ] ); diff --git a/src/Admin/AmpPlugins.php b/src/Admin/AmpPlugins.php index ab71d942a39..ad2056fa9fb 100644 --- a/src/Admin/AmpPlugins.php +++ b/src/Admin/AmpPlugins.php @@ -73,7 +73,7 @@ public static function get_requirements() { public static function is_needed() { if ( ! Services::get( 'dependency_support' )->has_support() ) { - return false; + return false; // @codeCoverageIgnore } /** This filter is documented in src/Admin/AmpThemes.php */ diff --git a/src/Admin/AmpThemes.php b/src/Admin/AmpThemes.php index 8a5a9582dc9..49be63041bb 100644 --- a/src/Admin/AmpThemes.php +++ b/src/Admin/AmpThemes.php @@ -51,7 +51,6 @@ class AmpThemes implements Service, HasRequirements, Registerable, Conditional, * @return string Registration action to use. */ public static function get_registration_action() { - return 'admin_init'; } @@ -72,7 +71,7 @@ public static function get_requirements() { public static function is_needed() { if ( ! Services::get( 'dependency_support' )->has_support() ) { - return false; + return false; // @codeCoverageIgnore } /** diff --git a/src/DevTools/UserAccess.php b/src/DevTools/UserAccess.php index 852ce74f140..0da4ed94c83 100644 --- a/src/DevTools/UserAccess.php +++ b/src/DevTools/UserAccess.php @@ -68,7 +68,7 @@ public function is_user_enabled( $user = null ) { // shown, and its the user interface which is particularly problematic on older versions of WP due to the // JavaScript dependencies. if ( ! $this->dependency_support->has_support() ) { - return false; + return false; // @codeCoverageIgnore } if ( null === $user ) { From 00ec54f0085a8eb20467a5cb3de1fe279533de4c Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 09:50:11 -0800 Subject: [PATCH 27/36] Further harden regex in amp_ensure_id_attribute_on_script_loader_tag() --- includes/amp-helper-functions.php | 32 +++++++++++------------ tests/php/test-amp-helper-functions.php | 34 ++++++++++++++++--------- 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 036fc4bdcda..1a86c4dfad6 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -94,7 +94,7 @@ function amp_bootstrap_plugin() { // Ensure ID attribute is present in WP<5.5. if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { - add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_script_loader_tag', 1000, 2 ); // @codeCoverageIgnore + add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag', 1000, 2 ); // @codeCoverageIgnore } // Ensure crossorigin=anonymous is added to font links. @@ -1165,8 +1165,8 @@ function amp_filter_script_loader_tag( $tag, $handle ) { /** * Ensure ID attribute is added to printed scripts. * - * Core started adding in WP 5.5. This is used both by validation logic for sourcing attribution as well as in the - * script and comments sanitizers. + * Core started adding the ID attribute in WP 5.5. This attribute is used both by validation logic for sourcing + * attribution as well as in the script and comments sanitizers. * * @link https://core.trac.wordpress.org/changeset/48295 * @since 2.2 @@ -1176,20 +1176,18 @@ function amp_filter_script_loader_tag( $tag, $handle ) { * @param string $handle The script's registered handle. * @return string Filtered script. */ -function amp_ensure_id_attribute_script_loader_tag( $tag, $handle ) { - if ( 0 !== strpos( $handle, 'amp-' ) ) { - $tag = preg_replace_callback( - '/(]*?\ssrc=["\'].*?["\'])([^>]*?>)/', - static function ( $matches ) use ( $handle ) { - if ( false === strpos( $matches[0], 'id=' ) ) { - return $matches[1] . sprintf( ' id="%s"', esc_attr( "$handle-js" ) ) . $matches[2]; - } - return $matches[0]; - }, - $tag, - 1 - ); - } +function amp_ensure_id_attribute_on_script_loader_tag( $tag, $handle ) { + $tag = preg_replace_callback( + '/(]*?\ssrc=(["\']).*?\2)([^>]*?>)/', + static function ( $matches ) use ( $handle ) { + if ( false === strpos( $matches[0], 'id=' ) ) { + return $matches[1] . sprintf( ' id="%s"', esc_attr( "$handle-js" ) ) . $matches[3]; + } + return $matches[0]; + }, + $tag, + 1 + ); return $tag; } diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index aeded3fcb1c..b41ef2b5676 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -129,9 +129,9 @@ public function test_amp_bootstrap_plugin() { $this->assertEquals( PHP_INT_MAX, has_filter( 'script_loader_tag', 'amp_filter_script_loader_tag' ) ); if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { - $this->assertEquals( 1000, has_filter( 'script_loader_tag', 'amp_ensure_id_attribute_script_loader_tag' ) ); + $this->assertEquals( 1000, has_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag' ) ); } else { - $this->assertFalse( has_filter( 'script_loader_tag', 'amp_ensure_id_attribute_script_loader_tag' ) ); + $this->assertFalse( has_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag' ) ); } $this->assertEquals( 10, has_filter( 'style_loader_tag', 'amp_filter_font_style_loader_tag_with_crossorigin_anonymous' ) ); $this->assertEquals( 10, has_filter( 'all_plugins', 'amp_modify_plugin_description' ) ); @@ -150,28 +150,38 @@ public function test_amp_bootstrap_plugin_amp_disabled() { } } - /** @covers ::amp_ensure_id_attribute_script_loader_tag() */ - public function test_amp_ensure_id_attribute_script_loader_tag() { + /** @covers ::amp_ensure_id_attribute_on_script_loader_tag() */ + public function test_amp_ensure_id_attribute_on_script_loader_tag() { $this->assertEquals( '', - amp_ensure_id_attribute_script_loader_tag( '', 'foo' ) + amp_ensure_id_attribute_on_script_loader_tag( '', 'foo' ) ); $this->assertEquals( '', - amp_ensure_id_attribute_script_loader_tag( '', 'foo' ) + amp_ensure_id_attribute_on_script_loader_tag( '', 'foo' ) + ); + + $this->assertEquals( + '', + amp_ensure_id_attribute_on_script_loader_tag( '', 'foo' ) + ); + + $this->assertEquals( + "", + amp_ensure_id_attribute_on_script_loader_tag( "", 'foo' ) ); $this->assertEquals( '', - amp_ensure_id_attribute_script_loader_tag( '', 'comment-reply' ) + amp_ensure_id_attribute_on_script_loader_tag( '', 'comment-reply' ) ); $inline_script_before = ''; $inline_script_after = ''; $this->assertEquals( $inline_script_before . '' . $inline_script_after, - amp_ensure_id_attribute_script_loader_tag( + amp_ensure_id_attribute_on_script_loader_tag( $inline_script_before . '' . $inline_script_after, 'foo' ) @@ -180,19 +190,19 @@ public function test_amp_ensure_id_attribute_script_loader_tag() { $foo_script = ''; $this->assertEquals( $foo_script, - amp_ensure_id_attribute_script_loader_tag( $foo_script, 'foo' ) + amp_ensure_id_attribute_on_script_loader_tag( $foo_script, 'foo' ) ); $foo_script = ''; $this->assertEquals( $foo_script, - amp_ensure_id_attribute_script_loader_tag( $foo_script, 'foo' ) + amp_ensure_id_attribute_on_script_loader_tag( $foo_script, 'foo' ) ); - $amp_runtime_script = ''; + $amp_runtime_script = ''; $this->assertEquals( $amp_runtime_script, - amp_ensure_id_attribute_script_loader_tag( $amp_runtime_script, 'amp-runtime' ) + amp_ensure_id_attribute_on_script_loader_tag( $amp_runtime_script, 'amp-runtime' ) ); } From 24943e9b7b9e0ea4145652e19fbdbf2eff596a8e Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 10:05:09 -0800 Subject: [PATCH 28/36] Restore wp_version in test tearDown method --- tests/php/src/Admin/AmpPluginsTest.php | 23 +++++++++++++--- tests/php/src/Admin/AmpThemesTest.php | 24 ++++++++++++++--- tests/php/src/Admin/SupportScreenTest.php | 32 ++++++++++++++++++----- 3 files changed, 65 insertions(+), 14 deletions(-) diff --git a/tests/php/src/Admin/AmpPluginsTest.php b/tests/php/src/Admin/AmpPluginsTest.php index c2f01ec3eee..ac1d032c5f0 100644 --- a/tests/php/src/Admin/AmpPluginsTest.php +++ b/tests/php/src/Admin/AmpPluginsTest.php @@ -25,20 +25,37 @@ class AmpPluginsTest extends TestCase { */ public $instance; + /** @var string */ + private $original_wp_version; + /** * Setup. * - * @inheritdoc + * @inheritDoc */ public function setUp() { parent::setUp(); - global $wp_scripts, $wp_styles; + global $wp_scripts, $wp_styles, $wp_version; $wp_scripts = null; $wp_styles = null; $this->instance = new AmpPlugins(); + + $this->original_wp_version = $wp_version; + } + + /** + * Tear down. + * + * @inheritDoc + */ + public function tearDown() { + parent::tearDown(); + + global $wp_version; + $wp_version = $this->original_wp_version; } /** @@ -135,7 +152,6 @@ public function test_get_registration_action() { */ public function test_is_needed() { global $wp_version; - $original_wp_version = $wp_version; // Test 1: Not admin request. $this->assertFalse( AmpPlugins::is_needed() ); @@ -165,7 +181,6 @@ static function ( $shown, $type ) { $this->assertFalse( AmpPlugins::is_needed() ); set_current_screen( 'front' ); - $wp_version = $original_wp_version; } /** diff --git a/tests/php/src/Admin/AmpThemesTest.php b/tests/php/src/Admin/AmpThemesTest.php index a2f914e5848..ccff732743b 100644 --- a/tests/php/src/Admin/AmpThemesTest.php +++ b/tests/php/src/Admin/AmpThemesTest.php @@ -25,19 +25,37 @@ class AmpThemesTest extends TestCase { */ public $instance; + /** @var string */ + private $original_wp_version; + /** * Setup. * - * @inheritdoc + * @inheritDoc */ public function setUp() { + parent::setUp(); - global $wp_scripts, $wp_styles; + global $wp_scripts, $wp_styles, $wp_version; $wp_scripts = null; $wp_styles = null; $this->instance = new AmpThemes(); + + $this->original_wp_version = $wp_version; + } + + /** + * Tear down. + * + * @inheritDoc + */ + public function tearDown() { + parent::tearDown(); + + global $wp_version; + $wp_version = $this->original_wp_version; } /** @@ -53,7 +71,6 @@ public function test_get_registration_action() { */ public function test_is_needed() { global $wp_version; - $original_wp_version = $wp_version; set_current_screen( 'front' ); @@ -86,7 +103,6 @@ static function ( $shown, $type ) { $this->assertFalse( AmpThemes::is_needed() ); set_current_screen( 'front' ); - $wp_version = $original_wp_version; } /** diff --git a/tests/php/src/Admin/SupportScreenTest.php b/tests/php/src/Admin/SupportScreenTest.php index 24ebfb556f4..7bf730e756e 100644 --- a/tests/php/src/Admin/SupportScreenTest.php +++ b/tests/php/src/Admin/SupportScreenTest.php @@ -34,24 +34,41 @@ class SupportScreenTest extends DependencyInjectedTestCase { */ public $instance; + /** @var string */ + private $original_wp_version; + /** * Setup. * * @inheritdoc */ public function setUp() { - parent::setUp(); if ( ! class_exists( 'WP_Site_Health' ) ) { $this->markTestSkipped( 'Test requires Site Health.' ); } + global $wp_version; + $this->original_wp_version = $wp_version; + $this->instance = $this->injector->make( SupportScreen::class ); $this->add_home_url_loopback_request_mocking(); } + /** + * Tear down. + * + * @inheritDoc + */ + public function tearDown() { + parent::tearDown(); + + global $wp_version; + $wp_version = $this->original_wp_version; + } + /** @covers ::__construct() */ public function test__construct() { @@ -73,18 +90,21 @@ public function test_get_registration_action() { * @covers ::check_core_version() */ public function test_check_core_version() { - global $wp_version; - $original_wp_version = $wp_version; + // This will always be true by default because setUp calls markTestSkipped if WP_Site_Health doesn't exist. + $this->assertTrue( SupportScreen::check_core_version() ); $wp_version = '4.9'; $this->assertFalse( SupportScreen::check_core_version() ); - $wp_version = '5.2'; - $this->assertTrue( SupportScreen::check_core_version() ); + $wp_version = '5.0'; + $this->assertFalse( SupportScreen::check_core_version() ); + + $wp_version = '5.1'; + $this->assertFalse( SupportScreen::check_core_version() ); - $wp_version = $original_wp_version; + $wp_version = '5.2'; $this->assertTrue( SupportScreen::check_core_version() ); } From e5c562b9b3001ce4164862dca2c3245cbd2bd44d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 10:29:21 -0800 Subject: [PATCH 29/36] Run amp_ensure_id_attribute_on_script_loader_tag at earliest priority Also remove invaid codeCoverageIgnore tags --- includes/amp-helper-functions.php | 2 +- src/Admin/AmpPlugins.php | 2 +- src/Admin/AmpThemes.php | 2 +- tests/php/test-amp-helper-functions.php | 5 ++++- tests/php/test-class-amp-theme-support.php | 4 ++-- 5 files changed, 9 insertions(+), 6 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 1a86c4dfad6..d44d3b68f60 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -94,7 +94,7 @@ function amp_bootstrap_plugin() { // Ensure ID attribute is present in WP<5.5. if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { - add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag', 1000, 2 ); // @codeCoverageIgnore + add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag', ~PHP_INT_MAX, 2 ); // @codeCoverageIgnore } // Ensure crossorigin=anonymous is added to font links. diff --git a/src/Admin/AmpPlugins.php b/src/Admin/AmpPlugins.php index ad2056fa9fb..ab71d942a39 100644 --- a/src/Admin/AmpPlugins.php +++ b/src/Admin/AmpPlugins.php @@ -73,7 +73,7 @@ public static function get_requirements() { public static function is_needed() { if ( ! Services::get( 'dependency_support' )->has_support() ) { - return false; // @codeCoverageIgnore + return false; } /** This filter is documented in src/Admin/AmpThemes.php */ diff --git a/src/Admin/AmpThemes.php b/src/Admin/AmpThemes.php index 49be63041bb..73aebd9ed7d 100644 --- a/src/Admin/AmpThemes.php +++ b/src/Admin/AmpThemes.php @@ -71,7 +71,7 @@ public static function get_requirements() { public static function is_needed() { if ( ! Services::get( 'dependency_support' )->has_support() ) { - return false; // @codeCoverageIgnore + return false; } /** diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index b41ef2b5676..fae37a06d6a 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -129,7 +129,10 @@ public function test_amp_bootstrap_plugin() { $this->assertEquals( PHP_INT_MAX, has_filter( 'script_loader_tag', 'amp_filter_script_loader_tag' ) ); if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { - $this->assertEquals( 1000, has_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag' ) ); + $this->assertEquals( + defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX, // phpcs:ignore PHPCompatibility.Constants.NewConstants + has_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag' ) + ); } else { $this->assertFalse( has_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag' ) ); } diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index d9097a8f86c..7120e31083a 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -1759,11 +1759,11 @@ function ( $dom, $effective_sandboxing_level ) { foreach ( $ordered_contains as $ordered_contain ) { if ( '#' === substr( $ordered_contain, 0, 1 ) ) { $this->assertEquals( 1, preg_match( $ordered_contain, $sanitized_html, $matches, PREG_OFFSET_CAPTURE ), "Failed to find: $ordered_contain" ); - $this->assertGreaterThan( $last_position, $matches[0][1], "'$ordered_contain' is not after '$prev_ordered_contain'" ); + $this->assertGreaterThan( $last_position, $matches[0][1], "'$ordered_contain' is not after '$prev_ordered_contain' in:\n$sanitized_html" ); $last_position = $matches[0][1]; } else { $this_position = strpos( $sanitized_html, $ordered_contain ); - $this->assertNotFalse( $this_position, "Failed to find: $ordered_contain" ); + $this->assertNotFalse( $this_position, "Failed to find: $ordered_contain in:\n$sanitized_html" ); $this->assertGreaterThan( $last_position, (int) $this_position, "'$ordered_contain' is not after '$prev_ordered_contain'" ); $last_position = $this_position; } From 0dcdc77880f34ca0b4f322a20a33096d652c70e6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 10:44:32 -0800 Subject: [PATCH 30/36] Fix tests in WP 4.9..5.1 --- tests/php/test-amp-helper-functions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index fae37a06d6a..f8da67709b2 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -1273,7 +1273,7 @@ public function test_script_registering() { add_filter( 'script_loader_tag', static function ( $script ) { - return preg_replace( "/ id='amp-[^']+?'/", '', $script ); + return preg_replace( '/ id=(["\'])amp-.*?\1/', '', $script ); } ); From 6d26e84e47e4553e8a728a830cae673c82e0de60 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 11:01:48 -0800 Subject: [PATCH 31/36] Fix test_prepare_response_standard in WP<5.5 --- tests/php/test-class-amp-theme-support.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 7120e31083a..f6e819fd0cc 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -2088,7 +2088,7 @@ private function get_original_html() { add_filter( 'script_loader_tag', static function ( $script ) { - return preg_replace( "/ id='amp-[^']+?'/", '', $script ); + return preg_replace( '/ id=(["\'])amp-.*?\1/', '', $script ); } ); From f1c62085dce1cc224a343dbab7820b485e232f70 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 11:36:40 -0800 Subject: [PATCH 32/36] Add tests for AMP_Customizer_Design_Settings --- .../class-amp-customizer-design-settings.php | 2 +- ...t-class-amp-customizer-design-settings.php | 108 +++++++++++++++--- 2 files changed, 96 insertions(+), 14 deletions(-) diff --git a/includes/settings/class-amp-customizer-design-settings.php b/includes/settings/class-amp-customizer-design-settings.php index f1fbc80491f..205315b9c3e 100644 --- a/includes/settings/class-amp-customizer-design-settings.php +++ b/includes/settings/class-amp-customizer-design-settings.php @@ -74,7 +74,7 @@ public static function init() { */ public static function init_customizer() { if ( ! Services::get( 'dependency_support' )->has_support() ) { - return; // @codeCoverageIgnore + return; } add_action( 'amp_customizer_register_settings', [ __CLASS__, 'register_customizer_settings' ] ); add_action( 'amp_customizer_register_ui', [ __CLASS__, 'register_customizer_ui' ] ); diff --git a/tests/php/test-class-amp-customizer-design-settings.php b/tests/php/test-class-amp-customizer-design-settings.php index f5f80f69e0c..6a9a3890875 100644 --- a/tests/php/test-class-amp-customizer-design-settings.php +++ b/tests/php/test-class-amp-customizer-design-settings.php @@ -11,7 +11,7 @@ /** * Class Test_AMP_Customizer_Design_Settings * - * @covers AMP_Customizer_Design_Settings + * @coversDefaultClass \AMP_Customizer_Design_Settings */ class Test_AMP_Customizer_Design_Settings extends TestCase { @@ -20,26 +20,108 @@ public static function setUpBeforeClass() { return parent::setUpBeforeClass(); } + /** @var string */ + private $original_wp_version; + + /** + * Setup. + * + * @inheritDoc + */ + public function setUp() { + parent::setUp(); + + global $wp_version; + $this->original_wp_version = $wp_version; + } + /** - * Test is_amp_customizer_enabled(). + * Tear down. * - * @covers AMP_Customizer_Design_Settings::is_amp_customizer_enabled + * @inheritDoc + */ + public function tearDown() { + parent::tearDown(); + + global $wp_version; + $wp_version = $this->original_wp_version; + } + + /** @return array */ + public function get_data_to_test_is_amp_customizer_enabled() { + return [ + 'transitional_mode' => [ + static function () { + AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::TRANSITIONAL_MODE_SLUG ); + }, + false, + ], + 'reader_mode' => [ + static function () { + AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG ); + }, + true, + ], + 'filter_disabled' => [ + static function () { + AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG ); + add_filter( 'amp_customizer_is_enabled', '__return_false' ); + }, + false, + ], + ]; + } + + /** + * @dataProvider get_data_to_test_is_amp_customizer_enabled + * @covers ::is_amp_customizer_enabled() + * @covers ::init() */ - public function test_is_amp_customizer_enabled() { - AMP_Options_Manager::update_option( Option::THEME_SUPPORT, 'foo' ); - $this->assertEquals( false, AMP_Customizer_Design_Settings::is_amp_customizer_enabled() ); + public function test_is_amp_customizer_enabled_and_init( callable $set_up, $enabled ) { + remove_all_actions( 'amp_customizer_init' ); + remove_all_actions( 'amp_customizer_get_settings' ); + + $set_up(); + $this->assertEquals( $enabled, AMP_Customizer_Design_Settings::is_amp_customizer_enabled() ); + AMP_Customizer_Design_Settings::init(); + + $this->assertEquals( $enabled ? 10 : false, has_action( 'amp_customizer_init', [ AMP_Customizer_Design_Settings::class, 'init_customizer' ] ) ); + $this->assertEquals( $enabled ? 10 : false, has_filter( 'amp_customizer_get_settings', [ AMP_Customizer_Design_Settings::class, 'append_settings' ] ) ); + + } - AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG ); - $this->assertEquals( true, AMP_Customizer_Design_Settings::is_amp_customizer_enabled() ); + /** @return array */ + public function get_data_to_test_init_customized() { + return [ + 'has_dependency_support' => [ true ], + 'not_has_dependency_support' => [ false ], + ]; + } + + /** + * @dataProvider get_data_to_test_init_customized + * @covers ::init_customizer() + */ + public function test_init_customizer( $has_dependency_support ) { + remove_all_actions( 'amp_customizer_register_settings' ); + remove_all_actions( 'amp_customizer_register_ui' ); + remove_all_actions( 'amp_customizer_enqueue_preview_scripts' ); - add_filter( 'amp_customizer_is_enabled', '__return_false' ); - $this->assertEquals( false, AMP_Customizer_Design_Settings::is_amp_customizer_enabled() ); + if ( $has_dependency_support ) { + $GLOBALS['wp_version'] = '5.6'; + } else { + $GLOBALS['wp_version'] = '5.5'; + } + AMP_Customizer_Design_Settings::init_customizer(); + $this->assertEquals( $has_dependency_support ? 10 : false, has_action( 'amp_customizer_register_settings', [ AMP_Customizer_Design_Settings::class, 'register_customizer_settings' ] ) ); + $this->assertEquals( $has_dependency_support ? 10 : false, has_action( 'amp_customizer_register_ui', [ AMP_Customizer_Design_Settings::class, 'register_customizer_ui' ] ) ); + $this->assertEquals( $has_dependency_support ? 10 : false, has_action( 'amp_customizer_enqueue_preview_scripts', [ AMP_Customizer_Design_Settings::class, 'enqueue_customizer_preview_scripts' ] ) ); } /** * Test register_customizer_settings(). * - * @covers AMP_Customizer_Design_Settings::register_customizer_settings + * @covers ::register_customizer_settings() */ public function test_register_customizer_settings() { $wp_customize = new WP_Customize_Manager(); @@ -58,7 +140,7 @@ public function test_register_customizer_settings() { /** * Test register_customizer_ui(). * - * @covers AMP_Customizer_Design_Settings::register_customizer_ui + * @covers ::register_customizer_ui() */ public function test_register_customizer_ui() { $wp_customize = new WP_Customize_Manager(); @@ -98,7 +180,7 @@ public function get_color_schemes() { /** * Test sanitize_color_scheme(). * - * @covers AMP_Customizer_Design_Settings::sanitize_color_scheme + * @covers ::sanitize_color_scheme() * @dataProvider get_color_schemes * * @param string $color_scheme Color scheme. From ccb9090bb4c17d35fbcedf60f297c678e22da2a7 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 11:42:21 -0800 Subject: [PATCH 33/36] Add tests for amp_bootstrap_plugin() --- includes/amp-helper-functions.php | 2 +- tests/php/test-amp-helper-functions.php | 34 ++++++++++++++++++++++--- 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index d44d3b68f60..5d7cf737f99 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -94,7 +94,7 @@ function amp_bootstrap_plugin() { // Ensure ID attribute is present in WP<5.5. if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { - add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag', ~PHP_INT_MAX, 2 ); // @codeCoverageIgnore + add_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag', ~PHP_INT_MAX, 2 ); } // Ensure crossorigin=anonymous is added to font links. diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php index f8da67709b2..b70d0881046 100644 --- a/tests/php/test-amp-helper-functions.php +++ b/tests/php/test-amp-helper-functions.php @@ -36,19 +36,29 @@ class Test_AMP_Helper_Functions extends DependencyInjectedTestCase { */ private $server_var_backup; + /** @var string */ + private $original_wp_version; + /** * Set up. + * + * @inheritDoc */ public function setUp() { parent::setUp(); $this->server_var_backup = $_SERVER; remove_theme_support( 'amp' ); + global $wp_version; + $this->original_wp_version = $wp_version; + $this->register_core_themes(); } /** - * After a test method runs, reset any state in WordPress the test method might have changed. + * Tear down. + * + * @inheritDoc */ public function tearDown() { AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG ); @@ -80,6 +90,9 @@ public function tearDown() { $this->remove_added_uploads(); } + global $wp_version; + $wp_version = $this->original_wp_version; + parent::tearDown(); } @@ -117,9 +130,21 @@ private function remove_bootstrapped_hooks() { } } - /** @covers ::amp_bootstrap_plugin() */ - public function test_amp_bootstrap_plugin() { + /** @return array */ + public function get_data_to_test_amp_bootstrap_plugin() { + return [ + '5.4' => [ '5.4', true ], + '5.5' => [ '5.5', false ], + ]; + } + + /** + * @dataProvider get_data_to_test_amp_bootstrap_plugin + * @covers ::amp_bootstrap_plugin() + */ + public function test_amp_bootstrap_plugin( $wp_version, $needs_script_loader_tag_filter ) { $this->remove_bootstrapped_hooks(); + $GLOBALS['wp_version'] = $wp_version; amp_bootstrap_plugin(); $this->assertEquals( 10, has_action( 'wp_default_scripts', 'amp_register_default_scripts' ) ); @@ -128,7 +153,8 @@ public function test_amp_bootstrap_plugin() { $this->assertEquals( 9, has_action( 'plugins_loaded', '_amp_bootstrap_customizer' ) ); $this->assertEquals( PHP_INT_MAX, has_filter( 'script_loader_tag', 'amp_filter_script_loader_tag' ) ); - if ( version_compare( get_bloginfo( 'version' ), '5.5', '<' ) ) { + + if ( $needs_script_loader_tag_filter ) { $this->assertEquals( defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX, // phpcs:ignore PHPCompatibility.Constants.NewConstants has_filter( 'script_loader_tag', 'amp_ensure_id_attribute_on_script_loader_tag' ) From 6dbd043de241cb84479616d4c39ac4e85834f651 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 11:52:45 -0800 Subject: [PATCH 34/36] Fix test_init_customizer to account for Gutenberg being active --- tests/php/test-class-amp-customizer-design-settings.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/php/test-class-amp-customizer-design-settings.php b/tests/php/test-class-amp-customizer-design-settings.php index 6a9a3890875..64d48551195 100644 --- a/tests/php/test-class-amp-customizer-design-settings.php +++ b/tests/php/test-class-amp-customizer-design-settings.php @@ -5,6 +5,7 @@ * @package AMP */ +use AmpProject\AmpWP\DependencySupport; use AmpProject\AmpWP\Option; use AmpProject\AmpWP\Tests\TestCase; @@ -112,6 +113,8 @@ public function test_init_customizer( $has_dependency_support ) { } else { $GLOBALS['wp_version'] = '5.5'; } + $has_dependency_support = ( new DependencySupport() )->has_support(); // To account for Gutenberg being active. + AMP_Customizer_Design_Settings::init_customizer(); $this->assertEquals( $has_dependency_support ? 10 : false, has_action( 'amp_customizer_register_settings', [ AMP_Customizer_Design_Settings::class, 'register_customizer_settings' ] ) ); $this->assertEquals( $has_dependency_support ? 10 : false, has_action( 'amp_customizer_register_ui', [ AMP_Customizer_Design_Settings::class, 'register_customizer_ui' ] ) ); From 779104f9550ae029dffb2292799a24d69e53c9ba Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 15:19:11 -0800 Subject: [PATCH 35/36] Use remove_all_filters instead of remove_all_actions for purity sake --- tests/php/test-class-amp-customizer-design-settings.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/test-class-amp-customizer-design-settings.php b/tests/php/test-class-amp-customizer-design-settings.php index 64d48551195..90f5018b340 100644 --- a/tests/php/test-class-amp-customizer-design-settings.php +++ b/tests/php/test-class-amp-customizer-design-settings.php @@ -80,7 +80,7 @@ static function () { */ public function test_is_amp_customizer_enabled_and_init( callable $set_up, $enabled ) { remove_all_actions( 'amp_customizer_init' ); - remove_all_actions( 'amp_customizer_get_settings' ); + remove_all_filters( 'amp_customizer_get_settings' ); $set_up(); $this->assertEquals( $enabled, AMP_Customizer_Design_Settings::is_amp_customizer_enabled() ); From ff7e7b7839d45c74b57d926f8d6fda2054c569fa Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Mon, 13 Dec 2021 15:59:55 -0800 Subject: [PATCH 36/36] Hide DevTools toggles when dependency_support is absent --- assets/src/settings-page/index.js | 4 +++- src/DevTools/UserAccess.php | 20 ++++++++++++++++++-- tests/php/src/DevTools/UserAccessTest.php | 15 +++++++++++---- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/assets/src/settings-page/index.js b/assets/src/settings-page/index.js index 0dd3a18e54b..fc22400a130 100644 --- a/assets/src/settings-page/index.js +++ b/assets/src/settings-page/index.js @@ -277,7 +277,9 @@ function Root( { appRoot } ) { initialOpen={ 'other-settings' === focusedSection } > - + { HAS_DEPENDENCY_SUPPORT && ( + + ) } diff --git a/src/DevTools/UserAccess.php b/src/DevTools/UserAccess.php index 0da4ed94c83..76b9fd9fc3a 100644 --- a/src/DevTools/UserAccess.php +++ b/src/DevTools/UserAccess.php @@ -146,13 +146,29 @@ public function register_rest_field() { ); } + /** + * Determine whether the option can be modified. + * + * @param int $user_id User ID. + * @return bool Whether the option can be modified. + */ + private function can_modify_option( $user_id ) { + return ( + $this->dependency_support->has_support() + && + current_user_can( 'edit_user', $user_id ) + && + AMP_Validation_Manager::has_cap( $user_id ) + ); + } + /** * Add the developer tools checkbox to the user edit screen. * * @param WP_User $profile_user Current user being edited. */ public function print_personal_options( $profile_user ) { - if ( ! current_user_can( 'edit_user', $profile_user->ID ) || ! AMP_Validation_Manager::has_cap( $profile_user ) ) { + if ( ! $this->can_modify_option( $profile_user->ID ) ) { return; } ?> @@ -177,7 +193,7 @@ public function print_personal_options( $profile_user ) { * @return bool Whether update was successful. */ public function update_user_setting( $user_id ) { - if ( ! current_user_can( 'edit_user', $user_id ) || ! AMP_Validation_Manager::has_cap( $user_id ) ) { + if ( ! $this->can_modify_option( $user_id ) ) { return false; } $enabled = isset( $_POST[ self::USER_FIELD_DEVELOPER_TOOLS_ENABLED ] ) && rest_sanitize_boolean( wp_unslash( $_POST[ self::USER_FIELD_DEVELOPER_TOOLS_ENABLED ] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Missing, phpcs:ignore WordPress.Security.ValidatedSanitizedInput.InputNotSanitized -- Nonce handled by user-edit.php; sanitization used is sanitized. diff --git a/tests/php/src/DevTools/UserAccessTest.php b/tests/php/src/DevTools/UserAccessTest.php index f74eae473ea..61bc5a36f9e 100644 --- a/tests/php/src/DevTools/UserAccessTest.php +++ b/tests/php/src/DevTools/UserAccessTest.php @@ -165,6 +165,7 @@ public function test_register_rest_field() { /** * Tests UserAccess::print_personal_options * + * @covers ::can_modify_option * @covers ::print_personal_options */ public function test_print_personal_options() { @@ -182,12 +183,18 @@ public function test_print_personal_options() { ob_start(); $this->dev_tools_user_access->print_personal_options( $admin_user ); - $this->assertStringContainsString( 'checkbox', ob_get_clean() ); + $output = ob_get_clean(); + if ( ( new DependencySupport() )->has_support() ) { + $this->assertStringContainsString( 'checkbox', $output ); + } else { + $this->assertStringNotContainsString( 'checkbox', $output ); + } } /** * Tests UserAccess::update_user_setting * + * @covers ::can_modify_option * @covers ::update_user_setting */ public function test_update_user_setting() { @@ -201,10 +208,10 @@ public function test_update_user_setting() { wp_set_current_user( $admin_user->ID ); $this->assertFalse( $this->dev_tools_user_access->update_user_setting( $editor_user->ID ) ); - $this->assertTrue( $this->dev_tools_user_access->update_user_setting( $admin_user->ID ) ); - $this->assertTrue( $this->dev_tools_user_access->get_user_enabled( $admin_user ) ); + $this->assertEquals( ( new DependencySupport() )->has_support(), $this->dev_tools_user_access->update_user_setting( $admin_user->ID ) ); + $this->assertEquals( ( new DependencySupport() )->has_support(), $this->dev_tools_user_access->get_user_enabled( $admin_user ) ); $_POST[ UserAccess::USER_FIELD_DEVELOPER_TOOLS_ENABLED ] = null; - $this->assertTrue( $this->dev_tools_user_access->update_user_setting( $admin_user->ID ) ); + $this->assertEquals( ( new DependencySupport() )->has_support(), $this->dev_tools_user_access->update_user_setting( $admin_user->ID ) ); $this->assertFalse( $this->dev_tools_user_access->get_user_enabled( $admin_user ) ); }