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

Add support for allowing a site subset to be native AMP #1235

Merged
merged 34 commits into from
Jul 2, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
143cd2a
WIP
westonruter Jun 22, 2018
6149da1
WIP2
westonruter Jun 24, 2018
fab22d4
WIP3
westonruter Jun 26, 2018
718e643
WIP4
westonruter Jun 27, 2018
1d2053c
WIP5
westonruter Jun 27, 2018
2eda94b
WIP6
westonruter Jun 28, 2018
3377ecf
WIP7
westonruter Jun 28, 2018
e5f4b77
Remove needless get_query_template
westonruter Jun 28, 2018
a7bf31e
WIP8
westonruter Jun 28, 2018
f440592
WIP9 [ci skip]
westonruter Jun 29, 2018
8f46ee7
WIP10 [ci skip]
westonruter Jun 29, 2018
0131c52
WIP11 [ci skip]
westonruter Jun 29, 2018
5288ccb
WIP12 [ci skip]
westonruter Jun 29, 2018
3f4ef4c
WIP13 [ci skip]
westonruter Jun 29, 2018
a616e84
Don't show post enable/disable if template is not available
westonruter Jun 30, 2018
a7371d1
Simplify logic in AMP_Post_Meta_Box::render_status
westonruter Jun 30, 2018
1adad3e
Fix tests regarding amp status metabox
westonruter Jun 30, 2018
fafb94e
Mock the gfycat requests during unit tests
westonruter Jun 30, 2018
95fd958
Let template_availability return supported when no templates match bu…
westonruter Jul 1, 2018
7e56c8b
Add unrecognized_templates_supported for Other templates
westonruter Jul 1, 2018
bfb202a
Let theme support args define templates_supported
westonruter Jul 1, 2018
2062816
Persist user selection even when checkbox disabled
westonruter Jul 1, 2018
1c759e5
Add tests for read_theme_support, is_support_added_via_option, get_th…
westonruter Jul 1, 2018
221c3f0
Improve testing of finish_init; add test stubs
westonruter Jul 2, 2018
3d07eac
Discard matching is_home when there are other matching templates
westonruter Jul 2, 2018
c32d76b
Elimiante Other/unrecognized template option since indistinguishable …
westonruter Jul 2, 2018
1bc4251
Restore the availability of AMP components in non-AMP documents in na…
westonruter Jul 2, 2018
11ea3d1
Remove unused variable and fix phpcs issue
westonruter Jul 2, 2018
4bde27d
Add tests for AMP_Theme_Support::get_supportable_templates()
westonruter Jul 2, 2018
c172763
Add test for AMP_Theme_Support::get_template_availability()
westonruter Jul 2, 2018
38b5546
Add test for custom template
westonruter Jul 2, 2018
acf9849
Restore available_callback but mark as deprecated
westonruter Jul 2, 2018
7091b35
Fix generation of AMP permalink for attachments
westonruter Jul 2, 2018
b54fcc0
Fix initialization of paried mode and Customizer, and fix support opt…
westonruter Jul 2, 2018
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
68 changes: 54 additions & 14 deletions amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -272,28 +272,67 @@ function amp_correct_query_when_is_front_page( WP_Query $query ) {
}

