Merge pull request #1132 from Automattic/update/warning-notice

Limit validation to AMP theme support
ampproject · May 8, 2018 · b5c53be · b5c53be
2 parents 4120ef7 + 801b9cf
commit b5c53be
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 71 deletions.
diff --git a/amp.php b/amp.php
@@ -110,7 +110,6 @@ function amp_init() {
 
 	add_rewrite_endpoint( amp_get_slug(), EP_PERMALINK );
 
-	AMP_Validation_Utils::init();
 	AMP_Theme_Support::init();
 	AMP_Post_Type_Support::add_post_type_support();
 	add_filter( 'request', 'amp_force_query_var_value' );

diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php
@@ -92,6 +92,8 @@ public static function init() {
 			return;
 		}
 
+		AMP_Validation_Utils::init();
+
 		self::purge_amp_query_vars();
 		self::handle_xhr_request();
 

diff --git a/includes/utils/class-amp-validation-utils.php b/includes/utils/class-amp-validation-utils.php
@@ -202,13 +202,10 @@ class AMP_Validation_Utils {
 	 * @return void
 	 */
 	public static function init() {
-		if ( current_theme_supports( 'amp' ) ) {
-			add_action( 'init', array( __CLASS__, 'register_post_type' ) );
-			add_filter( 'dashboard_glance_items', array( __CLASS__, 'filter_dashboard_glance_items' ) );
-			add_action( 'rightnow_end', array( __CLASS__, 'print_dashboard_glance_styles' ) );
-			add_action( 'save_post', array( __CLASS__, 'handle_save_post_prompting_validation' ), 10, 2 );
-		}
-
+		add_action( 'init', array( __CLASS__, 'register_post_type' ) );
+		add_filter( 'dashboard_glance_items', array( __CLASS__, 'filter_dashboard_glance_items' ) );
+		add_action( 'rightnow_end', array( __CLASS__, 'print_dashboard_glance_styles' ) );
+		add_action( 'save_post', array( __CLASS__, 'handle_save_post_prompting_validation' ), 10, 2 );
 		add_action( 'edit_form_top', array( __CLASS__, 'print_edit_form_validation_status' ), 10, 2 );
 		add_action( 'all_admin_notices', array( __CLASS__, 'plugin_notice' ) );
 		add_filter( 'manage_' . self::POST_TYPE_SLUG . '_posts_columns', array( __CLASS__, 'add_post_columns' ) );
@@ -535,64 +532,41 @@ public static function print_edit_form_validation_status( $post ) {
 			return;
 		}
 
-		$url                    = null;
-		$validation_status_post = null;
-		$validation_errors      = array();
-
-		// Incorporate frontend validation status if there is a known URL for the post.
-		if ( is_post_type_viewable( $post->post_type ) ) {
-			$url = amp_get_permalink( $post->ID );
-
-			$validation_status_post = self::get_validation_status_post( $url );
-			if ( $validation_status_post ) {
-				$data = json_decode( $validation_status_post->post_content, true );
-				if ( is_array( $data ) ) {
-					$validation_errors = array_merge( $validation_errors, $data );
-				}
-			}
+		// Skip if the post type is not viewable on the frontend, since we need a permalink to validate.
+		if ( ! is_post_type_viewable( $post->post_type ) ) {
+			return;
 		}
 
-		// If no results from URL are available, validate post content outside frontend context.
-		if ( empty( $validation_errors ) && post_type_supports( $post->post_type, 'editor' ) ) {
-			self::process_markup( $post->post_content );
-			$validation_errors = array_merge(
-				$validation_errors,
-				self::$validation_errors
-			);
-			self::reset_validation_results();
+		$url                    = amp_get_permalink( $post->ID );
+		$validation_status_post = self::get_validation_status_post( $url );
 
-			// Make sure original post is restored after applying shortcodes which could change it.
-			$GLOBALS['post'] = $post; // WPCS: override ok.
-			setup_postdata( $post );
+		// No validation status exists yet, so there is nothing to show.
+		if ( ! $validation_status_post ) {
+			return;
 		}
 
-		if ( empty( $validation_errors ) ) {
+		$validation_errors = json_decode( $validation_status_post->post_content, true );
+
+		// No validation errors so abort.
+		if ( empty( $validation_errors ) || ! is_array( $validation_errors ) ) {
 			return;
 		}
 
 		echo '<div class="notice notice-warning">';
 		echo '<p>';
 		esc_html_e( 'Warning: There is content which fails AMP validation; it will be stripped when served as AMP.', 'amp' );
-		if ( $validation_status_post || $url ) {
-			if ( $validation_status_post ) {
-				echo sprintf(
-					' <a href="%s" target="_blank">%s</a>',
-					esc_url( get_edit_post_link( $validation_status_post ) ),
-					esc_html__( 'Details', 'amp' )
-				);
-			}
-			if ( $url ) {
-				if ( $validation_status_post ) {
-					echo ' | ';
-				}
-				echo sprintf(
-					' <a href="%s" aria-label="%s" target="_blank">%s</a>',
-					esc_url( self::get_debug_url( $url ) ),
-					esc_attr__( 'Validate URL on frontend but without invalid elements/attributes removed', 'amp' ),
-					esc_html__( 'Debug', 'amp' )
-				);
-			}
-		}
+		echo sprintf(
+			' <a href="%s" target="_blank">%s</a>',
+			esc_url( get_edit_post_link( $validation_status_post ) ),
+			esc_html__( 'Details', 'amp' )
+		);
+		echo ' | ';
+		echo sprintf(
+			' <a href="%s" aria-label="%s" target="_blank">%s</a>',
+			esc_url( self::get_debug_url( $url ) ),
+			esc_attr__( 'Validate URL on frontend but without invalid elements/attributes removed', 'amp' ),
+			esc_html__( 'Debug', 'amp' )
+		);
 		echo '</p>';
 
 		$results      = self::summarize_validation_errors( array_unique( $validation_errors, SORT_REGULAR ) );

diff --git a/tests/test-class-amp-validation-utils.php b/tests/test-class-amp-validation-utils.php
@@ -266,6 +266,9 @@ public function test_reset_validation_results() {
 	 * @covers AMP_Validation_Utils::print_edit_form_validation_status()
 	 */
 	public function test_print_edit_form_validation_status() {
+		add_theme_support( 'amp' );
+
+		AMP_Validation_Utils::register_post_type();
 		$this->set_capability();
 		$post = $this->factory()->post->create_and_get();
 		ob_start();
@@ -275,26 +278,30 @@ public function test_print_edit_form_validation_status() {
 		$this->assertNotContains( 'notice notice-warning', $output );
 		$this->assertNotContains( 'Warning:', $output );
 
-		$post->post_content = $this->disallowed_tag;
+		$this->create_custom_post(
+			array(
+				array(
+					'code'            => AMP_Validation_Utils::INVALID_ELEMENT_CODE,
+					'node_name'       => $this->disallowed_tag_name,
+					'parent_name'     => 'div',
+					'node_attributes' => array(),
+					'sources'         => array(
+						array(
+							'type' => 'plugin',
+							'name' => $this->plugin_name,
+						),
+					),
+				),
+			),
+			amp_get_permalink( $post->ID )
+		);
 		ob_start();
 		AMP_Validation_Utils::print_edit_form_validation_status( $post );
 		$output = ob_get_clean();
 
 		$this->assertContains( 'notice notice-warning', $output );
 		$this->assertContains( 'Warning:', $output );
 		$this->assertContains( '<code>script</code>', $output );
-		AMP_Validation_Utils::reset_validation_results();
-
-		$youtube            = 'https://www.youtube.com/watch?v=GGS-tKTXw4Y';
-		$post->post_content = $youtube;
-		ob_start();
-		AMP_Validation_Utils::print_edit_form_validation_status( $post );
-		$output = ob_get_clean();
-
-		// The YouTube embed handler should convert the URL into a valid AMP element.
-		$this->assertNotContains( 'notice notice-warning', $output );
-		$this->assertNotContains( 'Warning:', $output );
-		AMP_Validation_Utils::reset_validation_results();
 	}
 
 	/**
@@ -1299,10 +1306,15 @@ public function test_get_recheck_link() {
 	/**
 	 * Creates and inserts a custom post.
 	 *
+	 * @param array  $errors Validation errors to populate.
+	 * @param string $url    URL that the errors occur on. Defaults to the home page.
 	 * @return int|WP_Error $error_post The ID of new custom post, or an error.
 	 */
-	public function create_custom_post() {
-		$content        = wp_json_encode( $this->get_mock_errors() );
+	public function create_custom_post( $errors = null, $url = null ) {
+		if ( ! $errors ) {
+			$errors = $this->get_mock_errors();
+		}
+		$content        = wp_json_encode( $errors );
 		$encoded_errors = md5( $content );
 		$post_args      = array(
 			'post_type'    => AMP_Validation_Utils::POST_TYPE_SLUG,
@@ -1311,7 +1323,9 @@ public function create_custom_post() {
 			'post_status'  => 'publish',
 		);
 		$error_post     = wp_insert_post( wp_slash( $post_args ) );
-		$url            = home_url( '/' );
+		if ( ! $url ) {
+			$url = home_url( '/' );
+		}
 		update_post_meta( $error_post, AMP_Validation_Utils::AMP_URL_META, $url );
 		return $error_post;
 	}