diff --git a/assets/src/settings-page/index.js b/assets/src/settings-page/index.js index d0fe6263bcc..a888b4ea9c5 100644 --- a/assets/src/settings-page/index.js +++ b/assets/src/settings-page/index.js @@ -44,6 +44,7 @@ import { PluginsContextProvider } from '../components/plugins-context-provider'; import { ThemesContextProvider } from '../components/themes-context-provider'; import { SiteScanContextProvider } from '../components/site-scan-context-provider'; import { Welcome } from './welcome'; +import { SandboxingLevel } from './sandboxing-level'; import { TemplateModes } from './template-modes'; import { SupportedTemplates } from './supported-templates'; import { SettingsFooter } from './settings-footer'; @@ -221,6 +222,7 @@ function Root( { appRoot } ) {

{ __( 'Advanced Settings', 'amp' ) }

+ diff --git a/assets/src/settings-page/sandboxing-level.js b/assets/src/settings-page/sandboxing-level.js new file mode 100644 index 00000000000..4923dd9ee29 --- /dev/null +++ b/assets/src/settings-page/sandboxing-level.js @@ -0,0 +1,105 @@ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * WordPress dependencies + */ +import { useContext } from '@wordpress/element'; +import { __ } from '@wordpress/i18n'; + +/** + * Internal dependencies + */ +import { Options } from '../components/options-context-provider'; +import { AMPDrawer } from '../components/amp-drawer'; +import { STANDARD } from '../common/constants'; + +/** + * Component rendering the sandboxing level. + * + * @param {Object} props Component props. + * @param {string} props.focusedSection Focused section. + */ +export function SandboxingLevel( { focusedSection } ) { + const { updateOptions, editedOptions: { + sandboxing_level: sandboxingLevel, + theme_support: themeSupport, + } } = useContext( Options ); + + if ( ! sandboxingLevel || STANDARD !== themeSupport ) { + return null; + } + + return ( + + { __( 'Sandboxing Level (Experimental)', 'amp' ) } + + ) } + hiddenTitle={ __( 'Sandboxing Level (Experimental)', 'amp' ) } + id="sandboxing-level" + initialOpen={ 'sandboxing-level' === focusedSection } + > +

+ { __( 'Try out a more flexible AMP by generating pages that use AMP components without requiring AMP validity! By selecting a sandboxing level, you are indicating the minimum degree of sanitization. For example, if you selected the loose leve but have a page without any POST form and no custom scripts, it will still be served as valid AMP—the same as if you had selected the strict level.', 'amp' ) } +

+
    +
  1. + { + updateOptions( { sandboxing_level: 1 } ); + } } + /> + +
    2. +
  2. + { + updateOptions( { sandboxing_level: 2 } ); + } } + /> + +
    3. +
  3. + { + updateOptions( { sandboxing_level: 3 } ); + } } + /> + +
    4. + +
+ + ); +} +SandboxingLevel.propTypes = { + focusedSection: PropTypes.string, +}; diff --git a/assets/src/settings-page/style.css b/assets/src/settings-page/style.css index d6b884f2c00..2d7af6455fe 100644 --- a/assets/src/settings-page/style.css +++ b/assets/src/settings-page/style.css @@ -367,6 +367,7 @@ li.error-kept { } } +#sandboxing-level .amp-drawer__panel-body-inner, #site-scan .amp-drawer__panel-body-inner, #site-review .amp-drawer__panel-body-inner, #plugin-suppression .amp-drawer__panel-body-inner, @@ -658,3 +659,31 @@ li.error-kept { .amp-other-settings p { font-size: 0.875rem; } + +/* Sandboxing Level */ +#sandboxing-level .amp-drawer__panel-body-inner p { + font-size: 14px; + margin-top: 0; +} + +#sandboxing-level .amp-drawer__panel-body-inner ol { + margin-top: 1.5rem; + margin-left: 0; +} + +#sandboxing-level .amp-drawer__panel-body-inner li { + list-style-type: none; +} + +#sandboxing-level .amp-drawer__panel-body-inner li:not(:last-child) { + margin-bottom: 12px; +} + +#sandboxing-level .amp-drawer__panel-body-inner input[type="radio"] { + margin-right: 12px; +} + +#sandboxing-level .amp-drawer__panel-body-inner input[type="radio"], +#sandboxing-level .amp-drawer__panel-body-inner label { + vertical-align: middle; +} diff --git a/assets/src/settings-page/template-modes.js b/assets/src/settings-page/template-modes.js index 624d3d0eb55..254ef861130 100644 --- a/assets/src/settings-page/template-modes.js +++ b/assets/src/settings-page/template-modes.js @@ -62,10 +62,8 @@ function NotRecommendedNotice() { export function TemplateModes( { focusReaderThemes } ) { const { editedOptions: { - sandboxing_level: sandboxingLevel, theme_support: editedThemeSupport, }, - updateOptions, } = useContext( Options ); const { selectedTheme, templateModeWasOverridden } = useContext( ReaderThemes ); const { templateModeRecommendation, staleTemplateModeRecommendation } = useTemplateModeRecommendation(); @@ -106,71 +104,7 @@ export function TemplateModes( { focusReaderThemes } ) { initialOpen={ false } mode={ STANDARD } labelExtra={ getLabelForTemplateMode( STANDARD ) } - > - { - sandboxingLevel && ( -
-

