Skip to content

Commit cdcb9e2

Browse files
westonruterwestonruter
committed
Defer gathering the required script components until the moment of return
1 parent 84221e8 commit cdcb9e2

File tree

1 file changed

+54
-44
lines changed

1 file changed

+54
-44
lines changed

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

Lines changed: 54 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -572,42 +572,11 @@ static function ( $validation_error ) {
572572
}
573573
}
574574

575-
$merged_attr_spec_list = array_merge(
575+
$attr_spec_list = array_merge(
576576
$this->globally_allowed_attributes,
577577
$attr_spec_list
578578
);
579579

580-
// Add required AMP component scripts.
581-
$script_components = [];
582-
if ( ! empty( $tag_spec['requires_extension'] ) ) {
583-
$script_components = array_merge( $script_components, $tag_spec['requires_extension'] );
584-
}
585-
586-
// Add required AMP components for attributes.
587-
foreach ( $node->attributes as $attribute ) {
588-
if ( isset( $merged_attr_spec_list[ $attribute->nodeName ]['requires_extension'] ) ) {
589-
$script_components = array_merge(
590-
$script_components,
591-
$merged_attr_spec_list[ $attribute->nodeName ]['requires_extension']
592-
);
593-
}
594-
}
595-
596-
// Manually add components for attributes; this is hard-coded because attributes do not have requires_extension like tags do. See <https://github.com/ampproject/amp-wp/issues/1808>.
597-
if ( $node->hasAttribute( 'lightbox' ) ) {
598-
$script_components[] = 'amp-lightbox-gallery';
599-
}
600-
601-
// Check if element needs amp-bind component.
602-
if ( $node instanceof DOMElement && ! in_array( 'amp-bind', $this->script_components, true ) ) {
603-
foreach ( $node->attributes as $name => $value ) {
604-
if ( Document::AMP_BIND_DATA_ATTR_PREFIX === substr( $name, 0, 14 ) ) {
605-
$script_components[] = 'amp-bind';
606-
break;
607-
}
608-
}
609-
}
610-
611580
// Remove element if it has illegal CDATA.
612581
if ( ! empty( $cdata ) && $node instanceof DOMElement ) {
613582
$validity = $this->validate_cdata_for_node( $node, $cdata );
@@ -619,18 +588,18 @@ static function ( $validation_error ) {
619588
[ 'spec_name' => $this->get_spec_name( $node, $tag_spec ) ]
620589
)
621590
);
622-
return $sanitized ? null : $script_components;
591+
return $sanitized ? null : $this->get_required_script_components( $node, $tag_spec, $attr_spec_list );
623592
}
624593
}
625594

626595
// Amend spec list with layout.
627596
if ( isset( $tag_spec['amp_layout'] ) ) {
628-
$merged_attr_spec_list = array_merge( $merged_attr_spec_list, $this->layout_allowed_attributes );
597+
$attr_spec_list = array_merge( $attr_spec_list, $this->layout_allowed_attributes );
629598

630599
if ( isset( $tag_spec['amp_layout']['supported_layouts'] ) ) {
631600
$layouts = wp_array_slice_assoc( Layout::FROM_SPEC, $tag_spec['amp_layout']['supported_layouts'] );
632601

633-
$merged_attr_spec_list['layout'][ AMP_Rule_Spec::VALUE_REGEX_CASEI ] = '(' . implode( '|', $layouts ) . ')';
602+
$attr_spec_list['layout'][ AMP_Rule_Spec::VALUE_REGEX_CASEI ] = '(' . implode( '|', $layouts ) . ')';
634603
}
635604
}
636605

