diff --git a/composer.json b/composer.json index 2d476100790..64c43960058 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ "ext-json": "*", "ext-libxml": "*", "ext-spl": "*", - "ampproject/amp-toolbox": "dev-main#a155c37", + "ampproject/amp-toolbox": "dev-add/enforced-css-max-byte-count-transformed-identifier-config#718d9dad", "cweagans/composer-patches": "~1.0", "fasterimage/fasterimage": "1.5.0", "sabberworm/php-css-parser": "dev-master#bfdd976" diff --git a/composer.lock b/composer.lock index e0664b3858f..6fab22aa2c7 100644 --- a/composer.lock +++ b/composer.lock @@ -4,20 +4,20 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "21fc63ecbbc5d31716e574af01f7f472", + "content-hash": "78b6a18b78105960db02188eba07a6de", "packages": [ { "name": "ampproject/amp-toolbox", - "version": "dev-main", + "version": "dev-add/enforced-css-max-byte-count-transformed-identifier-config", "source": { "type": "git", "url": "https://github.com/ampproject/amp-toolbox-php.git", - "reference": "a155c37" + "reference": "718d9dad" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/ampproject/amp-toolbox-php/zipball/a155c37", - "reference": "a155c37", + "url": "https://api.github.com/repos/ampproject/amp-toolbox-php/zipball/718d9dad", + "reference": "718d9dad", "shasum": "" }, "require": { @@ -46,7 +46,6 @@ "ext-mbstring": "Used by Dom\\Document to convert encoding to UTF-8 if needed.", "nette/php-generator": "Needed to generate the validator spec PHP classes and interfaces." }, - "default-branch": true, "bin": [ "bin/amp" ], @@ -72,9 +71,9 @@ "description": "A collection of AMP tools making it easier to publish and host AMP pages with PHP.", "support": { "issues": "https://github.com/ampproject/amp-toolbox-php/issues", - "source": "https://github.com/ampproject/amp-toolbox-php/tree/main" + "source": "https://github.com/ampproject/amp-toolbox-php/tree/add/enforced-css-max-byte-count-transformed-identifier-config" }, - "time": "2021-08-11T01:48:15+00:00" + "time": "2021-08-17T06:06:31+00:00" }, { "name": "cweagans/composer-patches", @@ -288,7 +287,7 @@ "packages-dev": [ { "name": "ampproject/php-css-parser-install-plugin", - "version": "dev-enhancement/phpunit-polyfills", + "version": "dev-add/custom-script-allowance", "dist": { "type": "path", "url": "./php-css-parser-install-composer-plugin", @@ -318,16 +317,16 @@ }, { "name": "antecedent/patchwork", - "version": "2.1.12", + "version": "2.1.13", "source": { "type": "git", "url": "https://github.com/antecedent/patchwork.git", - "reference": "b98e046dd4c0acc34a0846604f06f6111654d9ea" + "reference": "0158dbd62bd852b178307d17a5b43a9415ca391b" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/antecedent/patchwork/zipball/b98e046dd4c0acc34a0846604f06f6111654d9ea", - "reference": "b98e046dd4c0acc34a0846604f06f6111654d9ea", + "url": "https://api.github.com/repos/antecedent/patchwork/zipball/0158dbd62bd852b178307d17a5b43a9415ca391b", + "reference": "0158dbd62bd852b178307d17a5b43a9415ca391b", "shasum": "" }, "require": { @@ -360,9 +359,9 @@ ], "support": { "issues": "https://github.com/antecedent/patchwork/issues", - "source": "https://github.com/antecedent/patchwork/tree/2.1.12" + "source": "https://github.com/antecedent/patchwork/tree/2.1.13" }, - "time": "2019-12-22T17:52:09+00:00" + "time": "2021-08-15T16:32:48+00:00" }, { "name": "automattic/vipwpcs", @@ -2033,16 +2032,16 @@ }, { "name": "php-parallel-lint/php-parallel-lint", - "version": "v1.3.0", + "version": "v1.3.1", "source": { "type": "git", "url": "https://github.com/php-parallel-lint/PHP-Parallel-Lint.git", - "reference": "772a954e5f119f6f5871d015b23eabed8cbdadfb" + "reference": "761f3806e30239b5fcd90a0a45d41dc2138de192" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/php-parallel-lint/PHP-Parallel-Lint/zipball/772a954e5f119f6f5871d015b23eabed8cbdadfb", - "reference": "772a954e5f119f6f5871d015b23eabed8cbdadfb", + "url": "https://api.github.com/repos/php-parallel-lint/PHP-Parallel-Lint/zipball/761f3806e30239b5fcd90a0a45d41dc2138de192", + "reference": "761f3806e30239b5fcd90a0a45d41dc2138de192", "shasum": "" }, "require": { @@ -2056,7 +2055,7 @@ "require-dev": { "nette/tester": "^1.3 || ^2.0", "php-parallel-lint/php-console-highlighter": "~0.3", - "squizlabs/php_codesniffer": "^3.5" + "squizlabs/php_codesniffer": "^3.6" }, "suggest": { "php-parallel-lint/php-console-highlighter": "Highlight syntax in code snippet" @@ -2084,9 +2083,9 @@ "homepage": "https://github.com/php-parallel-lint/PHP-Parallel-Lint", "support": { "issues": "https://github.com/php-parallel-lint/PHP-Parallel-Lint/issues", - "source": "https://github.com/php-parallel-lint/PHP-Parallel-Lint/tree/v1.3.0" + "source": "https://github.com/php-parallel-lint/PHP-Parallel-Lint/tree/v1.3.1" }, - "time": "2021-04-07T14:42:48+00:00" + "time": "2021-08-13T05:35:13+00:00" }, { "name": "php-stubs/wordpress-stubs", @@ -5972,12 +5971,12 @@ "source": { "type": "git", "url": "https://github.com/wp-cli/wp-cli.git", - "reference": "da03bb1e8e929fe42a8f9e3814ef284028ee33fc" + "reference": "c4aba2085554e8872c0fb78ba25e7cf8d337e24a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/wp-cli/wp-cli/zipball/da03bb1e8e929fe42a8f9e3814ef284028ee33fc", - "reference": "da03bb1e8e929fe42a8f9e3814ef284028ee33fc", + "url": "https://api.github.com/repos/wp-cli/wp-cli/zipball/c4aba2085554e8872c0fb78ba25e7cf8d337e24a", + "reference": "c4aba2085554e8872c0fb78ba25e7cf8d337e24a", "shasum": "" }, "require": { @@ -6036,7 +6035,7 @@ "issues": "https://github.com/wp-cli/wp-cli/issues", "source": "https://github.com/wp-cli/wp-cli" }, - "time": "2021-08-06T13:54:16+00:00" + "time": "2021-08-12T15:46:55+00:00" }, { "name": "wp-cli/wp-cli-tests", diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 3e6b147637c..d7fbc6f0d4a 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -1478,6 +1478,11 @@ function amp_get_content_sanitizers( $post = null ) { $native_post_forms_allowed = amp_is_native_post_form_allowed(); $sanitizers = [ + // The AMP_Script_Sanitizer runs first because based on whether it allows custom scripts + // to be kept, it may impact the behavior of other sanitizers. For example, if custom + // scripts are kept then this is a signal that tree shaking in AMP_Style_Sanitizer cannot be + // performed. + AMP_Script_Sanitizer::class => [], AMP_Embed_Sanitizer::class => [ 'amp_to_amp_linking_enabled' => $amp_to_amp_linking_enabled, ], @@ -1515,8 +1520,9 @@ function amp_get_content_sanitizers( $post = null ) { 'native_img_used' => $native_img_used, ], AMP_Block_Sanitizer::class => [], // Note: Block sanitizer must come after embed / media sanitizers since its logic is using the already sanitized content. - AMP_Script_Sanitizer::class => [], - AMP_Style_Sanitizer::class => [], + AMP_Style_Sanitizer::class => [ + 'skip_tree_shaking' => is_customize_preview(), + ], AMP_Meta_Sanitizer::class => [], AMP_Layout_Sanitizer::class => [], AMP_Accessibility_Sanitizer::class => [], diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index d53afad3e04..30a233bfea4 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -18,6 +18,8 @@ use AmpProject\Dom\Document; use AmpProject\Extension; use AmpProject\Optimizer; +use AmpProject\Optimizer\Configuration\TransformedIdentifierConfiguration; +use AmpProject\Optimizer\Transformer\TransformedIdentifier; use AmpProject\RequestDestination; use AmpProject\Tag; use AmpProject\AmpWP\Optimizer\Transformer\AmpSchemaOrgMetadata; @@ -1531,7 +1533,11 @@ 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_native_post_form_allowed() && $dom->xpath->query( '//form[ @action and @method and translate( @method, "POST", "post" ) = "post" ]' )->length > 0 ) { + if ( + in_array( Extension::FORM, $script_handles, true ) + && + $dom->xpath->query( '//form[ @action and @method and translate( @method, "POST", "post" ) = "post" ]' )->length > 0 + ) { $superfluous_script_handles[] = Extension::FORM; } @@ -2075,6 +2081,9 @@ public static function prepare_response( $response, $args = [] ) { do_action( 'amp_server_timing_start', 'amp_optimizer' ); $errors = new Optimizer\ErrorCollection(); + + // @todo The dev mode attribute is being overloaded here. We should use something else. + $args['skip_css_max_byte_count_enforcement'] = $dom->documentElement->hasAttribute( DevMode::DEV_MODE_ATTRIBUTE ); self::get_optimizer( $args )->optimizeDom( $dom, $errors ); if ( count( $errors ) > 0 ) { @@ -2164,10 +2173,13 @@ static function () use ( $args ) { // Supply the Schema.org metadata, previously obtained just before output buffering began, to the AmpSchemaOrgMetadataConfiguration. add_filter( 'amp_optimizer_config', - function ( $config ) { + function ( $config ) use ( $args ) { if ( is_array( self::$metadata ) ) { $config[ AmpSchemaOrgMetadata::class ][ AmpSchemaOrgMetadataConfiguration::METADATA ] = self::$metadata; } + if ( ! empty( $args['skip_css_max_byte_count_enforcement'] ) ) { + $config[ TransformedIdentifier::class ][ TransformedIdentifierConfiguration::ENFORCED_CSS_MAX_BYTE_COUNT ] = false; + } return $config; }, defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : ~PHP_INT_MAX // phpcs:ignore PHPCompatibility.Constants.NewConstants.php_int_minFound diff --git a/includes/sanitizers/class-amp-base-sanitizer.php b/includes/sanitizers/class-amp-base-sanitizer.php index 6f246e6b78e..e668d7fd4c9 100644 --- a/includes/sanitizers/class-amp-base-sanitizer.php +++ b/includes/sanitizers/class-amp-base-sanitizer.php @@ -140,6 +140,17 @@ public function get_selector_conversion_mapping() { return []; } + /** + * Update args. + * + * Merges the supplied args with the existing args. + * + * @param array $args Args. + */ + public function update_args( $args ) { + $this->args = array_merge( $this->args, $args ); + } + /** * Run logic before any sanitizers are run. * diff --git a/includes/sanitizers/class-amp-script-sanitizer.php b/includes/sanitizers/class-amp-script-sanitizer.php index b630e293eee..cc19d93e576 100644 --- a/includes/sanitizers/class-amp-script-sanitizer.php +++ b/includes/sanitizers/class-amp-script-sanitizer.php @@ -6,7 +6,11 @@ * @package AMP */ +use AmpProject\Attribute; use AmpProject\DevMode; +use AmpProject\Dom\Element; +use AmpProject\Extension; +use AmpProject\Tag; /** * Class AMP_Script_Sanitizer @@ -17,24 +21,167 @@ class AMP_Script_Sanitizer extends AMP_Base_Sanitizer { /** - * Sanitize noscript elements. + * Error code for custom inline JS script tag. * - * Eventually this should also handle script elements, if there is a known AMP equivalent. - * If nothing is done with script elements, the validating sanitizer will deal with them ultimately. + * @var string + */ + const CUSTOM_INLINE_SCRIPT = 'CUSTOM_INLINE_SCRIPT'; + + /** + * Error code for custom external JS script tag. + * + * @var string + */ + const CUSTOM_EXTERNAL_SCRIPT = 'CUSTOM_EXTERNAL_SCRIPT'; + + /** + * Error code for JS event handler attribute. + * + * @var string + */ + const CUSTOM_EVENT_HANDLER_ATTR = 'CUSTOM_EVENT_HANDLER_ATTR'; + + /** + * Attribute which if present on a `noscript` element will prevent it from being unwrapped. + * + * @var string + */ + const NO_UNWRAP_ATTR = 'data-amp-no-unwrap'; + + /** + * Default args. + * + * @var array + */ + protected $DEFAULT_ARGS = [ + 'unwrap_noscripts' => true, + 'sanitize_js_scripts' => false, + ]; + + /** + * Array of flags used to control sanitization. + * + * @var array { + * @type bool $sanitize_js_scripts Whether to sanitize JS scripts (and not defer for final sanitizer). + * @type bool $unwrap_noscripts Whether to unwrap noscript elements. + * } + */ + protected $args; + + /** + * Number of scripts which were kept. * - * @todo Eventually this try to automatically convert script tags to AMP when they are recognized. See . - * @todo When a script has an adjacent noscript, consider removing the script here to prevent validation error later. See . + * This is used to determine whether noscript elements should be unwrapped. + * + * @var int + */ + protected $kept_script_count = 0; + + /** + * Style sanitizer. + * + * @var AMP_Style_Sanitizer + */ + protected $style_sanitizer; + + /** + * Image sanitizer. + * + * @var AMP_Img_Sanitizer + */ + protected $img_sanitizer; + + /** + * Form sanitizer. + * + * @var AMP_Form_Sanitizer + */ + protected $form_sanitizer; + + /** + * Init. + * + * @param AMP_Base_Sanitizer[] $sanitizers Sanitizers. + */ + public function init( $sanitizers ) { + parent::init( $sanitizers ); + + if ( + array_key_exists( AMP_Style_Sanitizer::class, $sanitizers ) + && + $sanitizers[ AMP_Style_Sanitizer::class ] instanceof AMP_Style_Sanitizer + ) { + $this->style_sanitizer = $sanitizers[ AMP_Style_Sanitizer::class ]; + } + + if ( + array_key_exists( AMP_Img_Sanitizer::class, $sanitizers ) + && + $sanitizers[ AMP_Img_Sanitizer::class ] instanceof AMP_Img_Sanitizer + ) { + $this->img_sanitizer = $sanitizers[ AMP_Img_Sanitizer::class ]; + } + + if ( + array_key_exists( AMP_Form_Sanitizer::class, $sanitizers ) + && + $sanitizers[ AMP_Form_Sanitizer::class ] instanceof AMP_Form_Sanitizer + ) { + $this->form_sanitizer = $sanitizers[ AMP_Form_Sanitizer::class ]; + } + } + + /** + * Sanitize script and noscript elements. * * @since 1.0 */ public function sanitize() { - $noscripts = $this->dom->getElementsByTagName( 'noscript' ); + if ( ! empty( $this->args['sanitize_js_scripts'] ) ) { + $this->sanitize_js_script_elements(); + } + + // 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'] ) ) { + $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. + // @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 ) { + if ( $this->style_sanitizer ) { + $this->style_sanitizer->update_args( [ 'skip_tree_shaking' => true ] ); + } + if ( $this->img_sanitizer ) { + $this->img_sanitizer->update_args( [ 'native_img_used' => true ] ); + } + if ( $this->form_sanitizer ) { + $this->form_sanitizer->update_args( [ 'native_post_forms_allowed' => true ] ); + } + } + } + + /** + * Unwrap noscript elements so their contents become the AMP version by default. + */ + protected function unwrap_noscript_elements() { + $noscripts = $this->dom->getElementsByTagName( Tag::NOSCRIPT ); for ( $i = $noscripts->length - 1; $i >= 0; $i-- ) { + /** @var Element $noscript */ $noscript = $noscripts->item( $i ); // Skip AMP boilerplate. - if ( $noscript->firstChild instanceof DOMElement && $noscript->firstChild->hasAttribute( 'amp-boilerplate' ) ) { + if ( $noscript->firstChild instanceof Element && $noscript->firstChild->hasAttribute( Attribute::AMP_BOILERPLATE ) ) { + continue; + } + + // Skip unwrapping