From 531de257af9fa9dcbbc9b74a9d11a937c55e4e3d Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Tue, 10 Aug 2021 16:57:12 -0700 Subject: [PATCH] Eliminate examination of collected validation errors to decide to omit amp-form script --- includes/class-amp-theme-support.php | 13 +---- tests/php/test-class-amp-theme-support.php | 65 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 11 deletions(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index ca9aaa7450a..5bc77cf5311 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1514,17 +1514,8 @@ public static function ensure_required_markup( Document $dom, $script_handles = } // When opting-in to POST forms, omit the amp-form component entirely since it blocks submission. - if ( amp_is_allowing_post_forms() ) { - foreach ( AMP_Validation_Manager::$validation_results as $validation_result ) { - if ( - ! $validation_result['sanitized'] - && - AMP_Form_Sanitizer::FORM_HAS_POST_METHOD === $validation_result['error']['code'] - ) { - $superfluous_script_handles[] = Extension::FORM; - break; - } - } + if ( amp_is_allowing_post_forms() && $dom->xpath->query( '//form[ @method = "post" and @action ]' )->length > 0 ) { + $superfluous_script_handles[] = Extension::FORM; } foreach ( $superfluous_script_handles as $superfluous_script_handle ) { diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 417c401421a..e7140d90a10 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -14,7 +14,10 @@ use AmpProject\AmpWP\Tests\Helpers\AssertContainsCompatibility; use AmpProject\AmpWP\Tests\Helpers\PrivateAccess; use AmpProject\AmpWP\Tests\Helpers\LoadsCoreThemes; +use AmpProject\Attribute; +use AmpProject\DevMode; use AmpProject\Dom\Document; +use AmpProject\Dom\Element; use org\bovigo\vfs; /** @@ -1796,6 +1799,68 @@ public function test_prepare_response_standard_mode_non_amp() { $this->assertStringContains( 'document.write = function', $sanitized_html, 'Override of document.write() is present.' ); } + /** @return array */ + public function get_data_for_allowing_post_forms() { + return [ + 'keep_post_forms' => [ false ], + 'convert_post_forms' => [ true ], + ]; + } + + /** + * Test prepare_response when allowing post forms. + * + * @dataProvider get_data_for_allowing_post_forms + * @covers AMP_Theme_Support::prepare_response() + * @param bool $converted Whether the POST form should be converted to amp-form. + */ + public function test_prepare_response_when_allowing_post_forms( $converted ) { + $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 === $error['code'] ) { + $sanitized = $converted; + } + return $sanitized; + }, + 10, + 2 + ); + + wp(); + add_filter( 'amp_allowing_native_post_forms', '__return_true' ); + AMP_Theme_Support::init(); + AMP_Theme_Support::finish_init(); + ob_start(); + ?> + + +
+ +
+ + + assertEquals( $converted, $dom->documentElement->hasAttribute( Attribute::AMP ) ); + $this->assertEquals( ! $converted, $dom->documentElement->hasAttribute( DevMode::DEV_MODE_ATTRIBUTE ) ); + $this->assertEquals( + $converted, + $dom->xpath->query( '//script[ @custom-element = "amp-form" ]' )->length > 0 + ); + $form = $dom->getElementsByTagName( 'form' )->item( 0 ); + $this->assertInstanceOf( Element::class, $form ); + $this->assertEquals( 'post', $form->getAttribute( Attribute::METHOD ) ); + $this->assertEquals( $converted, $form->hasAttribute( Attribute::ACTION_XHR ) ); + $this->assertEquals( ! $converted, $form->hasAttribute( Attribute::ACTION ) ); + $this->assertEquals( ! $converted, $form->hasAttribute( DevMode::DEV_MODE_ATTRIBUTE ) ); + } + /** * Test prepare_response when submitting form. *