Add options for enabling amp theme support and configuring validation…

… handling * Add admin settings for picking between disabled, paired, and native theme support. See #1196. * Add checkbox for automatically allowing tree shaking. * Add checkbox for automatically sanitizing all validation errors (including tree shaking). * Make explicitly clear that unaccepted validation errors will block rendering on AMP * Prevent serving dirty AMP by forcibly sanitizing all validation errors when amp_is_canonical(). See #1192. * When validation errors are automatically sanitized, ensure the terms' term_group is updated when re-checking. * Update AMP_Options_Manager::get_options() and to return default values.
ampproject · Jun 6, 2018 · 6e7712d · 6e7712d
1 parent 46342cd
commit 6e7712d
Show file tree

Hide file tree

Showing 12 changed files with 390 additions and 78 deletions.
diff --git a/assets/js/amp-block-validation.js b/assets/js/amp-block-validation.js
@@ -19,7 +19,8 @@ var ampBlockValidation = ( function() { // eslint-disable-line no-unused-vars
 		 */
 		data: {
 			i18n: {},
-			ampValidityRestField: ''
+			ampValidityRestField: '',
+			isCanonical: false
 		},
 
 		/**
@@ -179,7 +180,13 @@ var ampBlockValidation = ( function() { // eslint-disable-line no-unused-vars
 				);
 			}
 
-			noticeMessage += ' ' + wp.i18n.__( 'Non-accepted validation errors prevent AMP from being served.', 'amp' );
+			noticeMessage += ' ';
+			if ( module.data.isCanonical ) {
+				noticeMessage += wp.i18n.__( 'The invalid markup will be automatically sanitized to ensure a valid AMP response is served.', 'amp' );
+			} else {
+				noticeMessage += wp.i18n.__( 'Non-accepted validation errors prevent AMP from being served, and the user will be redirected to the non-AMP version.', 'amp' );
+			}
+
 			noticeElement = wp.element.createElement( 'p', {}, [
 				noticeMessage + ' ',
 				ampValidity.review_link && wp.element.createElement(

diff --git a/includes/admin/class-amp-editor-blocks.php b/includes/admin/class-amp-editor-blocks.php
@@ -43,8 +43,16 @@ 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 );
-			add_filter( 'the_content', array( $this, 'tally_content_requiring_amp_scripts' ) );
-			add_action( 'wp_print_footer_scripts', array( $this, 'print_dirty_amp_scripts' ) );
+
+			/*
+			 * Dirty AMP is currently disabled per <https://github.com/Automattic/amp-wp/issues/1192>.
+			 * Note that when not in native/canonical mode, AMP-specific Gutenberg blocks will not be
+			 * registered for use, and so users will not be likely attempting to serve AMP content in
+			 * non-AMP responses. When/if dirty AMP is allowed, the following can be re-enabled:
+			 *
+			 * add_filter( 'the_content', array( $this, 'tally_content_requiring_amp_scripts' ) );
+			 * add_action( 'wp_print_footer_scripts', array( $this, 'print_dirty_amp_scripts' ) );
+			*/
 		}
 	}
 