@@ -657,11 +626,11 @@ static function ( $validation_error ) {
657626
$layout_validity = $this->is_valid_layout( $tag_spec, $node );
658627
if ( true !== $layout_validity ) {
659628
$sanitized = $this->remove_invalid_child( $node, $layout_validity );
660-
return $sanitized ? null : $script_components;
629+
return $sanitized ? null : $this->get_required_script_components( $node, $tag_spec, $attr_spec_list );
661630
}
662631

663632
// Identify attribute values that don't conform to the attr_spec.
664-
$disallowed_attributes = $this->sanitize_disallowed_attribute_values_in_node( $node, $merged_attr_spec_list );
633+
$disallowed_attributes = $this->sanitize_disallowed_attribute_values_in_node( $node, $attr_spec_list );
665634

666635
// Remove all invalid attributes.
667636
if ( ! empty( $disallowed_attributes ) ) {
@@ -709,7 +678,7 @@ static function ( $validation_error ) {
709678

710679
if ( $this->should_sanitize_validation_error( $validation_error, [ 'node' => $attr_node ] ) ) {
711680
$properties = $this->parse_properties_attribute( $attr_node->nodeValue );
712-
if ( ! empty( $merged_attr_spec_list[ $attr_node->nodeName ]['value_properties'][ $error_data['name'] ]['mandatory'] ) ) {
681+
if ( ! empty( $attr_spec_list[ $attr_node->nodeName ]['value_properties'][ $error_data['name'] ]['mandatory'] ) ) {
713682
$properties[ $error_data['name'] ] = $error_data['required_value'];
714683
} else {
715684
unset( $properties[ $error_data['name'] ] );
@@ -727,7 +696,7 @@ static function ( $validation_error ) {
727696
$node->setAttribute( $attr_node->nodeName, $this->serialize_properties_attribute( $properties ) );
728697
}
729698
} else {
730-
$attr_spec = isset( $merged_attr_spec_list[ $attr_node->nodeName ] ) ? $merged_attr_spec_list[ $attr_node->nodeName ] : [];
699+
$attr_spec = isset( $attr_spec_list[ $attr_node->nodeName ] ) ? $attr_spec_list[ $attr_node->nodeName ] : [];
731700
if ( $this->remove_invalid_attribute( $node, $attr_node, $validation_error, $attr_spec ) ) {
732701
$removed_attributes[] = $attr_node;
733702
}
@@ -756,7 +725,7 @@ static function ( $validation_error ) {
756725
}
757726

758727
// After attributes have been sanitized (and potentially removed), if mandatory attribute(s) are missing, remove the element.
759-
$missing_mandatory_attributes = $this->get_missing_mandatory_attributes( $merged_attr_spec_list, $node );
728+
$missing_mandatory_attributes = $this->get_missing_mandatory_attributes( $attr_spec_list, $node );
760729
if ( ! empty( $missing_mandatory_attributes ) ) {
761730
$sanitized = $this->remove_invalid_child(
762731
$node,
@@ -766,7 +735,7 @@ static function ( $validation_error ) {
766735
'spec_name' => $this->get_spec_name( $node, $tag_spec ),
767736
]
768737
);
769-
return $sanitized ? null : $script_components;
738+
return $sanitized ? null : $this->get_required_script_components( $node, $tag_spec, $attr_spec_list );
770739
}
771740

772741
if ( ! empty( $tag_spec[ AMP_Rule_Spec::MANDATORY_ANYOF ] ) ) {
@@ -780,7 +749,7 @@ static function ( $validation_error ) {
780749
'spec_name' => $this->get_spec_name( $node, $tag_spec ),
781750
]
782751
);
783-
return $sanitized ? null : $script_components;
752+
return $sanitized ? null : $this->get_required_script_components( $node, $tag_spec, $attr_spec_list );
784753
}
785754
}
786755

@@ -795,7 +764,7 @@ static function ( $validation_error ) {
795764
'spec_name' => $this->get_spec_name( $node, $tag_spec ),
796765
]
797766
);
798-
return $sanitized ? null : $script_components;
767+
return $sanitized ? null : $this->get_required_script_components( $node, $tag_spec, $attr_spec_list );
799768
} elseif ( count( $oneof_attributes ) > 1 ) {
800769
$sanitized = $this->remove_invalid_child(
801770
$node,
@@ -805,10 +774,51 @@ static function ( $validation_error ) {
805774
'spec_name' => $this->get_spec_name( $node, $tag_spec ),
806775
]
807776
);
808-
return $sanitized ? null : $script_components;
777+
return $sanitized ? null : $this->get_required_script_components( $node, $tag_spec, $attr_spec_list );
778+
}
779+
}
780+
781+
return $this->get_required_script_components( $node, $tag_spec, $attr_spec_list );
782+
}
783+
784+
/**
785+
* Get required AMP component scripts.
786+
*
787+
* @param DOMElement $node Element.
788+
* @param array $tag_spec Tag spec.
789+
* @param array $attr_spec_list Attribute spec list.
790+
* @return string[] Script component handles.
791+
*/
792+
private function get_required_script_components( DOMElement $node, $tag_spec, $attr_spec_list ) {
793+
$script_components = [];
794+
if ( ! empty( $tag_spec['requires_extension'] ) ) {
795+
$script_components = array_merge( $script_components, $tag_spec['requires_extension'] );
796+
}
797+
798+
// Add required AMP components for attributes.
799+
foreach ( $node->attributes as $attribute ) {
800+
if ( isset( $attr_spec_list[ $attribute->nodeName ]['requires_extension'] ) ) {
801+
$script_components = array_merge(
802+
$script_components,
803+
$attr_spec_list[ $attribute->nodeName ]['requires_extension']
804+
);
809805
}
810806
}
811807

808+
// Manually add components for attributes; this is hard-coded because attributes do not have requires_extension like tags do. See <https://github.com/ampproject/amp-wp/issues/1808>.
809+
if ( $node->hasAttribute( 'lightbox' ) ) {
810+
$script_components[] = 'amp-lightbox-gallery';
811+
}
812+
813+
// Check if element needs amp-bind component.
814+
if ( $node instanceof DOMElement && ! in_array( 'amp-bind', $this->script_components, true ) ) {
815+
foreach ( $node->attributes as $name => $value ) {
816+
if ( Document::AMP_BIND_DATA_ATTR_PREFIX === substr( $name, 0, 14 ) ) {
817+
$script_components[] = 'amp-bind';
818+
break;
819+
}
820+
}
821+
}
812822
return $script_components;
813823
}
814824

0 commit comments

Comments
 (0)