Remove DISALLOWED_TAG_MULTIPLE_CHOICES validation error code

ampproject · Mar 20, 2020 · 3c29860 · 3c29860
1 parent 29eea3c
commit 3c29860
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 22 deletions.
diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php
@@ -474,6 +474,11 @@ public function remove_invalid_child( $node, $validation_error = [] ) {
 
  $should_remove = $this->should_sanitize_validation_error( $validation_error, compact( 'node' ) );
  if ( $should_remove ) {
+ if ( null === $node->parentNode ) {
+ // Node no longer exists.
+ return $should_remove;
+ }
+
  $node->parentNode->removeChild( $node );
  } else {
  $this->nodes_to_keep[ $node->nodeName ][] = $node;

diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
@@ -27,7 +27,6 @@
 class AMP_Tag_And_Attribute_Sanitizer extends AMP_Base_Sanitizer {
 
  const DISALLOWED_TAG = 'DISALLOWED_TAG';
- const DISALLOWED_TAG_MULTIPLE_CHOICES = 'DISALLOWED_TAG_MULTIPLE_CHOICES';
  const DISALLOWED_CHILD_TAG = 'DISALLOWED_CHILD_TAG';
  const DISALLOWED_FIRST_CHILD_TAG = 'DISALLOWED_FIRST_CHILD_TAG';
  const INCORRECT_NUM_CHILD_TAGS = 'INCORRECT_NUM_CHILD_TAGS';
@@ -505,7 +504,15 @@ private function process_node( DOMElement $node ) {
  array_unique(
  array_map(
  static function ( $validation_error ) {
- unset( $validation_error['spec_name'] );
+ unset(
+ $validation_error['spec_name'],
+ // Remove other keys that may not not make the error unique.
+ $validation_error['required_parent_name'],
+ $validation_error['required_ancestor_name'],
+ $validation_error['required_child_count'],
+ $validation_error['required_min_child_count'],
+ $validation_error['required_attr_value']
+ );
  return $validation_error;
  },
  $validation_errors
@@ -527,13 +534,12 @@ static function ( $validation_error ) {
  );
  } else {
  // Otherwise, we have a rare condition where multiple tag specs fail for different reasons.
- $this->remove_invalid_child(
- $node,
- [
- 'code' => self::DISALLOWED_TAG_MULTIPLE_CHOICES,
- 'errors' => $validation_errors,
- ]
- );
+ foreach ( $validation_errors as $validation_error ) {
+ $this->remove_invalid_child(
+ $node,
+ $validation_error
+ );
+ }
  }
  }
  return null;

diff --git a/tests/php/test-tag-and-attribute-sanitizer.php b/tests/php/test-tag-and-attribute-sanitizer.php
@@ -92,19 +92,14 @@ public function get_body_data() {
  [],
  [
  [
- 'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG_MULTIPLE_CHOICES,
- 'errors' => [
- [
- 'code' => AMP_Tag_And_Attribute_Sanitizer::WRONG_PARENT_TAG,
- 'spec_name' => 'noscript enclosure for boilerplate',
- 'required_parent_name' => 'head'
- ],
- [
- 'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG_ANCESTOR,
- 'disallowed_ancestor' => 'noscript',
- 'spec_name' => 'noscript',
- ],
- ],
+ 'code' => AMP_Tag_And_Attribute_Sanitizer::WRONG_PARENT_TAG,
+ 'spec_name' => 'noscript enclosure for boilerplate',
+ 'required_parent_name' => 'head'
+ ],
+ [
+ 'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG_ANCESTOR,
+ 'disallowed_ancestor' => 'noscript',
+ 'spec_name' => 'noscript',
  ],
  ],
  ],