diff --git a/includes/amp-frontend-actions.php b/includes/amp-frontend-actions.php
@@ -10,6 +10,7 @@
 /**
  * Add amphtml link to frontend.
  *
+ * @todo If there is a known amp_invalid_url post for this $amp_url that has unaccepted validation errors then this should output nothing.
  * @todo This function's name is incorrect. It's not about adding a canonical link but adding the amphtml link.
  *
  * @since 0.2

diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -96,6 +96,7 @@ class AMP_Theme_Support {
 	 * @since 0.7
 	 */
 	public static function init() {
+		self::apply_options();
 		if ( ! current_theme_supports( 'amp' ) ) {
 			return;
 		}
@@ -117,7 +118,7 @@ public static function init() {
 			$args = array_shift( $support );
 			if ( ! is_array( $args ) ) {
 				trigger_error( esc_html__( 'Expected AMP theme support arg to be array.', 'amp' ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
-			} elseif ( count( array_diff( array_keys( $args ), array( 'template_dir', 'available_callback', 'comments_live_list' ) ) ) !== 0 ) {
+			} elseif ( count( array_diff( array_keys( $args ), array( 'template_dir', 'available_callback', 'comments_live_list', '__added_via_option' ) ) ) !== 0 ) {
 				trigger_error( esc_html__( 'Expected AMP theme support to only have template_dir and/or available_callback.', 'amp' ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
 			}
 		}
@@ -132,6 +133,28 @@ public static function init() {
 		add_action( 'wp', array( __CLASS__, 'finish_init' ), PHP_INT_MAX );
 	}
 
+	/**
+	 * Apply options for whether theme support is enabled via admin and what sanitization is performed by default.
+	 *
+	 * @see AMP_Post_Type_Support::add_post_type_support() For where post type support is added, since it is irrespective of theme support.
+	 */
+	public static function apply_options() {
+		if ( ! current_theme_supports( 'amp' ) ) {
+			$theme_support_option = AMP_Options_Manager::get_option( 'theme_support' );
+			if ( 'disabled' === $theme_support_option ) {
+				return;
+			}
+
+			$args = array(
+				'__added_via_option' => true,
+			);
+			if ( 'paired' === $theme_support_option ) {
+				$args['template_dir'] = './';
+			}
+			add_theme_support( 'amp', $args );
+		}
+	}
+
 	/**
 	 * Finish initialization once query vars are set.
 	 *
@@ -1149,6 +1172,8 @@ public static function prepare_response( $response, $args = array() ) {
 		self::ensure_required_markup( $dom );
 
 		if ( ! AMP_Validation_Manager::should_validate_response() && AMP_Validation_Manager::has_blocking_validation_errors() ) {
+
+			// Note the canonical check will not currently ever be met because dirty AMP is not yet supported; all validation errors will forcibly be sanitized.
 			if ( amp_is_canonical() ) {
 				$dom->documentElement->removeAttribute( 'amp' );
 

diff --git a/includes/options/class-amp-options-manager.php b/includes/options/class-amp-options-manager.php
@@ -17,6 +17,19 @@ class AMP_Options_Manager {
 	 */
 	const OPTION_NAME = 'amp-options';
 
+	/**
+	 * Default option values.
+	 *
+	 * @var array
+	 */
+	protected static $defaults = array(
+		'theme_support'        => 'disabled',
+		'supported_post_types' => array(),
+		'analytics'            => array(),
+		'force_sanitization'   => false,
+		'accept_tree_shaking'  => false,
+	);
+
 	/**
 	 * Register settings.
 	 */
@@ -58,7 +71,11 @@ public static function maybe_flush_rewrite_rules( $old_options, $new_options ) {
 	 * @return array Options.
 	 */
 	public static function get_options() {
-		return get_option( self::OPTION_NAME, array() );
+		$options = get_option( self::OPTION_NAME, array() );
+		if ( empty( $options ) ) {
+			$options = array();
+		}
+		return array_merge( self::$defaults, $options );
 	}
 
 	/**
@@ -86,15 +103,20 @@ public static function get_option( $option, $default = false ) {
 	 * @return array Options.
 	 */
 	public static function validate_options( $new_options ) {
-		$defaults = array(
-			'supported_post_types' => array(),
-			'analytics'            => array(),
-		);
+		$options = self::get_options();
 
-		$options = array_merge(
-			$defaults,
-			self::get_options()
+		// Theme support.
+		$recognized_theme_supports = array(
+			'disabled',
+			'paired',
+			'native',
 		);
+		if ( isset( $new_options['theme_support'] ) && in_array( $new_options['theme_support'], $recognized_theme_supports, true ) ) {
+			$options['theme_support'] = $new_options['theme_support'];
+		}
+
+		$options['force_sanitization']  = ! empty( $new_options['force_sanitization'] );
+		$options['accept_tree_shaking'] = ! empty( $new_options['accept_tree_shaking'] );
 
 		// Validate post type support.
 		if ( isset( $new_options['supported_post_types'] ) ) {
@@ -156,7 +178,6 @@ public static function validate_options( $new_options ) {
 		return $options;
 	}
 
-
 	/**
 	 * Check for errors with updating the supported post types.
 	 *