Merge pull request #6125 from ampproject/fix/late-defined-slug-handling

Support late-defined slugs for paired URL structures and Reader themes
ampproject · Apr 28, 2021 · d6461d9 · d6461d9
2 parents 3eb0d4f + 3549c0a
commit d6461d9
Show file tree

Hide file tree

Showing 19 changed files with 450 additions and 176 deletions.
diff --git a/assets/src/components/reader-theme-carousel/index.js b/assets/src/components/reader-theme-carousel/index.js
@@ -1,8 +1,3 @@
-/**
- * External dependencies
- */
-import { AMP_QUERY_VAR, DEFAULT_AMP_QUERY_VAR, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.
-
 /**
  * WordPress dependencies
  */
@@ -155,25 +150,7 @@ export function ReaderThemeCarousel() {
 					hasUnavailableThemes && (
 						<AMPNotice type={ NOTICE_TYPE_INFO }>
 							<p>
-								{ AMP_QUERY_VAR_CUSTOMIZED_LATE
-								/* dangerouslySetInnerHTML reason: Injection of code tags. */
-									? (
-										<span
-											dangerouslySetInnerHTML={ {
-												__html: sprintf(
-													/* translators: 1: customized AMP query var, 2: default query var, 3: the AMP_QUERY_VAR constant name, 4: the amp_query_var filter, 5: the plugins_loaded action */
-													__( 'Some of the themes below are not available because your site (probably the active theme) has customized the AMP query var too late (it is set to %1$s as opposed to the default of %2$s). Please make sure that any customizations done by defining the %3$s constant or adding an %4$s filter are done before the %5$s action with priority 8.', 'amp' ),
-													`<code>${ AMP_QUERY_VAR }</code>`,
-													`<code>${ DEFAULT_AMP_QUERY_VAR }</code>`,
-													'<code>AMP_QUERY_VAR</code>',
-													'<code>amp_query_var</code>',
-													'<code>plugins_loaded</code>',
-												),
-											} }
-										/>
-									)
-									: __( 'Some supported themes cannot be installed automatically on this site. To use, please install them manually or contact your hosting provider.', 'amp' )
-								}
+								{ __( 'Some supported themes cannot be installed automatically on this site. To use, please install them manually or contact your hosting provider.', 'amp' ) }
 							</p>
 							<AMPSettingToggle
 								text={ __( 'Show unavailable themes', 'amp' ) }

diff --git a/assets/src/components/reader-theme-selection/index.js b/assets/src/components/reader-theme-selection/index.js
@@ -2,12 +2,11 @@
  * External dependencies
  */
 import PropTypes from 'prop-types';
-import { AMP_QUERY_VAR, DEFAULT_AMP_QUERY_VAR, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.
 
 /**
  * WordPress dependencies
  */
-import { __, sprintf } from '@wordpress/i18n';
+import { __ } from '@wordpress/i18n';
 import { useContext } from '@wordpress/element';
 
 /**
@@ -56,25 +55,7 @@ export function ReaderThemeSelection() {
 							{ __( 'Unavailable themes', 'amp' ) }
 						</h3>
 						<p>
-							{ AMP_QUERY_VAR_CUSTOMIZED_LATE
-								/* dangerouslySetInnerHTML reason: Injection of code tags. */
-								? (
-									<span
-										dangerouslySetInnerHTML={ {
-											__html: sprintf(
-												/* translators: 1: customized AMP query var, 2: default query var, 3: the AMP_QUERY_VAR constant name, 4: the amp_query_var filter, 5: the plugins_loaded action */
-												__( 'The following themes are not available because your site (probably the active theme) has customized the AMP query var too late (it is set to %1$s as opposed to the default of %2$s). Please make sure that any customizations done by defining the %3$s constant or adding an %4$s filter are done before the %5$s action with priority 8.', 'amp' ),
-												`<code>${ AMP_QUERY_VAR }</code>`,
-												`<code>${ DEFAULT_AMP_QUERY_VAR }</code>`,
-												'<code>AMP_QUERY_VAR</code>',
-												'<code>amp_query_var</code>',
-												'<code>plugins_loaded</code>',
-											),
-										} }
-									/>
-								)
-								: __( 'The following themes are compatible but cannot be installed automatically. Please install them manually, or contact your host if you are not able to do so.', 'amp' )
-							}
+							{ __( 'The following themes are compatible but cannot be installed automatically. Please install them manually, or contact your host if you are not able to do so.', 'amp' ) }
 						</p>
 						<ul className="choose-reader-theme__grid">
 							{ unavailableThemes.map( ( theme ) => (

diff --git a/assets/src/components/reader-themes-context-provider/index.js b/assets/src/components/reader-themes-context-provider/index.js
@@ -9,7 +9,7 @@ import { __ } from '@wordpress/i18n';
  * External dependencies
  */
 import PropTypes from 'prop-types';
-import { AMP_QUERY_VAR_CUSTOMIZED_LATE, USING_FALLBACK_READER_THEME, LEGACY_THEME_SLUG } from 'amp-settings';
+import { USING_FALLBACK_READER_THEME, LEGACY_THEME_SLUG } from 'amp-settings';
 
 /**
  * Internal dependencies
@@ -282,7 +282,7 @@ export function ReaderThemesContextProvider( { wpAjaxUrl, children, currentTheme
 	const { availableThemes, unavailableThemes } = useMemo(
 		() => ( filteredThemes || [] ).reduce(
 			( collections, theme ) => {
-				if ( ( AMP_QUERY_VAR_CUSTOMIZED_LATE && theme.slug !== LEGACY_THEME_SLUG ) || theme.availability === 'non-installable' ) {
+				if ( theme.availability === 'non-installable' ) {
 					collections.unavailableThemes.push( theme );
 				} else {
 					collections.availableThemes.push( theme );

diff --git a/assets/src/onboarding-wizard/pages/choose-reader-theme/index.js b/assets/src/onboarding-wizard/pages/choose-reader-theme/index.js
@@ -1,8 +1,3 @@
-/**
- * External dependencies
- */
-import { LEGACY_THEME_SLUG, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.
-
 /**
  * WordPress dependencies
  */
@@ -35,9 +30,7 @@ export function ChooseReaderTheme() {
 			themes &&
 			readerTheme &&
 			canGoForward === false &&
-			! AMP_QUERY_VAR_CUSTOMIZED_LATE
-				? themes.map( ( { slug } ) => slug ).includes( readerTheme )
-				: readerTheme === LEGACY_THEME_SLUG
+			themes.map( ( { slug } ) => slug ).includes( readerTheme )
 		) {
 			setCanGoForward( true );
 		}

diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php
@@ -6,6 +6,7 @@
  */
 
 use AmpProject\AmpWP\Admin\ReaderThemes;
+use AmpProject\AmpWP\AmpSlugCustomizationWatcher;
 use AmpProject\AmpWP\AmpWpPluginFactory;
 use AmpProject\AmpWP\Exception\InvalidService;
 use AmpProject\AmpWP\Icon;
@@ -531,10 +532,23 @@ function _amp_bootstrap_customizer() {
  * may be deprecated in the future. Normally the slug should be just 'amp'.
  *
  * @since 0.7
+ * @since 2.1 Added $ignore_late_defined_slug argument.
  *
+ * @param bool $ignore_late_defined_slug Whether to ignore the late defined slug.
  * @return string Slug used for query var, endpoint, and post type support.
  */
-function amp_get_slug() {
+function amp_get_slug( $ignore_late_defined_slug = false ) {
+
+	// When a slug was defined late according to AmpSlugCustomizationWatcher, the slug will be stored in the
+	// LATE_DEFINED_SLUG option by the PairedRouting service so that it can be used early. This is only needed until
+	// the after_setup_theme action fires, because at that time the late-defined slug will have been established.
+	if ( ! $ignore_late_defined_slug && ! did_action( AmpSlugCustomizationWatcher::LATE_DETERMINATION_ACTION ) ) {
+		$slug = AMP_Options_Manager::get_option( Option::LATE_DEFINED_SLUG );
+		if ( ! empty( $slug ) && is_string( $slug ) ) {
+			return $slug;
+		}
+	}
+
 	/**
 	 * Filter the AMP query variable.
 	 *

diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -833,6 +833,7 @@ public static function add_hooks() {
 		if ( has_action( 'wp_head', 'print_emoji_detection_script' ) ) {
 			remove_action( 'wp_head', 'print_emoji_detection_script', 7 );
 			remove_action( 'wp_print_styles', 'print_emoji_styles' );
+			remove_action( 'wp_footer', 'gutenberg_the_skip_link' ); // Temporary workaround for <https://github.com/ampproject/amp-wp/issues/6115>.
 			add_action( 'wp_print_styles', [ __CLASS__, 'print_emoji_styles' ] );
 			add_filter( 'the_title', 'wp_staticize_emoji' );
 			add_filter( 'the_excerpt', 'wp_staticize_emoji' );

diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php
@@ -60,28 +60,6 @@ public static function register_settings() {
 				'sanitize_callback' => [ __CLASS__, 'validate_options' ],
 			]
 		);
-
-		add_action( 'update_option_' . self::OPTION_NAME, [ __CLASS__, 'maybe_flush_rewrite_rules' ], 10, 2 );
-	}
-
-	/**
-	 * Flush rewrite rules if the supported_post_types have changed.
-	 *
-	 * @since 0.6.2
-	 *
-	 * @param array $old_options Old options.
-	 * @param array $new_options New options.
-	 */
-	public static function maybe_flush_rewrite_rules( $old_options, $new_options ) {
-		$old_post_types = isset( $old_options[ Option::SUPPORTED_POST_TYPES ] ) ? $old_options[ Option::SUPPORTED_POST_TYPES ] : [];
-		$new_post_types = isset( $new_options[ Option::SUPPORTED_POST_TYPES ] ) ? $new_options[ Option::SUPPORTED_POST_TYPES ] : [];
-		sort( $old_post_types );
-		sort( $new_post_types );
-		if ( $old_post_types !== $new_post_types ) {
-			// Flush rewrite rules.
-			add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );
-			flush_rewrite_rules( false );
-		}
 	}
 
 	/**

diff --git a/src/Admin/OnboardingWizardSubmenuPage.php b/src/Admin/OnboardingWizardSubmenuPage.php
@@ -220,8 +220,6 @@ public function enqueue_assets( $hook_suffix ) {
 		$setup_wizard_data = [
 			'AMP_OPTIONS_KEY'                    => AMP_Options_Manager::OPTION_NAME,
 			'AMP_QUERY_VAR'                      => amp_get_slug(),
-			'DEFAULT_AMP_QUERY_VAR'              => QueryVar::AMP,
-			'AMP_QUERY_VAR_CUSTOMIZED_LATE'      => $amp_slug_customization_watcher->did_customize_late(),
 			'LEGACY_THEME_SLUG'                  => ReaderThemes::DEFAULT_READER_THEME,
 			'APP_ROOT_ID'                        => self::APP_ROOT_ID,
 			'CUSTOMIZER_LINK'                    => add_query_arg(

diff --git a/src/Admin/OptionsMenu.php b/src/Admin/OptionsMenu.php
@@ -214,28 +214,26 @@ public function enqueue_assets( $hook_suffix ) {
 		$is_reader_theme = $this->reader_themes->theme_data_exists( get_stylesheet() );
 
 		$js_data = [
-			'AMP_QUERY_VAR'                 => amp_get_slug(),
-			'DEFAULT_AMP_QUERY_VAR'         => QueryVar::AMP,
-			'AMP_QUERY_VAR_CUSTOMIZED_LATE' => $amp_slug_customization_watcher->did_customize_late(),
-			'CURRENT_THEME'                 => [
+			'AMP_QUERY_VAR'               => amp_get_slug(),
+			'CURRENT_THEME'               => [
 				'name'            => $theme->get( 'Name' ),
 				'description'     => $theme->get( 'Description' ),
 				'is_reader_theme' => $is_reader_theme,
 				'screenshot'      => $theme->get_screenshot() ?: null,
 				'url'             => $theme->get( 'ThemeURI' ),
 			],
-			'OPTIONS_REST_PATH'             => '/amp/v1/options',
-			'READER_THEMES_REST_PATH'       => '/amp/v1/reader-themes',
-			'IS_CORE_THEME'                 => in_array(
+			'OPTIONS_REST_PATH'           => '/amp/v1/options',
+			'READER_THEMES_REST_PATH'     => '/amp/v1/reader-themes',
+			'IS_CORE_THEME'               => in_array(
 				get_stylesheet(),
 				AMP_Core_Theme_Sanitizer::get_supported_themes(),
 				true
 			),
-			'LEGACY_THEME_SLUG'             => ReaderThemes::DEFAULT_READER_THEME,
-			'USING_FALLBACK_READER_THEME'   => $this->reader_themes->using_fallback_theme(),
-			'THEME_SUPPORT_ARGS'            => AMP_Theme_Support::get_theme_support_args(),
-			'THEME_SUPPORTS_READER_MODE'    => AMP_Theme_Support::supports_reader_mode(),
-			'UPDATES_NONCE'                 => wp_create_nonce( 'updates' ),
+			'LEGACY_THEME_SLUG'           => ReaderThemes::DEFAULT_READER_THEME,
+			'USING_FALLBACK_READER_THEME' => $this->reader_themes->using_fallback_theme(),
+			'THEME_SUPPORT_ARGS'          => AMP_Theme_Support::get_theme_support_args(),
+			'THEME_SUPPORTS_READER_MODE'  => AMP_Theme_Support::supports_reader_mode(),
+			'UPDATES_NONCE'               => wp_create_nonce( 'updates' ),
 		];
 
 		wp_add_inline_script(