Skip to content

Commit

Permalink
Add initial actions for handling canonical AMP themes
Browse files Browse the repository at this point in the history
  • Loading branch information
westonruter committed Jan 12, 2018
1 parent b435107 commit 6ef3f86
Show file tree
Hide file tree
Showing 4 changed files with 228 additions and 1 deletion.
15 changes: 14 additions & 1 deletion amp.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,20 @@ function amp_force_query_var_value( $query_vars ) {
* @return void
*/
function amp_maybe_add_actions() {
if ( amp_is_canonical() || ! is_singular() || is_feed() ) {

// Add hooks for when a themes that support AMP.
if ( current_theme_supports( 'amp' ) ) {
if ( amp_is_canonical() ) {
AMP_Canonical_Mode_Actions::register_hooks();
} elseif ( is_amp_endpoint() ) {
AMP_Paired_Mode_Actions::register_hooks();
} else {
AMP_Frontend_Actions::register_hooks();
}
return;
}

if ( ! is_singular() || is_feed() ) {
return;
}

Expand Down
191 changes: 191 additions & 0 deletions includes/actions/class-amp-canonical-mode-actions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
<?php
/**
* Class AMP_Canonical_Mode_Actions
*
* @package AMP
*/

/**
* Class AMP_Canonical_Mode_Actions
*
* Callbacks for adding AMP-related things to the theme when in canonical mode.
*/
class AMP_Canonical_Mode_Actions {

/**
* Register hooks.
*/
public static function register_hooks() {

// Remove core actions which are invalid AMP.
remove_action( 'wp_head', 'locale_stylesheet' );
remove_action( 'wp_head', 'print_emoji_detection_script', 7 );
remove_action( 'wp_head', 'wp_print_styles', 8 );
remove_action( 'wp_head', 'wp_print_head_scripts', 9 );
remove_action( 'wp_head', 'wp_custom_css_cb', 101 );
remove_action( 'wp_footer', 'wp_print_footer_scripts', 20 );
remove_action( 'wp_print_styles', 'print_emoji_styles' );

// Replace core's canonical link functionality with one that outputs links for non-singular queries as well. See WP Core #18660.
remove_action( 'wp_head', 'rel_canonical' );
add_action( 'wp_head', array( __CLASS__, 'rel_canonical' ), 1 );

// Add additional markup required by AMP <https://www.ampproject.org/docs/reference/spec#required-markup>.
add_action( 'wp_head', array( __CLASS__, 'print_meta_charset' ), 0 );
add_action( 'wp_head', array( __CLASS__, 'print_meta_viewport' ), 2 );
add_action( 'wp_head', array( __CLASS__, 'print_amp_boilerplate_code' ), 3 );
add_action( 'wp_head', array( __CLASS__, 'print_amp_scripts' ), 4 );
add_action( 'wp_head', array( __CLASS__, 'print_amp_custom_style' ), 5 );

add_action( 'admin_bar_init', array( __CLASS__, 'admin_bar_init' ) );
add_theme_support( 'admin-bar', array( 'callback' => '__return_false' ) );

// @todo Add output buffering.
// @todo Add character conversion.
}

/**
* Print meta charset tag.
*
* @link https://www.ampproject.org/docs/reference/spec#chrs
*/
public static function print_meta_charset() {
echo '<meta charset="utf-8">';
}

/**
* Print meta charset tag.
*
* @link https://www.ampproject.org/docs/reference/spec#vprt
*/
public static function print_meta_viewport() {
echo '<meta name="viewport" content="width=device-width,minimum-scale=1">';
}

/**
* Print AMP boilerplate code.
*
* @link https://www.ampproject.org/docs/reference/spec#boilerplate
*/
public static function print_amp_boilerplate_code() {
echo '<style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style>';
echo '<noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>';
}

/**
* Print AMP script and placeholder for others.
*
* @link https://www.ampproject.org/docs/reference/spec#scrpt
*/
public static function print_amp_scripts() {
echo '<script async src="https://cdn.ampproject.org/v0.js"></script>'; // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript
echo '<!--AMP_PLUGIN_SCRIPTS_PLACEHOLDER-->'; // Replaced after output buffering with all AMP component scripts.
}

/**
* Add canonical link.
*
* Replaces `rel_canonical()` which only outputs canonical URLs for singular posts and pages.
* This can be removed once WP Core #18660 lands.
*
* @link https://www.ampproject.org/docs/reference/spec#canon.
* @link https://core.trac.wordpress.org/ticket/18660
*
* @see rel_canonical()
* @global WP $wp
* @global WP_Rewrite $wp_rewrite
*/
public static function rel_canonical() {
global $wp, $wp_rewrite;

$url = null;
if ( is_singular() ) {
$url = wp_get_canonical_url();
}

// For non-singular queries, make use of the request URI and public query vars to determine canonical URL.
if ( empty( $url ) ) {
$added_query_vars = $wp->query_vars;
if ( ! $wp_rewrite->permalink_structure || empty( $wp->request ) ) {
$url = home_url( '/' );
} else {
$url = home_url( user_trailingslashit( $wp->request ) );
parse_str( $wp->matched_query, $matched_query_vars );
foreach ( $wp->query_vars as $key => $value ) {

// Remove query vars that were matched in the rewrite rules for the request.
if ( isset( $matched_query_vars[ $key ] ) ) {
unset( $added_query_vars[ $key ] );
}
}
}
}

if ( ! empty( $added_query_vars ) ) {
$url = add_query_arg( $added_query_vars, $url );
}

echo '<link rel="canonical" href="' . esc_url( $url ) . '">' . "\n";
}

/**
* Print Custom AMP styles.
*
* @see wp_custom_css_cb()
*/
public static function print_amp_custom_style() {
echo '<style amp-custom>';

This comment has been minimized.

Copy link
@kopepasah

kopepasah Jan 19, 2018

Contributor

@westonruter styles inserted should be compressed to remove white space, as it contributes to the 50kb limit.

This comment has been minimized.

Copy link
@westonruter

westonruter Jan 19, 2018

Author Member

What whitespace is present?

This comment has been minimized.

Copy link
@westonruter

westonruter Jan 19, 2018

Author Member

Or just generally it should be compressed? See #688. In general I think that it should be up to the plugin to have a build step to minify CSS. That being said, if we do implement the logic to parse CSS rules out to strip what isn't actually used, then we can potentially do a degree of minification as well.


// @todo Grab source of all enqueued styles and concatenate here?
// @todo Print contents of get_locale_stylesheet_uri()?
// @todo Allow this to be filtered after output buffering is complete so additional styles can be added by widgets and other components just-in-time?
$path = get_template_directory() . '/style.css'; // @todo Honor filter in get_stylesheet_directory_uri()? Style must be local.

This comment has been minimized.

Copy link
@kopepasah

kopepasah Jan 19, 2018

Contributor

@westonruter we definitely need to honor get_stylesheet_uri(), as this makes working with a child theme impossible.

This comment has been minimized.

Copy link
@kopepasah

kopepasah Jan 19, 2018

Contributor

Once consideration is users often use @import in the child theme stylesheet to get the parent theme stylesheet, which is not compatible with AMP. We probably will have to parse the stylesheets, remove imports and get the file's contents another way.

This comment has been minimized.

Copy link
@westonruter

westonruter Jan 19, 2018

Author Member

we definitely need to honor get_stylesheet_uri(), as this makes working with a child theme impossible.

The main issue I see with this is that it is possible (though unlikely I suppose) that get_stylesheet_uri() returns a URL that points outside the WP file system. An external request would not be feasible as it would slow down the page load. But we could just make it a requirement that get_stylesheet_uri() start with get_stylesheet_directory_uri() or get_template_directory_uri().

Once consideration is users often use @import in the child theme stylesheet

Actually, using @import is now discouraged when building child themes. So we can safely require that if a theme says it supports amp that it not use @import. See Codex creating Child Themes:

The final step is to enqueue the parent and child theme stylesheets. Note that the previous method was to import the parent theme stylesheet using @import: this is no longer best practice, as it increases the amount of time it takes style sheets to load. The correct method of enqueuing the parent theme stylesheet is to add a wp_enqueue_scripts action and use wp_enqueue_style() in your child theme's functions.php.

This comment has been minimized.

Copy link
@kopepasah

kopepasah Jan 19, 2018

Contributor

Regarding @import: Awesome!.

I agree that making sure it is on the file system should be a requirement, though I still see instances where using get_template/stylesheet_directory() causes an issue for developers. At the very least, there should be a filter in AMP to change the location (if we are hard coding style.css).

This comment has been minimized.

Copy link
@westonruter

westonruter Jan 20, 2018

Author Member

I'm pretty happy with what I think is a good solution here: #887

$css = file_get_contents( $path ); // phpcs:ignore WordPress.WP.AlternativeFunctions -- It's not a remote file.
echo wp_strip_all_tags( $css ); // WPCS: XSS OK.

// Implement AMP version of wp_custom_css_cb().
$custom_css = trim( wp_get_custom_css() );
if ( ! empty( $custom_css ) ) {
echo '/* start:wp_get_custom_css */';

This comment has been minimized.

Copy link
@kopepasah

kopepasah Jan 19, 2018

Contributor

@westonruter perhaps we should refrain from inserting comments, as they contribute to the 50kb limit.

This comment has been minimized.

Copy link
@westonruter

westonruter Jan 19, 2018

Author Member

Yes. My thought here is that the Customizer could use the comment as a way to inject Custom CSS when modified. At least we can prevent showing this if not is_customize_preview().

echo wp_strip_all_tags( wp_get_custom_css() ); // WPCS: XSS OK.
echo '/* end:wp_get_custom_css */';
}
echo '</style>';
}

/**
* Fix up admin bar.
*/
public static function admin_bar_init() {
remove_action( 'wp_head', 'wp_admin_bar_header' );
add_action( 'admin_bar_menu', array( __CLASS__, 'remove_customize_support_script' ), 100 ); // See WP_Admin_Bar::add_menus().
add_filter( 'body_class', array( __CLASS__, array( __CLASS__, 'filter_body_class_to_force_customize_support' ) ) );
}

/**
* Let the body class include customize-support by default since support script won't be able to dynamically add it.
*
* @see wp_customize_support_script()
*
* @param array $classes Body classes.
* @return array Classes.
*/
public static function filter_body_class_to_force_customize_support( $classes ) {
$i = array_search( 'no-customize-support', $classes, true );
if ( false !== $i ) {
array_splice( $classes, $i, 1 );
}
$classes[] = 'customize-support';
return $classes;
}

/**
* Remove Customizer support script.
*
* @see WP_Admin_Bar::add_menus()
* @see wp_customize_support_script()
*/
public static function remove_customize_support_script() {
remove_action( 'wp_before_admin_bar_render', 'wp_customize_support_script' );
}
}
21 changes: 21 additions & 0 deletions includes/actions/class-amp-paired-mode-actions.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php
/**
* Class AMP_Paired_Mode_Actions
*
* @package AMP
*/

/**
* Class AMP_Paired_Mode_Actions
*
* Callbacks for adding AMP-related things to the theme when in paired mode, for themes that support amp.
*/
class AMP_Paired_Mode_Actions {

/**
* Register hooks.
*/
public static function register_hooks() {
// @todo See https://github.com/Automattic/amp-wp/issues/849.
}
}
2 changes: 2 additions & 0 deletions includes/class-amp-autoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class AMP_Autoloader {
'AMP_Actions' => 'includes/actions/class-amp-actions',
'AMP_Frontend_Actions' => 'includes/actions/class-amp-frontend-actions',
'AMP_Paired_Post_Actions' => 'includes/actions/class-amp-paired-post-actions',
'AMP_Paired_Mode_Actions' => 'includes/actions/class-amp-paired-mode-actions',
'AMP_Canonical_Mode_Actions' => 'includes/actions/class-amp-canonical-mode-actions',
'AMP_Template_Customizer' => 'includes/admin/class-amp-customizer',
'AMP_Post_Meta_Box' => 'includes/admin/class-amp-post-meta-box',
'AMP_Post_Type_Support' => 'includes/class-amp-post-type-support',
Expand Down

0 comments on commit 6ef3f86

Please sign in to comment.