Skip to content

Commit d6461d9

Browse files
authored
Merge pull request #6125 from ampproject/fix/late-defined-slug-handling
Support late-defined slugs for paired URL structures and Reader themes
2 parents 3eb0d4f + 3549c0a commit d6461d9

File tree

19 files changed

+450
-176
lines changed

19 files changed

+450
-176
lines changed

assets/src/components/reader-theme-carousel/index.js

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
/**
2-
* External dependencies
3-
*/
4-
import { AMP_QUERY_VAR, DEFAULT_AMP_QUERY_VAR, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.
5-
61
/**
72
* WordPress dependencies
83
*/
@@ -155,25 +150,7 @@ export function ReaderThemeCarousel() {
155150
hasUnavailableThemes && (
156151
<AMPNotice type={ NOTICE_TYPE_INFO }>
157152
<p>
158-
{ AMP_QUERY_VAR_CUSTOMIZED_LATE
159-
/* dangerouslySetInnerHTML reason: Injection of code tags. */
160-
? (
161-
<span
162-
dangerouslySetInnerHTML={ {
163-
__html: sprintf(
164-
/* 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 */
165-
__( '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' ),
166-
`<code>${ AMP_QUERY_VAR }</code>`,
167-
`<code>${ DEFAULT_AMP_QUERY_VAR }</code>`,
168-
'<code>AMP_QUERY_VAR</code>',
169-
'<code>amp_query_var</code>',
170-
'<code>plugins_loaded</code>',
171-
),
172-
} }
173-
/>
174-
)
175-
: __( 'Some supported themes cannot be installed automatically on this site. To use, please install them manually or contact your hosting provider.', 'amp' )
176-
}
153+
{ __( 'Some supported themes cannot be installed automatically on this site. To use, please install them manually or contact your hosting provider.', 'amp' ) }
177154
</p>
178155
<AMPSettingToggle
179156
text={ __( 'Show unavailable themes', 'amp' ) }

assets/src/components/reader-theme-selection/index.js

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,11 @@
22
* External dependencies
33
*/
44
import PropTypes from 'prop-types';
5-
import { AMP_QUERY_VAR, DEFAULT_AMP_QUERY_VAR, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.
65

76
/**
87
* WordPress dependencies
98
*/
10-
import { __, sprintf } from '@wordpress/i18n';
9+
import { __ } from '@wordpress/i18n';
1110
import { useContext } from '@wordpress/element';
1211

1312
/**
@@ -56,25 +55,7 @@ export function ReaderThemeSelection() {
5655
{ __( 'Unavailable themes', 'amp' ) }
5756
</h3>
5857
<p>
59-
{ AMP_QUERY_VAR_CUSTOMIZED_LATE
60-
/* dangerouslySetInnerHTML reason: Injection of code tags. */
61-
? (
62-
<span
63-
dangerouslySetInnerHTML={ {
64-
__html: sprintf(
65-
/* 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 */
66-
__( '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' ),
67-
`<code>${ AMP_QUERY_VAR }</code>`,
68-
`<code>${ DEFAULT_AMP_QUERY_VAR }</code>`,
69-
'<code>AMP_QUERY_VAR</code>',
70-
'<code>amp_query_var</code>',
71-
'<code>plugins_loaded</code>',
72-
),
73-
} }
74-
/>
75-
)
76-
: __( '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' )
77-
}
58+
{ __( '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' ) }
7859
</p>
7960
<ul className="choose-reader-theme__grid">
8061
{ unavailableThemes.map( ( theme ) => (

assets/src/components/reader-themes-context-provider/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { __ } from '@wordpress/i18n';
99
* External dependencies
1010
*/
1111
import PropTypes from 'prop-types';
12-
import { AMP_QUERY_VAR_CUSTOMIZED_LATE, USING_FALLBACK_READER_THEME, LEGACY_THEME_SLUG } from 'amp-settings';
12+
import { USING_FALLBACK_READER_THEME, LEGACY_THEME_SLUG } from 'amp-settings';
1313

1414
/**
1515
* Internal dependencies
@@ -282,7 +282,7 @@ export function ReaderThemesContextProvider( { wpAjaxUrl, children, currentTheme
282282
const { availableThemes, unavailableThemes } = useMemo(
283283
() => ( filteredThemes || [] ).reduce(
284284
( collections, theme ) => {
285-
if ( ( AMP_QUERY_VAR_CUSTOMIZED_LATE && theme.slug !== LEGACY_THEME_SLUG ) || theme.availability === 'non-installable' ) {
285+
if ( theme.availability === 'non-installable' ) {
286286
collections.unavailableThemes.push( theme );
287287
} else {
288288
collections.availableThemes.push( theme );

assets/src/onboarding-wizard/pages/choose-reader-theme/index.js

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,3 @@
1-
/**
2-
* External dependencies
3-
*/
4-
import { LEGACY_THEME_SLUG, AMP_QUERY_VAR_CUSTOMIZED_LATE } from 'amp-settings'; // From WP inline script.
5-
61
/**
72
* WordPress dependencies
83
*/
@@ -35,9 +30,7 @@ export function ChooseReaderTheme() {
3530
themes &&
3631
readerTheme &&
3732
canGoForward === false &&
38-
! AMP_QUERY_VAR_CUSTOMIZED_LATE
39-
? themes.map( ( { slug } ) => slug ).includes( readerTheme )
40-
: readerTheme === LEGACY_THEME_SLUG
33+
themes.map( ( { slug } ) => slug ).includes( readerTheme )
4134
) {
4235
setCanGoForward( true );
4336
}

includes/amp-helper-functions.php

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
use AmpProject\AmpWP\Admin\ReaderThemes;
9+
use AmpProject\AmpWP\AmpSlugCustomizationWatcher;
910
use AmpProject\AmpWP\AmpWpPluginFactory;
1011
use AmpProject\AmpWP\Exception\InvalidService;
1112
use AmpProject\AmpWP\Icon;
@@ -531,10 +532,23 @@ function _amp_bootstrap_customizer() {
531532
* may be deprecated in the future. Normally the slug should be just 'amp'.
532533
*
533534
* @since 0.7
535+
* @since 2.1 Added $ignore_late_defined_slug argument.
534536
*
537+
* @param bool $ignore_late_defined_slug Whether to ignore the late defined slug.
535538
* @return string Slug used for query var, endpoint, and post type support.
536539
*/
537-
function amp_get_slug() {
540+
function amp_get_slug( $ignore_late_defined_slug = false ) {
541+
542+
// When a slug was defined late according to AmpSlugCustomizationWatcher, the slug will be stored in the
543+
// LATE_DEFINED_SLUG option by the PairedRouting service so that it can be used early. This is only needed until
544+
// the after_setup_theme action fires, because at that time the late-defined slug will have been established.
545+
if ( ! $ignore_late_defined_slug && ! did_action( AmpSlugCustomizationWatcher::LATE_DETERMINATION_ACTION ) ) {
546+
$slug = AMP_Options_Manager::get_option( Option::LATE_DEFINED_SLUG );
547+
if ( ! empty( $slug ) && is_string( $slug ) ) {
548+
return $slug;
549+
}
550+
}
551+
538552
/**
539553
* Filter the AMP query variable.
540554
*

includes/class-amp-theme-support.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,7 @@ public static function add_hooks() {
833833
if ( has_action( 'wp_head', 'print_emoji_detection_script' ) ) {
834834
remove_action( 'wp_head', 'print_emoji_detection_script', 7 );
835835
remove_action( 'wp_print_styles', 'print_emoji_styles' );
836+
remove_action( 'wp_footer', 'gutenberg_the_skip_link' ); // Temporary workaround for <https://github.com/ampproject/amp-wp/issues/6115>.
836837
add_action( 'wp_print_styles', [ __CLASS__, 'print_emoji_styles' ] );
837838
add_filter( 'the_title', 'wp_staticize_emoji' );
838839
add_filter( 'the_excerpt', 'wp_staticize_emoji' );

includes/options/class-amp-options-manager.php

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -60,28 +60,6 @@ public static function register_settings() {
6060
'sanitize_callback' => [ __CLASS__, 'validate_options' ],
6161
]
6262
);
63-
64-
add_action( 'update_option_' . self::OPTION_NAME, [ __CLASS__, 'maybe_flush_rewrite_rules' ], 10, 2 );
65-
}
66-
67-
/**
68-
* Flush rewrite rules if the supported_post_types have changed.
69-
*
70-
* @since 0.6.2
71-
*
72-
* @param array $old_options Old options.
73-
* @param array $new_options New options.
74-
*/
75-
public static function maybe_flush_rewrite_rules( $old_options, $new_options ) {
76-
$old_post_types = isset( $old_options[ Option::SUPPORTED_POST_TYPES ] ) ? $old_options[ Option::SUPPORTED_POST_TYPES ] : [];
77-
$new_post_types = isset( $new_options[ Option::SUPPORTED_POST_TYPES ] ) ? $new_options[ Option::SUPPORTED_POST_TYPES ] : [];
78-
sort( $old_post_types );
79-
sort( $new_post_types );
80-
if ( $old_post_types !== $new_post_types ) {
81-
// Flush rewrite rules.
82-
add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );
83-
flush_rewrite_rules( false );
84-
}
8563
}
8664

8765
/**

src/Admin/OnboardingWizardSubmenuPage.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,6 @@ public function enqueue_assets( $hook_suffix ) {
220220
$setup_wizard_data = [
221221
'AMP_OPTIONS_KEY' => AMP_Options_Manager::OPTION_NAME,
222222
'AMP_QUERY_VAR' => amp_get_slug(),
223-
'DEFAULT_AMP_QUERY_VAR' => QueryVar::AMP,
224-
'AMP_QUERY_VAR_CUSTOMIZED_LATE' => $amp_slug_customization_watcher->did_customize_late(),
225223
'LEGACY_THEME_SLUG' => ReaderThemes::DEFAULT_READER_THEME,
226224
'APP_ROOT_ID' => self::APP_ROOT_ID,
227225
'CUSTOMIZER_LINK' => add_query_arg(

src/Admin/OptionsMenu.php

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -214,28 +214,26 @@ public function enqueue_assets( $hook_suffix ) {
214214
$is_reader_theme = $this->reader_themes->theme_data_exists( get_stylesheet() );
215215

216216
$js_data = [
217-
'AMP_QUERY_VAR' => amp_get_slug(),
218-
'DEFAULT_AMP_QUERY_VAR' => QueryVar::AMP,
219-
'AMP_QUERY_VAR_CUSTOMIZED_LATE' => $amp_slug_customization_watcher->did_customize_late(),
220-
'CURRENT_THEME' => [
217+
'AMP_QUERY_VAR' => amp_get_slug(),
218+
'CURRENT_THEME' => [
221219
'name' => $theme->get( 'Name' ),
222220
'description' => $theme->get( 'Description' ),
223221
'is_reader_theme' => $is_reader_theme,
224222
'screenshot' => $theme->get_screenshot() ?: null,
225223
'url' => $theme->get( 'ThemeURI' ),
226224
],
227-
'OPTIONS_REST_PATH' => '/amp/v1/options',
228-
'READER_THEMES_REST_PATH' => '/amp/v1/reader-themes',
229-
'IS_CORE_THEME' => in_array(
225+
'OPTIONS_REST_PATH' => '/amp/v1/options',
226+
'READER_THEMES_REST_PATH' => '/amp/v1/reader-themes',
227+
'IS_CORE_THEME' => in_array(
230228
get_stylesheet(),
231229
AMP_Core_Theme_Sanitizer::get_supported_themes(),
232230
true
233231
),
234-
'LEGACY_THEME_SLUG' => ReaderThemes::DEFAULT_READER_THEME,
235-
'USING_FALLBACK_READER_THEME' => $this->reader_themes->using_fallback_theme(),
236-
'THEME_SUPPORT_ARGS' => AMP_Theme_Support::get_theme_support_args(),
237-
'THEME_SUPPORTS_READER_MODE' => AMP_Theme_Support::supports_reader_mode(),
238-
'UPDATES_NONCE' => wp_create_nonce( 'updates' ),
232+
'LEGACY_THEME_SLUG' => ReaderThemes::DEFAULT_READER_THEME,
233+
'USING_FALLBACK_READER_THEME' => $this->reader_themes->using_fallback_theme(),
234+
'THEME_SUPPORT_ARGS' => AMP_Theme_Support::get_theme_support_args(),
235+
'THEME_SUPPORTS_READER_MODE' => AMP_Theme_Support::supports_reader_mode(),
236+
'UPDATES_NONCE' => wp_create_nonce( 'updates' ),
239237
];
240238

241239
wp_add_inline_script(

0 commit comments

Comments
 (0)