diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 54b8da57064..2ecb8ebc9dc 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1058,7 +1058,7 @@ public static function prepare_response( $response, $args = array() ) { 'allow_dirty_styles' => self::is_customize_preview_iframe(), // Dirty styles only needed when editing (e.g. for edit shortcodes). 'allow_dirty_scripts' => is_customize_preview(), // Scripts are always needed to inject changeset UUID. 'enable_response_caching' => ( - ( ! defined( 'WP_DEBUG' ) || true !== WP_DEBUG ) + ! ( defined( 'WP_DEBUG' ) && WP_DEBUG ) && ! AMP_Validation_Manager::should_validate_response() ), @@ -1080,8 +1080,20 @@ public static function prepare_response( $response, $args = array() ) { $response_cache = wp_cache_get( $response_cache_key, self::RESPONSE_CACHE_GROUP ); - if ( ! empty( $response_cache ) ) { - return $response_cache; + // Make sure that all of the validation errors should be sanitized in the same way; if not, then the cached body should be discarded. + if ( isset( $response_cache['validation_results'] ) ) { + foreach ( $response_cache['validation_results'] as $validation_result ) { + $should_sanitize = AMP_Validation_Error_Taxonomy::is_validation_error_sanitized( $validation_result['error'] ); + if ( $should_sanitize !== $validation_result['sanitized'] ) { + unset( $response_cache['body'] ); + break; + } + } + } + + // Short-circuit response with cached body. + if ( isset( $response_cache['body'] ) ) { + return $response_cache['body']; } } @@ -1102,14 +1114,13 @@ public static function prepare_response( $response, $args = array() ) { 1 ); } - $dom = AMP_DOM_Utils::get_dom( $response ); - - $xpath = new DOMXPath( $dom ); + $dom = AMP_DOM_Utils::get_dom( $response ); $head = $dom->getElementsByTagName( 'head' )->item( 0 ); + // Make sure scripts from the body get moved to the head. if ( isset( $head ) ) { - // Make sure scripts from the body get moved to the head. + $xpath = new DOMXPath( $dom ); foreach ( $xpath->query( '//body//script[ @custom-element or @custom-template ]' ) as $script ) { $head->appendChild( $script ); } @@ -1208,7 +1219,17 @@ public static function prepare_response( $response, $args = array() ) { // Cache response if enabled. if ( true === $args['enable_response_caching'] ) { - wp_cache_set( $response_cache_key, $response, self::RESPONSE_CACHE_GROUP, MONTH_IN_SECONDS ); + $response_cache = array( + 'body' => $response, + 'validation_results' => array_map( + function( $result ) { + unset( $result['error']['sources'] ); + return $result; + }, + AMP_Validation_Manager::$validation_results + ), + ); + wp_cache_set( $response_cache_key, $response_cache, self::RESPONSE_CACHE_GROUP, MONTH_IN_SECONDS ); } return $response; diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 665068fffc9..8504a36ffce 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -177,7 +177,7 @@ public static function add_admin_menu_new_invalid_url_count() { * * @type bool $ignore_accepted Exclude validation errors that are accepted. Default false. * } - * @return array List of errors. + * @return array List of errors, with keys for term, data, status, and (sanitization) forced. */ public static function get_invalid_url_validation_errors( $post, $args = array() ) { $args = array_merge( @@ -198,20 +198,18 @@ public static function get_invalid_url_validation_errors( $post, $args = array() continue; } $term = get_term_by( 'slug', $stored_validation_error['term_slug'], AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ); - if ( ! $term ) { - continue; - } - if ( $args['ignore_accepted'] && AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS === $term->term_group ) { + + $sanitization = AMP_Validation_Error_Taxonomy::get_validation_error_sanitization( $stored_validation_error['data'] ); + if ( $args['ignore_accepted'] && AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS === $sanitization['status'] ) { continue; } - $errors[] = array( - 'term' => $term, - 'data' => array_merge( - json_decode( $term->description, true ), - array( - 'sources' => $stored_validation_error['sources'], - ) + + $errors[] = array_merge( + array( + 'term' => $term, + 'data' => $stored_validation_error['data'], ), + $sanitization ); } return $errors; @@ -234,10 +232,8 @@ public static function get_invalid_url_validation_error_counts( $post ) { ); $validation_errors = self::get_invalid_url_validation_errors( $post ); - foreach ( wp_list_pluck( $validation_errors, 'term' ) as $term ) { - if ( isset( $counts[ $term->term_group ] ) ) { - $counts[ $term->term_group ]++; - } + foreach ( $validation_errors as $error ) { + $counts[ $error['status'] ]++; } return $counts; } @@ -318,15 +314,6 @@ public static function store_validation_errors( $validation_errors, $url ) { $terms = array(); foreach ( $validation_errors as $data ) { - /* - * Exclude sources from data since not available unless sources are being obtained, - * and thus not able to be matched when hashed. - */ - $sources = null; - if ( isset( $data['sources'] ) ) { - $sources = $data['sources']; - } - $term_data = AMP_Validation_Error_Taxonomy::prepare_validation_error_taxonomy_term( $data ); $term_slug = $term_data['slug']; if ( ! isset( $terms[ $term_slug ] ) ) { @@ -352,7 +339,7 @@ public static function store_validation_errors( $validation_errors, $url ) { $terms[ $term_slug ] = $term; } - $stored_validation_errors[] = compact( 'term_slug', 'sources' ); + $stored_validation_errors[] = compact( 'term_slug', 'data' ); } $post_content = wp_json_encode( $stored_validation_errors ); @@ -627,13 +614,11 @@ public static function filter_row_actions( $actions, $post ) { $view_url = add_query_arg( AMP_Validation_Manager::VALIDATE_QUERY_VAR, '', $url ); // Prevent redirection to non-AMP page. $actions['view'] = sprintf( '%s', esc_url( $view_url ), esc_html__( 'View', 'amp' ) ); - if ( ! empty( $url ) ) { - $actions[ self::RECHECK_ACTION ] = sprintf( - '%s', - self::get_recheck_url( $post, get_edit_post_link( $post->ID, 'raw' ), $url ), - esc_html__( 'Re-check', 'amp' ) - ); - } + $actions[ self::RECHECK_ACTION ] = sprintf( + '%s', + esc_url( self::get_recheck_url( $post, get_edit_post_link( $post->ID, 'raw' ) ) ), + esc_html__( 'Re-check', 'amp' ) + ); return $actions; } @@ -708,9 +693,9 @@ public static function print_admin_notice() { $errors_remain = ! empty( $_GET[ self::REMAINING_ERRORS ] ); // WPCS: CSRF ok. if ( $errors_remain ) { $class = 'notice-warning'; - $message = _n( 'The rechecked URL still has validation errors.', 'The rechecked URLs still have validation errors.', $count_urls_tested, 'amp' ); + $message = _n( 'The rechecked URL still has blocking validation errors.', 'The rechecked URLs still have validation errors.', $count_urls_tested, 'amp' ); } else { - $message = _n( 'The rechecked URL has no validation errors.', 'The rechecked URLs have no validation errors.', $count_urls_tested, 'amp' ); + $message = _n( 'The rechecked URL has no blocking validation errors.', 'The rechecked URLs have no validation errors.', $count_urls_tested, 'amp' ); $class = 'updated'; } @@ -722,8 +707,8 @@ public static function print_admin_notice() { ); } - if ( isset( $_GET['amp_taxonomy_terms_updated'] ) ) { // WPCS: CSRF ok. - $count = intval( $_GET['amp_taxonomy_terms_updated'] ); + $count = isset( $_GET['amp_taxonomy_terms_updated'] ) ? intval( $_GET['amp_taxonomy_terms_updated'] ) : 0; // WPCS: CSRF ok. + if ( $count > 0 ) { $class = 'updated'; printf( '

%s

', @@ -751,20 +736,14 @@ public static function print_admin_notice() { */ public static function handle_inline_recheck( $post_id ) { check_admin_referer( self::NONCE_ACTION . $post_id ); - $post = get_post( $post_id ); - $url = $post->post_title; - if ( isset( $_GET['recheck_url'] ) ) { - $url = wp_validate_redirect( wp_unslash( $_GET['recheck_url'] ) ); - } - $validation_errors = AMP_Validation_Manager::validate_url( $url ); - $remaining_errors = true; - if ( is_array( $validation_errors ) ) { - self::store_validation_errors( $validation_errors, $url ); - $remaining_errors = ! empty( $validation_errors ); - } + $validation_results = self::recheck_post( $post_id ); $redirect = wp_get_referer(); - if ( ! $redirect || empty( $validation_errors ) ) { + if ( $redirect ) { + $redirect = remove_query_arg( wp_removable_query_args(), $redirect ); + } + + if ( ! $redirect || is_wp_error( $validation_results ) || empty( $validation_results ) ) { // If there are no remaining errors and the post was deleted, redirect to edit.php instead of post.php. $redirect = add_query_arg( 'post_type', @@ -773,13 +752,48 @@ public static function handle_inline_recheck( $post_id ) { ); } $args = array( - self::URLS_TESTED => '1', - self::REMAINING_ERRORS => $remaining_errors ? '1' : '0', + self::URLS_TESTED => '1', ); + + // @todo For WP_Error case, see . + if ( ! is_wp_error( $validation_results ) ) { + $args[ self::REMAINING_ERRORS ] = count( array_filter( + $validation_results, + function( $result ) { + return ! $result['sanitized']; + } + ) ); + } + wp_safe_redirect( add_query_arg( $args, $redirect ) ); exit(); } + /** + * Re-check invalid URL post for whether it has blocking validation errors. + * + * @param int|WP_Post $post Post. + * @return array|WP_Error List of blocking validation resukts, or a WP_Error in the case of failure. + */ + public static function recheck_post( $post ) { + $post = get_post( $post ); + $url = $post->post_title; + + $validation_errors = AMP_Validation_Manager::validate_url( $url ); + if ( is_wp_error( $validation_errors ) ) { + return $validation_errors; + } + + $validation_results = array(); + self::store_validation_errors( $validation_errors, $url ); + foreach ( $validation_errors as $error ) { + $sanitized = AMP_Validation_Error_Taxonomy::is_validation_error_sanitized( $error ); + + $validation_results[] = compact( 'error', 'sanitized' ); + } + return $validation_results; + } + /** * Handle validation error status update. * @@ -792,6 +806,10 @@ public static function handle_validation_error_status_update() { if ( empty( $_POST[ AMP_Validation_Manager::VALIDATION_ERROR_TERM_STATUS_QUERY_VAR ] ) || ! is_array( $_POST[ AMP_Validation_Manager::VALIDATION_ERROR_TERM_STATUS_QUERY_VAR ] ) ) { return; } + $post = get_post(); + if ( ! $post || self::POST_TYPE_SLUG !== $post->post_type ) { + return; + } $updated_count = 0; $has_pre_term_description_filter = has_filter( 'pre_term_description', 'wp_filter_kses' ); @@ -819,7 +837,32 @@ public static function handle_validation_error_status_update() { $args = array( 'amp_taxonomy_terms_updated' => $updated_count, ); - wp_safe_redirect( add_query_arg( $args, wp_get_referer() ) ); + + /* + * Re-check the post after the validation status change. This is particularly important for validation errors like + * 'removed_unused_css_rules' since whether it is accepted will determine whether other validation errors are triggered + * such as in this case 'excessive_css'. + */ + if ( $updated_count > 0 ) { + $validation_results = self::recheck_post( $post->ID ); + // @todo For WP_Error case, see . + if ( ! is_wp_error( $validation_results ) ) { + $args[ self::REMAINING_ERRORS ] = count( array_filter( + $validation_results, + function( $result ) { + return ! $result['sanitized']; + } + ) ); + } + } + + $redirect = wp_get_referer(); + if ( ! $redirect ) { + $redirect = get_edit_post_link( $post->ID ); + } + + $redirect = remove_query_arg( wp_removable_query_args(), $redirect ); + wp_safe_redirect( add_query_arg( $args, $redirect ) ); exit(); } @@ -921,7 +964,7 @@ public static function print_validation_errors_meta_box( $post ) { $validation_errors = self::get_invalid_url_validation_errors( $post ); $can_serve_amp = 0 === count( array_filter( $validation_errors, function( $validation_error ) { - return AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS !== $validation_error['term']->term_group; + return AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS !== $validation_error['status']; } ) ); ?>