Remove scripts for components that were not detected in output buffer

bin/amphtml-update.py

@@ -211,6 +211,29 @@ def GenerateFooterPHP(out):
 		return self::$allowed_tags;
 	}
 
+	/**
+	 * Get extension specs.
+	 *
+	 * @since 1.5
+	 * @internal
+	 * @return array Extension specs, keyed by extension name.
+	 */
+	public static function get_extension_specs() {
+		static $extension_specs = [];
+
+		if ( ! empty( $extension_specs ) ) {
+			return $extension_specs;
+		}
+
+		foreach ( self::get_allowed_tag( 'script' ) as $script_spec ) {
+			if ( isset( $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec'] ) ) {
+				$extension_specs[ $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec']['name'] ] = $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec'];
+			}
+		}
+
+		return $extension_specs;
+	}
+
 	/**
 	 * Get allowed tag.
 	 *
@@ -463,11 +486,20 @@ def GetTagRules(tag_spec):
 			also_requires_tag_list.append(UnicodeEscape(also_requires_tag))
 		tag_rules['also_requires_tag'] = also_requires_tag_list
 
+	requires_extension_list = set()
 	if hasattr(tag_spec, 'requires_extension') and len( tag_spec.requires_extension ) != 0:
-		requires_extension_list = []
 		for requires_extension in tag_spec.requires_extension:
-			requires_extension_list.append(requires_extension)
-		tag_rules['requires_extension'] = requires_extension_list
+			requires_extension_list.add(requires_extension)
+
+	if hasattr(tag_spec, 'also_requires_tag_warning') and len( tag_spec.also_requires_tag_warning ) != 0:
+		for also_requires_tag_warning in tag_spec.also_requires_tag_warning:
+			matches = re.search( r'(amp-\S+) extension .js script', also_requires_tag_warning )
+			if not matches:
+				raise Exception( 'Unexpected also_requires_tag_warning format: ' + also_requires_tag_warning )
+			requires_extension_list.add(matches.group(1))
+
+	if len( requires_extension_list ) > 0:
+		tag_rules['requires_extension'] = list( requires_extension_list )
 
 	if hasattr(tag_spec, 'reference_points') and len( tag_spec.reference_points ) != 0:
 		tag_reference_points = {}
@@ -479,12 +511,6 @@ def GetTagRules(tag_spec):
 		if len( tag_reference_points ) > 0:
 			tag_rules['reference_points'] = tag_reference_points
 
-	if hasattr(tag_spec, 'also_requires_tag_warning') and len( tag_spec.also_requires_tag_warning ) != 0:
-		also_requires_tag_warning_list = []
-		for also_requires_tag_warning in tag_spec.also_requires_tag_warning:
-			also_requires_tag_warning_list.append(also_requires_tag_warning)
-		tag_rules['also_requires_tag_warning'] = also_requires_tag_warning_list
-
 	if tag_spec.disallowed_ancestor:
 		disallowed_ancestor_list = []
 		for disallowed_ancestor in tag_spec.disallowed_ancestor:
@@ -510,14 +536,43 @@ def GetTagRules(tag_spec):
 		return None
 
 	if tag_spec.HasField('extension_spec'):
-		extension_spec = {}
+		# See https://github.com/ampproject/amphtml/blob/e37f50d/validator/validator.proto#L430-L454
+		ERROR = 1
+		NONE = 3
+		extension_spec = {
+			'requires_usage': 1 # (ERROR=1)
+		}
 		for field in tag_spec.extension_spec.ListFields():
 			if isinstance(field[1], (list, google.protobuf.internal.containers.RepeatedScalarFieldContainer, google.protobuf.pyext._message.RepeatedScalarContainer)):
 				extension_spec[ field[0].name ] = []
 				for val in field[1]:
 					extension_spec[ field[0].name ].append( val )
 			else:
 				extension_spec[ field[0].name ] = field[1]
+
+		# Normalize ERROR and GRANDFATHERED as true, since we control which scripts are added (or removed) from the output.
+		extension_spec['requires_usage'] = ( extension_spec['requires_usage'] != 3 ) # NONE=3
+
+		if 'version' not in extension_spec:
+			raise Exception( 'Missing required version field' )
+		if 'name' not in extension_spec:
+			raise Exception( 'Missing required name field' )
+
+		# Get the versions and sort.
+		versions = set( extension_spec['version'] )
+		versions.remove( 'latest' )
+		extension_spec['version'] = sorted( versions, key=lambda version: map(int, version.split('.') ) )
+
+		# Unused since amp_filter_script_loader_tag() and \AMP_Tag_And_Attribute_Sanitizer::get_rule_spec_list_to_validate() just hard-codes the check for amp-mustache.
+		if 'extension_type' in extension_spec:
+			del extension_spec['extension_type']
+
+		if 'deprecated_version' in extension_spec:
+			del extension_spec['deprecated_version']
+
+		if 'deprecated_allow_duplicates' in extension_spec:
+			del extension_spec['deprecated_allow_duplicates']
+
 		tag_rules['extension_spec'] = extension_spec
 
 	if tag_spec.HasField('mandatory'):

includes/amp-helper-functions.php

@@ -464,39 +464,30 @@ function amp_register_default_scripts( $wp_scripts ) {
 		]
 	);
 
-	// Get all AMP components as defined in the spec.
-	$extensions = [];
-	foreach ( AMP_Allowed_Tags_Generated::get_allowed_tag( 'script' ) as $script_spec ) {
-		if ( isset( $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec']['name'], $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec']['version'] ) ) {
-			$versions = $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec']['version'];
-			array_pop( $versions );
-			$extensions[ $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec']['name'] ] = array_pop( $versions );
-		}
-	}
-
-	if ( isset( $extensions['amp-carousel'] ) ) {
-		/*
-		 * The 0.2 version of amp-carousel depends on the amp-base-carousel component, but this is still experimental.
-		 * Also, the validator spec does not currently specify what base dependencies a given component has.
-		 * @todo Revisit once amp-base-carousel is no longer experimental. Add support for obtaining a list of extensions that depend on other extensions to include in the script dependencies when registering below.
-		 */
-		$extensions['amp-carousel'] = '0.1';
-	}
-
-	foreach ( $extensions as $extension => $version ) {
+	// Register all AMP components as defined in the spec.
+	foreach ( AMP_Allowed_Tags_Generated::get_extension_specs() as $extension_name => $extension_spec ) {
 		$src = sprintf(
 			'https://cdn.ampproject.org/v0/%s-%s.js',
-			$extension,
-			$version
+			$extension_name,
+			end( $extension_spec['version'] )
 		);
 
 		$wp_scripts->add(
-			$extension,
+			$extension_name,
 			$src,
 			[ 'amp-runtime' ],
 			null
 		);
 	}
+
+	if ( $wp_scripts->query( 'amp-carousel', 'registered' ) ) {
+		/*
+		 * The 0.2 version of amp-carousel depends on the amp-base-carousel component, but this is still experimental.
+		 * Also, the validator spec does not currently specify what base dependencies a given component has.
+		 * @todo Revisit once amp-base-carousel is no longer experimental. Add support for obtaining a list of extensions that depend on other extensions to include in the script dependencies when registering below.
+		 */
+		$wp_scripts->registered['amp-carousel']->src = 'https://cdn.ampproject.org/v0/amp-carousel-0.1.js';
+	}
 }
 
 /**
@@ -576,6 +567,8 @@ function amp_filter_script_loader_tag( $tag, $handle ) {
 		/*
 		 * Per the spec, "Most extensions are custom-elements." In fact, there is only one custom template. So we hard-code it here.
 		 *
+		 * This could also be derived by looking at the extension_type in the extension_spec.
+		 *
 		 * @link https://github.com/ampproject/amphtml/blob/cd685d4e62153557519553ffa2183aedf8c93d62/validator/validator.proto#L326-L328
 		 * @link https://github.com/ampproject/amphtml/blob/cd685d4e62153557519553ffa2183aedf8c93d62/extensions/amp-mustache/validator-amp-mustache.protoascii#L27
 		 */