/**
* Whether this is in 'canonical mode.'
* Whether this is in 'canonical mode'.
*
* Themes can register support for this with `add_theme_support( 'amp' )`.
* Then, this will change the plugin from 'paired mode,' and it won't use its own templates.
* Nor output frontend markup like the 'rel' link. If the theme registers support for AMP with:
* `add_theme_support( 'amp', array( 'template_dir' => 'my-amp-templates' ) )`
* it will retain 'paired mode.
* Themes can register support for this with `add_theme_support( 'amp' )`:
*
* @return boolean Whether this is in AMP 'canonical mode'.
* add_theme_support( 'amp' );
*
* This will serve templates in native AMP by default but the user would be able to change the template mode
* from native to paired in the admin. To force only native to be available, such as when you are using AMP components
* in your theme templates, do:
*
* add_theme_support( 'amp', array(
* 'mode' => 'native',
* ) );
*
* If you want to force AMP to always be served on a given template, you can use the templates_supported arg,
* for example to always serve the Category template in AMP:
*
* add_theme_support( 'amp', array(
* 'templates_supported' => array(
* 'is_category' => true,
* ),
* ) );
*
* Or if you want to force AMP to be used on all templates:
*
* add_theme_support( 'amp', array(
* 'templates_supported' => 'all',
* ) );
*
* @see AMP_Theme_Support::read_theme_support()
* @return boolean Whether this is in AMP 'canonical' mode, that is whether it is native and there is not separate AMP URL current URL.
*/
function amp_is_canonical() {
$support = get_theme_support( 'amp' );
if ( true === $support ) {
return true;
if ( ! current_theme_supports( 'amp' ) ) {
return false;
}

$mode = 'native';
$support = get_theme_support( 'amp' );
Copy link
Member

Choose a reason for hiding this comment

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

May there be a more differentiating naming for current_theme_supports vs. get_theme_support? Probably I am missing some of the context but the naming appear a little confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

The current_theme_supports() and get_theme_support() are WordPress core functions. They are somewhat confusing. The former returns whether support has been added (boolean), and applies a filter to let plugins control whether support is present. The get_theme_support() on the other hand does not apply a filter, but rather it returns the args that were passed in for the original add_theme_support() call.

Copy link
Member

Choose a reason for hiding this comment

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

Got it. Makes sense now.

if ( is_array( $support ) ) {
$args = array_shift( $support );
if ( empty( $args['template_dir'] ) ) {
return true;
$args = array_shift( $support );
$support = AMP_Options_Manager::get_option( 'theme_support' );

// If support is optional, look at DB option if mode is not explicitly set in theme support.
if ( ! empty( $args['optional'] ) ) {
if ( 'disabled' === $support ) {
return false;
} elseif ( ! isset( $args['mode'] ) ) {
return 'native' === $support;
}
}

if ( isset( $args['mode'] ) ) {
$mode = $args['mode'];
} elseif ( 'disabled' !== $support ) {
$mode = $support; // Supplied via admin screen.
} elseif ( ! empty( $args['template_dir'] ) ) {
$mode = 'paired'; // If there is a template_dir, then paired mode is implied.
}
}
return false;
return 'native' === $mode;
}

/**
Expand Down Expand Up @@ -426,6 +465,7 @@ function _amp_bootstrap_customizer() {

/**
* Redirects the old AMP URL to the new AMP URL.
*
* If post slug is updated the amp page with old post slug will be redirected to the updated url.
*
* @since 0.5
Expand Down
44 changes: 44 additions & 0 deletions includes/admin/class-amp-editor-blocks.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,22 @@ public function init() {
if ( function_exists( 'gutenberg_init' ) ) {
add_action( 'enqueue_block_editor_assets', array( $this, 'enqueue_block_editor_assets' ) );
add_filter( 'wp_kses_allowed_html', array( $this, 'whitelist_block_atts_in_wp_kses_allowed_html' ), 10, 2 );

/*
* Dirty AMP is required when a site is in native mode but not all templates are being served
Copy link
Member

@amedina amedina Jul 2, 2018

Choose a reason for hiding this comment

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

This comment is hard to follow; perhaps not mentioning "Dirty AMP" first, but explaining directly the semantics of the filter and action functions may be clearer. Then the concept of Dirty AMP can be expressed. Just a thought.

Copy link
Member

Choose a reason for hiding this comment

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

No need to look/address this now.

* as AMP. In particular, if a single post is using AMP-specific Gutenberg Blocks which make
* use of AMP components, and the singular template is served as AMP but the blog page is not,
* then the non-AMP blog page need to load the AMP runtime scripts so that the AMP components
* in the posts displayed there will be rendered properly. This is only relevant on native AMP
* sites because the AMP Gutenberg blocks are only made available in that mode; they are not
* presented in the Gutenberg inserter in paired mode. In general, using AMP components in
* non-AMP documents is still not officially supported, so it's occurrence is being minimized
* as much as possible. For more, see <https://github.com/Automattic/amp-wp/issues/1192>.
*/
if ( amp_is_canonical() ) {
add_filter( 'the_content', array( $this, 'tally_content_requiring_amp_scripts' ) );
add_action( 'wp_print_footer_scripts', array( $this, 'print_dirty_amp_scripts' ) );
}
}
}

