From 0017edfb924002f0cf291d0e75f3325c928040f0 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Tue, 28 Aug 2018 14:23:05 -0500
Subject: [PATCH 01/29] Add a 'type' to all validation errors

Like Weston suggested, add this to the
amp_validation_error term description.
This will aid in searching that taxonomy page
by error type.
At the moment, there are only 3 types:
HTML, JS, and CSS.
---
 .../sanitizers/class-amp-base-sanitizer.php   |  8 ++++++
 .../sanitizers/class-amp-style-sanitizer.php  | 19 ++++++++++++++
 .../class-amp-validation-error-taxonomy.php   | 25 +++++++++++++++++++
 tests/test-class-amp-base-sanitizer.php       |  4 +++
 tests/test-tag-and-attribute-sanitizer.php    |  3 ++-
 5 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php
index b98c645b88b..8d77b6e6a62 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_ERROR_TYPE;
+			}
+
 			$error['node_attributes'] = array();
 			foreach ( $node->attributes as $attribute ) {
 				$error['node_attributes'][ $attribute->nodeName ] = $attribute->nodeValue;
@@ -456,6 +461,9 @@ 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'] ) ) {
+				$error['type'] = AMP_Validation_Error_Taxonomy::HTML_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-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 9c8414dc7b7..4a2cc0a5403 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -75,6 +75,31 @@ class AMP_Validation_Error_Taxonomy {
 	 */
 	const INVALID_ATTRIBUTE_CODE = 'invalid_attribute';
 
+	/**
+	 * The 'type' of error that applies to most errors with the 'code' of 'invalid_element' and 'invalid_attribute'.
+	 *
+	 * Except for 'invalid_element' errors for a <script>, which have the JS_ERROR_TYPE.
+	 * This allows filtering by type in the taxonomy page, like displaying only HTML errors, or only CSS errors.
+	 *
+	 * @var string
+	 */
+	const HTML_ERROR_TYPE = 'html_error';
+
+	/**
+	 * The 'type' of error that applies to the error 'code' of 'invalid_element' when the node is a <script>.
+	 * This applies both when enqueuing a script, and when a <script> is echoed directly.
+	 *
+	 * @var string
+	 */
+	const JS_ERROR_TYPE = 'js_error';
+
+	/**
+	 * The 'type' of all CSS errors, no matter what the 'code'.
+	 *
+	 * @var string
+	 */
+	const CSS_ERROR_TYPE = 'css_error';
+
 	/**
 	 * The key for removed elements.
 	 *
diff --git a/tests/test-class-amp-base-sanitizer.php b/tests/test-class-amp-base-sanitizer.php
index b6afcc8ebc5..d4ecb9df0c4 100644
--- a/tests/test-class-amp-base-sanitizer.php
+++ b/tests/test-class-amp-base-sanitizer.php
@@ -221,6 +221,7 @@ public function test_remove_invalid_child() {
 			),
 			'foo'             => 'bar',
 			'sources'         => null,
+			'type'            => AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE,
 		);
 
 		// Test sanitized.
@@ -250,6 +251,7 @@ public function test_remove_invalid_child() {
 		$expected_error['node_name'] = 'link';
 		unset( $expected_error['node_attributes']['src'] );
 		$expected_error['node_attributes']['href'] = 'http://example.com/bad.css?ver=__normalized__';
+		$expected_error['type']                    = AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE;
 		add_filter( 'amp_validation_error_sanitized', '__return_false' );
 		AMP_Validation_Manager::reset_validation_results();
 		$parent->appendChild( $child );
@@ -294,6 +296,7 @@ public function test_remove_invalid_attribute() {
 								'id'     => 'bar',
 								'onload' => 'someFunc()',
 							),
+						'type'               => AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE,
 					),
 					$error
 				);
@@ -320,6 +323,7 @@ public function test_remove_invalid_attribute() {
 								'id'     => 'bar',
 								'onload' => 'someFunc()',
 							),
+						'type'               => AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE,
 					),
 					$error
 				);
diff --git a/tests/test-tag-and-attribute-sanitizer.php b/tests/test-tag-and-attribute-sanitizer.php
index 3eb250a3063..8b169489fed 100644
--- a/tests/test-tag-and-attribute-sanitizer.php
+++ b/tests/test-tag-and-attribute-sanitizer.php
@@ -1114,7 +1114,8 @@ public function test_replace_node_with_children_validation_errors() {
 			$sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom, array(
 				'validation_error_callback' => function( $error, $context ) use ( $that, $expected_errors, &$error_index ) {
 					$expected = $expected_errors[ $error_index ];
-					$tag      = $expected['node_name'];
+					$expected['type'] = AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE;
+					$tag = $expected['node_name'];
 					$that->assertEquals( $expected, $error );
 					$that->assertInstanceOf( 'DOMElement', $context['node'] );
 					$that->assertEquals( $tag, $context['node']->tagName );

From 3e9a81e01947673a754ef6ad39c95cb1023e11f4 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Tue, 28 Aug 2018 16:02:38 -0500
Subject: [PATCH 02/29] If an on* attribute is removed, make type js_error

For example, onclick.
This will have JS in it, so this error code is more accurate.
On Weston's suggestion in code review.
---
 includes/sanitizers/class-amp-base-sanitizer.php | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php
index 8d77b6e6a62..9f1cd017557 100644
--- a/includes/sanitizers/class-amp-base-sanitizer.php
+++ b/includes/sanitizers/class-amp-base-sanitizer.php
@@ -462,7 +462,8 @@ public function prepare_validation_error( array $error = array(), array $data =
 				$error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ATTRIBUTE_CODE;
 			}
 			if ( ! isset( $error['type'] ) ) {
-				$error['type'] = AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE;
+				// If this is an attribute that begins with on, like onclick, it should be a js_error.
+				$error['type'] = preg_match( '/^on[a-zA-Z]/', $node->nodeName ) ? AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE : AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE;
 			}
 			$error['element_attributes'] = array();
 			if ( $node->parentNode && $node->parentNode->hasAttributes() ) {

From 0ae789d44c3ed2ca86014a24ddcdc229bb80fa7f Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Tue, 28 Aug 2018 16:20:09 -0500
Subject: [PATCH 03/29] In unit test assertions, change to js_error

These were for an onload attribute,
so update them for the latest commit.
---
 tests/test-class-amp-base-sanitizer.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/test-class-amp-base-sanitizer.php b/tests/test-class-amp-base-sanitizer.php
index d4ecb9df0c4..9ff15221b27 100644
--- a/tests/test-class-amp-base-sanitizer.php
+++ b/tests/test-class-amp-base-sanitizer.php
@@ -296,7 +296,7 @@ public function test_remove_invalid_attribute() {
 								'id'     => 'bar',
 								'onload' => 'someFunc()',
 							),
-						'type'               => AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE,
+						'type'               => AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE,
 					),
 					$error
 				);
@@ -323,7 +323,7 @@ public function test_remove_invalid_attribute() {
 								'id'     => 'bar',
 								'onload' => 'someFunc()',
 							),
-						'type'               => AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE,
+						'type'               => AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE,
 					),
 					$error
 				);

From 34337918a766dc7b17104db2559a2891df54e7e0 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Tue, 28 Aug 2018 19:11:01 -0500
Subject: [PATCH 04/29] Add a query var to filter by type

Borrowing heavily from:
add_group_terms_clauses_filter(),
filter the 'where' clause,
to filter the taxonomy page by type.
---
 .../class-amp-validation-error-taxonomy.php   | 31 +++++++++++++
 ...st-class-amp-validation-error-taxonomy.php | 46 +++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 4a2cc0a5403..c899922ffc6 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -61,6 +61,13 @@ 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';
+
 	/**
 	 * Validation code for an invalid element.
 	 *
@@ -478,6 +485,7 @@ public static function summarize_validation_errors( $validation_errors ) {
 	 */
 	public static function add_admin_hooks() {
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_group_terms_clauses_filter' ) );
+		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_error_type_clauses_filter' ) );
 		add_action( 'load-edit-tags.php', function() {
 			add_filter( 'user_has_cap', array( __CLASS__, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ), 10, 3 );
 		} );
@@ -502,6 +510,7 @@ public static function add_admin_hooks() {
 		// Add recognition of amp_validation_error_status query var (which will only apply in admin since post type is not publicly_queryable).
 		add_filter( 'query_vars', function( $query_vars ) {
 			$query_vars[] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR;
+			$query_vars[] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR;
 			return $query_vars;
 		} );
 
@@ -601,6 +610,28 @@ public static function add_group_terms_clauses_filter() {
 		}, 10, 2 );
 	}
 
+	/**
+	 * Filter amp_validation_error term query by type, like in the 'AMP Validation Errors' taxonomy page.
+	 * Allows viewing only a certain type at a time, like only JS errors.
+	 */
+	public static function add_error_type_clauses_filter() {
+		if ( self::TAXONOMY_SLUG !== get_current_screen()->taxonomy || ! isset( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) { // WPCS: CSRF ok.
+			return;
+		}
+
+		$type = strval( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ); // WPCS: CSRF ok.
+		if ( ! in_array( $type, array( self::HTML_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE ), true ) ) {
+			return;
+		}
+		add_filter( 'terms_clauses', function( $clauses, $taxonomies ) use ( $type ) {
+			global $wpdb;
+			if ( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG === $taxonomies[0] ) {
+				$clauses['where'] .= $wpdb->prepare( ' AND tt.description LIKE %s', '%"type":"' . $wpdb->esc_like( $type ) . '"%' );
+			}
+			return $clauses;
+		}, 10, 2 );
+	}
+
 	/**
 	 * Prevent user from being able to delete validation errors in order to disable the checkbox on the post list table.
 	 *
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 4d625b0f48a..8f948de28f0 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -322,6 +322,7 @@ public function test_add_admin_hooks() {
 		do_action( 'load-edit-tags.php' ); // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores
 
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_group_terms_clauses_filter' ) ) );
+		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_error_type_clauses_filter' ) ) );
 		$this->assertEquals( 10, has_filter( 'user_has_cap', array( self::TESTED_CLASS, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ) ) );
 		$this->assertEquals( 10, has_filter( 'terms_clauses', array( self::TESTED_CLASS, 'filter_terms_clauses_for_description_search' ) ) );
 		$this->assertEquals( 10, has_action( 'admin_notices', array( self::TESTED_CLASS, 'add_admin_notices' ) ) );
@@ -332,6 +333,12 @@ public function test_add_admin_hooks() {
 		$this->assertEquals( 10, has_filter( 'posts_where', array( self::TESTED_CLASS, 'filter_posts_where_for_validation_error_status' ) ) );
 		$this->assertEquals( 10, has_filter( 'handle_bulk_actions-edit-' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'handle_validation_error_update' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'handle_inline_edit_request' ) ) );
+
+		// Assert that the 'query_vars' callback adds these query vars.
+		$this->assertEmpty( array_diff(
+			array( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR ),
+			apply_filters( 'query_vars', array() )
+		) );
 	}
 
 	/**
@@ -359,6 +366,45 @@ public function test_add_group_terms_clauses_filter() {
 		$this->assertTrue( has_filter( $tested_filter ) );
 	}
 
+	/**
+	 * Test add_error_type_clauses_filter.
+	 *
+	 * @covers \AMP_Validation_Error_Taxonomy::add_error_type_clauses_filter()
+	 */
+	public function test_add_error_type_clauses_filter() {
+		global $current_screen;
+
+		$initial_where   = 'foo';
+		$initial_clauses = array( 'where' => $initial_where );
+		$type            = AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE;
+		$taxonomies      = array( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
+
+		set_current_screen( 'front' );
+		$tested_filter = 'terms_clauses';
+		remove_all_filters( $tested_filter );
+		AMP_Validation_Error_Taxonomy::add_error_type_clauses_filter();
+		$this->assertFalse( has_filter( $tested_filter ) );
+
+		// The VALIDATION_ERROR_TYPE_QUERY_VAR isn't present, so this should not add the filter.
+		$current_screen->taxonomy = AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG;
+		AMP_Validation_Error_Taxonomy::add_error_type_clauses_filter();
+		$this->assertFalse( has_filter( $tested_filter ) );
+
+		// Both parts of the conditional should be true, and this should add the filter.
+		$_GET[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR ] = $type;
+		AMP_Validation_Error_Taxonomy::add_error_type_clauses_filter();
+		$this->assertTrue( has_filter( $tested_filter ) );
+
+		// Assert that the filter works as expected.
+		$filtered_clauses = apply_filters( $tested_filter, $initial_clauses, $taxonomies );
+		$this->assertContains( $initial_where, $filtered_clauses['where'] );
+		$this->assertContains( 'AND tt.description LIKE', $filtered_clauses['where'] );
+
+		// If $taxonomies does not have the AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, the filter should return the clauses unchanged.
+		$taxonomies = array( 'post_tag' );
+		$this->assertEquals( $initial_clauses, apply_filters( $tested_filter, $initial_clauses, $taxonomies ) );
+	}
+
 	/**
 	 * Test filter_user_has_cap_for_hiding_term_list_table_checkbox.
 	 *

From 8dd3f01374cf3f3dc71da2512df2b2aae33f7b1b Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Wed, 29 Aug 2018 15:06:52 -0500
Subject: [PATCH 05/29] Filter the term redirect location, to possibly add a
 query var

On clicking the 'Filter' button on the 'AMP Validation Errors' taxonomy page,
edit-tags.php processes the POST request that this submits.
Then, it redirects to a URL to display the page again.
This filter callback looks for a value for VALIDATION_ERROR_TYPE_QUERY_VAR in the  request.
That means that the user filtered by error type, like 'js_error'.
It then passes that value to the redirect URL as a query var,
So that the taxonomy page will be filtered for that error type.
@see AMP_Validation_Error_Taxonomy::add_error_type_clauses_filter() for the filtering of the 'where' clause, based on that query var.
---
 .../class-amp-validation-error-taxonomy.php   | 35 +++++++++++++++++
 ...st-class-amp-validation-error-taxonomy.php | 39 +++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index c899922ffc6..a2a960a2156 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -484,6 +484,7 @@ public static function summarize_validation_errors( $validation_errors ) {
 	 * Add admin hooks.
 	 */
 	public static function add_admin_hooks() {
+		add_filter( 'redirect_term_location', array( __CLASS__, 'add_term_filter_query_var' ), 10, 2 );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_group_terms_clauses_filter' ) );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_error_type_clauses_filter' ) );
 		add_action( 'load-edit-tags.php', function() {
@@ -589,6 +590,40 @@ public static function add_admin_hooks() {
 		} );
 	}
 
+	/**
+	 * Filter the term redirect location, to possibly add a query var to filter by term type.
+	 *
+	 * On clicking the 'Filter' button on the 'AMP Validation Errors' taxonomy page,
+	 * edit-tags.php processes the POST request that this submits.
+	 * Then, it redirects to a URL to display the page again.
+	 * This filter callback looks for a value for VALIDATION_ERROR_TYPE_QUERY_VAR in the $_POST request.
+	 * That means that the user filtered by error type, like 'js_error'.
+	 * It then passes that value to the redirect URL as a query var,
+	 * So that the taxonomy page will be filtered for that error type.
+	 *
+	 * @see AMP_Validation_Error_Taxonomy::add_error_type_clauses_filter() for the filtering of the 'where' clause, based on that query var.
+	 * @param string      $url The $url to redirect to.
+	 * @param WP_Taxonomy $tax The WP_Taxonomy object.
+	 * @return string The filtered URL.
+	 */
+	public static function add_term_filter_query_var( $url, $tax ) {
+		if (
+			self::TAXONOMY_SLUG === $tax->name
+			&&
+			isset( $_POST['post_type'] ) && AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG === $_POST['post_type'] // WPCS: CSRF OK.
+			&&
+			! empty( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) // WPCS: CSRF OK.
+		) {
+			return add_query_arg(
+				self::VALIDATION_ERROR_TYPE_QUERY_VAR,
+				sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ), // WPCS: CSRF OK.
+				$url
+			);
+		}
+
+		return $url;
+	}
+
 	/**
 	 * Filter amp_validation_error term query by term group when requested.
 	 */
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 8f948de28f0..900d8df6d1d 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -321,6 +321,7 @@ public function test_add_admin_hooks() {
 		AMP_Validation_Error_Taxonomy::add_admin_hooks();
 		do_action( 'load-edit-tags.php' ); // phpcs:ignore WordPress.NamingConventions.ValidHookName.UseUnderscores
 
+		$this->assertEquals( 10, has_action( 'redirect_term_location', array( self::TESTED_CLASS, 'add_term_filter_query_var' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_group_terms_clauses_filter' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_error_type_clauses_filter' ) ) );
 		$this->assertEquals( 10, has_filter( 'user_has_cap', array( self::TESTED_CLASS, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ) ) );
@@ -366,6 +367,44 @@ public function test_add_group_terms_clauses_filter() {
 		$this->assertTrue( has_filter( $tested_filter ) );
 	}
 
+	/**
+	 * Test add_term_filter_query_var.
+	 *
+	 * @covers \AMP_Validation_Error_Taxonomy::add_term_filter_query_var()
+	 */
+	public function test_add_term_filter_query_var() {
+		$initial_url      = admin_url( 'edit-tags.php' );
+		$correct_taxonomy = new WP_Taxonomy( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG );
+		$wrong_taxonomy   = new WP_Taxonomy( 'category', 'post' );
+
+		// Because the VALIDATION_ERROR_TYPE_QUERY_VAR isn't present in the POST request, this should return the $initial_url unchanged, without adding the query var.
+		$this->assertEquals( $initial_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $wrong_taxonomy ) );
+
+		// The $_POST does not have a value for VALIDATION_ERROR_TYPE_QUERY_VAR, so this should again return $initial_url unchanged.
+		$wrong_query_var           = 'amp_incorrect_var';
+		$_POST[ $wrong_query_var ] = '1';
+		$this->assertEquals( $initial_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $wrong_taxonomy ) );
+
+		// The $_POST has the VALIDATION_ERROR_TYPE_QUERY_VAR, but does not have a $taxonomy of AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG.
+		$query_var_value = AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE;
+		$_POST[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR ] = $query_var_value;
+		$this->assertEquals( $initial_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $wrong_taxonomy ) );
+
+		// The $_POST has the taxonomy, but does not have the right 'post_type'.
+		$_POST['taxonomy']  = AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG;
+		$_POST['post_type'] = 'post';
+		$this->assertEquals( $initial_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $correct_taxonomy ) );
+
+		// The $_POST has correct 'post_type', so this should add the VALIDATION_ERROR_TYPE_QUERY_VAR to the $initial_url.
+		$_POST['post_type'] = AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG;
+		$expected_url       = add_query_arg(
+			AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR,
+			$query_var_value,
+			$initial_url
+		);
+		$this->assertEquals( $expected_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $correct_taxonomy ) );
+	}
+
 	/**
 	 * Test add_error_type_clauses_filter.
 	 *

From 5f92cc262032a461953c36b77595c18e01a5c670 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Wed, 29 Aug 2018 15:57:05 -0500
Subject: [PATCH 06/29] Render the taxonomy filter for this taxonomy type

Allows filtering by error_type,
like only viewing JS errors.
This UI now works for error_type,
though work is probably needed for accepted status.
---
 .../class-amp-validation-error-taxonomy.php   | 39 +++++++++++++++++++
 ...st-class-amp-validation-error-taxonomy.php | 30 ++++++++++++++
 2 files changed, 69 insertions(+)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index a2a960a2156..f82baff5e5b 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -487,6 +487,7 @@ public static function add_admin_hooks() {
 		add_filter( 'redirect_term_location', array( __CLASS__, 'add_term_filter_query_var' ), 10, 2 );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_group_terms_clauses_filter' ) );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_error_type_clauses_filter' ) );
+		add_action( sprintf( 'after-%s-table', self::TAXONOMY_SLUG ), array( __CLASS__, 'render_taxonomy_filter' ) );
 		add_action( 'load-edit-tags.php', function() {
 			add_filter( 'user_has_cap', array( __CLASS__, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ), 10, 3 );
 		} );
@@ -667,6 +668,44 @@ public static function add_error_type_clauses_filter() {
 		}, 10, 2 );
 	}
 
+	/**
+	 * Outputs the taxonomy filter UI for this taxonomy type.
+	 *
+	 * Similar to what appears on /wp-admin/edit.php for posts and pages,
+	 * this outputs a <select> element to choose the error type,
+	 * and a 'Filter' submit button that allows filtering for that type.
+	 * This allows viewing one type at a time, like only JS errors.
+	 *
+	 * @param string $taxonomy_name The name of the taxonomy.
+	 */
+	public static function render_taxonomy_filter( $taxonomy_name ) {
+		if ( self::TAXONOMY_SLUG !== $taxonomy_name ) {
+			return;
+		}
+
+		$error_type_filter_value = isset( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
+		$div_id                  = 'amp-tax-filter';
+		?>
+		<div id="<?php echo esc_attr( $div_id ); ?>" class="alignleft actions">
+			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
+			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>">
+				<option value="-1"><?php esc_html_e( 'All Error Types', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::HTML_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML Error', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>><?php esc_html_e( 'JS Error', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>><?php esc_html_e( 'CSS Error', 'amp' ); ?></option>
+			</select>
+			<input name="filter_action" type="submit" id="doaction" class="button action" value="<?php esc_html_e( 'Filter', 'amp' ); ?>">
+		</div>
+
+		<script>
+			( function ( $ ) {
+				// Move the filter after the 'Bulk Actions' <select>, as it looks like there's no way to do this with simply an action.
+				$( '#<?php echo $div_id; // WPCS: XSS OK. ?>' ).insertAfter( $( '.tablenav.top .bulkactions' ) );
+			} )( jQuery );
+		</script>
+		<?php
+	}
+
 	/**
 	 * Prevent user from being able to delete validation errors in order to disable the checkbox on the post list table.
 	 *
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 900d8df6d1d..2a1dd7c2c7d 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -324,6 +324,7 @@ public function test_add_admin_hooks() {
 		$this->assertEquals( 10, has_action( 'redirect_term_location', array( self::TESTED_CLASS, 'add_term_filter_query_var' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_group_terms_clauses_filter' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_error_type_clauses_filter' ) ) );
+		$this->assertEquals( 10, has_action( sprintf( 'after-%s-table', AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'render_taxonomy_filter' ) ) ) );
 		$this->assertEquals( 10, has_filter( 'user_has_cap', array( self::TESTED_CLASS, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ) ) );
 		$this->assertEquals( 10, has_filter( 'terms_clauses', array( self::TESTED_CLASS, 'filter_terms_clauses_for_description_search' ) ) );
 		$this->assertEquals( 10, has_action( 'admin_notices', array( self::TESTED_CLASS, 'add_admin_notices' ) ) );
@@ -444,6 +445,35 @@ public function test_add_error_type_clauses_filter() {
 		$this->assertEquals( $initial_clauses, apply_filters( $tested_filter, $initial_clauses, $taxonomies ) );
 	}
 
+	/**
+	 * Test render_taxonomy_filter.
+	 *
+	 * @covers \AMP_Validation_Error_Taxonomy::render_taxonomy_filter()
+	 */
+	public function test_render_taxonomy_filter() {
+		// When passing the wrong $taxonomy_name to the method, it should not output anything.
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_taxonomy_filter( 'category' );
+		$this->assertEmpty( ob_get_clean() );
+
+		// With the correct taxonomy name, the strings below should be present.
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_taxonomy_filter( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
+		$markup = ob_get_clean();
+
+		$expected_to_contain = array(
+			AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR,
+			AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE,
+			AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE,
+			AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
+			'<script>',
+		);
+
+		foreach ( $expected_to_contain as $expected ) {
+			$this->assertContains( $expected, $markup );
+		}
+	}
+
 	/**
 	 * Test filter_user_has_cap_for_hiding_term_list_table_checkbox.
 	 *

From 3d177343f8ce99af78c4d05fa9f5791f9f5f34c1 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Wed, 29 Aug 2018 16:28:51 -0500
Subject: [PATCH 07/29] Add a filter <select> element for error type, like
 'Accepted'

This still doesn't use the new terminology, like 'Identified.'
There wasn't much to change,
as filtering by error type worked thanks to Weston.
---
 .../class-amp-validation-error-taxonomy.php   | 36 +++++++++++++------
 ...st-class-amp-validation-error-taxonomy.php | 17 +++++++--
 2 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index f82baff5e5b..dfb5bdbabf8 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -612,14 +612,21 @@ public static function add_term_filter_query_var( $url, $tax ) {
 			self::TAXONOMY_SLUG === $tax->name
 			&&
 			isset( $_POST['post_type'] ) && AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG === $_POST['post_type'] // WPCS: CSRF OK.
-			&&
-			! empty( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) // WPCS: CSRF OK.
 		) {
-			return add_query_arg(
-				self::VALIDATION_ERROR_TYPE_QUERY_VAR,
-				sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ), // WPCS: CSRF OK.
-				$url
-			);
+			if ( isset( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) { // WPCS: CSRF OK.
+				$url = add_query_arg(
+					self::VALIDATION_ERROR_TYPE_QUERY_VAR,
+					sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ), // WPCS: CSRF OK.
+					$url
+				);
+			}
+			if ( isset( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ) { // WPCS: CSRF OK.
+				$url = add_query_arg(
+					self::VALIDATION_ERROR_STATUS_QUERY_VAR,
+					sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ), // WPCS: CSRF OK.
+					$url
+				);
+			}
 		}
 
 		return $url;
@@ -683,10 +690,19 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 			return;
 		}
 
-		$error_type_filter_value = isset( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
-		$div_id                  = 'amp-tax-filter';
+		$error_type_filter_value   = isset( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
+		$error_status_filter_value = isset( $_REQUEST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
+		$div_id                    = 'amp-tax-filter';
 		?>
 		<div id="<?php echo esc_attr( $div_id ); ?>" class="alignleft actions">
+			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error status', 'amp' ); ?></label>
+			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>">
+				<option value="-1"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php esc_html_e( 'New Error', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php esc_html_e( 'Accepted Error', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php esc_html_e( 'Rejected Error', 'amp' ); ?></option>
+			</select>
+
 			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
 			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>">
 				<option value="-1"><?php esc_html_e( 'All Error Types', 'amp' ); ?></option>
@@ -694,7 +710,7 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 				<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>><?php esc_html_e( 'JS Error', 'amp' ); ?></option>
 				<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>><?php esc_html_e( 'CSS Error', 'amp' ); ?></option>
 			</select>
-			<input name="filter_action" type="submit" id="doaction" class="button action" value="<?php esc_html_e( 'Filter', 'amp' ); ?>">
+			<input name="filter_action" type="submit" id="doaction" class="button action" value="<?php esc_html_e( 'Apply Filter', 'amp' ); ?>">
 		</div>
 
 		<script>
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 2a1dd7c2c7d..a01c3c676c3 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -387,8 +387,8 @@ public function test_add_term_filter_query_var() {
 		$this->assertEquals( $initial_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $wrong_taxonomy ) );
 
 		// The $_POST has the VALIDATION_ERROR_TYPE_QUERY_VAR, but does not have a $taxonomy of AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG.
-		$query_var_value = AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE;
-		$_POST[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR ] = $query_var_value;
+		$type_query_var_value = AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE;
+		$_POST[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR ] = $type_query_var_value;
 		$this->assertEquals( $initial_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $wrong_taxonomy ) );
 
 		// The $_POST has the taxonomy, but does not have the right 'post_type'.
@@ -400,7 +400,18 @@ public function test_add_term_filter_query_var() {
 		$_POST['post_type'] = AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG;
 		$expected_url       = add_query_arg(
 			AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR,
-			$query_var_value,
+			$type_query_var_value,
+			$initial_url
+		);
+		$this->assertEquals( $expected_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $correct_taxonomy ) );
+
+		// The $_POST has a value for the accepted status, so this method should pass that to the redirect URL as a query var.
+		$status_query_var_value = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS;
+		$_POST[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR ] = $status_query_var_value;
+		$_POST[ AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR ]   = null;
+		$expected_url = add_query_arg(
+			AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR,
+			$status_query_var_value,
 			$initial_url
 		);
 		$this->assertEquals( $expected_url, AMP_Validation_Error_Taxonomy::add_term_filter_query_var( $initial_url, $correct_taxonomy ) );

From c2656a364bb1af60851fd9807068531bc35d17b0 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Wed, 29 Aug 2018 16:55:02 -0500
Subject: [PATCH 08/29] Remove views for filtering taxonomy, but reuse their
 counts

Though these views are now applied in a <select> filter,
this uses much of their infrastructure.
Also, it uses their counts for each status.
For example, if there are 0 accepted errors,
this won't display the <option> for 'Accepted Error'.
---
 .../class-amp-validation-error-taxonomy.php   | 146 +++---------------
 ...st-class-amp-validation-error-taxonomy.php |  24 ---
 2 files changed, 23 insertions(+), 147 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index dfb5bdbabf8..cfb16c16506 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -496,7 +496,6 @@ public static function add_admin_hooks() {
 		add_filter( 'tag_row_actions', array( __CLASS__, 'filter_tag_row_actions' ), 10, 2 );
 		add_action( 'admin_menu', array( __CLASS__, 'add_admin_menu_validation_error_item' ) );
 		add_filter( 'manage_' . self::TAXONOMY_SLUG . '_custom_column', array( __CLASS__, 'filter_manage_custom_columns' ), 10, 3 );
-		add_filter( 'views_edit-' . self::TAXONOMY_SLUG, array( __CLASS__, 'filter_views_edit' ) );
 		add_filter( 'posts_where', array( __CLASS__, 'filter_posts_where_for_validation_error_status' ), 10, 2 );
 		add_filter( 'handle_bulk_actions-edit-' . self::TAXONOMY_SLUG, array( __CLASS__, 'handle_validation_error_update' ), 10, 3 );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'handle_inline_edit_request' ) );
@@ -620,7 +619,11 @@ public static function add_term_filter_query_var( $url, $tax ) {
 					$url
 				);
 			}
-			if ( isset( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ) { // WPCS: CSRF OK.
+			if (
+				isset( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) // WPCS: CSRF OK.
+				&&
+				in_array( intval( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ), array( self::VALIDATION_ERROR_NEW_STATUS, self::VALIDATION_ERROR_ACCEPTED_STATUS, self::VALIDATION_ERROR_REJECTED_STATUS ), true ) // WPCS: CSRF OK.
+			) {
 				$url = add_query_arg(
 					self::VALIDATION_ERROR_STATUS_QUERY_VAR,
 					sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ), // WPCS: CSRF OK.
@@ -690,17 +693,30 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 			return;
 		}
 
-		$error_type_filter_value   = isset( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
-		$error_status_filter_value = isset( $_REQUEST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
+
+		// Only display <option> for each status if it actually has errors associated with it.
+		$total_term_count          = self::get_validation_error_count();
+		$rejected_term_count       = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_REJECTED_STATUS ) );
+		$accepted_term_count       = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_ACCEPTED_STATUS ) );
+		$new_term_count            = $total_term_count - $rejected_term_count - $accepted_term_count;
+		$error_type_filter_value   = isset( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
+		$error_status_filter_value = isset( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ? intval( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) : ''; // WPCS: CSRF OK.
 		$div_id                    = 'amp-tax-filter';
+
 		?>
 		<div id="<?php echo esc_attr( $div_id ); ?>" class="alignleft actions">
 			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error status', 'amp' ); ?></label>
 			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>">
 				<option value="-1"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
-				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php esc_html_e( 'New Error', 'amp' ); ?></option>
-				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php esc_html_e( 'Accepted Error', 'amp' ); ?></option>
-				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php esc_html_e( 'Rejected Error', 'amp' ); ?></option>
+				<?php if ( $new_term_count ) : ?>
+					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php esc_html_e( 'New Error', 'amp' ); ?></option>
+				<?php endif; ?>
+				<?php if ( $accepted_term_count ) : ?>
+					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php esc_html_e( 'Accepted Error', 'amp' ); ?></option>
+				<?php endif; ?>
+				<?php if ( $rejected_term_count ) : ?>
+					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php esc_html_e( 'Rejected Error', 'amp' ); ?></option>
+				<?php endif; ?>
 			</select>
 
 			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
@@ -874,122 +890,6 @@ public static function add_admin_menu_validation_error_item() {
 		);
 	}
 
-	/**
-	 * Add views for filtering validation errors by status.
-	 *
-	 * @param array $views Views.
-	 * @return array Views.
-	 */
-	public static function filter_views_edit( $views ) {
-		$total_term_count    = self::get_validation_error_count();
-		$rejected_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_REJECTED_STATUS ) );
-		$accepted_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_ACCEPTED_STATUS ) );
-		$new_term_count      = $total_term_count - $rejected_term_count - $accepted_term_count;
-
-		$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[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ) { // WPCS: CSRF ok.
-			$value = intval( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ); // WPCS: CSRF ok.
-			if ( in_array( $value, array( self::VALIDATION_ERROR_NEW_STATUS, self::VALIDATION_ERROR_ACCEPTED_STATUS, self::VALIDATION_ERROR_REJECTED_STATUS ), true ) ) {
-				$current_status = $value;
-			}
-		}
-
-		$views['all'] = sprintf(
-			'<a href="%s" class="%s">%s</a>',
-			esc_url( remove_query_arg( self::VALIDATION_ERROR_STATUS_QUERY_VAR, $current_url ) ),
-			null === $current_status ? 'current' : '',
-			sprintf(
-				/* translators: %s: the term count. */
-				_nx(
-					'All <span class="count">(%s)</span>',
-					'All <span class="count">(%s)</span>',
-					$total_term_count,
-					'terms',
-					'amp'
-				),
-				number_format_i18n( $total_term_count )
-			)
-		);
-
-		$views['new'] = sprintf(
-			'<a href="%s" class="%s">%s</a>',
-			esc_url(
-				add_query_arg(
-					self::VALIDATION_ERROR_STATUS_QUERY_VAR,
-					self::VALIDATION_ERROR_NEW_STATUS,
-					$current_url
-				)
-			),
-			self::VALIDATION_ERROR_NEW_STATUS === $current_status ? 'current' : '',
-			sprintf(
-				/* translators: %s: the term count. */
-				_nx(
-					'New <span class="count">(%s)</span>',
-					'New <span class="count">(%s)</span>',
-					$new_term_count,
-					'terms',
-					'amp'
-				),
-				number_format_i18n( $new_term_count )
-			)
-		);
-
-		$views['rejected'] = sprintf(
-			'<a href="%s" class="%s">%s</a>',
-			esc_url(
-				add_query_arg(
-					self::VALIDATION_ERROR_STATUS_QUERY_VAR,
-					self::VALIDATION_ERROR_REJECTED_STATUS,
-					$current_url
-				)
-			),
-			self::VALIDATION_ERROR_REJECTED_STATUS === $current_status ? 'current' : '',
-			sprintf(
-				/* translators: %s: the term count. */
-				_nx(
-					'Rejected <span class="count">(%s)</span>',
-					'Rejected <span class="count">(%s)</span>',
-					$rejected_term_count,
-					'terms',
-					'amp'
-				),
-				number_format_i18n( $rejected_term_count )
-			)
-		);
-
-		$views['accepted'] = sprintf(
-			'<a href="%s" class="%s">%s</a>',
-			esc_url(
-				add_query_arg(
-					self::VALIDATION_ERROR_STATUS_QUERY_VAR,
-					self::VALIDATION_ERROR_ACCEPTED_STATUS,
-					$current_url
-				)
-			),
-			self::VALIDATION_ERROR_ACCEPTED_STATUS === $current_status ? 'current' : '',
-			sprintf(
-				/* translators: %s: the term count. */
-				_nx(
-					'Accepted <span class="count">(%s)</span>',
-					'Accepted <span class="count">(%s)</span>',
-					$accepted_term_count,
-					'terms',
-					'amp'
-				),
-				number_format_i18n( $accepted_term_count )
-			)
-		);
-		return $views;
-	}
-
 	/**
 	 * Supply the content for the custom columns.
 	 *
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index a01c3c676c3..0da5553758a 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -331,7 +331,6 @@ public function test_add_admin_hooks() {
 		$this->assertEquals( 10, has_filter( 'tag_row_actions', array( self::TESTED_CLASS, 'filter_tag_row_actions' ) ) );
 		$this->assertEquals( 10, has_action( 'admin_menu', array( self::TESTED_CLASS, 'add_admin_menu_validation_error_item' ) ) );
 		$this->assertEquals( 10, has_filter( 'manage_' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG . '_custom_column', array( self::TESTED_CLASS, 'filter_manage_custom_columns' ) ) );
-		$this->assertEquals( 10, has_filter( 'views_edit-' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'filter_views_edit' ) ) );
 		$this->assertEquals( 10, has_filter( 'posts_where', array( self::TESTED_CLASS, 'filter_posts_where_for_validation_error_status' ) ) );
 		$this->assertEquals( 10, has_filter( 'handle_bulk_actions-edit-' . AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'handle_validation_error_update' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'handle_inline_edit_request' ) ) );
@@ -620,29 +619,6 @@ public function test_add_admin_menu_validation_error_item() {
 		$this->assertEquals( $expected_submenu, end( $amp_options ) );
 	}
 
-	/**
-	 * Test filter_views_edit.
-	 *
-	 * @covers \AMP_Validation_Error_Taxonomy::filter_views_edit()
-	 */
-	public function test_filter_views_edit() {
-		AMP_Validation_Error_Taxonomy::register();
-		$views = AMP_Validation_Error_Taxonomy::filter_views_edit( array() );
-		$this->assertContains( 'Accepted', $views['accepted'] );
-		$this->assertContains( '(0)', $views['accepted'] );
-
-		$this->assertContains( 'All', $views['all'] );
-		$this->assertContains( '(0)', $views['all'] );
-
-		$this->assertContains( 'New', $views['new'] );
-		$this->assertContains( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, $views['new'] );
-		$this->assertContains( '(0)', $views['new'] );
-
-		$this->assertContains( 'Rejected', $views['rejected'] );
-		$this->assertContains( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, $views['rejected'] );
-		$this->assertContains( '(0)', $views['rejected'] );
-	}
-
 	/**
 	 * Test filter_manage_custom_columns.
 	 *

From fe43d31810621b86dbf11e24fa0496d571ca4ca3 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Wed, 29 Aug 2018 17:17:29 -0500
Subject: [PATCH 09/29] Add the option value for 'All Statuses' to the
 whitelist

Create a new constant, NO_FILTER_VALUE.
This is for the existing value of -1.
And add this to the whitelist: in_array().
---
 .../class-amp-validation-error-taxonomy.php   | 21 +++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index cfb16c16506..4074fedf57d 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -68,6 +68,16 @@ class AMP_Validation_Error_Taxonomy {
 	 */
 	const VALIDATION_ERROR_TYPE_QUERY_VAR = 'amp_validation_error_type';
 
