Feature description

Split out from #3758 via the conversation with @schlessera in #3758 (comment):

More generally, would it be possible to refactor the tag & meta sanitizer to strip the offending properties only, and then check if the entire thing is still valid or not? This way, we wouldn't have to duplicate this logic all over the place...

This is in relation to this code:

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

• Adding invalid properties to a meta[viewport] should not result in the entire attribute being removed, if there are other valid properties.
• A separate validation error should be raised for each invalid property in a meta[content] attribute.

Implementation brief

Instead of raising one single DISALLOWED_PROPERTY_IN_ATTR_VALUE for the entire meta[content] attribute and then causing the entire content attribute to be removed, we instead need to:

1. Raise a DISALLOWED_PROPERTY_IN_ATTR_VALUE validation error for each individual property in the content attribute.
2. Remove the invalid property from the content attribute if the validation error if \AMP_Base_Sanitizer::should_sanitize_validation_error() returns true.

What this means is that the \AMP_Tag_And_Attribute_Sanitizer::sanitize_disallowed_attribute_values_in_node() method will need to be augmented to not simply return a list of [ $attr_node, $error_code ] tuples. I the case of invalid properties, it will also need to return [ $attr_node, self::DISALLOWED_PROPERTY_IN_ATTR_VALUE, $property_name, $property_value ].

This resulting list of $disallowed_attributes will then need to be iterated over in the foreach ( $disallowed_attributes as $disallowed_attribute ) {…} here:

Where instead of calling the \AMP_Base_Sanitizer::remove_invalid_attribute(), there would need to be a direct call to AMP_Base_Sanitizer::should_sanitize_validation_error() and if it returns true, update the value of the attribute to omit that invalid property.

QA testing instructions

• Install the "Brovy" theme
• Visit an AMP URL
• Verify: The validation errors should contain a validation error "Invalid property value: user-scalable"

Demo

Changelog entry