Expand Down Expand Up @@ -145,4 +161,32 @@ public function enqueue_block_editor_assets() {
) ) )
);
}

/**
* Tally the AMP component scripts that are needed in a dirty AMP document.
*
* @param string $content Content.
* @return string Content (unmodified).
*/
public function tally_content_requiring_amp_scripts( $content ) {
if ( ! is_amp_endpoint() ) {
$pattern = sprintf( '/<(%s)\b.*?>/s', join( '|', $this->amp_blocks ) );
if ( preg_match_all( $pattern, $content, $matches ) ) {
$this->content_required_amp_scripts = array_merge(
$this->content_required_amp_scripts,
$matches[1]
);
}
}
return $content;
}

/**
* Print AMP scripts required for AMP components used in a non-AMP document (dirty AMP).
*/
public function print_dirty_amp_scripts() {
if ( ! is_amp_endpoint() && ! empty( $this->content_required_amp_scripts ) ) {
wp_scripts()->do_items( $this->content_required_amp_scripts );
}
}
}
36 changes: 29 additions & 7 deletions includes/admin/class-amp-post-meta-box.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,20 @@ public function enqueue_admin_assets() {
array( 'jquery' ),
AMP__VERSION
);

if ( current_theme_supports( 'amp' ) ) {
$availability = AMP_Theme_Support::get_template_availability( $post );
$support_errors = $availability['errors'];
} else {
$support_errors = AMP_Post_Type_Support::get_support_errors( $post );
}