includes/class-amp-theme-support.php

@@ -1534,7 +1534,6 @@ public static function filter_admin_bar_script_loader_tag( $tag, $handle ) {
 	 * @link https://www.ampproject.org/docs/reference/spec#required-markup
 	 * @link https://amp.dev/documentation/guides-and-tutorials/optimize-and-measure/optimize_amp/
 	 * @todo All of this might be better placed inside of a sanitizer.
-	 * @todo Consider removing any scripts that are not among the $script_handles.
 	 *
 	 * @param DOMDocument $dom            Document.
 	 * @param string[]    $script_handles AMP script handles for components identified during output buffering.
@@ -1727,7 +1726,7 @@ public static function ensure_required_markup( DOMDocument $dom, $script_handles
 			$script->parentNode->removeChild( $script ); // So we can move it.
 		}
 
-		// Create scripts for any components discovered from output buffering.
+		// Create scripts for any components discovered from output buffering that are missing.
 		foreach ( array_diff( $script_handles, array_keys( $amp_scripts ) ) as $missing_script_handle ) {
 			if ( ! wp_script_is( $missing_script_handle, 'registered' ) ) {
 				continue;
@@ -1745,6 +1744,18 @@ public static function ensure_required_markup( DOMDocument $dom, $script_handles
 			$amp_scripts[ $missing_script_handle ] = AMP_DOM_Utils::create_node( $dom, 'script', $attrs );
 		}
 
+		// Remove scripts that had already been added but couldn't be detected from output buffering.
+		$extension_specs            = AMP_Allowed_Tags_Generated::get_extension_specs();
+		$superfluous_script_handles = array_diff(
+			array_keys( $amp_scripts ),
+			array_merge( $script_handles, [ 'amp-runtime' ] )
+		);
+		foreach ( $superfluous_script_handles as $superfluous_script_handle ) {
+			if ( ! empty( $extension_specs[ $superfluous_script_handle ]['requires_usage'] ) ) {
+				unset( $amp_scripts[ $superfluous_script_handle ] );
+			}
+		}
+
 		/* phpcs:ignore Squiz.PHP.CommentedOutCode.Found
 		 *
 		 * "2. Next, preload the AMP runtime v0.js <script> tag with <link as=script href=https://cdn.ampproject.org/v0.js rel=preload>.