diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index b98c645b88b..aba63e03f2b 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -436,6 +436,11 @@ public function prepare_validation_error( array $error = array(), array $data = if ( ! isset( $error['code'] ) ) { $error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ELEMENT_CODE; } + + if ( ! isset( $error['type'] ) ) { + $error['type'] = 'script' === $node->nodeName ? AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE : AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE; + } + $error['node_attributes'] = array(); foreach ( $node->attributes as $attribute ) { $error['node_attributes'][ $attribute->nodeName ] = $attribute->nodeValue; @@ -456,6 +461,10 @@ public function prepare_validation_error( array $error = array(), array $data = if ( ! isset( $error['code'] ) ) { $error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ATTRIBUTE_CODE; } + if ( ! isset( $error['type'] ) ) { + // If this is an attribute that begins with on, like onclick, it should be a js_error. + $error['type'] = preg_match( '/^on\w+/', $node->nodeName ) ? AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE : AMP_Validation_Error_Taxonomy::HTML_ATTRIBUTE_ERROR_TYPE; + } $error['element_attributes'] = array(); if ( $node->parentNode && $node->parentNode->hasAttributes() ) { foreach ( $node->parentNode->attributes as $attribute ) { diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 050c24c69e1..6a7703f2dcf 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -673,6 +673,7 @@ private function process_link_element( DOMElement $element ) { $this->remove_invalid_child( $element, array( 'code' => $css_file_path->get_error_code(), 'message' => $css_file_path->get_error_message(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ) ); return; } else { @@ -682,6 +683,7 @@ private function process_link_element( DOMElement $element ) { $this->remove_invalid_child( $element, array( 'code' => $css_file_path->get_error_code(), 'message' => $css_file_path->get_error_message(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ) ); return; } else { @@ -691,6 +693,7 @@ private function process_link_element( DOMElement $element ) { if ( false === $stylesheet ) { $this->remove_invalid_child( $element, array( 'code' => 'stylesheet_file_missing', + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ) ); return; } @@ -887,6 +890,7 @@ private function parse_import_stylesheet( Import $item, CSSList $css_list, $opti $error = array( 'code' => $contents->get_error_code(), 'message' => $contents->get_error_message(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); if ( $sanitized ) { @@ -901,6 +905,7 @@ private function parse_import_stylesheet( Import $item, CSSList $css_list, $opti $error = array( 'code' => $css_file_path->get_error_code(), 'message' => $css_file_path->get_error_message(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); if ( $sanitized ) { @@ -988,6 +993,7 @@ function ( $value ) { $error = array( 'code' => 'css_parse_error', 'message' => $exception->getMessage(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); /* @@ -1239,6 +1245,7 @@ private function process_css_list( CSSList $css_list, $options ) { $error = array( 'code' => 'illegal_css_at_rule', 'at_rule' => $css_item->atRuleName(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); $results[] = compact( 'error', 'sanitized' ); @@ -1259,6 +1266,7 @@ private function process_css_list( CSSList $css_list, $options ) { $error = array( 'code' => 'illegal_css_at_rule', 'at_rule' => $css_item->atRuleName(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); $results[] = compact( 'error', 'sanitized' ); @@ -1275,6 +1283,7 @@ private function process_css_list( CSSList $css_list, $options ) { $error = array( 'code' => 'illegal_css_at_rule', 'at_rule' => $css_item->atRuleName(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); $results[] = compact( 'error', 'sanitized' ); @@ -1290,6 +1299,7 @@ private function process_css_list( CSSList $css_list, $options ) { $error = array( 'code' => 'illegal_css_at_rule', 'at_rule' => $css_item->atRuleName(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); $results[] = compact( 'error', 'sanitized' ); @@ -1297,6 +1307,7 @@ private function process_css_list( CSSList $css_list, $options ) { $error = array( 'code' => 'unrecognized_css', 'item' => get_class( $css_item ), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); $results[] = compact( 'error', 'sanitized' ); @@ -1382,6 +1393,7 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l 'code' => 'illegal_css_property', 'property_name' => $property->getRule(), 'property_value' => $property->getValue(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); if ( $sanitized ) { @@ -1398,6 +1410,7 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l 'code' => 'illegal_css_property', 'property_name' => $property->getRule(), 'property_value' => (string) $property->getValue(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); if ( $sanitized ) { @@ -1556,6 +1569,7 @@ private function process_css_keyframes( KeyFrame $css_list, $options ) { $error = array( 'code' => 'unrecognized_css', 'item' => get_class( $rules ), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); if ( $sanitized ) { @@ -1578,6 +1592,7 @@ private function process_css_keyframes( KeyFrame $css_list, $options ) { 'code' => 'illegal_css_property', 'property_name' => $property->getRule(), 'property_value' => (string) $property->getValue(), + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); if ( $sanitized ) { @@ -1623,6 +1638,7 @@ private function transform_important_qualifiers( RuleSet $ruleset, CSSList $css_ } else { $error = array( 'code' => 'illegal_css_important', + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); $sanitized = $this->should_sanitize_validation_error( $error ); if ( $sanitized ) { @@ -1901,6 +1917,7 @@ private function finalize_styles() { if ( ! $body ) { $this->should_sanitize_validation_error( array( 'code' => 'missing_body_element', + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ) ); } else { $style_element = $this->dom->createElement( 'style' ); @@ -1997,6 +2014,7 @@ private function finalize_stylesheet_set( $stylesheet_set ) { if ( $is_too_much_css && $should_tree_shake && empty( $this->args['accept_tree_shaking'] ) ) { $should_tree_shake = $this->should_sanitize_validation_error( array( 'code' => self::TREE_SHAKING_ERROR_CODE, + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ) ); } @@ -2066,6 +2084,7 @@ private function finalize_stylesheet_set( $stylesheet_set ) { if ( $final_size + $sheet_size > $stylesheet_set['cdata_spec']['max_bytes'] ) { $validation_error = array( 'code' => 'excessive_css', + 'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE, ); if ( isset( $pending_stylesheet['sources'] ) ) { $validation_error['sources'] = $pending_stylesheet['sources']; diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php index 39b88dfe1a8..f99504d336f 100644 --- a/includes/validation/class-amp-invalid-url-post-type.php +++ b/includes/validation/class-amp-invalid-url-post-type.php @@ -131,8 +131,8 @@ public static function add_admin_hooks() { add_action( 'add_meta_boxes', array( __CLASS__, 'add_meta_boxes' ) ); add_action( 'edit_form_top', array( __CLASS__, 'print_url_as_title' ) ); add_filter( 'the_title', array( __CLASS__, 'filter_the_title_in_post_list_table' ), 10, 2 ); + add_action( 'restrict_manage_posts', array( __CLASS__, 'render_post_filters' ), 10, 2 ); - add_filter( 'views_edit-' . self::POST_TYPE_SLUG, array( __CLASS__, 'filter_views_edit' ) ); add_filter( 'manage_' . self::POST_TYPE_SLUG . '_posts_columns', array( __CLASS__, 'add_post_columns' ) ); add_action( 'manage_posts_custom_column', array( __CLASS__, 'output_custom_column' ), 10, 2 ); add_filter( 'post_row_actions', array( __CLASS__, 'filter_row_actions' ), 10, 2 ); @@ -497,122 +497,6 @@ public static function get_post_staleness( $post ) { return $staleness; } - /** - * Add views for filtering validation errors by status. - * - * @param array $views Views. - * @return array Views - */ - public static function filter_views_edit( $views ) { - unset( $views['publish'] ); - - $args = array( - 'post_type' => self::POST_TYPE_SLUG, - 'update_post_meta_cache' => false, - 'update_post_term_cache' => false, - ); - - $with_new_query = new WP_Query( array_merge( - $args, - array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS ) - ) ); - $with_rejected_query = new WP_Query( array_merge( - $args, - array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS ) - ) ); - $with_accepted_query = new WP_Query( array_merge( - $args, - array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS ) - ) ); - - $current_url = remove_query_arg( - array_merge( - wp_removable_query_args(), - array( 's' ) // For some reason behavior of posts list table is to not persist the search query. - ), - wp_unslash( $_SERVER['REQUEST_URI'] ) - ); - - $current_status = null; - if ( isset( $_GET[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ) { // WPCS: CSRF ok. - $value = intval( $_GET[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR ] ); // WPCS: CSRF ok. - if ( in_array( $value, array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS ), true ) ) { - $current_status = $value; - } - } - - $views['new'] = sprintf( - '%s', - esc_url( - add_query_arg( - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS, - $current_url - ) - ), - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS === $current_status ? 'current' : '', - sprintf( - /* translators: %s: the post count. */ - _nx( - 'With New Errors (%s)', - 'With New Errors (%s)', - $with_new_query->found_posts, - 'posts', - 'amp' - ), - number_format_i18n( $with_new_query->found_posts ) - ) - ); - - $views['rejected'] = sprintf( - '%s', - esc_url( - add_query_arg( - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS, - $current_url - ) - ), - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_REJECTED_STATUS === $current_status ? 'current' : '', - sprintf( - /* translators: %s: the post count. */ - _nx( - 'With Rejected Errors (%s)', - 'With Rejected Errors (%s)', - $with_rejected_query->found_posts, - 'posts', - 'amp' - ), - number_format_i18n( $with_rejected_query->found_posts ) - ) - ); - - $views['accepted'] = sprintf( - '%s', - esc_url( - add_query_arg( - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS, - $current_url - ) - ), - AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS === $current_status ? 'current' : '', - sprintf( - /* translators: %s: the post count. */ - _nx( - 'With Accepted Errors (%s)', - 'With Accepted Errors (%s)', - $with_accepted_query->found_posts, - 'posts', - 'amp' - ), - number_format_i18n( $with_accepted_query->found_posts ) - ) - ); - - return $views; - } - /** * Adds post columns to the UI for the validation errors. * @@ -1475,6 +1359,19 @@ public static function filter_the_title_in_post_list_table( $title, $post ) { return $title; } + /** + * Renders the filters on the invalid URL post type edit.php page. + * + * @param string $post_type The slug of the post type. + * @param string $which The location for the markup, either 'top' or 'bottom'. + */ + public static function render_post_filters( $post_type, $which ) { + if ( self::POST_TYPE_SLUG === $post_type && 'top' === $which ) { + AMP_Validation_Error_Taxonomy::render_error_status_filter(); + AMP_Validation_Error_Taxonomy::render_error_type_filter(); + } + } + /** * Gets the URL to recheck the post for AMP validity. * diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php index 8505e0f5669..f02c25ea56e 100644 --- a/includes/validation/class-amp-validation-error-taxonomy.php +++ b/includes/validation/class-amp-validation-error-taxonomy.php @@ -61,6 +61,30 @@ class AMP_Validation_Error_Taxonomy { */ const VALIDATION_ERROR_STATUS_QUERY_VAR = 'amp_validation_error_status'; + /** + * Query var used when filtering for the validation error type. + * + * @var string + */ + const VALIDATION_ERROR_TYPE_QUERY_VAR = 'amp_validation_error_type'; + + /** + * The