diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php index ada07196abb..47b93f0803e 100644 --- a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php +++ b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php @@ -1850,7 +1850,7 @@ private function remove_disallowed_descendants( $node, $allowed_descendants ) { } /** - * Loop through node's children and remove the ones that are not whitelisted. + * Check whether the node validates the constraints for children. * * @param DOMNode $node Node. * @param array $child_tags { @@ -1878,24 +1878,22 @@ private function check_valid_children( $node, $child_tags ) { } // Verify that all of the child are among the set of allowed elements. - $removed_count = 0; if ( isset( $child_tags['child_tag_name_oneof'] ) ) { foreach ( $child_elements as $child_element ) { if ( ! in_array( $child_element->nodeName, $child_tags['child_tag_name_oneof'], true ) ) { - $removed_count++; - $this->remove_invalid_child( $child_element ); + return false; } } } // If there aren't the exact number of elements, then mark this $node as being invalid. if ( isset( $child_tags['mandatory_num_child_tags'] ) ) { - return ( count( $child_elements ) - $removed_count ) === $child_tags['mandatory_num_child_tags']; + return count( $child_elements ) === $child_tags['mandatory_num_child_tags']; } // If there aren't enough elements, then mark this $node as being invalid. if ( isset( $child_tags['mandatory_min_num_child_tags'] ) ) { - return ( count( $child_elements ) - $removed_count ) >= $child_tags['mandatory_min_num_child_tags']; + return count( $child_elements ) >= $child_tags['mandatory_min_num_child_tags']; } return true; diff --git a/tests/php/test-tag-and-attribute-sanitizer.php b/tests/php/test-tag-and-attribute-sanitizer.php index 04b12cce509..df6931b5cdb 100644 --- a/tests/php/test-tag-and-attribute-sanitizer.php +++ b/tests/php/test-tag-and-attribute-sanitizer.php @@ -72,17 +72,23 @@ public function get_body_data() { ], 'amp-animation' => [ - 'badvery bad', '', + null, [ 'amp-animation' ], ], - 'amp-call-tracking' => [ - 'bad--and not great: +1 (23) 456-789more badnot great', - '--and not great: +1 (23) 456-789not great', + 'amp-call-tracking-ok' => [ + '+1 (23) 456-789', + null, [ 'amp-call-tracking' ], ], + 'amp-call-tracking-bad-children' => [ + 'bad--and not great: +1 (23) 456-789more badnot great', + '', + [], + ], + 'amp-call-tracking_blacklisted_config' => [ '+1 (23) 456-789', '', @@ -278,9 +284,7 @@ static function () { '', ' - bad - bad @@ -295,9 +299,7 @@ static function () { - bad - bad @@ -325,9 +327,7 @@ static function () { - bad - bad ' @@ -568,7 +568,6 @@ static function() { ' ok -

bad

Section 1

Bunch of awesome content.

@@ -585,8 +584,6 @@ static function() {
ok -
bad
- ok
' ); @@ -735,7 +732,7 @@ static function() { ], 'remove_node_with_disallowed_ancestor_and_disallowed_child_nodes' => [ - 'This node is not allowed here.This node is not allowed here.', + 'This node is not allowed here.This node is not allowed here.', '', [ 'amp-sidebar' ], ], @@ -790,6 +787,23 @@ static function() { [ 'amp-sidebar' ], ], + 'amp_sidebar_with_div_inside_nav' => [ + ' + + + + ', + null, + [ 'amp-sidebar' ], + ], + 'remove_node_without_mandatory_ancestor' => [ '
All I have is this div, when all you want is a noscript tag.
', '
All I have is this div, when all you want is a noscript tag.
', @@ -1328,8 +1342,8 @@ static function() { ], 'amp-image-slider' => [ - 'Not allowedforbidden
This apple is green
not allowed
This apple is red
not ok
', '
This apple is green
This apple is red
', + null, [ 'amp-image-slider' ], ], @@ -1339,6 +1353,12 @@ static function() { [], ], + 'amp-image-slider-more-bad-children' => [ + 'Not allowedforbidden
This apple is green
not allowed
This apple is red
not ok
', + '', + [], + ], + 'amp-fx-collection' => [ '

A title that moves faster than other content.

', null,