Skip to content

Commit b0ae412

Browse files
westonruterwestonruter
committed
Improve validation error details for invalid properties; improve tests
1 parent 8cf95cd commit b0ae412

File tree

3 files changed

+48
-7
lines changed

3 files changed

+48
-7
lines changed

‎includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -721,7 +721,8 @@ static function ( $validation_error ) {
721721
$node->setAttribute( $attr_node->nodeName, $this->serialize_properties_attribute( $properties ) );
722722
}
723723
} elseif ( self::MISSING_MANDATORY_PROPERTY === $error_code ) {
724-
$validation_error['meta_property_name'] = $error_data['name'];
724+
$validation_error['meta_property_name'] = $error_data['name'];
725+
$validation_error['meta_property_required_value'] = $error_data['required_value'];
725726
if ( $this->should_sanitize_validation_error( $validation_error, [ 'node' => $attr_node ] ) ) {
726727
$properties = array_merge(
727728
$this->parse_properties_attribute( $attr_node->nodeValue ),

‎includes/validation/class-amp-validation-error-taxonomy.php‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2102,6 +2102,9 @@ public static function render_single_url_error_details( $validation_error, $term
21022102
</p>
21032103
</dd>
21042104

2105+
<dt><?php esc_html_e( 'Error code', 'amp' ); ?></dt>
2106+
<dd><code><?php echo esc_html( $validation_error['code'] ); ?></code></dd>
2107+
21052108
<?php if ( AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG === $validation_error['code'] && isset( $validation_error['node_attributes'] ) ) : ?>
21062109
<dt><?php esc_html_e( 'Invalid markup', 'amp' ); ?></dt>
21072110
<dd class="detailed">
@@ -2144,7 +2147,7 @@ public static function render_single_url_error_details( $validation_error, $term
21442147
if ( $is_element_attributes && empty( $value ) ) {
21452148
continue;
21462149
}
2147-
if ( in_array( $key, [ 'code', 'type', 'css_property_value', 'mandatory_anyof_attrs', 'mandatory_oneof_attrs' ], true ) ) {
2150+
if ( in_array( $key, [ 'code', 'type', 'css_property_value', 'mandatory_anyof_attrs', 'meta_property_value', 'meta_property_required_value', 'mandatory_oneof_attrs' ], true ) ) {
21482151
continue; // Handled above.
21492152
}
21502153
if ( 'spec_name' === $key ) {
@@ -2167,6 +2170,20 @@ public static function render_single_url_error_details( $validation_error, $term
21672170
printf( '<code>%s</code>', esc_html( $value ) );
21682171
}
21692172
?>
2173+
<?php elseif ( 'meta_property_name' === $key ) : ?>
2174+
<?php
2175+
printf( '<code>%s</code>', esc_html( $value ) );
2176+
if ( isset( $validation_error['meta_property_value'] ) ) {
2177+
printf( ': <code>%s</code>', esc_html( $validation_error['meta_property_value'] ) );
2178+
}
2179+
if ( isset( $validation_error['meta_property_required_value'] ) ) {
2180+
printf(
2181+
' (%s: <code>%s</code>)',
2182+
esc_html__( 'required', 'amp' ),
2183+
esc_html( $validation_error['meta_property_required_value'] )
2184+
);
2185+
}
2186+
?>
21702187
<?php elseif ( 'text' === $key ) : ?>
21712188
<details>
21722189
<summary>
@@ -2992,7 +3009,7 @@ public static function get_error_title_from_code( $validation_error ) {
29923009
case AMP_Tag_And_Attribute_Sanitizer::MISSING_REQUIRED_PROPERTY_VALUE:
29933010
$title = sprintf(
29943011
/* translators: %1$s is the property name, %2$s is the value for the property */
2995-
wp_kses( __( 'Missing required value for <code>%1$s</code> property: <code>%2$s</code>', 'amp' ), [ 'code' => '' ] ),
3012+
wp_kses( __( 'Invalid value for <code>%1$s</code> property: <code>%2$s</code>', 'amp' ), [ 'code' => '' ] ),
29963013
esc_html( $validation_error['meta_property_name'] ),
29973014
esc_html( $validation_error['meta_property_value'] )
29983015
);

‎tests/php/test-tag-and-attribute-sanitizer.php‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2329,10 +2329,10 @@ public function get_html_data() {
23292329
[ AMP_Tag_And_Attribute_Sanitizer::INVALID_ATTR_VALUE_REGEX, AMP_Tag_And_Attribute_Sanitizer::ATTR_REQUIRED_BUT_MISSING ],
23302330
],
23312331
'bad_meta_ua_compatible' => [
2332-
'<html amp><head><meta charset="utf-8"><meta http-equiv="X-UA-Compatible" content="IE=9,chrome=1"></head><body></body></html>',
2332+
'<html amp><head><meta charset="utf-8"><meta http-equiv="X-UA-Compatible" content="IE=6,chrome=1,netscape=4"></head><body></body></html>',
23332333
'<html amp><head><meta charset="utf-8"><meta http-equiv="X-UA-Compatible" content="chrome=1"></head><body></body></html>',
23342334
[],
2335-
[ AMP_Tag_And_Attribute_Sanitizer::MISSING_REQUIRED_PROPERTY_VALUE ],
2335+
[ AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_PROPERTY_IN_ATTR_VALUE, AMP_Tag_And_Attribute_Sanitizer::MISSING_REQUIRED_PROPERTY_VALUE ],
23362336
],
23372337
'bad_meta_width_property' => [
23382338
'<html amp><head><meta charset="utf-8"><meta name="viewport" content="width=600, initial-scale=1.0"></head><body></body></html>',
@@ -2344,13 +2344,36 @@ public function get_html_data() {
23442344
'<html amp><head><meta charset="utf-8"><meta name="viewport" content="width=600, initial-scale=1.0, bad=yes"></head><body></body></html>',
23452345
'<html amp><head><meta charset="utf-8"><meta name="viewport" content="width=device-width,initial-scale=1.0"></head><body></body></html>',
23462346
[],
2347-
[ AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_PROPERTY_IN_ATTR_VALUE, AMP_Tag_And_Attribute_Sanitizer::MISSING_REQUIRED_PROPERTY_VALUE ],
2347+
[
2348+
[
2349+
'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_PROPERTY_IN_ATTR_VALUE,
2350+
'meta_property_name' => 'bad',
2351+
'meta_property_value' => 'yes',
2352+
],
2353+
[
2354+
'code' => AMP_Tag_And_Attribute_Sanitizer::MISSING_REQUIRED_PROPERTY_VALUE,
2355+
'meta_property_name' => 'width',
2356+
'meta_property_value' => '600',
2357+
'meta_property_required_value' => 'device-width',
2358+
],
2359+
],
23482360
],
23492361
'missing_meta_width_and_unknown_property' => [
23502362
'<html amp><head><meta charset="utf-8"><meta name="viewport" content="initial-scale=1.0, bad=yes"></head><body></body></html>',
23512363
'<html amp><head><meta charset="utf-8"><meta name="viewport" content="initial-scale=1.0,width=device-width"></head><body></body></html>',
23522364
[],
2353-
[ AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_PROPERTY_IN_ATTR_VALUE, AMP_Tag_And_Attribute_Sanitizer::MISSING_MANDATORY_PROPERTY ],
2365+
[
2366+
[
2367+
'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_PROPERTY_IN_ATTR_VALUE,
2368+
'meta_property_name' => 'bad',
2369+
'meta_property_value' => 'yes',
2370+
],
2371+
[
2372+
'code' => AMP_Tag_And_Attribute_Sanitizer::MISSING_MANDATORY_PROPERTY,
2373+
'meta_property_name' => 'width',
2374+
'meta_property_required_value' => 'device-width',
2375+
],
2376+
],
23542377
],
23552378
'bad_meta_charset' => [
23562379
'<html amp><head><meta charset="latin-1"><title>Mojibake?</title></head><body></body></html>',

0 commit comments

Comments
 (0)