wp_add_inline_script( self::ASSETS_HANDLE, sprintf( 'ampPostMetaBox.boot( %s );',
wp_json_encode( array(
'previewLink' => esc_url_raw( add_query_arg( amp_get_slug(), '', get_preview_post_link( $post ) ) ),
'canonical' => amp_is_canonical(),
'enabled' => post_supports_amp( $post ),
'canSupport' => count( AMP_Post_Type_Support::get_support_errors( $post ) ) === 0,
'enabled' => empty( $support_errors ),
Copy link
Member

Choose a reason for hiding this comment

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

Very nice semantics.

'canSupport' => 0 === count( array_diff( $support_errors, array( 'post-status-disabled' ) ) ),
'statusInputName' => self::STATUS_INPUT_NAME,
'l10n' => array(
'ampPreviewBtnLabel' => __( 'Preview changes in AMP (opens in new window)', 'amp' ),
Expand All @@ -170,23 +178,37 @@ public function render_status( $post ) {
is_post_type_viewable( $post->post_type )
&&
current_user_can( 'edit_post', $post->ID )
&&
! amp_is_canonical()
);

if ( true !== $verify ) {
return;
}

$errors = AMP_Post_Type_Support::get_support_errors( $post );
$status = post_supports_amp( $post ) ? self::ENABLED_STATUS : self::DISABLED_STATUS;
/*
* When theme support is present then theme templates can be served in AMP and we check first if the template is available.
* Checking for template availability will include a check for get_support_errors. Otherwise, if theme support is not present
* then we just check get_support_errors.
*/
if ( current_theme_supports( 'amp' ) ) {
$availability = AMP_Theme_Support::get_template_availability( $post );
$status = $availability['supported'] ? self::ENABLED_STATUS : self::DISABLED_STATUS;
$errors = array_diff( $availability['errors'], array( 'post-status-disabled' ) ); // Subtract the status which the metabox will allow to be toggled.
if ( true === $availability['immutable'] ) {
$errors[] = 'status_immutable';
}
} else {
$errors = AMP_Post_Type_Support::get_support_errors( $post );
$status = empty( $errors ) ? self::ENABLED_STATUS : self::DISABLED_STATUS;
$errors = array_diff( $errors, array( 'post-status-disabled' ) ); // Subtract the status which the metabox will allow to be toggled.
}

$labels = array(
'enabled' => __( 'Enabled', 'amp' ),
'disabled' => __( 'Disabled', 'amp' ),
);

// The preceding variables are used inside the following amp-status.php template.
include_once AMP__DIR__ . '/templates/admin/amp-status.php';
include AMP__DIR__ . '/templates/admin/amp-status.php';
}

/**
Expand Down
84 changes: 29 additions & 55 deletions includes/amp-helper-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ function amp_get_permalink( $post_id ) {
||
// If the post type is hierarchical then the /amp/ endpoint isn't available.
is_post_type_hierarchical( get_post_type( $post_id ) )
||
// Attachment pages don't accept the /amp/ endpoint.
'attachment' === get_post_type( $post_id )
);
if ( $use_query_var ) {
$amp_url = add_query_arg( amp_get_slug(), '', $permalink );
Expand Down Expand Up @@ -239,61 +242,20 @@ function amp_add_amphtml_link() {
* @see AMP_Post_Type_Support::get_support_errors()
*
* @param WP_Post $post Post.
*
* @return bool Whether the post supports AMP.
*/
function post_supports_amp( $post ) {
if ( amp_is_canonical() ) {
return true;
}

$errors = AMP_Post_Type_Support::get_support_errors( $post );

// Return false if an error is found.
if ( ! empty( $errors ) ) {
return false;
}

switch ( get_post_meta( $post->ID, AMP_Post_Meta_Box::STATUS_POST_META_KEY, true ) ) {
case AMP_Post_Meta_Box::ENABLED_STATUS:
return true;

case AMP_Post_Meta_Box::DISABLED_STATUS:
return false;

// Disabled by default for custom page templates, page on front and page for posts.
default:
$enabled = (
! (bool) get_page_template_slug( $post )
&&
! (
'page' === $post->post_type
&&
'page' === get_option( 'show_on_front' )
&&
in_array( (int) $post->ID, array(
(int) get_option( 'page_on_front' ),
(int) get_option( 'page_for_posts' ),
), true )
)
);

/**
* Filters whether default AMP status should be enabled or not.
*
* @since 0.6
*
* @param string $status Status.
* @param WP_Post $post Post.
*/
return apply_filters( 'amp_post_status_default_enabled', $enabled, $post );
}
return 0 === count( AMP_Post_Type_Support::get_support_errors( $post ) );
}

/**
* Are we currently on an AMP URL?
* Determine whether the current response being served as AMP.
*
* @since 1.0 This function can be called before the `parse_query` action because the 'amp' query var is specifically and exclusively used when 'amp' theme support is added.
* This function cannot be called before the parse_query action because it needs to be able
* to determine the queried object is able to be served as AMP. If 'amp' theme support is not
* present, this function returns true just if the query var is present. If theme support is
* present, then it returns true in paired mode if an AMP template is available and the query
* var is present, or else in native mode if just the template is available.
*
* @return bool Whether it is the AMP endpoint.
*/
Expand All @@ -302,17 +264,29 @@ function is_amp_endpoint() {
return false;
}

// When 'amp' theme support is (or will be added) then these are the conditions that are key to be checked.
if ( amp_is_canonical() || isset( $_GET[ amp_get_slug() ] ) ) { // WPCS: CSRF OK.
return true;
$did_parse_query = did_action( 'parse_query' );

if ( ! $did_parse_query ) {
_doing_it_wrong( __FUNCTION__, sprintf( esc_html__( "is_amp_endpoint() was called before the 'parse_query' hook was called. This function will always return 'false' before the 'parse_query' hook is called.", 'amp' ) ), '0.4.2' );
}

// Condition for non-theme support when /amp/ endpoint is used.
if ( false !== get_query_var( amp_get_slug(), false ) ) {
return true;
$has_amp_query_var = (
isset( $_GET[ amp_get_slug() ] ) // WPCS: CSRF OK.
||
false !== get_query_var( amp_get_slug(), false )
);

if ( ! current_theme_supports( 'amp' ) ) {
return $has_amp_query_var;
}

// When there is no query var and AMP is not canonical/native, then this is definitely not an AMP endpoint.
if ( ! $has_amp_query_var && ! amp_is_canonical() ) {
return false;
}

return false;
$availability = AMP_Theme_Support::get_template_availability();
return amp_is_canonical() ? $availability['supported'] : ( $has_amp_query_var && $availability['supported'] );
}

/**
Expand Down
Loading