Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add filters on taxonomy and post page #1373

Merged
merged 30 commits into from
Sep 5, 2018
Merged

Add filters on taxonomy and post page #1373

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0017edf
Add a 'type' to all validation errors
kienstra Aug 28, 2018
3e9a81e
If an on* attribute is removed, make type js_error
kienstra Aug 28, 2018
0ae789d
In unit test assertions, change to js_error
kienstra Aug 28, 2018
3433791
Add a query var to filter by type
kienstra Aug 29, 2018
8dd3f01
Filter the term redirect location, to possibly add a query var
kienstra Aug 29, 2018
5f92cc2
Render the taxonomy filter for this taxonomy type
kienstra Aug 29, 2018
3d17734
Add a filter <select> element for error type, like 'Accepted'
kienstra Aug 29, 2018
c2656a3
Remove views for filtering taxonomy, but reuse their counts
kienstra Aug 29, 2018
fe43d31
Add the option value for 'All Statuses' to the whitelist
kienstra Aug 29, 2018
1e135d2
Use the new terminology for the error taxonomy page name
kienstra Aug 29, 2018
f1d4bea
Address failed unit tests in Travis build
kienstra Aug 29, 2018
caa2a26
Restore error counts by status in <option>
kienstra Aug 30, 2018
7b937b1
Merge branch 'develop' into add/1360-taxonomy-error-type
kienstra Aug 30, 2018
13d5caf
Add error types for HTML (Element) and HTML (Attribute)
kienstra Aug 30, 2018
b74854c
Refactor logic in render_taxonomy_filters() into 2 methods
kienstra Aug 31, 2018
0397a52
Add filters on the 'Errors by URL' page
kienstra Aug 31, 2018
228eb81
Filter for error type on Errors by URL page
kienstra Sep 3, 2018
a567047
Output the 'View errors by URL' link from the design
kienstra Sep 3, 2018
3bcfc03
Simplify add_term_filter_query_var()
kienstra Sep 3, 2018
13a8648
Make the error status counts accurate on the Errors by URL page
kienstra Sep 3, 2018
6c5c06d
On the 'Errors by URL' page, use 'With New Errors' instead of 'New Er…
kienstra Sep 3, 2018
6c262f0
Remove extra empty line to address Travis failure
kienstra Sep 3, 2018
3694bb9
Fix an issue where filtering by 'All Error Types' wasn't reflected in…
kienstra Sep 3, 2018
8443fd8
Improve inline documentation, address Travis error.
kienstra Sep 3, 2018
a13c91e
Skip showing status dropdown if there will be no options
westonruter Sep 5, 2018
61c6095
Simplify logic conditional in render_post_filters
westonruter Sep 5, 2018
364944e
Only add WHERE conditions when required in filter_posts_where_for_val…
westonruter Sep 5, 2018
ea15a2e
Fix translatability of strings which vary 'with'
westonruter Sep 5, 2018
2882262
Harden reading of input vars
westonruter Sep 5, 2018
3b87595
Opt to output all status options even when empty to prevent de-select…
westonruter Sep 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions includes/sanitizers/class-amp-base-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also needs to be a js_error if it is an invalidate attribute and the attribute starts with on.

Copy link
Contributor Author

@kienstra kienstra Aug 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. 3e9a81e sets a js_error for on* attributes:

type-js-error

}

$error['node_attributes'] = array();
foreach ( $node->attributes as $attribute ) {
$error['node_attributes'][ $attribute->nodeName ] = $attribute->nodeValue;
Expand All @@ -456,6 +461,10 @@ public function prepare_validation_error( array $error = array(), array $data =
if ( ! isset( $error['code'] ) ) {
$error['code'] = AMP_Validation_Error_Taxonomy::INVALID_ATTRIBUTE_CODE;
}
if ( ! isset( $error['type'] ) ) {
// If this is an attribute that begins with on, like onclick, it should be a js_error.
$error['type'] = preg_match( '/^on[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() ) {
foreach ( $node->parentNode->attributes as $attribute ) {
Expand Down
19 changes: 19 additions & 0 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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 ) {
Expand All @@ -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 ) {
Expand Down Expand Up @@ -988,6 +993,7 @@ function ( $value ) {
$error = array(
'code' => 'css_parse_error',
'message' => $exception->getMessage(),
'type' => AMP_Validation_Error_Taxonomy::CSS_ERROR_TYPE,
);

/*
Expand Down Expand Up @@ -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' );
Expand All @@ -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' );
Expand All @@ -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' );
Expand All @@ -1290,13 +1299,15 @@ 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' );
} else {
$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' );
Expand Down Expand Up @@ -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 ) {
Expand All @@ -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 ) {
Expand Down Expand Up @@ -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 ) {
Expand All @@ -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 ) {
Expand Down Expand Up @@ -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 ) {
Expand Down Expand Up @@ -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' );
Expand Down Expand Up @@ -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,
) );
}

Expand Down Expand Up @@ -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'];
Expand Down
25 changes: 25 additions & 0 deletions includes/validation/class-amp-validation-error-taxonomy.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
4 changes: 4 additions & 0 deletions tests/test-class-amp-base-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ public function test_remove_invalid_child() {
),
'foo' => 'bar',
'sources' => null,
'type' => AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE,
);

// Test sanitized.
Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -294,6 +296,7 @@ public function test_remove_invalid_attribute() {
'id' => 'bar',
'onload' => 'someFunc()',
),
'type' => AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE,
),
$error
);
Expand All @@ -320,6 +323,7 @@ public function test_remove_invalid_attribute() {
'id' => 'bar',
'onload' => 'someFunc()',
),
'type' => AMP_Validation_Error_Taxonomy::JS_ERROR_TYPE,
),
$error
);
Expand Down
3 changes: 2 additions & 1 deletion tests/test-tag-and-attribute-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down