Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass entire Reader mode templates through post-processor #3781

Merged
merged 28 commits into from
Nov 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
6717e4f
Pass entire Reader mode templates through post-processor
westonruter Nov 16, 2019
0c34778
Fix tests in WP<5.2
westonruter Nov 16, 2019
8070c56
Transition away from AMP_Post_Template to traditional template tags
westonruter Nov 16, 2019
531d807
Show 'View Comments' link if comments closed but comments are present
westonruter Nov 16, 2019
68c26e5
Fix passing featured image to Reader template
westonruter Nov 16, 2019
22c725b
Prevent dev mode in Reader mode template
westonruter Nov 16, 2019
82cd73c
Fix AMP_Render_Post_Test (again) by setting up postdata
westonruter Nov 16, 2019
6da568c
Revert changes to meta-comments-link to preserve prior behavior
westonruter Nov 16, 2019
b1555eb
Ensure amp_post_template_data filter-supplied data takes precedence
westonruter Nov 16, 2019
aed6c0e
Restore use of post template data and getter
westonruter Nov 17, 2019
29abc85
fixup! Prevent dev mode in Reader mode template
westonruter Nov 17, 2019
a3cb92a
Remove obsolete back-compat prior to WP<4.9
westonruter Nov 17, 2019
8da4f68
Improve phpdoc comments including deprecated note
westonruter Nov 18, 2019
93cede4
Eliminate bogus else condition
westonruter Nov 18, 2019
c9891b5
Use add_filter() instead of add_action(); add static keyword
westonruter Nov 18, 2019
d536c8f
Fix phpdoc for amp_post_template_footer
westonruter Nov 18, 2019
c74ba97
Move to lazy gathering of AMP_Post_Template properties
westonruter Nov 18, 2019
d9a3883
Remove unused private method merge_data_for_key; add phpdoc
westonruter Nov 18, 2019
ca89bf9
Remove unused (and deprecated) functions amp_load_fasterimage_classes…
westonruter Nov 19, 2019
5b0ec96
Move deprecated functions to deprecated.php
westonruter Nov 19, 2019
e8f290b
Add deprecation flags
westonruter Nov 19, 2019
fcb344c
Move additional deprecated files to deprecated.php
westonruter Nov 19, 2019
c31f190
Remove usage of deprecated function
westonruter Nov 19, 2019
9fb3514
Discontinue requiring unused amp-post-template-functions.php in theme…
westonruter Nov 19, 2019
56a7f1c
Remove restrictions on use of important and 50KB style limit when dir…
westonruter Nov 19, 2019
4e64c14
Use AMP__DIR__ instead of __DIR__
westonruter Nov 20, 2019
968f594
Merge branch 'develop' of github.com:ampproject/amp-wp into update/re…
westonruter Nov 20, 2019
d10ab18
Add E2E tests to allow_failures
westonruter Nov 20, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ jobs:
# PHP unit tests (7.4, WordPress trunk)
- env: WP_VERSION=trunk DEV_LIB_ONLY=phpunit INSTALL_PWA_PLUGIN=1
php: 7.4snapshot
# E2E tests, since flaky.
- env: WP_VERSION=latest DEV_LIB_SKIP=phpcs,eslint,xmllint,phpsyntax,phpunit PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=
include:
- stage: lint
name: Lint (PHP, JavaScript, and configuration files)
Expand Down
172 changes: 1 addition & 171 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ function _amp_xdebug_admin_notice() {
require_once AMP__DIR__ . '/back-compat/back-compat.php';
require_once AMP__DIR__ . '/includes/amp-helper-functions.php';
require_once AMP__DIR__ . '/includes/admin/functions.php';
require_once AMP__DIR__ . '/includes/deprecated.php';

register_activation_hook( __FILE__, 'amp_activate' );

Expand Down Expand Up @@ -379,9 +380,6 @@ function amp_init() {
add_action( 'wp_loaded', 'amp_editor_core_blocks' );
add_filter( 'request', 'amp_force_query_var_value' );

// Add actions for reader mode templates.
add_action( 'wp', 'amp_maybe_add_actions' );

// Redirect the old url of amp page to the updated url.
add_filter( 'old_slug_redirect_url', 'amp_redirect_old_slug_to_new_url' );
}
Expand Down Expand Up @@ -437,71 +435,6 @@ function amp_force_query_var_value( $query_vars ) {
return $query_vars;
}

/**
* Conditionally add AMP actions or render the transitional mode template(s).
*
* If the request is for an AMP page and this is in 'canonical mode,' redirect to the non-AMP page.
* It won't need this plugin's template system, nor the frontend actions like the 'rel' link.
*
* @deprecated This function is not used when 'amp' theme support is added.
* @global WP_Query $wp_query
* @since 0.2
* @return void
*/
function amp_maybe_add_actions() {

// Short-circuit when theme supports AMP, as everything is handled by AMP_Theme_Support.
if ( current_theme_supports( AMP_Theme_Support::SLUG ) ) {
return;
}

// The remaining logic here is for transitional mode running in themes that don't support AMP, the template system in AMP<=0.6.
global $wp_query;
if ( ! ( is_singular() || $wp_query->is_posts_page ) || is_feed() ) {
return;
}

if ( is_singular( AMP_Story_Post_Type::POST_TYPE_SLUG ) ) {
return;
}

$is_amp_endpoint = is_amp_endpoint();

/**
* Queried post object.
*
* @var WP_Post $post
*/
$post = get_queried_object();
if ( ! post_supports_amp( $post ) ) {
if ( $is_amp_endpoint ) {
/*
* Temporary redirect is used for admin users because reader mode and AMP support can be enabled by user at any time,
* so they will be able to make AMP available for this URL and see the change without wrestling with the redirect cache.
*/
wp_safe_redirect( get_permalink( $post->ID ), current_user_can( 'manage_options' ) ? 302 : 301 );
exit;
}
return;
}

if ( $is_amp_endpoint ) {

// Prevent infinite URL space under /amp/ endpoint.
global $wp;
$path_args = [];
wp_parse_str( $wp->matched_query, $path_args );
if ( isset( $path_args[ amp_get_slug() ] ) && '' !== $path_args[ amp_get_slug() ] ) {
wp_safe_redirect( amp_get_permalink( $post->ID ), 301 );
exit;
}

amp_prepare_render();
} else {
amp_add_frontend_actions();
}
}

/**
* Fix up WP_Query for front page when amp query var is present.
*
Expand Down Expand Up @@ -601,16 +534,6 @@ function amp_is_canonical() {
return empty( $args['template_dir'] );
}

/**
* Load classes.
*
* @since 0.2
* @deprecated As of 0.6 since autoloading is now employed.
*/
function amp_load_classes() {
_deprecated_function( __FUNCTION__, '0.6' );
}

/**
* Add frontend actions.
*
Expand All @@ -620,98 +543,6 @@ function amp_add_frontend_actions() {
add_action( 'wp_head', 'amp_add_amphtml_link' );
}

/**
* Add post template actions.
*
* @since 0.2
* @deprecated This function is not used when 'amp' theme support is added.
*/
function amp_add_post_template_actions() {
require_once AMP__DIR__ . '/includes/amp-post-template-functions.php';
amp_post_template_init_hooks();
}

/**
* Add action to do post template rendering at template_redirect action.
*
* @since 0.2
* @since 1.0 The amp_render() function is called at template_redirect action priority 11 instead of priority 10.
* @deprecated This function is not used when 'amp' theme support is added.
*/
function amp_prepare_render() {
add_action( 'template_redirect', 'amp_render', 11 );
}

/**
* Render AMP for queried post.
*
* @since 0.1
* @deprecated This function is not used when 'amp' theme support is added.
*/
function amp_render() {
// Note that queried object is used instead of the ID so that the_preview for the queried post can apply.
$post = get_queried_object();
if ( $post instanceof WP_Post ) {
amp_render_post( $post );
exit;
}
}

/**
* Render AMP post template.
*
* @since 0.5
* @deprecated This function is not used when 'amp' theme support is added.
*
* @param WP_Post|int $post Post.
* @global WP_Query $wp_query
*/
function amp_render_post( $post ) {
global $wp_query;

if ( ! ( $post instanceof WP_Post ) ) {
$post = get_post( $post );
if ( ! $post ) {
return;
}
}
$post_id = $post->ID;

/*
* If amp_render_post is called directly outside of the standard endpoint, is_amp_endpoint() will return false,
* which is not ideal for any code that expects to run in an AMP context.
* Let's force the value to be true while we render AMP.
*/
$was_set = isset( $wp_query->query_vars[ amp_get_slug() ] );
if ( ! $was_set ) {
$wp_query->query_vars[ amp_get_slug() ] = true;
}

// Prevent New Relic from causing invalid AMP responses due the NREUM script it injects after the meta charset.
if ( extension_loaded( 'newrelic' ) ) {
newrelic_disable_autorum();
}

/**
* Fires before rendering a post in AMP.
*
* This action is not triggered when 'amp' theme support is present. Instead, you should use 'template_redirect' action and check if `is_amp_endpoint()`.
*
* @since 0.2
*
* @param int $post_id Post ID.
*/
do_action( 'pre_amp_render_post', $post_id );

amp_add_post_template_actions();
$template = new AMP_Post_Template( $post );
$template->load();

if ( ! $was_set ) {
unset( $wp_query->query_vars[ amp_get_slug() ] );
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, it's great to see amp.php simplified.

/**
* Bootstraps the AMP customizer.
*
Expand All @@ -734,7 +565,6 @@ function _amp_bootstrap_customizer() {
* If post slug is updated the amp page with old post slug will be redirected to the updated url.
*
* @since 0.5
* @deprecated This function is irrelevant when 'amp' theme support is added.
*
* @param string $link New URL of the post.
* @return string URL to be redirected.
Expand Down
10 changes: 7 additions & 3 deletions includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -838,8 +838,8 @@ function amp_is_dev_mode() {
(
// For the few sites that forcibly show the admin bar even when the user is logged out, only enable dev
// mode if the user is actually logged in. This prevents the dev mode from being served to crawlers
// when they index the AMP version.
( is_admin_bar_showing() && is_user_logged_in() )
// when they index the AMP version. The theme support check disables dev mode in Reader mode.
( is_admin_bar_showing() && is_user_logged_in() && current_theme_supports( 'amp' ) )
||
is_customize_preview()
)
Expand Down Expand Up @@ -1264,7 +1264,11 @@ function amp_add_admin_bar_view_link( $wp_admin_bar ) {
}

// Show nothing if there are rejected validation errors for this URL.
if ( ! is_amp_endpoint() && count( AMP_Validated_URL_Post_Type::get_invalid_url_validation_errors( amp_get_current_url(), [ 'ignore_accepted' => true ] ) ) > 0 ) {
if (
! is_amp_endpoint() &&
AMP_Theme_Support::READER_MODE_SLUG !== AMP_Theme_Support::get_support_mode() &&
count( AMP_Validated_URL_Post_Type::get_invalid_url_validation_errors( amp_get_current_url(), [ 'ignore_accepted' => true ] ) ) > 0
) {
return;
}

Expand Down
42 changes: 0 additions & 42 deletions includes/amp-post-template-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
function amp_post_template_init_hooks() {
add_action( 'amp_post_template_head', 'amp_post_template_add_title' );
add_action( 'amp_post_template_head', 'amp_post_template_add_canonical' );
add_action( 'amp_post_template_head', 'amp_post_template_add_scripts' );
add_action( 'amp_post_template_head', 'amp_post_template_add_fonts' );
add_action( 'amp_post_template_head', 'amp_post_template_add_boilerplate_css' );
add_action( 'amp_post_template_head', 'amp_print_schemaorg_metadata' );
add_action( 'amp_post_template_head', 'amp_add_generator_metadata' );
add_action( 'amp_post_template_head', 'wp_generator' );
Expand Down Expand Up @@ -44,26 +42,6 @@ function amp_post_template_add_canonical( $amp_template ) {
<?php
}

/**
* Print scripts.
*
* @see amp_register_default_scripts()
* @see amp_filter_script_loader_tag()
* @param AMP_Post_Template $amp_template Template.
*/
function amp_post_template_add_scripts( $amp_template ) {
// phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
echo amp_render_scripts(
array_merge(
[
// Just in case the runtime has been overridden by amp_post_template_data filter.
'amp-runtime' => $amp_template->get( 'amp_runtime_script' ),
],
$amp_template->get( 'amp_component_scripts', [] )
)
);
}

/**
* Print fonts.
*
Expand All @@ -76,26 +54,6 @@ function amp_post_template_add_fonts( $amp_template ) {
}
}

/**
* Print boilerplate CSS.
*
* @since 0.3
* @see amp_get_boilerplate_code()
*/
function amp_post_template_add_boilerplate_css() {
echo amp_get_boilerplate_code(); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
}

/**
* Print Schema.org metadata.
*
* @deprecated Since 0.7
*/
function amp_post_template_add_schemaorg_metadata() {
_deprecated_function( __FUNCTION__, '0.7', 'amp_print_schemaorg_metadata' );
amp_print_schemaorg_metadata();
}

/**
* Print styles.
*
Expand Down
Loading