+	/**
+	 * The <option> value to not filter at all, like for 'All Statuses'.
+	 *
+	 * This is also used in WP_List_Table, like for the 'Bulk Actions' option.
+	 * When this is present, this ensures that this isn't filtered.
+	 *
+	 * @var int
+	 */
+	const NO_FILTER_VALUE = -1;
+
 	/**
 	 * Validation code for an invalid element.
 	 *
@@ -622,7 +632,11 @@ public static function add_term_filter_query_var( $url, $tax ) {
 			if (
 				isset( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) // WPCS: CSRF OK.
 				&&
-				in_array( intval( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ), array( self::VALIDATION_ERROR_NEW_STATUS, self::VALIDATION_ERROR_ACCEPTED_STATUS, self::VALIDATION_ERROR_REJECTED_STATUS ), true ) // WPCS: CSRF OK.
+				in_array(
+					intval( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ), // WPCS: CSRF OK.
+					array( self::VALIDATION_ERROR_NEW_STATUS, self::VALIDATION_ERROR_ACCEPTED_STATUS, self::VALIDATION_ERROR_REJECTED_STATUS, self::NO_FILTER_VALUE ),
+					true
+				)
 			) {
 				$url = add_query_arg(
 					self::VALIDATION_ERROR_STATUS_QUERY_VAR,
@@ -693,7 +707,6 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 			return;
 		}
 
-
 		// Only display <option> for each status if it actually has errors associated with it.
 		$total_term_count          = self::get_validation_error_count();
 		$rejected_term_count       = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_REJECTED_STATUS ) );
@@ -707,7 +720,7 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 		<div id="<?php echo esc_attr( $div_id ); ?>" class="alignleft actions">
 			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error status', 'amp' ); ?></label>
 			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>">
-				<option value="-1"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
 				<?php if ( $new_term_count ) : ?>
 					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php esc_html_e( 'New Error', 'amp' ); ?></option>
 				<?php endif; ?>
@@ -721,7 +734,7 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 
 			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
 			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>">
-				<option value="-1"><?php esc_html_e( 'All Error Types', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Error Types', 'amp' ); ?></option>
 				<option value="<?php echo esc_attr( self::HTML_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML Error', 'amp' ); ?></option>
 				<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>><?php esc_html_e( 'JS Error', 'amp' ); ?></option>
 				<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>><?php esc_html_e( 'CSS Error', 'amp' ); ?></option>

From 1e135d2e602e18e6fd68c6575fba669b67ef36f8 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Wed, 29 Aug 2018 17:37:29 -0500
Subject: [PATCH 10/29] Use the new terminology for the error taxonomy page
 name

Change this to 'Errors by Type,'
to match the Sketch design.
---
 .../validation/class-amp-validation-error-taxonomy.php    | 8 ++++----
 .../test-class-amp-validation-error-taxonomy.php          | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 4074fedf57d..64c2a6748bd 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -164,13 +164,13 @@ public static function register() {
 
 		register_taxonomy( self::TAXONOMY_SLUG, AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, array(
 			'labels'             => array(
-				'name'                  => _x( 'AMP Validation Errors', 'taxonomy general name', 'amp' ),
+				'name'                  => _x( 'Errors by Type', 'taxonomy general name', 'amp' ),
 				'singular_name'         => _x( 'AMP Validation Error', 'taxonomy singular name', 'amp' ),
 				'search_items'          => __( 'Search AMP Validation Errors', 'amp' ),
 				'all_items'             => __( 'All AMP Validation Errors', 'amp' ),
 				'edit_item'             => __( 'Edit AMP Validation Error', 'amp' ),
 				'update_item'           => __( 'Update AMP Validation Error', 'amp' ),
-				'menu_name'             => __( 'Validation Errors', 'amp' ),
+				'menu_name'             => __( 'Errors by Type', 'amp' ),
 				'back_to_items'         => __( 'Back to AMP Validation Errors', 'amp' ),
 				'popular_items'         => __( 'Frequent Validation Errors', 'amp' ),
 				'view_item'             => __( 'View Validation Error', 'amp' ),
@@ -884,7 +884,7 @@ public static function filter_tag_row_actions( $actions, WP_Term $tag ) {
 	 * Show AMP validation errors under AMP admin menu.
 	 */
 	public static function add_admin_menu_validation_error_item() {
-		$menu_item_label = esc_html__( 'Validation Errors', 'amp' );
+		$menu_item_label = esc_html__( 'Errors by Type', 'amp' );
 		$new_error_count = self::get_validation_error_count( array(
 			'group' => self::VALIDATION_ERROR_NEW_STATUS,
 		) );
@@ -895,7 +895,7 @@ public static function add_admin_menu_validation_error_item() {
 		$taxonomy_caps = (object) get_taxonomy( self::TAXONOMY_SLUG )->cap; // Yes, cap is an object not an array.
 		add_submenu_page(
 			AMP_Options_Manager::OPTION_NAME,
-			esc_html__( 'Validation Errors', 'amp' ),
+			esc_html__( 'Errors by Type', 'amp' ),
 			$menu_item_label,
 			$taxonomy_caps->manage_terms,
 			// The following esc_attr() is sadly needed due to <https://github.com/WordPress/wordpress-develop/blob/4.9.5/src/wp-admin/menu-header.php#L201>.
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 0da5553758a..4b5a37123c6 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -55,7 +55,7 @@ public function test_register() {
 		$this->assertFalse( $taxonomy_object->hierarchical );
 		$this->assertTrue( $taxonomy_object->show_in_menu );
 		$this->assertFalse( $taxonomy_object->meta_box_cb );
-		$this->assertEquals( 'AMP Validation Errors', $taxonomy_object->label );
+		$this->assertEquals( 'Errors by Type', $taxonomy_object->label );
 		$this->assertEquals( 'do_not_allow', $taxonomy_object->cap->assign_terms );
 		$this->assertEquals( array( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ), $taxonomy_object->object_type );
 
@@ -66,7 +66,7 @@ public function test_register() {
 		$this->assertEquals( 'All AMP Validation Errors', $labels->all_items );
 		$this->assertEquals( 'Edit AMP Validation Error', $labels->edit_item );
 		$this->assertEquals( 'Update AMP Validation Error', $labels->update_item );
-		$this->assertEquals( 'Validation Errors', $labels->menu_name );
+		$this->assertEquals( 'Errors by Type', $labels->menu_name );
 		$this->assertEquals( 'Back to AMP Validation Errors', $labels->back_to_items );
 		$this->assertEquals( 'Frequent Validation Errors', $labels->popular_items );
 		$this->assertEquals( 'View Validation Error', $labels->view_item );
@@ -610,10 +610,10 @@ public function test_add_admin_menu_validation_error_item() {
 		wp_set_current_user( $this->factory()->user->create( array( 'role' => 'administrator' ) ) );
 		AMP_Validation_Error_Taxonomy::add_admin_menu_validation_error_item();
 		$expected_submenu = array(
-			'Validation Errors',
+			'Errors by Type',
 			'manage_categories',
 			'edit-tags.php?taxonomy=amp_validation_error&amp;post_type=amp_invalid_url',
-			'Validation Errors',
+			'Errors by Type',
 		);
 		$amp_options      = $submenu[ AMP_Options_Manager::OPTION_NAME ];
 		$this->assertEquals( $expected_submenu, end( $amp_options ) );

From f1d4bea8a6b7b2bd4e354a87bddc67053bb65b39 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Wed, 29 Aug 2018 18:02:32 -0500
Subject: [PATCH 11/29] Address failed unit tests in Travis build

---
 includes/validation/class-amp-validation-error-taxonomy.php   | 2 +-
 tests/validation/test-class-amp-validation-error-taxonomy.php | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 64c2a6748bd..0ba571513ab 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -895,7 +895,7 @@ public static function add_admin_menu_validation_error_item() {
 		$taxonomy_caps = (object) get_taxonomy( self::TAXONOMY_SLUG )->cap; // Yes, cap is an object not an array.
 		add_submenu_page(
 			AMP_Options_Manager::OPTION_NAME,
-			esc_html__( 'Errors by Type', 'amp' ),
+			$menu_item_label,
 			$menu_item_label,
 			$taxonomy_caps->manage_terms,
 			// The following esc_attr() is sadly needed due to <https://github.com/WordPress/wordpress-develop/blob/4.9.5/src/wp-admin/menu-header.php#L201>.
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 4b5a37123c6..9cfd6cdbb83 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -60,7 +60,7 @@ public function test_register() {
 		$this->assertEquals( array( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ), $taxonomy_object->object_type );
 
 		$labels = $taxonomy_object->labels;
-		$this->assertEquals( 'AMP Validation Errors', $labels->name );
+		$this->assertEquals( 'Errors by Type', $labels->name );
 		$this->assertEquals( 'AMP Validation Error', $labels->singular_name );
 		$this->assertEquals( 'Search AMP Validation Errors', $labels->search_items );
 		$this->assertEquals( 'All AMP Validation Errors', $labels->all_items );

From caa2a261884c27b1307b6556fbdccad5c2b38b6f Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Thu, 30 Aug 2018 13:39:19 -0500
Subject: [PATCH 12/29] Restore error counts by status in <option>

There were counts earlier,
when filtering by status was done by clicking an <a>.
Thanks to Weston's work.
Mainly copy the previous logic to add the counts.
---
 .../class-amp-validation-error-taxonomy.php   | 53 ++++++++++++++++---
 ...st-class-amp-validation-error-taxonomy.php | 21 ++++++++
 2 files changed, 67 insertions(+), 7 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 0ba571513ab..b67a016d562 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -721,14 +721,53 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error status', 'amp' ); ?></label>
 			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>">
 				<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
-				<?php if ( $new_term_count ) : ?>
-					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php esc_html_e( 'New Error', 'amp' ); ?></option>
-				<?php endif; ?>
-				<?php if ( $accepted_term_count ) : ?>
-					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php esc_html_e( 'Accepted Error', 'amp' ); ?></option>
+				<?php
+				if ( $new_term_count ) :
+					$new_term_text = sprintf(
+						/* translators: %s: the new term count. */
+						_nx(
+							'New Error <span class="count">(%s)</span>',
+							'New Errors <span class="count">(%s)</span>',
+							$new_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $new_term_count )
+					);
+					?>
+					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php echo wp_kses_post( $new_term_text ); ?></option>
 				<?php endif; ?>
-				<?php if ( $rejected_term_count ) : ?>
-					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php esc_html_e( 'Rejected Error', 'amp' ); ?></option>
+				<?php
+				if ( $accepted_term_count ) :
+					$accepted_term_text = sprintf(
+						/* translators: %s: the accepted term count. */
+						_nx(
+							'Accepted Error <span class="count">(%s)</span>',
+							'Accepted Errors <span class="count">(%s)</span>',
+							$accepted_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $accepted_term_count )
+					);
+					?>
+					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php echo wp_kses_post( $accepted_term_text ); ?></option>
+					<?php
+				endif;
+				if ( $rejected_term_count ) :
+					$rejected_term_text = sprintf(
+						/* translators: %s: the rejected term count. */
+						_nx(
+							'Rejected Error <span class="count">(%s)</span>',
+							'Rejected Errors <span class="count">(%s)</span>',
+							$rejected_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $rejected_term_count )
+					);
+					?>
+					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php echo wp_kses_post( $rejected_term_text ); ?></option>
 				<?php endif; ?>
 			</select>
 
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 9cfd6cdbb83..9edb344cb95 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -461,6 +461,14 @@ public function test_add_error_type_clauses_filter() {
 	 * @covers \AMP_Validation_Error_Taxonomy::render_taxonomy_filter()
 	 */
 	public function test_render_taxonomy_filter() {
+		AMP_Validation_Error_Taxonomy::register();
+		// Create one new error.
+		$this->factory()->term->create( array(
+			'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
+			'description' => wp_json_encode( $this->get_mock_error() ),
+			'term_group'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS,
+		) );
+
 		// When passing the wrong $taxonomy_name to the method, it should not output anything.
 		ob_start();
 		AMP_Validation_Error_Taxonomy::render_taxonomy_filter( 'category' );
@@ -482,6 +490,19 @@ public function test_render_taxonomy_filter() {
 		foreach ( $expected_to_contain as $expected ) {
 			$this->assertContains( $expected, $markup );
 		}
+
+		// When there is one new error, the <option> for this status in the filter should have (1) after the status name.
+		$this->assertContains( 'New Error <span class="count">(1)</span>', $markup );
+
+		// When there are two new errors, the <option> text should be plural, and have a count of (2).
+		$this->factory()->term->create( array(
+			'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
+			'description' => wp_json_encode( $this->get_mock_error() ),
+			'term_group'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS,
+		) );
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_taxonomy_filter( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
+		$this->assertContains( 'New Errors <span class="count">(2)</span>', ob_get_clean() );
 	}
 
 	/**

From 13d5cafd2cc9516bc5608511e83c6fe5f65e009d Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Thu, 30 Aug 2018 14:03:19 -0500
Subject: [PATCH 13/29] Add error types for HTML (Element) and HTML (Attribute)

Before, there was only HTML_ERROR_TYPE.
Based on comments in issues like 1361,
this adds 2 types of errors for HTML.
---
 .../sanitizers/class-amp-base-sanitizer.php   |  4 ++--
 .../class-amp-validation-error-taxonomy.php   | 23 +++++++++++++++----
 tests/test-class-amp-base-sanitizer.php       |  2 +-
 tests/test-tag-and-attribute-sanitizer.php    |  2 +-
 ...st-class-amp-validation-error-taxonomy.php |  3 ++-
 5 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php
index 9f1cd017557..8c5d47094df 100644
--- a/includes/sanitizers/class-amp-base-sanitizer.php
+++ b/includes/sanitizers/class-amp-base-sanitizer.php
@@ -438,7 +438,7 @@ public function prepare_validation_error( array $error = array(), array $data =
 			}
 
 			if ( ! isset( $error['type'] ) ) {
-				$error['type'] = 'script' === $node->nodeName ? AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE : AMP_Validation_Error_Taxonomy::HTML_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();
@@ -463,7 +463,7 @@ public function prepare_validation_error( array $error = array(), array $data =
 			}
 			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[a-zA-Z]/', $node->nodeName ) ? AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE : AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE;
+				$error['type'] = preg_match( '/^on[a-zA-Z]/', $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() ) {
diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index a3ef572da0d..263cd3fa6db 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -93,14 +93,26 @@ class AMP_Validation_Error_Taxonomy {
 	const INVALID_ATTRIBUTE_CODE = 'invalid_attribute';
 
 	/**
-	 * The 'type' of error that applies to most errors with the 'code' of 'invalid_element' and 'invalid_attribute'.
+	 * The 'type' of error for invalid HTML elements, like <frame>.
 	 *
+	 * These usually have the 'code' of 'invalid_element'.
 	 * Except for 'invalid_element' errors for a <script>, which have the JS_ERROR_TYPE.
-	 * This allows filtering by type in the taxonomy page, like displaying only HTML errors, or only CSS errors.
+	 * This allows filtering by type in the taxonomy page, like displaying only HTML element errors, or only CSS errors.
 	 *
 	 * @var string
 	 */
-	const HTML_ERROR_TYPE = 'html_error';
+	const HTML_ELEMENT_ERROR_TYPE = 'html_element_error';
+
+	/**
+	 * The 'type' of error for invalid HTML attributes.
+	 *
+	 * These usually have the 'code' of 'invalid_attribute'.
+	 * Banned attributes include i-amp-*.
+	 * But on* attributes, like onclick, have the JS_ERROR_TYPE.
+	 *
+	 * @var string
+	 */
+	const HTML_ATTRIBUTE_ERROR_TYPE = 'html_attribute_error';
 
 	/**
 	 * The 'type' of error that applies to the error 'code' of 'invalid_element' when the node is a <script>.
@@ -695,7 +707,7 @@ public static function add_error_type_clauses_filter() {
 		}
 
 		$type = strval( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ); // WPCS: CSRF ok.
-		if ( ! in_array( $type, array( self::HTML_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE ), true ) ) {
+		if ( ! in_array( $type, array( self::HTML_ELEMENT_ERROR_TYPE, self::HTML_ATTRIBUTE_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE ), true ) ) {
 			return;
 		}
 		add_filter( 'terms_clauses', function( $clauses, $taxonomies ) use ( $type ) {
@@ -789,7 +801,8 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
 			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>">
 				<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Error Types', 'amp' ); ?></option>
-				<option value="<?php echo esc_attr( self::HTML_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML Error', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::HTML_ELEMENT_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ELEMENT_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML (Element) Error', 'amp' ); ?></option>
+				<option value="<?php echo esc_attr( self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML (Attribute) Error', 'amp' ); ?></option>
 				<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>><?php esc_html_e( 'JS Error', 'amp' ); ?></option>
 				<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>><?php esc_html_e( 'CSS Error', 'amp' ); ?></option>
 			</select>
diff --git a/tests/test-class-amp-base-sanitizer.php b/tests/test-class-amp-base-sanitizer.php
index 9ff15221b27..5f898b8aafd 100644
--- a/tests/test-class-amp-base-sanitizer.php
+++ b/tests/test-class-amp-base-sanitizer.php
@@ -251,7 +251,7 @@ public function test_remove_invalid_child() {
 		$expected_error['node_name'] = 'link';
 		unset( $expected_error['node_attributes']['src'] );
 		$expected_error['node_attributes']['href'] = 'http://example.com/bad.css?ver=__normalized__';
-		$expected_error['type']                    = AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE;
+		$expected_error['type']                    = AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE;
 		add_filter( 'amp_validation_error_sanitized', '__return_false' );
 		AMP_Validation_Manager::reset_validation_results();
 		$parent->appendChild( $child );
diff --git a/tests/test-tag-and-attribute-sanitizer.php b/tests/test-tag-and-attribute-sanitizer.php
index 8b169489fed..00dc35195f3 100644
--- a/tests/test-tag-and-attribute-sanitizer.php
+++ b/tests/test-tag-and-attribute-sanitizer.php
@@ -1114,7 +1114,7 @@ public function test_replace_node_with_children_validation_errors() {
 			$sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom, array(
 				'validation_error_callback' => function( $error, $context ) use ( $that, $expected_errors, &$error_index ) {
 					$expected = $expected_errors[ $error_index ];
-					$expected['type'] = AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE;
+					$expected['type'] = AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE;
 					$tag = $expected['node_name'];
 					$that->assertEquals( $expected, $error );
 					$that->assertInstanceOf( 'DOMElement', $context['node'] );
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 827b6f4d5ad..a2f44109a99 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -505,7 +505,8 @@ public function test_render_taxonomy_filter() {
 
 		$expected_to_contain = array(
 			AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR,
-			AMP_Validation_Error_Taxonomy::HTML_ERROR_TYPE,
+			AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE,
+			AMP_Validation_Error_Taxonomy::HTML_ATTRIBUTE_ERROR_TYPE,
 			AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE,
 			AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
 			'<script>',

From b74854c3f6ffe7eff4fb2fda6793b2d3e5c4bfb2 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Thu, 30 Aug 2018 21:10:27 -0500
Subject: [PATCH 14/29] Refactor logic in render_taxonomy_filters() into 2
 methods

The AMP validation error page also needs these filters,
but it doesn't need all of
render_taxonomy_filters().
So abstract it into 2 functions that can be reused.
---
 .../class-amp-validation-error-taxonomy.php   | 202 +++++++++++-------
 ...st-class-amp-validation-error-taxonomy.php | 109 ++++++++--
 2 files changed, 220 insertions(+), 91 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 263cd3fa6db..4331fc68659 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -522,7 +522,7 @@ public static function add_admin_hooks() {
 		add_filter( 'redirect_term_location', array( __CLASS__, 'add_term_filter_query_var' ), 10, 2 );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_group_terms_clauses_filter' ) );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_error_type_clauses_filter' ) );
-		add_action( sprintf( 'after-%s-table', self::TAXONOMY_SLUG ), array( __CLASS__, 'render_taxonomy_filter' ) );
+		add_action( sprintf( 'after-%s-table', self::TAXONOMY_SLUG ), array( __CLASS__, 'render_taxonomy_filters' ) );
 		add_action( 'load-edit-tags.php', function() {
 			add_filter( 'user_has_cap', array( __CLASS__, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ), 10, 3 );
 		} );
@@ -729,83 +729,18 @@ public static function add_error_type_clauses_filter() {
 	 *
 	 * @param string $taxonomy_name The name of the taxonomy.
 	 */
-	public static function render_taxonomy_filter( $taxonomy_name ) {
+	public static function render_taxonomy_filters( $taxonomy_name ) {
 		if ( self::TAXONOMY_SLUG !== $taxonomy_name ) {
 			return;
 		}
 
-		// Only display <option> for each status if it actually has errors associated with it.
-		$total_term_count          = self::get_validation_error_count();
-		$rejected_term_count       = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_REJECTED_STATUS ) );
-		$accepted_term_count       = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_ACCEPTED_STATUS ) );
-		$new_term_count            = $total_term_count - $rejected_term_count - $accepted_term_count;
-		$error_type_filter_value   = isset( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
-		$error_status_filter_value = isset( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ? intval( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) : ''; // WPCS: CSRF OK.
-		$div_id                    = 'amp-tax-filter';
-
+		$div_id = 'amp-tax-filter';
 		?>
 		<div id="<?php echo esc_attr( $div_id ); ?>" class="alignleft actions">
-			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error status', 'amp' ); ?></label>
-			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>">
-				<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
-				<?php
-				if ( $new_term_count ) :
-					$new_term_text = sprintf(
-						/* translators: %s: the new term count. */
-						_nx(
-							'New Error <span class="count">(%s)</span>',
-							'New Errors <span class="count">(%s)</span>',
-							$new_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $new_term_count )
-					);
-					?>
-					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php echo wp_kses_post( $new_term_text ); ?></option>
-				<?php endif; ?>
-				<?php
-				if ( $accepted_term_count ) :
-					$accepted_term_text = sprintf(
-						/* translators: %s: the accepted term count. */
-						_nx(
-							'Accepted Error <span class="count">(%s)</span>',
-							'Accepted Errors <span class="count">(%s)</span>',
-							$accepted_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $accepted_term_count )
-					);
-					?>
-					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php echo wp_kses_post( $accepted_term_text ); ?></option>
-					<?php
-				endif;
-				if ( $rejected_term_count ) :
-					$rejected_term_text = sprintf(
-						/* translators: %s: the rejected term count. */
-						_nx(
-							'Rejected Error <span class="count">(%s)</span>',
-							'Rejected Errors <span class="count">(%s)</span>',
-							$rejected_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $rejected_term_count )
-					);
-					?>
-					<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php echo wp_kses_post( $rejected_term_text ); ?></option>
-				<?php endif; ?>
-			</select>
-
-			<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
-			<select name="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>">
-				<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Error Types', 'amp' ); ?></option>
-				<option value="<?php echo esc_attr( self::HTML_ELEMENT_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ELEMENT_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML (Element) Error', 'amp' ); ?></option>
-				<option value="<?php echo esc_attr( self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML (Attribute) Error', 'amp' ); ?></option>
-				<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>><?php esc_html_e( 'JS Error', 'amp' ); ?></option>
-				<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>><?php esc_html_e( 'CSS Error', 'amp' ); ?></option>
-			</select>
+			<?php
+			self::render_error_status_filter();
+			self::render_error_type_filter();
+			?>
 			<input name="filter_action" type="submit" id="doaction" class="button action" value="<?php esc_html_e( 'Apply Filter', 'amp' ); ?>">
 		</div>
 
@@ -818,6 +753,129 @@ public static function render_taxonomy_filter( $taxonomy_name ) {
 		<?php
 	}
 
+	/**
+	 * Renders the error status filter <select> element that allows filtering by error status.
+	 *
+	 * There is a difference how the errors are counted, depending on which screen this is on.
+	 * For example: Accepted Errors (10).
+	 * This status filter <select> element is rendered on the validation error post page (Errors by URL),
+	 * and the validation error taxonomy page (Errors by Type).
+	 * On the taxonomy page, this simply needs to count the number of errors with a given type.
+	 * On the post page, this needs to count the number of posts that have at least one error of a given type.
+	 */
+	public static function render_error_status_filter() {
+		$screen_base = get_current_screen()->base;
+		if ( 'edit-tags' === $screen_base ) {
+			$total_term_count    = self::get_validation_error_count();
+			$rejected_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_REJECTED_STATUS ) );
+			$accepted_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_ACCEPTED_STATUS ) );
+			$new_term_count      = $total_term_count - $rejected_term_count - $accepted_term_count;
+		} elseif ( 'edit' === $screen_base ) {
+			$args = array(
+				'post_type'              => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
+				'update_post_meta_cache' => false,
+				'update_post_term_cache' => false,
+			);
+
+			$with_new_query = new WP_Query( array_merge(
+				$args,
+				array( self::VALIDATION_ERROR_STATUS_QUERY_VAR => self::VALIDATION_ERROR_NEW_STATUS )
+			) );
+			$new_term_count = $with_new_query->found_posts;
+
+			$with_rejected_query = new WP_Query( array_merge(
+				$args,
+				array( self::VALIDATION_ERROR_STATUS_QUERY_VAR => self::VALIDATION_ERROR_REJECTED_STATUS )
+			) );
+			$rejected_term_count = $with_rejected_query->found_posts;
+			$with_accepted_query = new WP_Query( array_merge(
+				$args,
+				array( self::VALIDATION_ERROR_STATUS_QUERY_VAR => self::VALIDATION_ERROR_ACCEPTED_STATUS )
+			) );
+			$accepted_term_count = $with_accepted_query->found_posts;
+		} else {
+			return;
+		}
+
+		$error_status_filter_value = isset( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ? intval( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) : ''; // WPCS: CSRF OK.
+
+		?>
+		<label for="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error status', 'amp' ); ?></label>
+		<select name="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>">
+			<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
+			<?php
+			if ( $new_term_count ) :
+				$new_term_text = sprintf(
+					/* translators: %s: the new term count. */
+					_nx(
+						'New Error <span class="count">(%s)</span>',
+						'New Errors <span class="count">(%s)</span>',
+						$new_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $new_term_count )
+				);
+				?>
+				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php echo wp_kses_post( $new_term_text ); ?></option>
+			<?php endif; ?>
+			<?php
+			if ( $accepted_term_count ) :
+				$accepted_term_text = sprintf(
+					/* translators: %s: the accepted term count. */
+					_nx(
+						'Accepted Error <span class="count">(%s)</span>',
+						'Accepted Errors <span class="count">(%s)</span>',
+						$accepted_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $accepted_term_count )
+				);
+				?>
+				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php echo wp_kses_post( $accepted_term_text ); ?></option>
+				<?php
+			endif;
+			if ( $rejected_term_count ) :
+				$rejected_term_text = sprintf(
+					/* translators: %s: the rejected term count. */
+					_nx(
+						'Rejected Error <span class="count">(%s)</span>',
+						'Rejected Errors <span class="count">(%s)</span>',
+						$rejected_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $rejected_term_count )
+				);
+				?>
+				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php echo wp_kses_post( $rejected_term_text ); ?></option>
+			<?php endif; ?>
+		</select>
+		<?php
+	}
+
+	/**
+	 * Renders the filter for error type.
+	 *
+	 * This type filter <select> element is rendered on the validation error post page (Errors by URL),
+	 * and the validation error taxonomy page (Errors by Type).
+	 */
+	public static function render_error_type_filter() {
+		$error_type_filter_value = isset( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
+
+		?>
+		<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
+		<select name="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>">
+			<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Error Types', 'amp' ); ?></option>
+			<option value="<?php echo esc_attr( self::HTML_ELEMENT_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ELEMENT_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML (Element) Error', 'amp' ); ?></option>
+			<option value="<?php echo esc_attr( self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML (Attribute) Error', 'amp' ); ?></option>
+			<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>><?php esc_html_e( 'JS Error', 'amp' ); ?></option>
+			<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>><?php esc_html_e( 'CSS Error', 'amp' ); ?></option>
+		</select>
+		<?php
+	}
+
 	/**
 	 * Prevent user from being able to delete validation errors in order to disable the checkbox on the post list table.
 	 *
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index a2f44109a99..956ebef212f 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -348,7 +348,7 @@ public function test_add_admin_hooks() {
 		$this->assertEquals( 10, has_action( 'redirect_term_location', array( self::TESTED_CLASS, 'add_term_filter_query_var' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_group_terms_clauses_filter' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_error_type_clauses_filter' ) ) );
-		$this->assertEquals( 10, has_action( sprintf( 'after-%s-table', AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'render_taxonomy_filter' ) ) ) );
+		$this->assertEquals( 10, has_action( sprintf( 'after-%s-table', AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'render_taxonomy_filters' ) ) ) );
 		$this->assertEquals( 10, has_filter( 'user_has_cap', array( self::TESTED_CLASS, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ) ) );
 		$this->assertEquals( 10, has_filter( 'terms_clauses', array( self::TESTED_CLASS, 'filter_terms_clauses_for_description_search' ) ) );
 		$this->assertEquals( 10, has_action( 'admin_notices', array( self::TESTED_CLASS, 'add_admin_notices' ) ) );
@@ -480,12 +480,13 @@ public function test_add_error_type_clauses_filter() {
 	}
 
 	/**
-	 * Test render_taxonomy_filter.
+	 * Test render_taxonomy_filters.
 	 *
-	 * @covers \AMP_Validation_Error_Taxonomy::render_taxonomy_filter()
+	 * @covers \AMP_Validation_Error_Taxonomy::render_taxonomy_filters()
 	 */
-	public function test_render_taxonomy_filter() {
+	public function test_render_taxonomy_filters() {
 		AMP_Validation_Error_Taxonomy::register();
+		set_current_screen( 'edit-tags.php' );
 		// Create one new error.
 		$this->factory()->term->create( array(
 			'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
@@ -495,12 +496,86 @@ public function test_render_taxonomy_filter() {
 
 		// When passing the wrong $taxonomy_name to the method, it should not output anything.
 		ob_start();
-		AMP_Validation_Error_Taxonomy::render_taxonomy_filter( 'category' );
+		AMP_Validation_Error_Taxonomy::render_taxonomy_filters( 'category' );
 		$this->assertEmpty( ob_get_clean() );
 
-		// With the correct taxonomy name, the strings below should be present.
+		// When there are two new errors, the <option> text should be plural, and have a count of (2).
+		$this->factory()->term->create( array(
+			'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
+			'description' => wp_json_encode( $this->get_mock_error() ),
+			'term_group'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS,
+		) );
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_taxonomy_filters( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
+		$this->assertContains( 'New Errors <span class="count">(2)</span>', ob_get_clean() );
+	}
+
+	/**
+	 * Test render_error_status_filter.
+	 *
+	 * @covers \AMP_Validation_Error_Taxonomy::render_error_status_filter()
+	 */
+	public function test_render_error_status_filter() {
+		AMP_Validation_Error_Taxonomy::register();
+		set_current_screen( 'post.php' );
+
+		// When this is not on the correct screen, this should not render anything.
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_error_status_filter();
+		$this->assertEmpty( ob_get_clean() );
+
+		/*
+		 * This is now on the correct screen, so it shouldn't be empty anymore.
+		 * When no validation error exists, there should not be an <option> for any specific error status, like 'New Error'.
+		 */
+		set_current_screen( 'edit.php' );
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_error_status_filter();
+		$this->assertNotContains( 'New Error', ob_get_clean() );
+
+		$number_of_errors = 10;
+		for ( $i = 0; $i < $number_of_errors; $i++ ) {
+			$this->factory()->term->create( array(
+				'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
+				'description' => wp_json_encode( $this->get_mock_error() ),
+			) );
+		}
+
+		/*
+		 * This is on another accepted screen, so this should again render markup.
+		 * When there are 10 accepted errors, the <option> element for it should end with (10).
+		 */
+		set_current_screen( 'edit-tags.php' );
 		ob_start();
-		AMP_Validation_Error_Taxonomy::render_taxonomy_filter( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
+		AMP_Validation_Error_Taxonomy::render_error_status_filter();
+		$this->assertContains(
+			sprintf(
+				'New Errors <span class="count">(%d)</span>',
+				$number_of_errors
+			),
+			ob_get_clean()
+		);
+	}
+
+	/**
+	 * Test render_error_type_filter.
+	 *
+	 * @covers \AMP_Validation_Error_Taxonomy::render_error_type_filter()
+	 */
+	public function test_render_error_type_filter() {
+		set_current_screen( 'edit-tags.php' );
+		$number_of_errors = 10;
+		for ( $i = 0; $i < $number_of_errors; $i++ ) {
+			$this->factory()->term->create( array(
+				'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
+				'description' => wp_json_encode( $this->get_mock_error() ),
+				'term_group'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS,
+			) );
+		}
+
+		// The strings below should be present.
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_taxonomy_filters( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
 		$markup = ob_get_clean();
 
 		$expected_to_contain = array(
@@ -516,18 +591,14 @@ public function test_render_taxonomy_filter() {
 			$this->assertContains( $expected, $markup );
 		}
 
-		// When there is one new error, the <option> for this status in the filter should have (1) after the status name.
-		$this->assertContains( 'New Error <span class="count">(1)</span>', $markup );
-
-		// When there are two new errors, the <option> text should be plural, and have a count of (2).
-		$this->factory()->term->create( array(
-			'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
-			'description' => wp_json_encode( $this->get_mock_error() ),
-			'term_group'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS,
-		) );
-		ob_start();
-		AMP_Validation_Error_Taxonomy::render_taxonomy_filter( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
-		$this->assertContains( 'New Errors <span class="count">(2)</span>', ob_get_clean() );
+		// When there are 10 errors with this status, its <option> element should have (10).
+		$this->assertContains(
+			sprintf(
+				'New Errors <span class="count">(%d)</span>',
+				$number_of_errors
+			),
+			$markup
+		);
 	}
 
 	/**

From 0397a52a4bd7f46fe3d32ad17cebb5793566523d Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Thu, 30 Aug 2018 22:19:03 -0500
Subject: [PATCH 15/29] Add filters on the 'Errors by URL' page

Using the functions created earlier,
output the filter <select> elements on this page.
There's more work remaining,
like ensuring the query vars work.
---
 .../class-amp-invalid-url-post-type.php       | 137 +++---------------
 .../test-class-amp-invalid-url-post-type.php  | 111 +++++++-------
 ...st-class-amp-validation-error-taxonomy.php |  11 +-
 3 files changed, 80 insertions(+), 179 deletions(-)

diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php
index 39b88dfe1a8..5a3c4071b96 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(
-			'<a href="%s" class="%s">%s</a>',
-			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 <span class="count">(%s)</span>',
-					'With New Errors <span class="count">(%s)</span>',
-					$with_new_query->found_posts,
-					'posts',
-					'amp'
-				),
-				number_format_i18n( $with_new_query->found_posts )
-			)
-		);
-
-		$views['rejected'] = sprintf(
-			'<a href="%s" class="%s">%s</a>',
-			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 <span class="count">(%s)</span>',
-					'With Rejected Errors <span class="count">(%s)</span>',
-					$with_rejected_query->found_posts,
-					'posts',
-					'amp'
-				),
-				number_format_i18n( $with_rejected_query->found_posts )
-			)
-		);
-
-		$views['accepted'] = sprintf(
-			'<a href="%s" class="%s">%s</a>',
-			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 <span class="count">(%s)</span>',
-					'With Accepted Errors <span class="count">(%s)</span>',
-					$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,25 @@ 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
+		) {
+			return;
+		}
+
+		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/tests/validation/test-class-amp-invalid-url-post-type.php b/tests/validation/test-class-amp-invalid-url-post-type.php
index ea2196c2b4a..b7297fab178 100644
--- a/tests/validation/test-class-amp-invalid-url-post-type.php
+++ b/tests/validation/test-class-amp-invalid-url-post-type.php
@@ -86,8 +86,8 @@ public function test_add_admin_hooks() {
 		$this->assertEquals( 10, has_action( 'add_meta_boxes', array( self::TESTED_CLASS, 'add_meta_boxes' ) ) );
 		$this->assertEquals( 10, has_action( 'edit_form_top', array( self::TESTED_CLASS, 'print_url_as_title' ) ) );
 		$this->assertEquals( 10, has_filter( 'the_title', array( self::TESTED_CLASS, 'filter_the_title_in_post_list_table' ) ) );
+		$this->assertEquals( 10, has_filter( 'restrict_manage_posts', array( self::TESTED_CLASS, 'render_post_filters' ), 10, 2 ) );
 
-		$this->assertEquals( 10, has_filter( 'views_edit-' . AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, array( self::TESTED_CLASS, 'filter_views_edit' ) ) );
 		$this->assertEquals( 10, has_filter( 'manage_' . AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG . '_posts_columns', array( self::TESTED_CLASS, 'add_post_columns' ) ) );
 		$this->assertEquals( 10, has_action( 'manage_posts_custom_column', array( self::TESTED_CLASS, 'output_custom_column' ) ) );
 		$this->assertEquals( 10, has_filter( 'post_row_actions', array( self::TESTED_CLASS, 'filter_row_actions' ) ) );
@@ -433,66 +433,6 @@ public function test_get_post_staleness() {
 		$this->assertEmpty( AMP_Invalid_URL_Post_Type::get_post_staleness( $invalid_url_post_id ) );
 	}
 
-	/**
-	 * Test filter_views_edit.
-	 *
-	 * @covers \AMP_Invalid_URL_Post_Type::filter_views_edit()
-	 */
-	public function test_filter_views_edit() {
-		$_SERVER['REQUEST_URI'] = '/wp-admin/edit.php?post_type=amp_invalid_url&amp_validation_error_status=0';
-
-		add_theme_support( 'amp', array( 'paired' => true ) );
-		AMP_Validation_Manager::init();
-		AMP_Validation_Error_Taxonomy::add_admin_hooks();
-		$post = $this->factory()->post->create();
-		$this->assertEmpty( AMP_Invalid_URL_Post_Type::get_invalid_url_validation_errors( get_permalink( $post ) ) );
-
-		add_filter( 'amp_validation_error_sanitized', function( $sanitized, $error ) {
-			if ( 'accepted' === $error['code'] ) {
-				$sanitized = true;
-			} elseif ( 'rejected' === $error['code'] ) {
-				$sanitized = false;
-			}
-			return $sanitized;
-		}, 10, 2 );
-
-		$errors = array(
-			array( 'code' => 'accepted' ),
-			array( 'code' => 'rejected' ),
-			array( 'code' => 'new' ),
-		);
-
-		$invalid_url_post_id = AMP_Invalid_URL_Post_Type::store_validation_errors(
-			$errors,
-			get_permalink( $post )
-		);
-		$this->assertNotInstanceOf( 'WP_Error', $invalid_url_post_id );
-
-		$views = AMP_Invalid_URL_Post_Type::filter_views_edit( array(
-			'publish' => 'Published',
-		) );
-
-		$this->assertArrayNotHasKey( 'publish', $views );
-		$this->assertContains( '(1)', $views['new'] );
-		$this->assertContains( '(1)', $views['rejected'] );
-		$this->assertContains( '(1)', $views['accepted'] );
-
-		$terms = get_terms( array( 'taxonomy' => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG ) );
-		foreach ( $terms as $term ) {
-			wp_update_term( $term->term_id, AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array(
-				'term_group' => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_ACCEPTED_STATUS,
-			) );
-		}
-
-		$views = AMP_Invalid_URL_Post_Type::filter_views_edit( array(
-			'publish' => 'Published',
-		) );
-		$this->assertArrayNotHasKey( 'publish', $views );
-		$this->assertContains( '(0)', $views['new'] );
-		$this->assertContains( '(0)', $views['rejected'] );
-		$this->assertContains( '(1)', $views['accepted'] );
-	}
-
 	/**
 	 * Test for add_post_columns()
 	 *
@@ -1135,6 +1075,55 @@ public function test_filter_the_title_in_post_list_table() {
 		$this->assertEquals( '/baz', AMP_Invalid_URL_Post_Type::filter_the_title_in_post_list_table( $title, $post_correct_post_type ) );
 	}
 
+	/**
+	 * Test render_post_filters.
+	 *
+	 * @covers \AMP_Validation_Error_Taxonomy::render_post_filters()
+	 */
+	public function test_render_post_filters() {
+		set_current_screen( 'edit.php' );
+		$number_of_errors = 20;
+		for ( $i = 0; $i < $number_of_errors; $i++ ) {
+			$invalid_url_post      = $this->factory()->post->create( array( 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) );
+			$validation_error_term = $this->factory()->term->create( array(
+				'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
+				'description' => wp_json_encode( $this->get_mock_errors() ),
+				'term_group'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS,
+			) );
+
+			// Associate the validation error term with a URL.
+			wp_set_post_terms(
+				$invalid_url_post,
+				$validation_error_term,
+				AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG
+			);
+		}
+
+		$new_error_count               = sprintf(
+			'New Errors <span class="count">(%d)</span>',
+			$number_of_errors
+		);
+		$correct_post_type             = AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG;
+		$wrong_post_type               = 'page';
+		$correct_which_second_argument = 'top';
+		$wrong_which_second_argument   = 'bottom';
+
+		// This has an incorrect post type as the first argument, so it should not output anything.
+		ob_start();
+		AMP_Invalid_URL_Post_Type::render_post_filters( $wrong_post_type, $correct_which_second_argument );
+		$this->assertEmpty( ob_get_clean() );
+
+		// This has an incorrect second argument, so again it should not output anything.
+		ob_start();
+		AMP_Invalid_URL_Post_Type::render_post_filters( $correct_post_type, $wrong_which_second_argument );
+		$this->assertEmpty( ob_get_clean() );
+
+		// This is now on the invalid URL post type edit.php screen, so it should output a <select> element.
+		ob_start();
+		AMP_Invalid_URL_Post_Type::render_post_filters( $correct_post_type, $correct_which_second_argument );
+		$this->assertContains( $new_error_count, ob_get_clean() );
+	}
+
 	/**
 	 * Test for get_recheck_url()
 	 *
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 956ebef212f..1d54a3e4baa 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -535,10 +535,19 @@ public function test_render_error_status_filter() {
 
 		$number_of_errors = 10;
 		for ( $i = 0; $i < $number_of_errors; $i++ ) {
-			$this->factory()->term->create( array(
+			$invalid_url_post      = $this->factory()->post->create( array( 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) );
+			$validation_error_term = $this->factory()->term->create( array(
 				'taxonomy'    => AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG,
 				'description' => wp_json_encode( $this->get_mock_error() ),
+				'term_group'  => AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_STATUS,
 			) );
+
+			// Associate the validation error term with a URL so that it appears in a query.
+			wp_set_post_terms(
+				$invalid_url_post,
+				$validation_error_term,
+				AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG
+			);
 		}
 
 		/*

From 228eb8138824a2f4ba4a8f8d3fbc25d11734223e Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Mon, 3 Sep 2018 14:06:33 -0500
Subject: [PATCH 16/29] Filter for error type on Errors by URL page

Mainly by using the method that Weston wrote:
filter_posts_where_for_validation_error_status()
Look for the query var of the error type.
And if that's present, add to the WHERE clause.
---
 .../class-amp-validation-error-taxonomy.php   | 81 +++++++++++++------
 ...st-class-amp-validation-error-taxonomy.php | 13 ++-
 2 files changed, 67 insertions(+), 27 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 4331fc68659..dcc95be3bb7 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -426,9 +426,10 @@ public static function get_validation_error_count( $args = array() ) {
 	}
 
 	/**
-	 * Add support for querying posts by amp_validation_error_status.
+	 * Add support for querying posts by amp_validation_error_status and by error type.
 	 *
 	 * Add recognition of amp_validation_error_status query var for amp_invalid_url post queries.
+	 * Also, conditionally filter for error type, like js_error or css_error.
 	 *
 	 * @see WP_Tax_Query::get_sql_for_clause()
 	 *
@@ -438,29 +439,54 @@ public static function get_validation_error_count( $args = array() ) {
 	 */
 	public static function filter_posts_where_for_validation_error_status( $where, WP_Query $query ) {
 		global $wpdb;
-		if (
-			in_array( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, (array) $query->get( 'post_type' ), true )
-			&&
-			is_numeric( $query->get( self::VALIDATION_ERROR_STATUS_QUERY_VAR ) )
-		) {
-			$where .= $wpdb->prepare(
-				" AND (
-					SELECT 1
-					FROM $wpdb->term_relationships
-					INNER JOIN $wpdb->term_taxonomy ON $wpdb->term_taxonomy.term_taxonomy_id = $wpdb->term_relationships.term_taxonomy_id
-					INNER JOIN $wpdb->terms ON $wpdb->terms.term_id = $wpdb->term_taxonomy.term_id
-					WHERE
-						$wpdb->term_taxonomy.taxonomy = %s
-						AND
-						$wpdb->term_relationships.object_id = $wpdb->posts.ID
-						AND
-						$wpdb->terms.term_group = %d
-					LIMIT 1
-				)",
-				self::TAXONOMY_SLUG,
-				$query->get( self::VALIDATION_ERROR_STATUS_QUERY_VAR )
-			);
+
+		// If the post type is not correct, return the $where clause unchanged.
+		if ( ! in_array( AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG, (array) $query->get( 'post_type' ), true ) ) {
+			return $where;
+		}
+
+		$error_status = $query->get( self::VALIDATION_ERROR_STATUS_QUERY_VAR );
+		$error_type   = $query->get( self::VALIDATION_ERROR_TYPE_QUERY_VAR );
+
+		/*
+		 * Selecting the 'All Statuses' <option> sends a value of '-1' to indicate that this should not filter.
+		 * So ensure that the value is_numeric(), and is not '-1'.
+		 */
+		$is_error_status_present = is_numeric( $error_status ) && self::NO_FILTER_VALUE !== intval( $error_status );
+		$is_error_type_present   = in_array( $error_type, array( self::HTML_ELEMENT_ERROR_TYPE, self::HTML_ATTRIBUTE_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE ), true );
+
+		// If neither the error status nor the type is present, there is no need to filter the $where clause.
+		if ( ! $is_error_status_present && ! $is_error_type_present ) {
+			return $where;
 		}
+
+		// If there is no valid error status query var, use the wildcard so that any status will match.
+		$error_status = $is_error_status_present ? $error_status : '*';
+
+		// If there is no valid error type query var, use %s so that anything will match.
+		$error_type = $is_error_type_present ? '%"type":"' . $wpdb->esc_like( $error_type ) . '"%' : '%';
+
+		$where .= $wpdb->prepare(
+			" AND (
+				SELECT 1
+				FROM $wpdb->term_relationships
+				INNER JOIN $wpdb->term_taxonomy ON $wpdb->term_taxonomy.term_taxonomy_id = $wpdb->term_relationships.term_taxonomy_id
+				INNER JOIN $wpdb->terms ON $wpdb->terms.term_id = $wpdb->term_taxonomy.term_id
+				WHERE
+					$wpdb->term_taxonomy.taxonomy = %s
+					AND
+					$wpdb->term_relationships.object_id = $wpdb->posts.ID
+					AND
+					$wpdb->terms.term_group = %s
+					AND
+					$wpdb->term_taxonomy.description LIKE %s
+				LIMIT 1
+			)",
+			self::TAXONOMY_SLUG,
+			$error_status,
+			$error_type
+		);
+
 		return $where;
 	}
 
@@ -545,7 +571,7 @@ public static function add_admin_hooks() {
 			return $query_vars;
 		} );
 
-		// Add recognition of amp_validation_error_status query var (which will only apply in admin since post type is not publicly_queryable).
+		// Add recognition of amp_validation_error_status and type query vars (which will only apply in admin since post type is not publicly_queryable).
 		add_filter( 'query_vars', function( $query_vars ) {
 			$query_vars[] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR;
 			$query_vars[] = AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR;
@@ -754,22 +780,25 @@ public static function render_taxonomy_filters( $taxonomy_name ) {
 	}
 
 	/**
-	 * Renders the error status filter <select> element that allows filtering by error status.
+	 * Renders the error status filter <select> element.
 	 *
 	 * There is a difference how the errors are counted, depending on which screen this is on.
 	 * For example: Accepted Errors (10).
 	 * This status filter <select> element is rendered on the validation error post page (Errors by URL),
 	 * and the validation error taxonomy page (Errors by Type).
-	 * On the taxonomy page, this simply needs to count the number of errors with a given type.
+	 * On the taxonomy page, this simply needs to count the number of terms with a given type.
 	 * On the post page, this needs to count the number of posts that have at least one error of a given type.
 	 */
 	public static function render_error_status_filter() {
 		$screen_base = get_current_screen()->base;
+
+		// The taxonomy page.
 		if ( 'edit-tags' === $screen_base ) {
 			$total_term_count    = self::get_validation_error_count();
 			$rejected_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_REJECTED_STATUS ) );
 			$accepted_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_ACCEPTED_STATUS ) );
 			$new_term_count      = $total_term_count - $rejected_term_count - $accepted_term_count;
+			// The post page.
 		} elseif ( 'edit' === $screen_base ) {
 			$args = array(
 				'post_type'              => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 1d54a3e4baa..96369604a27 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -276,12 +276,23 @@ public function test_filter_posts_where_for_validation_error_status() {
 		$this->assertEquals( $initial_where, AMP_Validation_Error_Taxonomy::filter_posts_where_for_validation_error_status( $initial_where, $wp_query ) );
 
 		// The entire conditional should now be true, so this should filter the WHERE clause.
-		$wp_query->set( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, 1 );
+		$error_status = 1;
+		$wp_query->set( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR, $error_status );
 		$filtered_where = AMP_Validation_Error_Taxonomy::filter_posts_where_for_validation_error_status( $initial_where, $wp_query );
 		$this->assertContains( 'SELECT 1', $filtered_where );
 		$this->assertContains( 'INNER JOIN', $filtered_where );
 		$this->assertContains( $wpdb->term_relationships, $filtered_where );
 		$this->assertContains( $wpdb->term_taxonomy, $filtered_where );
+		$this->assertContains( strval( $error_status ), $filtered_where );
+
+		// Now that there is a query var for error type, that should also appear in the filtered WHERE clause.
+		$error_type         = 'js_error';
+		$escaped_error_type = 'js\\\\_error';
+		$wp_query->set( AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_TYPE_QUERY_VAR, $error_type );
+		$filtered_where = AMP_Validation_Error_Taxonomy::filter_posts_where_for_validation_error_status( $initial_where, $wp_query );
+		$this->assertContains( 'SELECT 1', $filtered_where );
+		$this->assertContains( strval( $error_status ), $filtered_where );
+		$this->assertContains( $escaped_error_type, $filtered_where );
 	}
 
 	/**

From a567047205614cf269cceaa1f2a000cba2ca1caa Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Mon, 3 Sep 2018 16:25:23 -0500
Subject: [PATCH 17/29] Output the 'View errors by URL' link from the design

This is at the top of the taxonomy page (Errors by Type),
and links to the post page (Errors by URL).
---
 .../class-amp-validation-error-taxonomy.php   | 41 +++++++++++++++++--
 ...st-class-amp-validation-error-taxonomy.php | 27 ++++++++++++
 2 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index dcc95be3bb7..4c38e244c1c 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -78,6 +78,13 @@ class AMP_Validation_Error_Taxonomy {
 	 */
 	const NO_FILTER_VALUE = -1;
 
+	/**
+	 * The ID for the link to the Errors by URL.
+	 *
+	 * @var string
+	 */
+	const ID_LINK_ERRORS_BY_URL = 'link-errors-url';
+
 	/**
 	 * Validation code for an invalid element.
 	 *
@@ -549,6 +556,7 @@ public static function add_admin_hooks() {
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_group_terms_clauses_filter' ) );
 		add_action( 'load-edit-tags.php', array( __CLASS__, 'add_error_type_clauses_filter' ) );
 		add_action( sprintf( 'after-%s-table', self::TAXONOMY_SLUG ), array( __CLASS__, 'render_taxonomy_filters' ) );
+		add_action( sprintf( 'after-%s-table', self::TAXONOMY_SLUG ), array( __CLASS__, 'render_link_to_errors_by_url' ) );
 		add_action( 'load-edit-tags.php', function() {
 			add_filter( 'user_has_cap', array( __CLASS__, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ), 10, 3 );
 		} );
@@ -749,7 +757,7 @@ public static function add_error_type_clauses_filter() {
 	 * Outputs the taxonomy filter UI for this taxonomy type.
 	 *
 	 * Similar to what appears on /wp-admin/edit.php for posts and pages,
-	 * this outputs a <select> element to choose the error type,
+	 * this outputs <select> elements to choose the error status and type,
 	 * and a 'Filter' submit button that allows filtering for that type.
 	 * This allows viewing one type at a time, like only JS errors.
 	 *
@@ -772,13 +780,40 @@ public static function render_taxonomy_filters( $taxonomy_name ) {
 
 		<script>
 			( function ( $ ) {
-				// Move the filter after the 'Bulk Actions' <select>, as it looks like there's no way to do this with simply an action.
-				$( '#<?php echo $div_id; // WPCS: XSS OK. ?>' ).insertAfter( $( '.tablenav.top .bulkactions' ) );
+				$( function() {
+					// Move the filter after the 'Bulk Actions' <select>, as it looks like there's no way to do this with simply an action.
+					$( '#<?php echo $div_id; // WPCS: XSS OK. ?>' ).insertAfter( $( '.tablenav.top .bulkactions' ) );
+
+					// Move the link to 'View errors by URL' to after the heading, as there is no hook to output it there.
+					$( '#<?php echo self::ID_LINK_ERRORS_BY_URL; // WPCS: XSS OK. ?>' ).insertAfter( $( '.wp-heading-inline' ) );
+				} );
 			} )( jQuery );
 		</script>
 		<?php
 	}
 
+	/**
+	 * On the 'Errors by Type' taxonomy page, renders a link to the 'Errors by URL' page.
+	 *
+	 * @param string $taxonomy_name The name of the taxonomy.
+	 */
+	public static function render_link_to_errors_by_url( $taxonomy_name ) {
+		if ( self::TAXONOMY_SLUG !== $taxonomy_name ) {
+			return;
+		}
+
+		printf(
+			'<a href="%s" class="page-title-action" id="%s" style="margin-left: 1rem;">%s</a>',
+			esc_attr( add_query_arg(
+				'post_type',
+				AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
+				admin_url( 'edit.php' )
+			) ),
+			esc_attr( self::ID_LINK_ERRORS_BY_URL ),
+			esc_html__( 'View errors by URL', 'amp' )
+		);
+	}
+
 	/**
 	 * Renders the error status filter <select> element.
 	 *
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 96369604a27..b97e2c0c07e 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -360,6 +360,7 @@ public function test_add_admin_hooks() {
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_group_terms_clauses_filter' ) ) );
 		$this->assertEquals( 10, has_action( 'load-edit-tags.php', array( self::TESTED_CLASS, 'add_error_type_clauses_filter' ) ) );
 		$this->assertEquals( 10, has_action( sprintf( 'after-%s-table', AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'render_taxonomy_filters' ) ) ) );
+		$this->assertEquals( 10, has_action( sprintf( 'after-%s-table', AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG, array( self::TESTED_CLASS, 'render_link_to_errors_by_url' ) ) ) );
 		$this->assertEquals( 10, has_filter( 'user_has_cap', array( self::TESTED_CLASS, 'filter_user_has_cap_for_hiding_term_list_table_checkbox' ) ) );
 		$this->assertEquals( 10, has_filter( 'terms_clauses', array( self::TESTED_CLASS, 'filter_terms_clauses_for_description_search' ) ) );
 		$this->assertEquals( 10, has_action( 'admin_notices', array( self::TESTED_CLASS, 'add_admin_notices' ) ) );
@@ -521,6 +522,32 @@ public function test_render_taxonomy_filters() {
 		$this->assertContains( 'New Errors <span class="count">(2)</span>', ob_get_clean() );
 	}
 
+	/**
+	 * Test render_link_to_errors_by_url.
+	 *
+	 * @covers \AMP_Validation_Error_Taxonomy::render_link_to_errors_by_url()
+	 */
+	public function test_render_link_to_errors_by_url() {
+		// When passing the wrong $taxomomy argument, this should not render anything.
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_link_to_errors_by_url( 'category' );
+		$this->assertEmpty( ob_get_clean() );
+
+		// When passing the correct taxonomy, this should render the link.
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_link_to_errors_by_url( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
+		$output = ob_get_clean();
+		$this->assertContains( 'View errors by URL', $output );
+		$this->assertContains(
+			add_query_arg(
+				'post_type',
+				AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
+				admin_url( 'edit.php' )
+			),
+			$output
+		);
+	}
+
 	/**
 	 * Test render_error_status_filter.
 	 *

From 3bcfc036145f84a01f3fcee3aa5750327e50b36a Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Mon, 3 Sep 2018 17:13:56 -0500
Subject: [PATCH 18/29] Simplify add_term_filter_query_var()

Instead of an if block for most of the method,
simply return if the condition is false.
Then, the remaining if blocks aren't nested.
---
 .../class-amp-invalid-url-post-type.php       |  2 +-
 .../class-amp-validation-error-taxonomy.php   | 65 ++++++++++++-------
 2 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php
index 5a3c4071b96..00c3a3491f5 100644
--- a/includes/validation/class-amp-invalid-url-post-type.php
+++ b/includes/validation/class-amp-invalid-url-post-type.php
@@ -131,7 +131,7 @@ 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_action( 'restrict_manage_posts', array( __CLASS__, 'render_post_filters' ), 10, 2 );
 
 		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 );
diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 4c38e244c1c..fc9b69d0758 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -662,7 +662,7 @@ public static function add_admin_hooks() {
 	}
 
 	/**
-	 * Filter the term redirect location, to possibly add a query var to filter by term type.
+	 * Filter the term redirect URL, to possibly add query vars to filter by term status or type.
 	 *
 	 * On clicking the 'Filter' button on the 'AMP Validation Errors' taxonomy page,
 	 * edit-tags.php processes the POST request that this submits.
@@ -679,32 +679,47 @@ public static function add_admin_hooks() {
 	 */
 	public static function add_term_filter_query_var( $url, $tax ) {
 		if (
-			self::TAXONOMY_SLUG === $tax->name
+			self::TAXONOMY_SLUG !== $tax->name
+			||
+			! isset( $_POST['post_type'] ) // WPCS: CSRF OK.
+			||
+			AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG !== $_POST['post_type'] // WPCS: CSRF OK.
+		) {
+			return $url;
+		}
+
+		// If the error type query var is valid, pass it along in the redirect $url.
+		if (
+			isset( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) // WPCS: CSRF OK.
 			&&
-			isset( $_POST['post_type'] ) && AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG === $_POST['post_type'] // WPCS: CSRF OK.
+			in_array(
+				$_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ], // WPCS: CSRF OK.
+				array( self::HTML_ELEMENT_ERROR_TYPE, self::HTML_ATTRIBUTE_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE ),
+				true
+			)
 		) {
-			if ( isset( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) { // WPCS: CSRF OK.
-				$url = add_query_arg(
-					self::VALIDATION_ERROR_TYPE_QUERY_VAR,
-					sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ), // WPCS: CSRF OK.
-					$url
-				);
-			}
-			if (
-				isset( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) // WPCS: CSRF OK.
-				&&
-				in_array(
-					intval( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ), // WPCS: CSRF OK.
-					array( self::VALIDATION_ERROR_NEW_STATUS, self::VALIDATION_ERROR_ACCEPTED_STATUS, self::VALIDATION_ERROR_REJECTED_STATUS, self::NO_FILTER_VALUE ),
-					true
-				)
-			) {
-				$url = add_query_arg(
-					self::VALIDATION_ERROR_STATUS_QUERY_VAR,
-					sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ), // WPCS: CSRF OK.
-					$url
-				);
-			}
+			$url = add_query_arg(
+				self::VALIDATION_ERROR_TYPE_QUERY_VAR,
+				sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ), // WPCS: CSRF OK.
+				$url
+			);
+		}
+
+		// If the error status query var is valid, pass it along in the redirect $url.
+		if (
+			isset( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) // WPCS: CSRF OK.
+			&&
+			in_array(
+				intval( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ), // WPCS: CSRF OK.
+				array( self::VALIDATION_ERROR_NEW_STATUS, self::VALIDATION_ERROR_ACCEPTED_STATUS, self::VALIDATION_ERROR_REJECTED_STATUS, self::NO_FILTER_VALUE ),
+				true
+			)
+		) {
+			$url = add_query_arg(
+				self::VALIDATION_ERROR_STATUS_QUERY_VAR,
+				sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ), // WPCS: CSRF OK.
+				$url
+			);
 		}
 
 		return $url;

From 13a86487ab1f773a6bdafc244553a420940e3985 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Mon, 3 Sep 2018 17:32:00 -0500
Subject: [PATCH 19/29] Make the error status counts accurate on the Errors by
 URL page

Before, that didn't take account of the filter for error type
So if that filtered for js_error,
the total count of error status was the same as if it
were for 'All Error Types.'
So look for the query var for the error type,
and pass that to the WP_Query if it's valid.
---
 .../class-amp-validation-error-taxonomy.php   | 21 ++++++++++++++++---
 ...st-class-amp-validation-error-taxonomy.php | 12 +++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index fc9b69d0758..9d0979d9e62 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -460,7 +460,7 @@ public static function filter_posts_where_for_validation_error_status( $where, W
 		 * So ensure that the value is_numeric(), and is not '-1'.
 		 */
 		$is_error_status_present = is_numeric( $error_status ) && self::NO_FILTER_VALUE !== intval( $error_status );
-		$is_error_type_present   = in_array( $error_type, array( self::HTML_ELEMENT_ERROR_TYPE, self::HTML_ATTRIBUTE_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE ), true );
+		$is_error_type_present   = in_array( $error_type, self::get_error_types(), true );
 
 		// If neither the error status nor the type is present, there is no need to filter the $where clause.
 		if ( ! $is_error_status_present && ! $is_error_type_present ) {
@@ -694,7 +694,7 @@ public static function add_term_filter_query_var( $url, $tax ) {
 			&&
 			in_array(
 				$_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ], // WPCS: CSRF OK.
-				array( self::HTML_ELEMENT_ERROR_TYPE, self::HTML_ATTRIBUTE_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE ),
+				self::get_error_types(),
 				true
 			)
 		) {
@@ -756,7 +756,7 @@ public static function add_error_type_clauses_filter() {
 		}
 
 		$type = strval( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ); // WPCS: CSRF ok.
-		if ( ! in_array( $type, array( self::HTML_ELEMENT_ERROR_TYPE, self::HTML_ATTRIBUTE_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE ), true ) ) {
+		if ( ! in_array( $type, self::get_error_types(), true ) ) {
 			return;
 		}
 		add_filter( 'terms_clauses', function( $clauses, $taxonomies ) use ( $type ) {
@@ -840,6 +840,7 @@ public static function render_link_to_errors_by_url( $taxonomy_name ) {
 	 * On the post page, this needs to count the number of posts that have at least one error of a given type.
 	 */
 	public static function render_error_status_filter() {
+		global $wp_query;
 		$screen_base = get_current_screen()->base;
 
 		// The taxonomy page.
@@ -856,6 +857,11 @@ public static function render_error_status_filter() {
 				'update_post_term_cache' => false,
 			);
 
+			$error_type = $wp_query->get( self::VALIDATION_ERROR_TYPE_QUERY_VAR );
+			if ( $error_type && in_array( $error_type, self::get_error_types(), true ) ) {
+				$args[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] = $error_type;
+			}
+
 			$with_new_query = new WP_Query( array_merge(
 				$args,
 				array( self::VALIDATION_ERROR_STATUS_QUERY_VAR => self::VALIDATION_ERROR_NEW_STATUS )
@@ -934,6 +940,15 @@ public static function render_error_status_filter() {
 		<?php
 	}
 
+	/**
+	 * Gets all of the possible error types.
+	 *
+	 * @return array Error types.
+	 */
+	public static function get_error_types() {
+		return array( self::HTML_ELEMENT_ERROR_TYPE, self::HTML_ATTRIBUTE_ERROR_TYPE, self::JS_ERROR_TYPE, self::CSS_ERROR_TYPE );
+	}
+
 	/**
 	 * Renders the filter for error type.
 	 *
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index b97e2c0c07e..281c11e98e2 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -604,6 +604,18 @@ public function test_render_error_status_filter() {
 		);
 	}
 
+	/**
+	 * Test get_error_types.
+	 *
+	 * @covers \AMP_Validation_Error_Taxonomy::get_error_types()
+	 */
+	public function test_get_error_types() {
+		$this->assertEquals(
+			array( 'html_element_error', 'html_attribute_error', 'js_error', 'css_error' ),
+			AMP_Validation_Error_Taxonomy::get_error_types()
+		);
+	}
+
 	/**
 	 * Test render_error_type_filter.
 	 *

From 6c5c06df29c5b1a820cf5f4d1217abd1118318a7 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Mon, 3 Sep 2018 18:02:43 -0500
Subject: [PATCH 20/29] On the 'Errors by URL' page, use 'With New Errors'
 instead of 'New Errors'

This is how the text was when it was inside a link.
Check for the current screen,
and conditionally add the 'With'.
---
 .../class-amp-validation-error-taxonomy.php   | 68 ++++++++++++++-----
 ...st-class-amp-validation-error-taxonomy.php |  9 +++
 2 files changed, 61 insertions(+), 16 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 9d0979d9e62..b1674c0c423 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -843,14 +843,18 @@ public static function render_error_status_filter() {
 		global $wp_query;
 		$screen_base = get_current_screen()->base;
 
-		// The taxonomy page.
+
 		if ( 'edit-tags' === $screen_base ) {
+			// The taxonomy page.
+			$possible_with_text  = '';
 			$total_term_count    = self::get_validation_error_count();
 			$rejected_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_REJECTED_STATUS ) );
 			$accepted_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_ACCEPTED_STATUS ) );
 			$new_term_count      = $total_term_count - $rejected_term_count - $accepted_term_count;
-			// The post page.
+
 		} elseif ( 'edit' === $screen_base ) {
+			// The post page should have <option> text like 'With New Errors'.
+			$possible_with_text = esc_html__( 'With', 'amp' );
 			$args = array(
 				'post_type'              => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
 				'update_post_meta_cache' => false,
@@ -891,14 +895,15 @@ public static function render_error_status_filter() {
 			<?php
 			if ( $new_term_count ) :
 				$new_term_text = sprintf(
-					/* translators: %s: the new term count. */
+					/* translators: %s: possibly the word With, %s: the new term count. */
 					_nx(
-						'New Error <span class="count">(%s)</span>',
-						'New Errors <span class="count">(%s)</span>',
+						'%1$s New Error <span class="count">(%2$s)</span>',
+						'%1$s New Errors <span class="count">(%2$s)</span>',
 						$new_term_count,
 						'terms',
 						'amp'
 					),
+					$possible_with_text,
 					number_format_i18n( $new_term_count )
 				);
 				?>
@@ -907,14 +912,15 @@ public static function render_error_status_filter() {
 			<?php
 			if ( $accepted_term_count ) :
 				$accepted_term_text = sprintf(
-					/* translators: %s: the accepted term count. */
+					/* translators: %s: possibly the word With, %s: the accepted term count. */
 					_nx(
-						'Accepted Error <span class="count">(%s)</span>',
-						'Accepted Errors <span class="count">(%s)</span>',
+						'%1$s Accepted Error <span class="count">(%2$s)</span>',
+						'%1$s Accepted Errors <span class="count">(%2$s)</span>',
 						$accepted_term_count,
 						'terms',
 						'amp'
 					),
+					$possible_with_text,
 					number_format_i18n( $accepted_term_count )
 				);
 				?>
@@ -923,14 +929,15 @@ public static function render_error_status_filter() {
 			endif;
 			if ( $rejected_term_count ) :
 				$rejected_term_text = sprintf(
-					/* translators: %s: the rejected term count. */
+					/* translators: %s: possibly the word With, %s: the rejected term count. */
 					_nx(
-						'Rejected Error <span class="count">(%s)</span>',
-						'Rejected Errors <span class="count">(%s)</span>',
+						'%1$s Rejected Error <span class="count">(%2$s)</span>',
+						'%1$s Rejected Errors <span class="count">(%2$s)</span>',
 						$rejected_term_count,
 						'terms',
 						'amp'
 					),
+					$possible_with_text,
 					number_format_i18n( $rejected_term_count )
 				);
 				?>
@@ -957,15 +964,44 @@ public static function get_error_types() {
 	 */
 	public static function render_error_type_filter() {
 		$error_type_filter_value = isset( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
+		$screen_base             = get_current_screen()->base;
+
+		/*
+		 * On the 'Errors by URL' page, the <option> text should be different.
+		 * For example, it should be 'With JS Errors' instead of 'JS Errors'.
+		 */
+		$possible_with_text = 'edit' === $screen_base ? __( 'With', 'amp' ) : '';
 
 		?>
 		<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
 		<select name="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>">
-			<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Error Types', 'amp' ); ?></option>
-			<option value="<?php echo esc_attr( self::HTML_ELEMENT_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ELEMENT_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML (Element) Error', 'amp' ); ?></option>
-			<option value="<?php echo esc_attr( self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>><?php esc_html_e( 'HTML (Attribute) Error', 'amp' ); ?></option>
-			<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>><?php esc_html_e( 'JS Error', 'amp' ); ?></option>
-			<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>><?php esc_html_e( 'CSS Error', 'amp' ); ?></option>
+			<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>">
+				<?php esc_html_e( 'All Error Types', 'amp' ); ?>
+			</option>
+			<option value="<?php echo esc_attr( self::HTML_ELEMENT_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ELEMENT_ERROR_TYPE ); ?>>
+				<?php
+				/* translators: %s: possibly the word With */
+				echo esc_html( sprintf( __( '%s HTML (Element) Errors', 'amp' ), $possible_with_text ) );
+				?>
+			</option>
+			<option value="<?php echo esc_attr( self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>>
+				<?php
+				/* translators: %s: possibly the word With */
+				echo esc_html( sprintf( __( '%s HTML (Attribute) Errors', 'amp' ), $possible_with_text ) );
+				?>
+			</option>
+			<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>>
+				<?php
+				/* translators: %s: possibly the word With */
+				echo esc_html( sprintf( __( '%s JS Errors', 'amp' ), $possible_with_text ) );
+				?>
+			</option>
+			<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>>
+				<?php
+				/* translators: %s: possibly the word With */
+				echo esc_html( sprintf( __( '%s CSS Errors', 'amp' ), $possible_with_text ) );
+				?>
+			</option>
 		</select>
 		<?php
 	}
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 281c11e98e2..45dda31d2ad 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -658,6 +658,15 @@ public function test_render_error_type_filter() {
 			),
 			$markup
 		);
+
+		// On the edit-tags.php page, the <option> text should not have 'With', like 'With JS Errors'.
+		$this->assertNotContains( 'With', $markup );
+
+		// On the edit.php page (Errors by URL), the <option> text should have 'With', like 'With JS Errors'.
+		set_current_screen( 'edit.php' );
+		ob_start();
+		AMP_Validation_Error_Taxonomy::render_taxonomy_filters( AMP_Validation_Error_Taxonomy::TAXONOMY_SLUG );
+		$this->assertContains( 'With', ob_get_clean() );
 	}
 
 	/**

From 6c262f0b6ca2da1c3010ef13983fb7fe8997b981 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Mon, 3 Sep 2018 18:13:44 -0500
Subject: [PATCH 21/29] Remove extra empty line to address Travis failure

---
 includes/validation/class-amp-validation-error-taxonomy.php | 1 -
 1 file changed, 1 deletion(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index b1674c0c423..7696ddd8cd5 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -843,7 +843,6 @@ public static function render_error_status_filter() {
 		global $wp_query;
 		$screen_base = get_current_screen()->base;
 
-
 		if ( 'edit-tags' === $screen_base ) {
 			// The taxonomy page.
 			$possible_with_text  = '';

From 3694bb94eb3e0bee3b59233ec2ce9089cb9383de Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Mon, 3 Sep 2018 18:35:43 -0500
Subject: [PATCH 22/29] Fix an issue where filtering by 'All Error Types'
 wasn't reflected in redirect

Before, the filter for the redirect URL
did not accept -1,
which is for 'All Error Types' and 'All Statuses'.
This means that the URL sometimes did not redirect,
and it sometimes showed the same results as before.
---
 includes/validation/class-amp-validation-error-taxonomy.php | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 7696ddd8cd5..e69462a8be3 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -672,7 +672,7 @@ public static function add_admin_hooks() {
 	 * It then passes that value to the redirect URL as a query var,
 	 * So that the taxonomy page will be filtered for that error type.
 	 *
-	 * @see AMP_Validation_Error_Taxonomy::add_error_type_clauses_filter() for the filtering of the 'where' clause, based on that query var.
+	 * @see AMP_Validation_Error_Taxonomy::add_error_type_clauses_filter() for the filtering of the 'where' clause, based on the query vars.
 	 * @param string      $url The $url to redirect to.
 	 * @param WP_Taxonomy $tax The WP_Taxonomy object.
 	 * @return string The filtered URL.
@@ -694,7 +694,7 @@ public static function add_term_filter_query_var( $url, $tax ) {
 			&&
 			in_array(
 				$_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ], // WPCS: CSRF OK.
-				self::get_error_types(),
+				array_merge( self::get_error_types(), array( strval( self::NO_FILTER_VALUE ) ) ),
 				true
 			)
 		) {

From 8443fd878769a956a1ee1e12ca79e5cdf67ce7d4 Mon Sep 17 00:00:00 2001
From: Ryan Kienstra <kienstraryan@gmail.com>
Date: Mon, 3 Sep 2018 18:55:50 -0500
Subject: [PATCH 23/29] Improve inline documentation, address Travis error.

Make PHP DocBlocks more clear,
especially when they needed to be updated.
---
 .../class-amp-validation-error-taxonomy.php          | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index e69462a8be3..d45f6de3c9c 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -747,7 +747,7 @@ public static function add_group_terms_clauses_filter() {
 	}
 
 	/**
-	 * Filter amp_validation_error term query by type, like in the 'AMP Validation Errors' taxonomy page.
+	 * Filters amp_validation_error term query by type, like in the 'AMP Validation Errors' taxonomy page.
 	 * Allows viewing only a certain type at a time, like only JS errors.
 	 */
 	public static function add_error_type_clauses_filter() {
@@ -773,8 +773,7 @@ public static function add_error_type_clauses_filter() {
 	 *
 	 * Similar to what appears on /wp-admin/edit.php for posts and pages,
 	 * this outputs <select> elements to choose the error status and type,
-	 * and a 'Filter' submit button that allows filtering for that type.
-	 * This allows viewing one type at a time, like only JS errors.
+	 * and a 'Filter' submit button that filters for them.
 	 *
 	 * @param string $taxonomy_name The name of the taxonomy.
 	 */
@@ -796,10 +795,10 @@ public static function render_taxonomy_filters( $taxonomy_name ) {
 		<script>
 			( function ( $ ) {
 				$( function() {
-					// Move the filter after the 'Bulk Actions' <select>, as it looks like there's no way to do this with simply an action.
+					// Move the filter UI after the 'Bulk Actions' <select>, as it looks like there's no way to do this with only an action.
 					$( '#<?php echo $div_id; // WPCS: XSS OK. ?>' ).insertAfter( $( '.tablenav.top .bulkactions' ) );
 
-					// Move the link to 'View errors by URL' to after the heading, as there is no hook to output it there.
+					// Move the link to 'View errors by URL' to after the heading, as it also looks like there's no action for this.
 					$( '#<?php echo self::ID_LINK_ERRORS_BY_URL; // WPCS: XSS OK. ?>' ).insertAfter( $( '.wp-heading-inline' ) );
 				} );
 			} )( jQuery );
@@ -854,7 +853,7 @@ public static function render_error_status_filter() {
 		} elseif ( 'edit' === $screen_base ) {
 			// The post page should have <option> text like 'With New Errors'.
 			$possible_with_text = esc_html__( 'With', 'amp' );
-			$args = array(
+			$args               = array(
 				'post_type'              => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
 				'update_post_meta_cache' => false,
 				'update_post_term_cache' => false,
@@ -876,6 +875,7 @@ public static function render_error_status_filter() {
 				array( self::VALIDATION_ERROR_STATUS_QUERY_VAR => self::VALIDATION_ERROR_REJECTED_STATUS )
 			) );
 			$rejected_term_count = $with_rejected_query->found_posts;
+
 			$with_accepted_query = new WP_Query( array_merge(
 				$args,
 				array( self::VALIDATION_ERROR_STATUS_QUERY_VAR => self::VALIDATION_ERROR_ACCEPTED_STATUS )

From a13c91e888e74b8fe476161d99db8d3f87c1e7e7 Mon Sep 17 00:00:00 2001
From: Weston Ruter <weston@xwp.co>
Date: Tue, 4 Sep 2018 17:19:16 -0700
Subject: [PATCH 24/29] Skip showing status dropdown if there will be no
 options

---
 includes/validation/class-amp-validation-error-taxonomy.php | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index d45f6de3c9c..fe2cf49018d 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -887,6 +887,9 @@ public static function render_error_status_filter() {
 
 		$error_status_filter_value = isset( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ? intval( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) : ''; // WPCS: CSRF OK.
 
+		if ( 0 === $new_term_count && 0 === $accepted_term_count && 0 === $rejected_term_count ) {
+			return;
+		}
 		?>
 		<label for="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error status', 'amp' ); ?></label>
 		<select name="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>">

From 61c6095960e254957853f3fb4d8799d54c253b02 Mon Sep 17 00:00:00 2001
From: Weston Ruter <weston@xwp.co>
Date: Tue, 4 Sep 2018 17:20:03 -0700
Subject: [PATCH 25/29] Simplify logic conditional in render_post_filters

---
 includes/sanitizers/class-amp-base-sanitizer.php     |  2 +-
 .../validation/class-amp-invalid-url-post-type.php   | 12 +++---------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php
index 8c5d47094df..aba63e03f2b 100644
--- a/includes/sanitizers/class-amp-base-sanitizer.php
+++ b/includes/sanitizers/class-amp-base-sanitizer.php
@@ -463,7 +463,7 @@ public function prepare_validation_error( array $error = array(), array $data =
 			}
 			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[a-zA-Z]/', $node->nodeName ) ? AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE : AMP_Validation_Error_Taxonomy::HTML_ATTRIBUTE_ERROR_TYPE;
+				$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() ) {
diff --git a/includes/validation/class-amp-invalid-url-post-type.php b/includes/validation/class-amp-invalid-url-post-type.php
index 00c3a3491f5..f99504d336f 100644
--- a/includes/validation/class-amp-invalid-url-post-type.php
+++ b/includes/validation/class-amp-invalid-url-post-type.php
@@ -1366,16 +1366,10 @@ public static function filter_the_title_in_post_list_table( $title, $post ) {
 	 * @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
-		) {
-			return;
+		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();
 		}
-
-		AMP_Validation_Error_Taxonomy::render_error_status_filter();
-		AMP_Validation_Error_Taxonomy::render_error_type_filter();
 	}
 
 	/**

From 364944edac12365a9bc26170134d684f3441214c Mon Sep 17 00:00:00 2001
From: Weston Ruter <weston@xwp.co>
Date: Tue, 4 Sep 2018 17:47:34 -0700
Subject: [PATCH 26/29] Only add WHERE conditions when required in
 filter_posts_where_for_validation_error_status

---
 .../class-amp-validation-error-taxonomy.php   | 39 +++++++++++--------
 1 file changed, 22 insertions(+), 17 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index fe2cf49018d..f5b05949db1 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -467,14 +467,8 @@ public static function filter_posts_where_for_validation_error_status( $where, W
 			return $where;
 		}
 
-		// If there is no valid error status query var, use the wildcard so that any status will match.
-		$error_status = $is_error_status_present ? $error_status : '*';
-
-		// If there is no valid error type query var, use %s so that anything will match.
-		$error_type = $is_error_type_present ? '%"type":"' . $wpdb->esc_like( $error_type ) . '"%' : '%';
-
-		$where .= $wpdb->prepare(
-			" AND (
+		$sql_select = $wpdb->prepare(
+			"
 				SELECT 1
 				FROM $wpdb->term_relationships
 				INNER JOIN $wpdb->term_taxonomy ON $wpdb->term_taxonomy.term_taxonomy_id = $wpdb->term_relationships.term_taxonomy_id
@@ -483,17 +477,28 @@ public static function filter_posts_where_for_validation_error_status( $where, W
 					$wpdb->term_taxonomy.taxonomy = %s
 					AND
 					$wpdb->term_relationships.object_id = $wpdb->posts.ID
-					AND
-					$wpdb->terms.term_group = %s
-					AND
-					$wpdb->term_taxonomy.description LIKE %s
-				LIMIT 1
-			)",
-			self::TAXONOMY_SLUG,
-			$error_status,
-			$error_type
+			",
+			self::TAXONOMY_SLUG
 		);
 
+		if ( $is_error_status_present ) {
+			$sql_select .= $wpdb->prepare(
+				" AND $wpdb->terms.term_group = %s ",
+				$error_status
+			);
+		}
+
+		if ( $is_error_type_present ) {
+			$sql_select .= $wpdb->prepare(
+				" AND $wpdb->term_taxonomy.description LIKE %s ",
+				'%"type":"' . $wpdb->esc_like( $error_type ) . '"%'
+			);
+		}
+
+		$sql_select .= ' LIMIT 1 ';
+
+		$where .= " AND ( $sql_select ) ";
+
 		return $where;
 	}
 

From ea15a2efce89e79b98b40a326f79ac42361d684c Mon Sep 17 00:00:00 2001
From: Weston Ruter <weston@xwp.co>
Date: Tue, 4 Sep 2018 18:01:06 -0700
Subject: [PATCH 27/29] Fix translatability of strings which vary 'with'

---
 .../class-amp-validation-error-taxonomy.php   | 156 +++++++++++-------
 1 file changed, 97 insertions(+), 59 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index f5b05949db1..3883b6e1478 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -848,17 +848,13 @@ public static function render_error_status_filter() {
 		$screen_base = get_current_screen()->base;
 
 		if ( 'edit-tags' === $screen_base ) {
-			// The taxonomy page.
-			$possible_with_text  = '';
 			$total_term_count    = self::get_validation_error_count();
 			$rejected_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_REJECTED_STATUS ) );
 			$accepted_term_count = self::get_validation_error_count( array( 'group' => self::VALIDATION_ERROR_ACCEPTED_STATUS ) );
 			$new_term_count      = $total_term_count - $rejected_term_count - $accepted_term_count;
 
 		} elseif ( 'edit' === $screen_base ) {
-			// The post page should have <option> text like 'With New Errors'.
-			$possible_with_text = esc_html__( 'With', 'amp' );
-			$args               = array(
+			$args = array(
 				'post_type'              => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG,
 				'update_post_meta_cache' => false,
 				'update_post_term_cache' => false,
@@ -901,52 +897,91 @@ public static function render_error_status_filter() {
 			<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
 			<?php
 			if ( $new_term_count ) :
-				$new_term_text = sprintf(
-					/* translators: %s: possibly the word With, %s: the new term count. */
-					_nx(
-						'%1$s New Error <span class="count">(%2$s)</span>',
-						'%1$s New Errors <span class="count">(%2$s)</span>',
-						$new_term_count,
-						'terms',
-						'amp'
-					),
-					$possible_with_text,
-					number_format_i18n( $new_term_count )
-				);
+				if ( 'edit' === $screen_base ) {
+					$new_term_text = sprintf(
+						/* translators: %s: the new term count. */
+						_nx(
+							'With New Error <span class="count">(%s)</span>',
+							'With New Errors <span class="count">(%s)</span>',
+							$new_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $new_term_count )
+					);
+				} else {
+					$new_term_text = sprintf(
+						/* translators: %s: the new term count. */
+						_nx(
+							'New Error <span class="count">(%s)</span>',
+							'New Errors <span class="count">(%s)</span>',
+							$new_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $new_term_count )
+					);
+				}
 				?>
 				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php echo wp_kses_post( $new_term_text ); ?></option>
 			<?php endif; ?>
 			<?php
 			if ( $accepted_term_count ) :
-				$accepted_term_text = sprintf(
-					/* translators: %s: possibly the word With, %s: the accepted term count. */
-					_nx(
-						'%1$s Accepted Error <span class="count">(%2$s)</span>',
-						'%1$s Accepted Errors <span class="count">(%2$s)</span>',
-						$accepted_term_count,
-						'terms',
-						'amp'
-					),
-					$possible_with_text,
-					number_format_i18n( $accepted_term_count )
-				);
+				if ( 'edit' === $screen_base ) {
+					$accepted_term_text = sprintf(
+						/* translators: %s: the accepted term count. */
+						_nx(
+							'With Accepted Error <span class="count">(%s)</span>',
+							'With Accepted Errors <span class="count">(%s)</span>',
+							$accepted_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $accepted_term_count )
+					);
+				} else {
+					$accepted_term_text = sprintf(
+						/* translators: %s: the accepted term count. */
+						_nx(
+							'Accepted Error <span class="count">(%s)</span>',
+							'Accepted Errors <span class="count">(%s)</span>',
+							$accepted_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $accepted_term_count )
+					);
+				}
 				?>
 				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php echo wp_kses_post( $accepted_term_text ); ?></option>
 				<?php
 			endif;
 			if ( $rejected_term_count ) :
-				$rejected_term_text = sprintf(
-					/* translators: %s: possibly the word With, %s: the rejected term count. */
-					_nx(
-						'%1$s Rejected Error <span class="count">(%2$s)</span>',
-						'%1$s Rejected Errors <span class="count">(%2$s)</span>',
-						$rejected_term_count,
-						'terms',
-						'amp'
-					),
-					$possible_with_text,
-					number_format_i18n( $rejected_term_count )
-				);
+				if ( 'edit' === $screen_base ) {
+					$rejected_term_text = sprintf(
+						/* translators: %s: the rejected term count. */
+						_nx(
+							'With Rejected Error <span class="count">(%s)</span>',
+							'With Rejected Errors <span class="count">(%s)</span>',
+							$rejected_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $rejected_term_count )
+					);
+				} else {
+					$rejected_term_text = sprintf(
+						/* translators: %s: the rejected term count. */
+						_nx(
+							'Rejected Error <span class="count">(%s)</span>',
+							'Rejected Errors <span class="count">(%s)</span>',
+							$rejected_term_count,
+							'terms',
+							'amp'
+						),
+						number_format_i18n( $rejected_term_count )
+					);
+				}
 				?>
 				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php echo wp_kses_post( $rejected_term_text ); ?></option>
 			<?php endif; ?>
@@ -977,8 +1012,7 @@ public static function render_error_type_filter() {
 		 * On the 'Errors by URL' page, the <option> text should be different.
 		 * For example, it should be 'With JS Errors' instead of 'JS Errors'.
 		 */
-		$possible_with_text = 'edit' === $screen_base ? __( 'With', 'amp' ) : '';
-
+		$screen_base = get_current_screen()->base;
 		?>
 		<label for="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error type', 'amp' ); ?></label>
 		<select name="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_TYPE_QUERY_VAR ); ?>">
@@ -986,28 +1020,32 @@ public static function render_error_type_filter() {
 				<?php esc_html_e( 'All Error Types', 'amp' ); ?>
 			</option>
 			<option value="<?php echo esc_attr( self::HTML_ELEMENT_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ELEMENT_ERROR_TYPE ); ?>>
-				<?php
-				/* translators: %s: possibly the word With */
-				echo esc_html( sprintf( __( '%s HTML (Element) Errors', 'amp' ), $possible_with_text ) );
-				?>
+				<?php if ( 'edit' === $screen_base ) : ?>
+					<?php esc_html_e( 'With HTML (Element) Errors', 'amp' ); ?>
+				<?php else : ?>
+					<?php esc_html_e( 'HTML (Element) Errors', 'amp' ); ?>
+				<?php endif; ?>
 			</option>
 			<option value="<?php echo esc_attr( self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::HTML_ATTRIBUTE_ERROR_TYPE ); ?>>
-				<?php
-				/* translators: %s: possibly the word With */
-				echo esc_html( sprintf( __( '%s HTML (Attribute) Errors', 'amp' ), $possible_with_text ) );
-				?>
+				<?php if ( 'edit' === $screen_base ) : ?>
+					<?php esc_html_e( 'With HTML (Attribute) Errors', 'amp' ); ?>
+				<?php else : ?>
+					<?php esc_html_e( 'HTML (Attribute) Errors', 'amp' ); ?>
+				<?php endif; ?>
 			</option>
 			<option value="<?php echo esc_attr( self::JS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::JS_ERROR_TYPE ); ?>>
-				<?php
-				/* translators: %s: possibly the word With */
-				echo esc_html( sprintf( __( '%s JS Errors', 'amp' ), $possible_with_text ) );
-				?>
+				<?php if ( 'edit' === $screen_base ) : ?>
+					<?php esc_html_e( 'With JS Errors', 'amp' ); ?>
+				<?php else : ?>
+					<?php esc_html_e( 'JS Errors', 'amp' ); ?>
+				<?php endif; ?>
 			</option>
 			<option value="<?php echo esc_attr( self::CSS_ERROR_TYPE ); ?>" <?php selected( $error_type_filter_value, self::CSS_ERROR_TYPE ); ?>>
-				<?php
-				/* translators: %s: possibly the word With */
-				echo esc_html( sprintf( __( '%s CSS Errors', 'amp' ), $possible_with_text ) );
-				?>
+				<?php if ( 'edit' === $screen_base ) : ?>
+					<?php esc_html_e( 'With CSS Errors', 'amp' ); ?>
+				<?php else : ?>
+					<?php esc_html_e( 'CSS Errors', 'amp' ); ?>
+				<?php endif; ?>
 			</option>
 		</select>
 		<?php

From 2882262cd2792309a2a2052e48a20ca572befd11 Mon Sep 17 00:00:00 2001
From: Weston Ruter <weston@xwp.co>
Date: Tue, 4 Sep 2018 18:24:46 -0700
Subject: [PATCH 28/29] Harden reading of input vars

---
 .../class-amp-validation-error-taxonomy.php     | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 3883b6e1478..8eea7b798da 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -452,8 +452,8 @@ public static function filter_posts_where_for_validation_error_status( $where, W
 			return $where;
 		}
 
-		$error_status = $query->get( self::VALIDATION_ERROR_STATUS_QUERY_VAR );
-		$error_type   = $query->get( self::VALIDATION_ERROR_TYPE_QUERY_VAR );
+		$error_status = sanitize_key( $query->get( self::VALIDATION_ERROR_STATUS_QUERY_VAR ) );
+		$error_type   = sanitize_key( $query->get( self::VALIDATION_ERROR_TYPE_QUERY_VAR ) );
 
 		/*
 		 * Selecting the 'All Statuses' <option> sends a value of '-1' to indicate that this should not filter.
@@ -705,7 +705,7 @@ public static function add_term_filter_query_var( $url, $tax ) {
 		) {
 			$url = add_query_arg(
 				self::VALIDATION_ERROR_TYPE_QUERY_VAR,
-				sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ), // WPCS: CSRF OK.
+				sanitize_key( wp_unslash( $_POST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ), // WPCS: CSRF OK.
 				$url
 			);
 		}
@@ -722,7 +722,7 @@ public static function add_term_filter_query_var( $url, $tax ) {
 		) {
 			$url = add_query_arg(
 				self::VALIDATION_ERROR_STATUS_QUERY_VAR,
-				sanitize_text_field( wp_unslash( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ), // WPCS: CSRF OK.
+				intval( $_POST[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ), // WPCS: CSRF OK.
 				$url
 			);
 		}
@@ -752,7 +752,7 @@ public static function add_group_terms_clauses_filter() {
 	}
 
 	/**
-	 * Filters amp_validation_error term query by type, like in the 'AMP Validation Errors' taxonomy page.
+	 * Adds filter for amp_validation_error term query by type, like in the 'AMP Validation Errors' taxonomy page.
 	 * Allows viewing only a certain type at a time, like only JS errors.
 	 */
 	public static function add_error_type_clauses_filter() {
@@ -760,7 +760,7 @@ public static function add_error_type_clauses_filter() {
 			return;
 		}
 
-		$type = strval( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ); // WPCS: CSRF ok.
+		$type = sanitize_key( wp_unslash( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ); // WPCS: CSRF ok.
 		if ( ! in_array( $type, self::get_error_types(), true ) ) {
 			return;
 		}
@@ -860,7 +860,7 @@ public static function render_error_status_filter() {
 				'update_post_term_cache' => false,
 			);
 
-			$error_type = $wp_query->get( self::VALIDATION_ERROR_TYPE_QUERY_VAR );
+			$error_type = sanitize_key( $wp_query->get( self::VALIDATION_ERROR_TYPE_QUERY_VAR ) );
 			if ( $error_type && in_array( $error_type, self::get_error_types(), true ) ) {
 				$args[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] = $error_type;
 			}
@@ -1005,8 +1005,7 @@ public static function get_error_types() {
 	 * and the validation error taxonomy page (Errors by Type).
 	 */
 	public static function render_error_type_filter() {
-		$error_type_filter_value = isset( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_text_field( wp_unslash( $_REQUEST[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
-		$screen_base             = get_current_screen()->base;
+		$error_type_filter_value = isset( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ? sanitize_key( wp_unslash( $_GET[ self::VALIDATION_ERROR_TYPE_QUERY_VAR ] ) ) : ''; // WPCS: CSRF OK.
 
 		/*
 		 * On the 'Errors by URL' page, the <option> text should be different.

From 3b87595f1fdadcaf945b93316e281c7f128b91c7 Mon Sep 17 00:00:00 2001
From: Weston Ruter <weston@xwp.co>
Date: Tue, 4 Sep 2018 18:31:17 -0700
Subject: [PATCH 29/29] Opt to output all status options even when empty to
 prevent de-selection when changing selected error type

---
 .../class-amp-validation-error-taxonomy.php   | 173 +++++++++---------
 ...st-class-amp-validation-error-taxonomy.php |   8 -
 2 files changed, 82 insertions(+), 99 deletions(-)

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index 8eea7b798da..f02c25ea56e 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -888,103 +888,94 @@ public static function render_error_status_filter() {
 
 		$error_status_filter_value = isset( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) ? intval( $_GET[ self::VALIDATION_ERROR_STATUS_QUERY_VAR ] ) : ''; // WPCS: CSRF OK.
 
-		if ( 0 === $new_term_count && 0 === $accepted_term_count && 0 === $rejected_term_count ) {
-			return;
-		}
 		?>
 		<label for="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" class="screen-reader-text"><?php esc_html_e( 'Filter by error status', 'amp' ); ?></label>
 		<select name="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>" id="<?php echo esc_attr( self::VALIDATION_ERROR_STATUS_QUERY_VAR ); ?>">
 			<option value="<?php echo esc_attr( self::NO_FILTER_VALUE ); ?>"><?php esc_html_e( 'All Statuses', 'amp' ); ?></option>
 			<?php
-			if ( $new_term_count ) :
-				if ( 'edit' === $screen_base ) {
-					$new_term_text = sprintf(
-						/* translators: %s: the new term count. */
-						_nx(
-							'With New Error <span class="count">(%s)</span>',
-							'With New Errors <span class="count">(%s)</span>',
-							$new_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $new_term_count )
-					);
-				} else {
-					$new_term_text = sprintf(
-						/* translators: %s: the new term count. */
-						_nx(
-							'New Error <span class="count">(%s)</span>',
-							'New Errors <span class="count">(%s)</span>',
-							$new_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $new_term_count )
-					);
-				}
-				?>
-				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php echo wp_kses_post( $new_term_text ); ?></option>
-			<?php endif; ?>
+			if ( 'edit' === $screen_base ) {
+				$new_term_text = sprintf(
+					/* translators: %s: the new term count. */
+					_nx(
+						'With New Error <span class="count">(%s)</span>',
+						'With New Errors <span class="count">(%s)</span>',
+						$new_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $new_term_count )
+				);
+			} else {
+				$new_term_text = sprintf(
+					/* translators: %s: the new term count. */
+					_nx(
+						'New Error <span class="count">(%s)</span>',
+						'New Errors <span class="count">(%s)</span>',
+						$new_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $new_term_count )
+				);
+			}
+			?>
+			<option value="<?php echo esc_attr( self::VALIDATION_ERROR_NEW_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_NEW_STATUS ); ?>><?php echo wp_kses_post( $new_term_text ); ?></option>
 			<?php
-			if ( $accepted_term_count ) :
-				if ( 'edit' === $screen_base ) {
-					$accepted_term_text = sprintf(
-						/* translators: %s: the accepted term count. */
-						_nx(
-							'With Accepted Error <span class="count">(%s)</span>',
-							'With Accepted Errors <span class="count">(%s)</span>',
-							$accepted_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $accepted_term_count )
-					);
-				} else {
-					$accepted_term_text = sprintf(
-						/* translators: %s: the accepted term count. */
-						_nx(
-							'Accepted Error <span class="count">(%s)</span>',
-							'Accepted Errors <span class="count">(%s)</span>',
-							$accepted_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $accepted_term_count )
-					);
-				}
-				?>
-				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php echo wp_kses_post( $accepted_term_text ); ?></option>
-				<?php
-			endif;
-			if ( $rejected_term_count ) :
-				if ( 'edit' === $screen_base ) {
-					$rejected_term_text = sprintf(
-						/* translators: %s: the rejected term count. */
-						_nx(
-							'With Rejected Error <span class="count">(%s)</span>',
-							'With Rejected Errors <span class="count">(%s)</span>',
-							$rejected_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $rejected_term_count )
-					);
-				} else {
-					$rejected_term_text = sprintf(
-						/* translators: %s: the rejected term count. */
-						_nx(
-							'Rejected Error <span class="count">(%s)</span>',
-							'Rejected Errors <span class="count">(%s)</span>',
-							$rejected_term_count,
-							'terms',
-							'amp'
-						),
-						number_format_i18n( $rejected_term_count )
-					);
-				}
-				?>
-				<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php echo wp_kses_post( $rejected_term_text ); ?></option>
-			<?php endif; ?>
+			if ( 'edit' === $screen_base ) {
+				$accepted_term_text = sprintf(
+					/* translators: %s: the accepted term count. */
+					_nx(
+						'With Accepted Error <span class="count">(%s)</span>',
+						'With Accepted Errors <span class="count">(%s)</span>',
+						$accepted_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $accepted_term_count )
+				);
+			} else {
+				$accepted_term_text = sprintf(
+					/* translators: %s: the accepted term count. */
+					_nx(
+						'Accepted Error <span class="count">(%s)</span>',
+						'Accepted Errors <span class="count">(%s)</span>',
+						$accepted_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $accepted_term_count )
+				);
+			}
+			?>
+			<option value="<?php echo esc_attr( self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_ACCEPTED_STATUS ); ?>><?php echo wp_kses_post( $accepted_term_text ); ?></option>
+			<?php
+			if ( 'edit' === $screen_base ) {
+				$rejected_term_text = sprintf(
+					/* translators: %s: the rejected term count. */
+					_nx(
+						'With Rejected Error <span class="count">(%s)</span>',
+						'With Rejected Errors <span class="count">(%s)</span>',
+						$rejected_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $rejected_term_count )
+				);
+			} else {
+				$rejected_term_text = sprintf(
+					/* translators: %s: the rejected term count. */
+					_nx(
+						'Rejected Error <span class="count">(%s)</span>',
+						'Rejected Errors <span class="count">(%s)</span>',
+						$rejected_term_count,
+						'terms',
+						'amp'
+					),
+					number_format_i18n( $rejected_term_count )
+				);
+			}
+			?>
+			<option value="<?php echo esc_attr( self::VALIDATION_ERROR_REJECTED_STATUS ); ?>" <?php selected( $error_status_filter_value, self::VALIDATION_ERROR_REJECTED_STATUS ); ?>><?php echo wp_kses_post( $rejected_term_text ); ?></option>
 		</select>
 		<?php
 	}
diff --git a/tests/validation/test-class-amp-validation-error-taxonomy.php b/tests/validation/test-class-amp-validation-error-taxonomy.php
index 45dda31d2ad..775240046dd 100644
--- a/tests/validation/test-class-amp-validation-error-taxonomy.php
+++ b/tests/validation/test-class-amp-validation-error-taxonomy.php
@@ -562,15 +562,7 @@ public function test_render_error_status_filter() {
 		AMP_Validation_Error_Taxonomy::render_error_status_filter();
 		$this->assertEmpty( ob_get_clean() );
 
-		/*
-		 * This is now on the correct screen, so it shouldn't be empty anymore.
-		 * When no validation error exists, there should not be an <option> for any specific error status, like 'New Error'.
-		 */
 		set_current_screen( 'edit.php' );
-		ob_start();
-		AMP_Validation_Error_Taxonomy::render_error_status_filter();
-		$this->assertNotContains( 'New Error', ob_get_clean() );
-
 		$number_of_errors = 10;
 		for ( $i = 0; $i < $number_of_errors; $i++ ) {
 			$invalid_url_post      = $this->factory()->post->create( array( 'post_type' => AMP_Invalid_URL_Post_Type::POST_TYPE_SLUG ) );