- { __( 'Sandboxing Level (Experimental)', 'amp' ) } -

-

- { __( 'Try out a more flexible AMP by generating pages that use AMP components without requiring AMP validity! By selecting a sandboxing level, you are indicating the minimum degree of sanitization. For example, if you selected level 1 but have a page without any POST form and no custom scripts, it will still be served as valid AMP, the same as if you had selected level 3.', 'amp' ) } -

-
    -
  1. - { - updateOptions( { sandboxing_level: 1 } ); - } } - /> - -
    2. -
  2. - { - updateOptions( { sandboxing_level: 2 } ); - } } - /> - -
    3. -
  3. - { - updateOptions( { sandboxing_level: 3 } ); - } } - /> - -
    4. - -
-
- ) - } - + /> finalize_document( $dom, $effective_sandboxing_level ); - } + /** + * Fires immediately before the DOM is serialized to send as the response body. + * + * @since 2.2 + * @internal + * + * @param Document $dom Document prior to serialization. + * @param int $effective_sandboxing_level Effective sandboxing level. + */ + do_action( 'amp_finalize_dom', $dom, $effective_sandboxing_level ); $response = $dom->saveHTML(); diff --git a/includes/sanitizers/class-amp-comments-sanitizer.php b/includes/sanitizers/class-amp-comments-sanitizer.php index bdda9ca6b00..1ce80107e50 100644 --- a/includes/sanitizers/class-amp-comments-sanitizer.php +++ b/includes/sanitizers/class-amp-comments-sanitizer.php @@ -59,69 +59,90 @@ public function init( $sanitizers ) { * @since 0.7 */ public function sanitize() { - foreach ( $this->dom->getElementsByTagName( Tag::FORM ) as $comment_form ) { - $action = $comment_form->getAttribute( Attribute::ACTION_XHR ); + + // Find the comment form (which may not have the commentform ID). + $comment_form = null; + foreach ( $this->dom->getElementsByTagName( Tag::FORM ) as $form ) { + $action = $form->getAttribute( Attribute::ACTION_XHR ); if ( ! $action ) { - $action = $comment_form->getAttribute( Attribute::ACTION ); + $action = $form->getAttribute( Attribute::ACTION ); } $action_path = wp_parse_url( $action, PHP_URL_PATH ); if ( $action_path && 'wp-comments-post.php' === basename( $action_path ) ) { - $this->ampify_threaded_comments( $comment_form ); + $comment_form = $form; + break; } } - if ( $this->args['comments_live_list'] ) { - $comments = $this->dom->xpath->query( '//amp-live-list/*[ @items ]/*[ starts-with( @id, "comment-" ) ]' ); - - foreach ( $comments as $comment ) { - $this->add_amp_live_list_comment_attributes( $comment ); - } - } - } - - /** - * Ampify threaded comments by utilizing amp-bind to implement comment reply functionality. - * - * The logic here is only needed if: - * 1. Threaded comments is enabled, and - * 2. The comment-reply script was not added to the page. - * - * @param Element $comment_form Comment form. - */ - protected function ampify_threaded_comments( Element $comment_form ) { - if ( ! get_option( 'thread_comments' ) ) { - return; - } - - $comment_reply_script = $this->dom->xpath->query( '//script[ @id = "comment-reply-js" ]' )->item( 0 ); - if ( 'never' === $this->args['ampify_comment_threading'] ) { - if ( $comment_reply_script instanceof Element ) { - $this->prepare_native_comment_reply( $comment_reply_script ); - } - return; - } - + // Handle the comment reply script. + $comment_reply_script = $this->get_comment_reply_script(); + $should_ampify_comment_threading = false; if ( $comment_reply_script instanceof Element ) { - if ( + if ( 'never' === $this->args['ampify_comment_threading'] ) { + $this->prepare_native_comment_reply( $comment_reply_script ); + $should_ampify_comment_threading = false; + } elseif ( 'always' === $this->args['ampify_comment_threading'] || ( 'conditionally' === $this->args['ampify_comment_threading'] && - $comment_form->hasAttribute( Attribute::ACTION_XHR ) + ( + // If there isn't even a comment form on the page, then the comment-reply script shouldn't be here at all. + ! $comment_form instanceof Element + || + // If the form has an action-xhr attribute, then it's going to be an AMP page and we need ampified comment threading. + $comment_form->hasAttribute( Attribute::ACTION_XHR ) + ) ) ) { // Remove the script and then proceed with the amp-bind implementation below. $comment_reply_script->parentNode->removeChild( $comment_reply_script ); + $should_ampify_comment_threading = true; } else { // This is the conditionally-no case. $this->prepare_native_comment_reply( $comment_reply_script ); // Do not proceed with the AMP-bind implementation for threaded comments since the comment-reply script was included. - return; + $should_ampify_comment_threading = false; } } + // Now based on the comment reply script handling above, ampify the comment form. + if ( get_option( 'thread_comments' ) && $comment_form && $should_ampify_comment_threading ) { + $this->ampify_threaded_comments( $comment_form ); + } + + if ( $this->args['comments_live_list'] ) { + $comments = $this->dom->xpath->query( '//amp-live-list/*[ @items ]/*[ starts-with( @id, "comment-" ) ]' ); + + foreach ( $comments as $comment ) { + $this->add_amp_live_list_comment_attributes( $comment ); + } + } + } + + /** + * Get comment reply script. + * + * @return Element|null + */ + protected function get_comment_reply_script() { + $element = $this->dom->getElementById( 'comment-reply-js' ); + if ( $element instanceof Element && Tag::SCRIPT === $element->tagName ) { + return $element; + } else { + return null; + } + } + + /** + * Ampify threaded comments by utilizing amp-bind to implement comment reply functionality. + * + * @param Element $comment_form Comment form. + */ + protected function ampify_threaded_comments( Element $comment_form ) { + // Create reply state. $amp_state = $this->dom->createElement( Extension::STATE ); $comment_form->insertBefore( $amp_state, $comment_form->firstChild ); diff --git a/src/Sandboxing.php b/src/Sandboxing.php index 95bc3d48374..ced3e11034f 100644 --- a/src/Sandboxing.php +++ b/src/Sandboxing.php @@ -176,6 +176,8 @@ static function ( $error ) use ( $sandboxing_level ) { return $error; } ); + + add_action( 'amp_finalize_dom', [ $this, 'finalize_document' ], 10, 2 ); } /** @@ -197,11 +199,49 @@ public static function get_effective_level( Document $dom ) { } } + /** + * Remove required AMP markup if not used. + * + * @param Document $dom Document. + * @param int $effective_sandboxing_level Effective sandboxing level. + */ + private function remove_required_amp_markup_if_not_used( Document $dom, $effective_sandboxing_level ) { + if ( 3 === $effective_sandboxing_level ) { + // When valid AMP is the target, don't remove the scripts since it won't be valid AMP. + return; + } + + $amp_scripts = $dom->xpath->query( '//script[ @custom-element or @custom-template ]' ); + if ( $amp_scripts->length > 0 ) { + return; + } + + // Remove runtime script(s). + $runtime_scripts = $dom->xpath->query( '//script[ @async and @src and starts-with( @src, "https://cdn.ampproject.org/" ) and contains( @src, "v0" ) ]' ); + foreach ( $runtime_scripts as $runtime_script ) { + if ( $runtime_script instanceof Element ) { + $runtime_script->parentNode->removeChild( $runtime_script ); + } + } + + // Remove runtime style. + $runtime_style = $dom->xpath->query( './style[ @amp-runtime ]', $dom->head )->item( 0 ); + if ( $runtime_style instanceof Element ) { + $dom->head->removeChild( $runtime_style ); + } + + // Remove preconnect link. + $preconnect_link = $dom->xpath->query( './link[ @href = "https://cdn.ampproject.org" ]', $dom->head )->item( 0 ); + if ( $preconnect_link instanceof Element ) { + $dom->head->removeChild( $preconnect_link ); + } + } + /** * Finalize document. * - * @param Document $dom Document. - * @param int|null $effective_sandboxing_level Effective sandboxing level. + * @param Document $dom Document. + * @param int $effective_sandboxing_level Effective sandboxing level. */ public function finalize_document( Document $dom, $effective_sandboxing_level ) { $actual_sandboxing_level = AMP_Options_Manager::get_option( self::OPTION_LEVEL ); @@ -211,6 +251,8 @@ public function finalize_document( Document $dom, $effective_sandboxing_level ) $meta_generator->nodeValue .= "; sandboxing-level={$actual_sandboxing_level}:{$effective_sandboxing_level}"; } + $this->remove_required_amp_markup_if_not_used( $dom, $effective_sandboxing_level ); + $amp_admin_bar_menu_item = $dom->xpath->query( '//div[ @id = "wpadminbar" ]//li[ @id = "wp-admin-bar-amp" ]/a' )->item( 0 ); if ( $amp_admin_bar_menu_item instanceof Element ) { $span = $dom->createElement( Tag::SPAN ); diff --git a/tests/php/src/SandboxingTest.php b/tests/php/src/SandboxingTest.php index 94502c5a673..301063eb5a1 100644 --- a/tests/php/src/SandboxingTest.php +++ b/tests/php/src/SandboxingTest.php @@ -196,25 +196,35 @@ public function test_add_hooks( $level, $expected_sanitizer_args ) { /** @return array */ public function get_data_to_test_finalize_document() { return [ - 'level_3_to_3' => [ - 'min_level' => 3, - 'body' => '
', - 'expected_level' => 3, + 'level_3_to_3' => [ + 'min_level' => 3, + 'body' => '
', + 'expected_level' => 3, + 'expected_required_amp_markup' => true, ], - 'level_1_to_2' => [ - 'min_level' => 1, - 'body' => sprintf( '
', ValidationExemption::PX_VERIFIED_TAG_ATTRIBUTE ), - 'expected_level' => 2, + 'level_1_to_2' => [ + 'min_level' => 1, + 'body' => sprintf( '
', ValidationExemption::PX_VERIFIED_TAG_ATTRIBUTE ), + 'expected_level' => 2, + 'expected_required_amp_markup' => false, ], - 'level_1_to_1' => [ - 'min_level' => 1, - 'body' => sprintf( '
', ValidationExemption::AMP_UNVALIDATED_TAG_ATTRIBUTE ), - 'expected_level' => 1, + 'level_1_to_2_with_v0' => [ + 'min_level' => 1, + 'body' => sprintf( '
', ValidationExemption::PX_VERIFIED_TAG_ATTRIBUTE ), + 'expected_level' => 2, + 'expected_required_amp_markup' => true, ], - 'level_1_to_1_with_2' => [ - 'min_level' => 1, - 'body' => sprintf( '
', ValidationExemption::AMP_UNVALIDATED_TAG_ATTRIBUTE, ValidationExemption::PX_VERIFIED_TAG_ATTRIBUTE ), - 'expected_level' => 1, + 'level_1_to_1' => [ + 'min_level' => 1, + 'body' => sprintf( '
', ValidationExemption::AMP_UNVALIDATED_TAG_ATTRIBUTE ), + 'expected_level' => 1, + 'expected_required_amp_markup' => false, + ], + 'level_1_to_1_with_2' => [ + 'min_level' => 1, + 'body' => sprintf( '
', ValidationExemption::AMP_UNVALIDATED_TAG_ATTRIBUTE, ValidationExemption::PX_VERIFIED_TAG_ATTRIBUTE ), + 'expected_level' => 1, + 'expected_required_amp_markup' => false, ], ]; } @@ -223,8 +233,9 @@ public function get_data_to_test_finalize_document() { * @dataProvider get_data_to_test_finalize_document * @covers ::get_effective_level() * @covers ::finalize_document() + * @covers ::remove_required_amp_markup_if_not_used() */ - public function test_finalize_document_and_get_effective_level( $min_level, $body, $expected_level ) { + public function test_finalize_document_and_get_effective_level( $min_level, $body, $expected_level, $expected_required_amp_markup ) { AMP_Options_Manager::update_option( Sandboxing::OPTION_LEVEL, $min_level ); $dom = Document::fromHtml( @@ -232,6 +243,10 @@ public function test_finalize_document_and_get_effective_level( $min_level, $bod ' + + + + %s @@ -249,5 +264,16 @@ public function test_finalize_document_and_get_effective_level( $min_level, $bod $meta = $dom->xpath->query( '//meta[ @name = "generator" ]' )->item( 0 ); $this->assertInstanceOf( Element::class, $meta ); $this->assertStringEndsWith( "foo=bar; sandboxing-level={$min_level}:{$expected_level}", $meta->getAttribute( Attribute::CONTENT ) ); + + $expressions = [ + '//link[ @rel = "preconnect" and @href = "https://cdn.ampproject.org" ]', + '//style[ @amp-runtime ]', + '//script[ @src = "https://cdn.ampproject.org/v0.mjs" ]', + '//script[ @src = "https://cdn.ampproject.org/v0.js" ]', + ]; + foreach ( $expressions as $expression ) { + $this->assertEquals( $expected_required_amp_markup ? 1 : 0, $dom->xpath->query( $expression )->length, $expression ); + } + } } diff --git a/tests/php/test-class-amp-comments-sanitizer.php b/tests/php/test-class-amp-comments-sanitizer.php index 2ef1fcfd9cb..99fb4817f4d 100644 --- a/tests/php/test-class-amp-comments-sanitizer.php +++ b/tests/php/test-class-amp-comments-sanitizer.php @@ -110,6 +110,7 @@ public function get_data_to_test_ampify_threaded_comments_always_and_conditional * @covers ::sanitize() * @covers ::ampify_threaded_comments() * @covers ::prepare_native_comment_reply() + * @covers ::get_comment_reply_script() */ public function test_ampify_threaded_comments( $ampify_comment_threading, $comments_form_has_action_xhr, $expect_ampify_comment_threading ) { if ( version_compare( get_bloginfo( 'version' ), '5.2', '<' ) ) { @@ -228,6 +229,7 @@ public function test_ampify_threaded_comments( $ampify_comment_threading, $comme /** * Test AMP_Comments_Sanitizer::add_amp_live_list_comment_attributes. * + * @covers ::sanitize() * @covers ::add_amp_live_list_comment_attributes() */ public function test_add_amp_live_list_comment_attributes() { diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 621b3c5c502..b2d0df46c8d 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -1682,8 +1682,22 @@ static function ( $url ) { return AMP_Theme_Support::prepare_response( $original_html ); }; + $amp_finalize_dom_count = did_action( 'amp_finalize_dom' ); + add_action( + 'amp_finalize_dom', + function ( $dom, $effective_sandboxing_level ) { + $this->assertInstanceOf( Document::class, $dom ); + $this->assertIsInt( $effective_sandboxing_level ); + $this->assertSame( 3, $effective_sandboxing_level ); + }, + 10, + 2 + ); + $sanitized_html = $call_prepare_response(); + $this->assertSame( $amp_finalize_dom_count + 1, did_action( 'amp_finalize_dom' ) ); + $this->assertStringNotContainsString( 'handle=', $sanitized_html ); $this->assertEquals( 2, did_action( 'wp_print_scripts' ) ); @@ -1922,8 +1936,23 @@ static function ( $sanitizers ) use ( $converted ) { assertInstanceOf( Document::class, $dom ); + $this->assertIsInt( $effective_sandboxing_level ); + $this->assertSame( $converted ? 3 : 2, $effective_sandboxing_level ); + }, + 10, + 2 + ); + $html = AMP_Theme_Support::prepare_response( ob_get_clean() ); + $this->assertSame( $amp_finalize_dom_count + 1, did_action( 'amp_finalize_dom' ) ); + $dom = Document::fromHtml( $html ); $this->assertEquals( $converted, $dom->documentElement->hasAttribute( Attribute::AMP ) );