since the native_img_used arg is the opt-in to not do any such conversion.
- // @todo Remove once https://github.com/ampproject/amphtml/issues/30442 lands.
- $node->setAttributeNode(
- $this->dom->createAttribute( AMP_Validation_Manager::AMP_UNVALIDATED_TAG_ATTRIBUTE )
- );
- $this->dom->documentElement->setAttributeNode(
- $this->dom->createAttribute( AMP_Validation_Manager::AMP_NON_VALID_DOC_ATTRIBUTE )
+ // If native post forms are used, then mark any POST forms as being unvalidated for AMP. Note that it is
+ // an all or nothing proposition with forms, where there cannot be some POST forms with [action] and
+ // others with [action-xhr]. The former is incompatible with the amp-form extension but the latter
+ // fundamentally depends on it. So it's one or the other.
+ if ( $this->args['native_post_forms_used'] ) {
+ if ( $xhr_action ) {
+ // @todo Consider rewriting action-xhr to action? Or include a shim which implements the amp-form functionality?
+ $this->remove_invalid_child(
+ $node,
+ [ 'code' => self::POST_FORM_HAS_ACTION_XHR_WHEN_NATIVE_USED ]
);
continue;
}
+
+ $node->setAttributeNode(
+ $this->dom->createAttribute( AMP_Validation_Manager::AMP_UNVALIDATED_TAG_ATTRIBUTE )
+ );
+ $this->dom->documentElement->setAttributeNode(
+ $this->dom->createAttribute( AMP_Validation_Manager::AMP_NON_VALID_DOC_ATTRIBUTE )
+ );
+ continue;
}
$node->removeAttribute( 'action' );
diff --git a/includes/sanitizers/class-amp-script-sanitizer.php b/includes/sanitizers/class-amp-script-sanitizer.php
index 9f59965653c..11916f5f5aa 100644
--- a/includes/sanitizers/class-amp-script-sanitizer.php
+++ b/includes/sanitizers/class-amp-script-sanitizer.php
@@ -77,6 +77,13 @@ class AMP_Script_Sanitizer extends AMP_Base_Sanitizer {
*/
protected $kept_script_count = 0;
+ /**
+ * Number of kept nodes which were PX-verified.
+ *
+ * @var int
+ */
+ protected $px_verified_kept_node_count = 0;
+
/**
* Sanitizers.
*
@@ -110,28 +117,41 @@ public function sanitize() {
// If custom scripts were kept (after sanitize_js_script_elements() ran) it's important that noscripts not be
// unwrapped or else this could result in the JS and no-JS fallback experiences both being present on the page.
// So unwrapping is only done when no custom scripts were retained (and the sanitizer arg opts-in to unwrap).
- if ( 0 === $this->kept_script_count && ! empty( $this->args['unwrap_noscripts'] ) ) {
+ if ( 0 === $this->kept_script_count && 0 === $this->px_verified_kept_node_count && ! empty( $this->args['unwrap_noscripts'] ) ) {
$this->unwrap_noscript_elements();
}
- // When there are kept custom scripts, skip tree shaking since it's likely JS will toggle classes that have
- // associated style rules.
+ $sanitizer_arg_updates = [];
+
+ // When there are kept custom scripts, turn off conversion to AMP components since scripts may be attempting to
+ // query for them directly, and skip tree shaking since it's likely JS will toggle classes that have associated
+ // style rules.
// @todo There should be an attribute on script tags that opt-in to keeping tree shaking and/or to indicate what class names need to be included.
// @todo Depending on the size of the underlying stylesheets, this may need to retain the use of external styles to prevent inlining excessive CSS. This may involve writing minified CSS to disk, or skipping style processing altogether if no selector conversions are needed.
if ( $this->kept_script_count > 0 ) {
- $sanitizer_arg_updates = [
- AMP_Style_Sanitizer::class => [ 'skip_tree_shaking' => true ],
- AMP_Img_Sanitizer::class => [ 'native_img_used' => true ],
- AMP_Video_Sanitizer::class => [ 'native_video_used' => true ],
- AMP_Audio_Sanitizer::class => [ 'native_audio_used' => true ],
- AMP_Iframe_Sanitizer::class => [ 'native_iframe_used' => true ],
- AMP_Form_Sanitizer::class => [ 'native_post_forms_allowed' => true ],
- AMP_Tag_And_Attribute_Sanitizer::class => [ 'prefer_bento' => true ],
- ];
- foreach ( $sanitizer_arg_updates as $sanitizer_class => $sanitizer_args ) {
- if ( array_key_exists( $sanitizer_class, $this->sanitizers ) ) {
- $this->sanitizers[ $sanitizer_class ]->update_args( $sanitizer_args );
- }
+ $sanitizer_arg_updates[ AMP_Style_Sanitizer::class ]['skip_tree_shaking'] = true;
+ $sanitizer_arg_updates[ AMP_Form_Sanitizer::class ]['native_post_forms_used'] = true;
+ $sanitizer_arg_updates[ AMP_Video_Sanitizer::class ]['native_video_used'] = true;
+ $sanitizer_arg_updates[ AMP_Audio_Sanitizer::class ]['native_audio_used'] = true;
+ $sanitizer_arg_updates[ AMP_Iframe_Sanitizer::class ]['native_iframe_used'] = true;
+
+ // Once amp-img is deprecated, these won't be needed and an ![]()
won't prevent strict sandboxing level for valid AMP.
+ // Note that AMP_Core_Theme_Sanitizer would have already run, so we can't update it here. Nevertheless,
+ // the native_img_used flag was already enabled by the SandboxingLevels service.
+ $sanitizer_arg_updates[ AMP_Gallery_Block_Sanitizer::class ]['native_img_used'] = true;
+ $sanitizer_arg_updates[ AMP_Img_Sanitizer::class ]['native_img_used'] = true;
+ }
+
+ // When custom scripts are on the page, use Bento AMP components whenever possible and turn off some CSS
+ // processing is unnecessary for a valid AMP page and which can break custom scripts.
+ if ( $this->px_verified_kept_node_count > 0 || $this->kept_script_count > 0 ) {
+ $sanitizer_arg_updates[ AMP_Tag_And_Attribute_Sanitizer::class ]['prefer_bento'] = true;
+ $sanitizer_arg_updates[ AMP_Style_Sanitizer::class ]['transform_important_qualifiers'] = false;
+ }
+
+ foreach ( $sanitizer_arg_updates as $sanitizer_class => $sanitizer_args ) {
+ if ( array_key_exists( $sanitizer_class, $this->sanitizers ) ) {
+ $this->sanitizers[ $sanitizer_class ]->update_args( $sanitizer_args );
}
}
}
@@ -220,6 +240,12 @@ protected function sanitize_js_script_elements() {
continue;
}
+ if ( $script->hasAttribute( AMP_Validation_Manager::PX_VERIFIED_TAG_ATTRIBUTE ) ) {
+ $this->px_verified_kept_node_count++;
+ // @todo Consider forcing any PX-verified script to have async/defer if not module. For inline scripts, hack via data: URL?
+ continue;
+ }
+
if ( $script->hasAttribute( Attribute::SRC ) ) {
// Skip external AMP CDN scripts.
if ( 0 === strpos( $script->getAttribute( Attribute::SRC ), 'https://cdn.ampproject.org/' ) ) {
@@ -280,6 +306,20 @@ protected function sanitize_js_script_elements() {
continue;
}
+ // Since the attribute has been PX-verified, move along.
+ if (
+ $element->hasAttribute( AMP_Validation_Manager::PX_VERIFIED_ATTRS_ATTRIBUTE )
+ &&
+ in_array(
+ $event_handler_attribute->nodeName,
+ preg_split( '/\s+/', $element->getAttribute( AMP_Validation_Manager::PX_VERIFIED_ATTRS_ATTRIBUTE ) ),
+ true
+ )
+ ) {
+ $this->px_verified_kept_node_count++;
+ continue;
+ }
+
$removed = $this->remove_invalid_attribute(
$element,
$event_handler_attribute,
diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php
index 0cd1d3a15d4..3d649e25c9f 100644
--- a/includes/sanitizers/class-amp-style-sanitizer.php
+++ b/includes/sanitizers/class-amp-style-sanitizer.php
@@ -122,15 +122,16 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
* Array of flags used to control sanitization.
*
* @var array {
- * @type string[] $dynamic_element_selectors Selectors for elements (or their ancestors) which contain dynamic content; selectors containing these will not be filtered.
- * @type bool $use_document_element Whether the root of the document should be used rather than the body.
- * @type bool $require_https_src Require HTTPS URLs.
- * @type callable $validation_error_callback Function to call when a validation error is encountered.
- * @type bool $should_locate_sources Whether to locate the sources when reporting validation errors.
- * @type string $parsed_cache_variant Additional value by which to vary parsed cache.
- * @type string[] $focus_within_classes Class names in selectors that should be replaced with :focus-within pseudo classes.
- * @type string[] $low_priority_plugins Plugin slugs of the plugins to deprioritize when hitting the CSS limit.
- * @type bool $allow_transient_caching Whether to allow caching parsed CSS in transients. This may need to be disabled when there is highly-variable CSS content.
+ * @type string[] $dynamic_element_selectors Selectors for elements (or their ancestors) which contain dynamic content; selectors containing these will not be filtered.
+ * @type bool $use_document_element Whether the root of the document should be used rather than the body.
+ * @type bool $require_https_src Require HTTPS URLs.
+ * @type callable $validation_error_callback Function to call when a validation error is encountered.
+ * @type bool $should_locate_sources Whether to locate the sources when reporting validation errors.
+ * @type string $parsed_cache_variant Additional value by which to vary parsed cache.
+ * @type string[] $focus_within_classes Class names in selectors that should be replaced with :focus-within pseudo classes.
+ * @type string[] $low_priority_plugins Plugin slugs of the plugins to deprioritize when hitting the CSS limit.
+ * @type bool $allow_transient_caching Whether to allow caching parsed CSS in transients. This may need to be disabled when there is highly-variable CSS content.
+ * @type bool $transform_important_qualifiers Whether !important rules should be transformed. This also necessarily transform inline style attributes.
* }
*/
protected $args;
@@ -141,19 +142,20 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
* @var array
*/
protected $DEFAULT_ARGS = [
- 'dynamic_element_selectors' => [
+ 'dynamic_element_selectors' => [
'amp-list',
'amp-live-list',
'[submit-error]',
'[submit-success]',
'amp-script',
],
- 'should_locate_sources' => false,
- 'parsed_cache_variant' => null,
- 'focus_within_classes' => [ 'focus' ],
- 'low_priority_plugins' => [ 'query-monitor' ],
- 'allow_transient_caching' => true,
- 'skip_tree_shaking' => false,
+ 'should_locate_sources' => false,
+ 'parsed_cache_variant' => null,
+ 'focus_within_classes' => [ 'focus' ],
+ 'low_priority_plugins' => [ 'query-monitor' ],
+ 'allow_transient_caching' => true,
+ 'skip_tree_shaking' => false,
+ 'transform_important_qualifiers' => true,
];
/**
@@ -899,6 +901,7 @@ public function sanitize() {
* in document order. DOMNode::compareDocumentPosition() is not yet implemented.
*/
+ // @todo Also consider skipping the processing of link and style elements that have data-px-verified-tag.
$dev_mode_predicate = '';
if ( DevMode::isActiveForDocument( $this->dom ) ) {
$dev_mode_predicate = sprintf( ' and not ( @%s )', AMP_Rule_Spec::DEV_MODE_ATTRIBUTE );
@@ -955,12 +958,14 @@ public function sanitize() {
}
}
- $elements = [];
- foreach ( $this->dom->xpath->query( "//*[ @style $dev_mode_predicate ]" ) as $element ) {
- $elements[] = $element;
- }
- foreach ( $elements as $element ) {
- $this->collect_inline_styles( $element );
+ if ( $this->args['transform_important_qualifiers'] ) {
+ $elements = [];
+ foreach ( $this->dom->xpath->query( "//*[ @style $dev_mode_predicate ]" ) as $element ) {
+ $elements[] = $element;
+ }
+ foreach ( $elements as $element ) {
+ $this->collect_inline_styles( $element );
+ }
}
$this->finalize_styles();
@@ -1570,11 +1575,21 @@ private function get_parsed_stylesheet( $stylesheet, $options = [] ) {
$cache_impacting_options = array_merge(
wp_array_slice_assoc(
$options,
- [ 'property_allowlist', 'property_denylist', 'stylesheet_url', 'allowed_at_rules' ]
+ [
+ 'property_allowlist',
+ 'property_denylist',
+ 'stylesheet_url',
+ 'allowed_at_rules',
+ ]
),
wp_array_slice_assoc(
$this->args,
- [ 'should_locate_sources', 'parsed_cache_variant', 'dynamic_element_selectors' ]
+ [
+ 'should_locate_sources',
+ 'parsed_cache_variant',
+ 'dynamic_element_selectors',
+ 'transform_important_qualifiers',
+ ]
),
[
'language' => get_bloginfo( 'language' ), // Used to tree-shake html[lang] selectors.
@@ -2382,10 +2397,12 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l
$this->process_font_face_at_rule( $ruleset, $options );
}
- $results = array_merge(
- $results,
- $this->transform_important_qualifiers( $ruleset, $css_list, $options )
- );
+ if ( $this->args['transform_important_qualifiers'] ) {
+ $results = array_merge(
+ $results,
+ $this->transform_important_qualifiers( $ruleset, $css_list, $options )
+ );
+ }
// Remove the ruleset if it is now empty.
if ( 0 === count( $ruleset->getRules() ) ) {
@@ -2605,10 +2622,12 @@ private function process_css_keyframes( KeyFrame $css_list, $options ) {
continue;
}
- $results = array_merge(
- $results,
- $this->transform_important_qualifiers( $rules, $css_list, $options )
- );
+ if ( $this->args['transform_important_qualifiers'] ) {
+ $results = array_merge(
+ $results,
+ $this->transform_important_qualifiers( $rules, $css_list, $options )
+ );
+ }
$properties = $rules->getRules();
foreach ( $properties as $property ) {
@@ -3203,15 +3222,17 @@ static function ( $matches ) {
}
// Replace the somewhat-meta [style] attribute selectors with attribute selector using the data attribute the original styles are copied into.
- $selector = preg_replace(
- '/(?<=\[)style(?=([*$~]?=.*?)?])/is',
- self::ORIGINAL_STYLE_ATTRIBUTE_NAME,
- $selector,
- -1,
- $count
- );
- if ( $count > 0 ) {
- $has_changed_selectors = true;
+ if ( $this->args['transform_important_qualifiers'] ) {
+ $selector = preg_replace(
+ '/(?<=\[)style(?=([*$~]?=.*?)?])/is',
+ self::ORIGINAL_STYLE_ATTRIBUTE_NAME,
+ $selector,
+ - 1,
+ $count
+ );
+ if ( $count > 0 ) {
+ $has_changed_selectors = true;
+ }
}
/*
diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
index ea0599befad..fd9fb76de6a 100644
--- a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
+++ b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php
@@ -10,6 +10,7 @@
use AmpProject\Amp;
use AmpProject\Attribute;
use AmpProject\CssLength;
+use AmpProject\DevMode;
use AmpProject\Dom\Document;
use AmpProject\Layout;
use AmpProject\Extension;
@@ -420,7 +421,7 @@ private function process_node( DOMElement $node ) {
// If it's not an allowed tag, replace the node with it's children.
$this->replace_node_with_children( $node );
- // Return early since this node no longer exists.
+ // Return early since we don't know anything about this node to validate it.
return null;
}
@@ -2558,14 +2559,52 @@ private function get_ancestor_with_matching_spec_name( DOMElement $node, $ancest
* @see https://github.com/ampproject/amp-wp/issues/1100
*
* @param DOMElement $node Node.
+ * @return bool Whether replacement was done.
*/
private function replace_node_with_children( DOMElement $node ) {
+ if ( DevMode::isExemptFromValidation( $node ) ) {
+ return false;
+ }
+
+ // Prevent removing a tag which was exempted from validation.
+ if (
+ $node instanceof DOMElement
+ &&
+ $node->hasAttribute( AMP_Validation_Manager::AMP_UNVALIDATED_TAG_ATTRIBUTE )
+ &&
+ $node->ownerDocument
+ &&
+ $node->ownerDocument->documentElement->hasAttribute( AMP_Validation_Manager::AMP_NON_VALID_DOC_ATTRIBUTE )
+ ) {
+ return false;
+ }
+
+ // Prevent removing a tag which was verified for PX.
+ if (
+ $node instanceof DOMElement
+ &&
+ $node->hasAttribute( AMP_Validation_Manager::PX_VERIFIED_TAG_ATTRIBUTE )
+ ) {
+ return false;
+ }
+
+ // Prevent double-reporting nodes that are rejected for sanitization.
+ if ( isset( $this->should_not_replace_nodes[ $node->nodeName ] ) && in_array( $node, $this->should_not_replace_nodes[ $node->nodeName ], true ) ) {
+ return false;
+ }
// If node has no children or no parent, just remove the node.
if ( ! $node->hasChildNodes() || ! $node->parentNode ) {
- $this->remove_node( $node );
+ if ( $this->remove_node( $node ) ) {
+ return true;
+ }
+ }
- return;
+ // Replace node with fragment.
+ $should_replace = $this->should_sanitize_validation_error( [], compact( 'node' ) );
+ if ( ! $should_replace ) {
+ $this->should_not_replace_nodes[ $node->nodeName ][] = $node;
+ return false;
}
/*
@@ -2582,18 +2621,9 @@ private function replace_node_with_children( DOMElement $node ) {
$child = $node->firstChild;
}
- // Prevent double-reporting nodes that are rejected for sanitization.
- if ( isset( $this->should_not_replace_nodes[ $node->nodeName ] ) && in_array( $node, $this->should_not_replace_nodes[ $node->nodeName ], true ) ) {
- return;
- }
+ $node->parentNode->replaceChild( $fragment, $node );
+ return true;
- // Replace node with fragment.
- $should_replace = $this->should_sanitize_validation_error( [], compact( 'node' ) );
- if ( $should_replace ) {
- $node->parentNode->replaceChild( $fragment, $node );
- } else {
- $this->should_not_replace_nodes[ $node->nodeName ][] = $node;
- }
}
/**
@@ -2605,6 +2635,7 @@ private function replace_node_with_children( DOMElement $node ) {
* @since 0.5
*
* @param DOMElement $node Node.
+ * @return bool Whether removal was done.
*/
private function remove_node( DOMElement $node ) {
/**
@@ -2614,7 +2645,9 @@ private function remove_node( DOMElement $node ) {
*/
$parent = $node->parentNode;
if ( $node && $parent ) {
- $this->remove_invalid_child( $node );
+ if ( ! $this->remove_invalid_child( $node ) ) {
+ return false;
+ }
}
// @todo Does this parent removal even make sense anymore? Perhaps limit to only.
@@ -2625,6 +2658,8 @@ private function remove_node( DOMElement $node ) {
$parent->removeChild( $node );
}
}
+
+ return true;
}
/**
diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
index d8c5e96a69a..6060b6dc78e 100644
--- a/includes/validation/class-amp-validation-error-taxonomy.php
+++ b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -3341,10 +3341,10 @@ static function ( $attribute_name ) {
'' . esc_html( $validation_error['node_name'] ) . ''
);
- case AMP_Form_Sanitizer::FORM_HAS_POST_METHOD_WITHOUT_ACTION_XHR_ATTR:
+ case AMP_Form_Sanitizer::POST_FORM_HAS_ACTION_XHR_WHEN_NATIVE_USED:
return sprintf(
/* translators: %1$s is 'POST', %2$s is 'action-xhr' */
- esc_html__( 'Form has %1$s method without %2$s attribute. (Mark as kept to prevent conversion to AMP.)', 'amp' ),
+ esc_html__( 'Native %1$s form has %2$s attribute.', 'amp' ),
'POST',
'action-xhr'
);
diff --git a/includes/validation/class-amp-validation-manager.php b/includes/validation/class-amp-validation-manager.php
index 2dc2cd89fc0..3c49dc2dddb 100644
--- a/includes/validation/class-amp-validation-manager.php
+++ b/includes/validation/class-amp-validation-manager.php
@@ -84,6 +84,16 @@ class AMP_Validation_Manager {
*/
const AMP_UNVALIDATED_TAG_ATTRIBUTE = 'data-amp-unvalidated-tag';
+ /**
+ * HTML attribute to indicate an tag/element has been verified for PX.
+ *
+ * The difference here with `data-amp-unvalidated-tag` is that the PX-verified means that the tag will work
+ * properly Bento components and CSS tree shaking.
+ *
+ * @var string
+ */
+ const PX_VERIFIED_TAG_ATTRIBUTE = 'data-px-verified-tag';
+
/**
* HTML attribute to indicate one or more attributes are exempted from AMP validation.
*
@@ -93,6 +103,13 @@ class AMP_Validation_Manager {
*/
const AMP_UNVALIDATED_ATTRS_ATTRIBUTE = 'data-amp-unvalidated-attrs';
+ /**
+ * HTML attribute to indicate one or more attributes have been verified for PX from AMP validation.
+ *
+ * @var string
+ */
+ const PX_VERIFIED_ATTRS_ATTRIBUTE = 'data-px-verified-attrs';
+
/**
* The errors encountered when validating.
*
diff --git a/src/SandboxingLevels.php b/src/SandboxingLevels.php
index c2630298903..4b841dc8250 100644
--- a/src/SandboxingLevels.php
+++ b/src/SandboxingLevels.php
@@ -7,12 +7,16 @@
namespace AmpProject\AmpWP;
+use AMP_Comments_Sanitizer;
+use AMP_Core_Theme_Sanitizer;
+use AMP_Form_Sanitizer;
+use AMP_Gallery_Block_Sanitizer;
+use AMP_Img_Sanitizer;
use AMP_Options_Manager;
-use AmpProject\AmpWP\Infrastructure\Registerable;
+use AMP_Script_Sanitizer;
use AmpProject\AmpWP\Infrastructure\Conditional;
+use AmpProject\AmpWP\Infrastructure\Registerable;
use AmpProject\AmpWP\Infrastructure\Service;
-use AMP_Form_Sanitizer;
-use AMP_Script_Sanitizer;
/**
* Experimental service to facilitate flexible AMP.
@@ -136,14 +140,21 @@ public function add_hooks() {
$sandboxing_level = AMP_Options_Manager::get_option( self::OPTION_SANDBOXING_LEVEL );
- // Allow native POST forms, but they won't be converted by default (unless on level 3, per below).
- add_filter( 'amp_native_post_form_allowed', '__return_true' );
-
// Opt-in to the new script sanitization logic in the script sanitizer.
add_filter(
'amp_content_sanitizers',
- static function ( $sanitizers ) {
+ static function ( $sanitizers ) use ( $sandboxing_level ) {
$sanitizers[ AMP_Script_Sanitizer::class ]['sanitize_js_scripts'] = true;
+
+ if ( $sandboxing_level < 3 ) {
+ $sanitizers[ AMP_Form_Sanitizer::class ]['native_post_forms_used'] = true;
+ $sanitizers[ AMP_Comments_Sanitizer::class ]['allow_commenting_scripts'] = true;
+
+ // Once amp-img is deprecated, these won't be needed and an ![]()
won't prevent strict sandboxing level for valid AMP.
+ $sanitizers[ AMP_Img_Sanitizer::class ]['native_img_used'] = true;
+ $sanitizers[ AMP_Core_Theme_Sanitizer::class ]['native_img_used'] = true;
+ $sanitizers[ AMP_Gallery_Block_Sanitizer::class ]['native_img_used'] = true;
+ }
return $sanitizers;
}
);
@@ -153,21 +164,6 @@ static function ( $sanitizers ) {
add_filter( 'amp_validation_error_default_sanitized', '__return_false' );
}
- if ( $sandboxing_level < 3 ) {
- // Prevent conversion of POST forms to use action-xhr by default.
- add_filter(
- 'amp_validation_error_default_sanitized',
- static function ( $sanitized, $error ) {
- if ( isset( $error['code'] ) && AMP_Form_Sanitizer::FORM_HAS_POST_METHOD_WITHOUT_ACTION_XHR_ATTR === $error['code'] ) {
- $sanitized = false;
- }
- return $sanitized;
- },
- 20,
- 2
- );
- }
-
// To facilitate testing, vary the errors by the sandboxing level.
add_filter(
'amp_validation_error',
diff --git a/tests/php/test-amp-form-sanitizer.php b/tests/php/test-amp-form-sanitizer.php
index f47cdd23e20..34c3a074d94 100644
--- a/tests/php/test-amp-form-sanitizer.php
+++ b/tests/php/test-amp-form-sanitizer.php
@@ -194,33 +194,23 @@ public function get_data() {
'add_dev_mode' => true,
],
],
- 'form_with_post_action_converted' => [
- '
',
- '',
- [
- 'native_post_forms_allowed' => true,
- ],
- [ AMP_Form_Sanitizer::FORM_HAS_POST_METHOD_WITHOUT_ACTION_XHR_ATTR ],
- ],
- 'form_with_post_action_kept' => [
+ 'native_form_with_post_action' => [
'',
sprintf( '', AMP_Validation_Manager::AMP_UNVALIDATED_TAG_ATTRIBUTE ),
[
- 'native_post_forms_allowed' => true,
- 'keep_post_forms' => true,
- 'expected_non_valid_doc' => true,
+ 'native_post_forms_used' => true,
+ 'expected_non_valid_doc' => true,
],
- [ AMP_Form_Sanitizer::FORM_HAS_POST_METHOD_WITHOUT_ACTION_XHR_ATTR ],
+ [],
],
- 'form_with_post_action-xhr_ok' => [
+ 'native_form_with_post_action-xhr' => [
'',
- '',
+ '',
[
- 'native_post_forms_allowed' => true,
- 'keep_post_forms' => true,
- 'expected_non_valid_doc' => false,
+ 'native_post_forms_used' => true,
+ 'expected_non_valid_doc' => false,
],
- [],
+ [ AMP_Form_Sanitizer::POST_FORM_HAS_ACTION_XHR_WHEN_NATIVE_USED ],
],
];
}
@@ -243,13 +233,9 @@ public function test_converter( $source, $expected = null, $args = [], $expected
$dom->documentElement->setAttribute( AMP_Rule_Spec::DEV_MODE_ATTRIBUTE, '' );
}
- $actual_errors = [];
-
- $args['validation_error_callback'] = static function( $error ) use ( &$actual_errors, $args ) {
+ $actual_errors = [];
+ $args['validation_error_callback'] = static function( $error ) use ( &$actual_errors ) {
$actual_errors[] = $error;
- if ( AMP_Form_Sanitizer::FORM_HAS_POST_METHOD_WITHOUT_ACTION_XHR_ATTR === $error['code'] && ! empty( $args['keep_post_forms'] ) ) {
- return false;
- }
return true;
};
diff --git a/tests/php/test-amp-helper-functions.php b/tests/php/test-amp-helper-functions.php
index 0aa9b168350..f57ccd73cf7 100644
--- a/tests/php/test-amp-helper-functions.php
+++ b/tests/php/test-amp-helper-functions.php
@@ -1701,21 +1701,6 @@ public function test_amp_is_native_img_used() {
$this->assertFalse( amp_is_native_img_used() );
}
- /**
- * Test amp_is_native_post_form_allowed().
- *
- * @covers ::amp_is_native_post_form_allowed()
- */
- public function test_amp_is_native_post_form_allowed() {
- $this->assertFalse( amp_is_native_post_form_allowed(), 'Expected to be disabled by default for now.' );
-
- add_filter( 'amp_native_post_form_allowed', '__return_true' );
- $this->assertTrue( amp_is_native_post_form_allowed() );
-
- add_filter( 'amp_native_post_form_allowed', '__return_false', 20 );
- $this->assertFalse( amp_is_native_post_form_allowed() );
- }
-
/**
* Test deprecated $post param for amp_get_content_embed_handlers().
*
@@ -1770,6 +1755,10 @@ static function( $classes ) {
array_search( 'Even_After_Validating_Sanitizer', $ordered_sanitizers, true ),
array_search( AMP_Script_Sanitizer::class, $ordered_sanitizers, true )
);
+ $this->assertGreaterThan(
+ array_search( AMP_Core_Theme_Sanitizer::class, $ordered_sanitizers, true ),
+ array_search( AMP_Script_Sanitizer::class, $ordered_sanitizers, true )
+ );
$this->assertEquals( AMP_Layout_Sanitizer::class, $ordered_sanitizers[ count( $ordered_sanitizers ) - 4 ] );
$this->assertEquals( AMP_Style_Sanitizer::class, $ordered_sanitizers[ count( $ordered_sanitizers ) - 3 ] );
$this->assertEquals( AMP_Meta_Sanitizer::class, $ordered_sanitizers[ count( $ordered_sanitizers ) - 2 ] );
diff --git a/tests/php/test-amp-script-sanitizer.php b/tests/php/test-amp-script-sanitizer.php
index fd61018dee5..41968ab6dd0 100644
--- a/tests/php/test-amp-script-sanitizer.php
+++ b/tests/php/test-amp-script-sanitizer.php
@@ -434,7 +434,7 @@ public function test_cascading_sanitizer_argument_changes_with_custom_scripts( $
AMP_Form_Sanitizer::class => new AMP_Form_Sanitizer(
$dom,
[
- 'native_post_forms_allowed' => false, // Overridden by AMP_Script_Sanitizer when there is a kept script.
+ 'native_post_forms_used' => false, // Overridden by AMP_Script_Sanitizer when there is a kept script.
'validation_error_callback' => function () use ( $remove_custom_scripts ) {
return $remove_custom_scripts;
},
diff --git a/tests/php/test-amp-style-sanitizer.php b/tests/php/test-amp-style-sanitizer.php
index e4e0984595c..7fbde898c08 100644
--- a/tests/php/test-amp-style-sanitizer.php
+++ b/tests/php/test-amp-style-sanitizer.php
@@ -975,6 +975,80 @@ public function test_tree_shaking_disabled() {
$this->assertGreaterThan( 75000, strlen( implode( '', $actual_stylesheets ) ) );
}
+ /** @return array */
+ public function get_data_to_test_transform_important_qualifiers_arg() {
+ return [
+ 'transform' => [
+ true,
+ [
+ ':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .foo{color:red}',
+ ':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .foo[data-amp-original-style*="blue"]{outline:solid 2px green}',
+ ':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-9605c4d{background:blue}',
+ ],
+ null,
+ ],
+ 'no_transform' => [
+ false,
+ [
+ '.foo{color:red !important}',
+ '.foo[style*="blue"]{outline:solid 2px green !important}',
+ ],
+ 'background: blue !important',
+ ],
+ ];
+ }
+
+ /**
+ * Test that transformation of !important qualifiers (and processing of style attributes) can be turned off.
+ *
+ * @dataProvider get_data_to_test_transform_important_qualifiers_arg
+ * @param bool $transform_important_qualifiers Sanitizer args.
+ * @param array $expected_stylesheets Expected stylesheets.
+ * @param string|null $expected_style_attr Inline style attribute value, or null if not expected.
+ */
+ public function test_transform_important_qualifiers_arg( $transform_important_qualifiers, $expected_stylesheets, $expected_style_attr ) {
+ $dom = Document::fromHtml(
+ '
+
+
+
+
+
+
+
+
+
+ ',
+ Options::DEFAULTS
+ );
+
+ $args = [
+ 'use_document_element' => true,
+ ];
+
+ $sanitizer = new AMP_Style_Sanitizer( $dom, array_merge( $args, compact( 'transform_important_qualifiers' ) ) );
+ $sanitizer->sanitize();
+
+ $validating_sanitizer = new AMP_Tag_And_Attribute_Sanitizer( $dom, $args );
+ $validating_sanitizer->sanitize();
+
+ $this->assertEquals(
+ $expected_stylesheets,
+ array_values( array_filter( $sanitizer->get_stylesheets() ) )
+ );
+
+ $style_attr = $dom->xpath->query( '//*/@style' )->item( 0 );
+ if ( $style_attr instanceof DOMAttr ) {
+ $style_attr = $style_attr->nodeValue;
+ }
+
+ $this->assertEquals( $expected_style_attr, $style_attr );
+ }
+
/**
* Data for testing AMP selector conversion.
*
diff --git a/tests/php/test-class-amp-comments-sanitizer.php b/tests/php/test-class-amp-comments-sanitizer.php
index e4d907ccb10..2ad985dafb5 100644
--- a/tests/php/test-class-amp-comments-sanitizer.php
+++ b/tests/php/test-class-amp-comments-sanitizer.php
@@ -60,20 +60,22 @@ public function test_sanitize_incorrect_action() {
* @covers AMP_Comments_Sanitizer::sanitize()
*/
public function test_sanitize_allowed_action_xhr() {
+ update_option( 'thread_comments', '1' );
$form_sanitizer = new AMP_Form_Sanitizer( $this->dom );
- $comments_sanitizer = new AMP_Comments_Sanitizer( $this->dom );
+ $comments_sanitizer = new AMP_Comments_Sanitizer(
+ $this->dom,
+ [ 'thread_comments' => '1' ]
+ );
// Use an allowed action.
$form = $this->create_form( '/wp-comments-post.php' );
$form_sanitizer->sanitize();
$comments_sanitizer->sanitize();
- $on = $form->getAttribute( 'on' );
- $this->assertStringContainsString( 'submit:AMP.setState(', $on );
- $this->assertStringContainsString( 'submit-error:AMP.setState(', $on );
- foreach ( $this->get_form_element_names() as $name ) {
- $this->assertStringContainsString( $name, $on );
- }
+ $this->assertEquals(
+ 'submit-success:i-amp-0.clear,AMP.setState({ampCommentThreading: {"replyTo":"","commentParent":"0"}})',
+ $form->getAttribute( 'on' )
+ );
}
/**
@@ -96,7 +98,8 @@ public function test_sanitize_native_post_form() {
* @covers AMP_Comments_Sanitizer::process_comment_form()
*/
public function test_process_comment_form() {
- $instance = new AMP_Comments_Sanitizer( $this->dom );
+ update_option( 'thread_comments', '1' );
+ $instance = new AMP_Comments_Sanitizer( $this->dom, [ 'thread_comments' => true ] );
$form = $this->create_form( '/wp-comments-post.php' );
$this->call_private_method( $instance, 'process_comment_form', [ $form ] );
@@ -104,26 +107,16 @@ public function test_process_comment_form() {
$on = $form->getAttribute( 'on' );
$amp_state = $this->dom->getElementsByTagName( 'amp-state' )->item( 0 );
- $this->assertStringContainsString( 'submit:AMP.setState(', $on );
- $this->assertStringContainsString( 'submit-error:AMP.setState(', $on );
- $this->assertStringContainsString( 'submit-success:AMP.setState(', $on );
- $this->assertStringContainsString( strval( $GLOBALS['post']->ID ), $on );
+ $this->assertEquals(
+ $on,
+ 'submit-success:i-amp-0.clear,AMP.setState({ampCommentThreading: {"replyTo":"","commentParent":"0"}})'
+ );
$this->assertEquals( 'script', $amp_state->firstChild->nodeName );
- foreach ( $this->get_form_element_names() as $name ) {
- $this->assertStringContainsString( $name, $on );
- $this->assertStringContainsString( $name, $amp_state->nodeValue );
- }
- foreach ( $form->getElementsByTagName( 'input' ) as $input ) {
- /**
- * Input.
- *
- * @var DOMElement $input
- */
- $on = $input->getAttribute( 'on' );
- $this->assertStringContainsString( 'change:AMP.setState(', $on );
- $this->assertStringContainsString( strval( $GLOBALS['post']->ID ), $on );
- }
+ $this->assertEquals(
+ '{"replyTo":"","commentParent":"0"}',
+ $amp_state->nodeValue
+ );
}
/**
diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php
index a518b4c3725..9c05e22c267 100644
--- a/tests/php/test-class-amp-theme-support.php
+++ b/tests/php/test-class-amp-theme-support.php
@@ -779,10 +779,6 @@ public function test_add_hooks() {
$priority = defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX; // phpcs:ignore PHPCompatibility.Constants.NewConstants.php_int_minFound
$this->assertEquals( $priority, has_action( 'template_redirect', [ self::TESTED_CLASS, 'start_output_buffering' ] ) );
- $this->assertEquals( PHP_INT_MAX, has_filter( 'comment_form_defaults', [ self::TESTED_CLASS, 'filter_comment_form_defaults' ] ) );
- $this->assertEquals( 10, has_filter( 'comment_reply_link', [ self::TESTED_CLASS, 'filter_comment_reply_link' ] ) );
- $this->assertEquals( 10, has_filter( 'cancel_comment_reply_link', [ self::TESTED_CLASS, 'filter_cancel_comment_reply_link' ] ) );
- $this->assertFalse( has_action( 'comment_form', 'wp_comment_form_unfiltered_html_nonce' ) );
$this->assertEquals( PHP_INT_MAX, has_filter( 'get_header_image_tag', [ self::TESTED_CLASS, 'amend_header_image_with_video_header' ] ) );
}
@@ -881,6 +877,7 @@ public function test_get_current_canonical_url_in_paired_amp() {
* Test get_comment_form_state_id.
*
* @covers AMP_Theme_Support::get_comment_form_state_id()
+ * @expectedDeprecated AMP_Theme_Support::get_comment_form_state_id
*/
public function test_get_comment_form_state_id() {
$post_id = 54;
@@ -893,6 +890,8 @@ public function test_get_comment_form_state_id() {
* Test filter_comment_form_defaults.
*
* @covers AMP_Theme_Support::filter_comment_form_defaults()
+ * @expectedDeprecated AMP_Theme_Support::filter_comment_form_defaults
+ * @expectedDeprecated AMP_Theme_Support::get_comment_form_state_id
*/
public function test_filter_comment_form_defaults() {
global $post;
@@ -914,6 +913,8 @@ public function test_filter_comment_form_defaults() {
* Test filter_comment_reply_link.
*
* @covers AMP_Theme_Support::filter_comment_reply_link()
+ * @expectedDeprecated AMP_Theme_Support::filter_comment_reply_link
+ * @expectedDeprecated AMP_Theme_Support::get_comment_form_state_id
*/
public function test_filter_comment_reply_link() {
global $post;
@@ -949,6 +950,8 @@ public function test_filter_comment_reply_link() {
* Test filter_cancel_comment_reply_link.
*
* @covers AMP_Theme_Support::filter_cancel_comment_reply_link()
+ * @expectedDeprecated AMP_Theme_Support::filter_cancel_comment_reply_link
+ * @expectedDeprecated AMP_Theme_Support::get_comment_form_state_id
*/
public function test_filter_cancel_comment_reply_link() {
global $post;
@@ -1867,19 +1870,14 @@ public function test_prepare_response_when_allowing_native_post_forms( $converte
$this->set_template_mode( AMP_Theme_Support::STANDARD_MODE_SLUG );
add_filter(
- 'amp_validation_error_default_sanitized',
- static function ( $sanitized, $error ) use ( $converted ) {
- if ( AMP_Form_Sanitizer::FORM_HAS_POST_METHOD_WITHOUT_ACTION_XHR_ATTR === $error['code'] ) {
- $sanitized = $converted;
- }
- return $sanitized;
- },
- 10,
- 2
+ 'amp_content_sanitizers',
+ static function ( $sanitizers ) use ( $converted ) {
+ $sanitizers[ AMP_Form_Sanitizer::class ]['native_post_forms_used'] = ! $converted;
+ return $sanitizers;
+ }
);
wp();
- add_filter( 'amp_native_post_form_allowed', '__return_true' );
AMP_Theme_Support::init();
AMP_Theme_Support::finish_init();
ob_start();
diff --git a/tests/php/test-tag-and-attribute-sanitizer.php b/tests/php/test-tag-and-attribute-sanitizer.php
index 0ff1339e007..c3a0fb40c91 100644
--- a/tests/php/test-tag-and-attribute-sanitizer.php
+++ b/tests/php/test-tag-and-attribute-sanitizer.php
@@ -3993,6 +3993,54 @@ public function get_data_for_replace_node_with_children_validation_errors() {
],
],
],
+
+ 'custom_elements-removed' => [
+ 'Hello!',
+ 'Hello!',
+ [
+ [
+ 'node_name' => 'bar-baz',
+ 'parent_name' => 'foo-bar',
+ 'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG,
+ 'node_attributes' => [],
+ 'type' => AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE,
+ 'node_type' => XML_ELEMENT_NODE,
+ ],
+ [
+ 'node_name' => 'foo-bar',
+ 'parent_name' => 'body',
+ 'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG,
+ 'node_attributes' => [],
+ 'type' => AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE,
+ 'node_type' => XML_ELEMENT_NODE,
+ ],
+ ],
+ true,
+ ],
+
+ 'custom_elements-kept' => [
+ 'Hello!',
+ 'Hello!',
+ [
+ [
+ 'node_name' => 'bar-baz',
+ 'parent_name' => 'foo-bar',
+ 'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG,
+ 'node_attributes' => [],
+ 'type' => AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE,
+ 'node_type' => XML_ELEMENT_NODE,
+ ],
+ [
+ 'node_name' => 'foo-bar',
+ 'parent_name' => 'body',
+ 'code' => AMP_Tag_And_Attribute_Sanitizer::DISALLOWED_TAG,
+ 'node_attributes' => [],
+ 'type' => AMP_Validation_Error_Taxonomy::HTML_ELEMENT_ERROR_TYPE,
+ 'node_type' => XML_ELEMENT_NODE,
+ ],
+ ],
+ false,
+ ],
];
}
@@ -4005,20 +4053,20 @@ public function get_data_for_replace_node_with_children_validation_errors() {
* @param string $source_content Source DOM content.
* @param string $expected_content Expected content after sanitization.
* @param array[] $expected_errors Expected errors.
+ * @param bool $sanitize Whether the invalid markup should be sanitized.
*/
- public function test_replace_node_with_children_validation_errors( $source_content, $expected_content, $expected_errors ) {
+ public function test_replace_node_with_children_validation_errors( $source_content, $expected_content, $expected_errors, $sanitize = true ) {
$dom = AMP_DOM_Utils::get_dom_from_content( $source_content );
$sanitizer = new AMP_Tag_And_Attribute_Sanitizer(
$dom,
[
- 'validation_error_callback' => function( $error, $context ) use ( &$expected_errors ) {
+ 'validation_error_callback' => function( $error, $context ) use ( &$expected_errors, $sanitize ) {
$expected = array_shift( $expected_errors );
$tag = $expected['node_name'];
$this->assertEquals( $expected, $error );
$this->assertInstanceOf( 'DOMElement', $context['node'] );
$this->assertEquals( $tag, $context['node']->tagName );
- $this->assertEquals( $tag, $context['node']->nodeName );
- return true;
+ return $sanitize;
},
]
);