From cee8d58f921cdbfe553a654796ee79e7a85c377e Mon Sep 17 00:00:00 2001 From: Alain Schlesser Date: Sun, 10 Nov 2019 13:47:23 +0100 Subject: [PATCH 1/9] Remove scripts for components that were not detected in output buffer --- includes/class-amp-theme-support.php | 26 ++++++++++++++- tests/php/test-class-amp-theme-support.php | 37 ++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 63f28aec370..41b41fa7de6 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1727,7 +1727,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 +1745,30 @@ 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. + foreach ( array_diff( array_keys( $amp_scripts ), $script_handles ) as $superfluous_script_handle ) { + $src = $amp_scripts[ $superfluous_script_handle ]->getAttribute( 'src' ); + + // Skip scripts unrelated to AMP. + if ( ! $src || 0 !== strpos( $src, 'https://cdn.ampproject.org/' ) ) { + continue; + } + + // Skip AMP runtime script. + if ( 'https://cdn.ampproject.org/v0.js' === $src ) { + continue; + } + + $custom_element = $amp_scripts[ $superfluous_script_handle ]->getAttribute( 'custom-element' ); + + // Skip dynamic CSS classes script, it has no associated element. + if ( 'amp-dynamic-css-classes' === $custom_element ) { + continue; + } + + unset( $amp_scripts[ $superfluous_script_handle ] ); + } + /* phpcs:ignore Squiz.PHP.CommentedOutCode.Found * * "2. Next, preload the AMP runtime v0.js + From 8744c8981a268f6595e5af79b8e614df39f3ca47 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 13 Nov 2019 12:05:11 -0800 Subject: [PATCH 2/9] Eliminate extracting distict also_requires_tag_warning from spec --- bin/amphtml-update.py | 21 +++++++----- .../class-amp-allowed-tags-generated.php | 34 +++++-------------- .../class-amp-tag-and-attribute-sanitizer.php | 3 -- 3 files changed, 20 insertions(+), 38 deletions(-) diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py index 5ad04233eb3..083fe51321e 100644 --- a/bin/amphtml-update.py +++ b/bin/amphtml-update.py @@ -463,11 +463,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 +488,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: diff --git a/includes/sanitizers/class-amp-allowed-tags-generated.php b/includes/sanitizers/class-amp-allowed-tags-generated.php index b60135580fc..a769f71a034 100644 --- a/includes/sanitizers/class-amp-allowed-tags-generated.php +++ b/includes/sanitizers/class-amp-allowed-tags-generated.php @@ -719,9 +719,6 @@ class AMP_Allowed_Tags_Generated { ), ), 'tag_spec' => array( - 'also_requires_tag_warning' => array( - 'amp-ad extension .js script', - ), 'amp_layout' => array( 'supported_layouts' => array( 6, @@ -769,9 +766,6 @@ class AMP_Allowed_Tags_Generated { ), ), 'tag_spec' => array( - 'also_requires_tag_warning' => array( - 'amp-ad extension .js script', - ), 'amp_layout' => array( 'supported_layouts' => array( 6, @@ -826,9 +820,6 @@ class AMP_Allowed_Tags_Generated { ), ), 'tag_spec' => array( - 'also_requires_tag_warning' => array( - 'amp-ad extension .js script', - ), 'amp_layout' => array( 'supported_layouts' => array( 6, @@ -886,9 +877,6 @@ class AMP_Allowed_Tags_Generated { ), ), 'tag_spec' => array( - 'also_requires_tag_warning' => array( - 'amp-ad extension .js script', - ), 'amp_layout' => array( 'supported_layouts' => array( 6, @@ -1548,8 +1536,8 @@ class AMP_Allowed_Tags_Generated { ), ), 'requires_extension' => array( - 'amp-base-carousel', 'amp-lightbox-gallery', + 'amp-base-carousel', ), 'spec_name' => 'AMP-BASE-CAROUSEL [lightbox]', 'spec_url' => 'https://amp.dev/documentation/components/amp-base-carousel', @@ -2699,9 +2687,6 @@ class AMP_Allowed_Tags_Generated { ), ), 'tag_spec' => array( - 'also_requires_tag_warning' => array( - 'amp-ad extension .js script', - ), 'amp_layout' => array( 'supported_layouts' => array( 6, @@ -2755,9 +2740,6 @@ class AMP_Allowed_Tags_Generated { ), ), 'tag_spec' => array( - 'also_requires_tag_warning' => array( - 'amp-ad extension .js script', - ), 'amp_layout' => array( 'supported_layouts' => array( 6, @@ -5761,9 +5743,6 @@ class AMP_Allowed_Tags_Generated { ), ), 'tag_spec' => array( - 'also_requires_tag_warning' => array( - 'amp-video extension .js script', - ), 'amp_layout' => array( 'supported_layouts' => array( 6, @@ -5777,6 +5756,9 @@ class AMP_Allowed_Tags_Generated { 'disallowed_ancestor' => array( 'amp-story', ), + 'requires_extension' => array( + 'amp-video', + ), 'spec_url' => 'https://amp.dev/documentation/components/amp-video', ), ), @@ -5874,9 +5856,6 @@ class AMP_Allowed_Tags_Generated { ), ), 'tag_spec' => array( - 'also_requires_tag_warning' => array( - 'amp-video extension .js script', - ), 'amp_layout' => array( 'supported_layouts' => array( 6, @@ -5888,6 +5867,9 @@ class AMP_Allowed_Tags_Generated { ), ), 'mandatory_ancestor' => 'amp-story-page-attachment', + 'requires_extension' => array( + 'amp-video', + ), 'spec_name' => 'amp-story >> amp-story-page-attachment >> amp-video', 'spec_url' => 'https://amp.dev/documentation/components/amp-video', ), @@ -8948,8 +8930,8 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'mandatory_parent' => 'amp-autocomplete', 'requires_extension' => array( - 'amp-autocomplete', 'amp-form', + 'amp-autocomplete', ), 'spec_name' => 'amp-autocomplete > input', ), diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php index 3c6a56251a1..6dcd9c3edaf 100644 --- a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php +++ b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php @@ -606,9 +606,6 @@ private function process_node( DOMElement $node ) { // Add required AMP component scripts. $script_components = []; - if ( ! empty( $tag_spec['also_requires_tag_warning'] ) ) { - $script_components[] = strtok( $tag_spec['also_requires_tag_warning'][0], ' ' ); - } if ( ! empty( $tag_spec['requires_extension'] ) ) { $script_components = array_merge( $script_components, $tag_spec['requires_extension'] ); } From dba4eade4c315b029d926cd923b1dd77e37f7251 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 13 Nov 2019 12:22:35 -0800 Subject: [PATCH 3/9] Normalize required_usage in extension_spec --- bin/amphtml-update.py | 11 +- .../class-amp-allowed-tags-generated.php | 171 ++++++++++++------ 2 files changed, 126 insertions(+), 56 deletions(-) diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py index 083fe51321e..7c88ab8ca21 100644 --- a/bin/amphtml-update.py +++ b/bin/amphtml-update.py @@ -513,7 +513,12 @@ 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 ] = [] @@ -521,6 +526,10 @@ def GetTagRules(tag_spec): 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 + tag_rules['extension_spec'] = extension_spec if tag_spec.HasField('mandatory'): diff --git a/includes/sanitizers/class-amp-allowed-tags-generated.php b/includes/sanitizers/class-amp-allowed-tags-generated.php index a769f71a034..37c88d2551f 100644 --- a/includes/sanitizers/class-amp-allowed-tags-generated.php +++ b/includes/sanitizers/class-amp-allowed-tags-generated.php @@ -11549,6 +11549,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-3d-gltf', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11574,6 +11575,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-3q-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11599,7 +11601,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-access-laterpay', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', '0.2', @@ -11629,7 +11631,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-access-poool', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -11658,7 +11660,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-access-scroll', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -11688,7 +11690,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-access', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11747,7 +11749,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-accordion', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11773,6 +11775,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-action-macro', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11798,6 +11801,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-ad-custom', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11825,7 +11829,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-ad', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11852,6 +11856,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-addthis', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11878,7 +11883,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-analytics', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11931,7 +11936,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-anim', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -11957,6 +11962,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-animation', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12008,7 +12014,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-apester-media', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12035,6 +12041,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-app-banner', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12061,7 +12068,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-audio', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12087,6 +12094,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-auto-ads', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12112,6 +12120,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-autocomplete', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12162,6 +12171,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-base-carousel', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12187,6 +12197,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-beopinion', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12212,7 +12223,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-bind', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -12266,6 +12277,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-bodymovin-animation', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12292,7 +12304,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-brid-player', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12319,7 +12331,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-brightcove', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12345,6 +12357,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-byside-content', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12370,7 +12383,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-call-tracking', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12397,7 +12410,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-carousel', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', '0.2', @@ -12424,6 +12437,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-connatix-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12449,6 +12463,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-consent', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12501,7 +12516,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-dailymotion', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12527,6 +12542,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-date-countdown', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12552,6 +12568,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-date-display', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12577,6 +12594,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-date-picker', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12602,6 +12620,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-delight-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12628,7 +12647,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-dynamic-css-classes', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -12654,6 +12673,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-embedly-card', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12680,7 +12700,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-experiment', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', '1.0', @@ -12732,6 +12752,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-facebook-comments', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12757,6 +12778,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-facebook-like', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12782,6 +12804,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-facebook-page', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12808,7 +12831,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-facebook', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12835,7 +12858,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-fit-text', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12862,7 +12885,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-font', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12889,7 +12912,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-form', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12915,7 +12938,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-fx-collection', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -12942,7 +12965,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-fx-flying-carpet', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -12968,6 +12991,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-geo', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13020,7 +13044,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-gfycat', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13046,6 +13070,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-gist', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13071,6 +13096,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-google-document-embed', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13096,6 +13122,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-hulu', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13122,7 +13149,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-iframe', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13148,6 +13175,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-ima-video', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13174,7 +13202,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-image-lightbox', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13200,6 +13228,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-image-slider', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13225,6 +13254,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-imgur', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13250,7 +13280,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-inputmask', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -13277,7 +13307,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-instagram', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13304,7 +13334,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-install-serviceworker', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13330,7 +13360,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-izlesene', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13357,7 +13387,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-jwplayer', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13384,7 +13414,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-kaltura-player', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13410,7 +13440,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-lightbox-gallery', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -13437,7 +13467,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-lightbox', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13463,6 +13493,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-link-rewriter', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13514,7 +13545,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-list', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13540,7 +13571,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-live-list', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13568,6 +13599,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-mathml', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13593,6 +13625,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-megaphone', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13618,6 +13651,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-minute-media-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13643,6 +13677,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-mowplayer', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13673,7 +13708,7 @@ class AMP_Allowed_Tags_Generated { ), 'extension_type' => 2, 'name' => 'amp-mustache', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', '0.2', @@ -13738,6 +13773,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-next-page', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13782,6 +13818,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-nexxtv-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13808,7 +13845,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-o2-player', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13834,6 +13871,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-ooyala-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13859,6 +13897,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-orientation-observer', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13884,6 +13923,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-pan-zoom', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13910,7 +13950,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-pinterest', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13936,6 +13976,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-playbuzz', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13961,6 +14002,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-position-observer', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -13986,6 +14028,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-powr-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14012,7 +14055,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-reach-player', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14038,6 +14081,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-recaptcha-input', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14064,6 +14108,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-reddit', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14089,6 +14134,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-riddle-quiz', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14114,6 +14160,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-script', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14176,7 +14223,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-selector', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14203,7 +14250,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-sidebar', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14229,6 +14276,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-skimlinks', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14254,6 +14302,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-smartlinks', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14280,7 +14329,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-social-share', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14307,7 +14356,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-soundcloud', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14334,7 +14383,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-springboard-player', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14363,7 +14412,7 @@ class AMP_Allowed_Tags_Generated { '0.1', ), 'name' => 'amp-sticky-ad', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', '1.0', @@ -14390,6 +14439,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-story-auto-ads', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14441,6 +14491,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-story', + 'requires_usage' => true, 'version' => array( '1.0', 'latest', @@ -14537,7 +14588,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-subscriptions', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -14595,7 +14646,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-subscriptions-google', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -14624,6 +14675,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-timeago', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14649,6 +14701,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-truncate-text', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14675,7 +14728,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-twitter', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14701,6 +14754,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-user-location', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14753,7 +14807,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-user-notification', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14779,6 +14833,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-video-docking', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14805,6 +14860,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-video-iframe', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14830,7 +14886,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-video', - 'requires_usage' => 3, + 'requires_usage' => false, 'version' => array( '0.1', 'latest', @@ -14858,7 +14914,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-vimeo', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14885,7 +14941,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-vine', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14911,6 +14967,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-viqeo-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14936,6 +14993,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-vk', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14961,6 +15019,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-web-push', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -14986,6 +15045,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-wistia-player', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -15011,6 +15071,7 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'name' => 'amp-yotpo', + 'requires_usage' => true, 'version' => array( '0.1', 'latest', @@ -15038,7 +15099,7 @@ class AMP_Allowed_Tags_Generated { 'extension_spec' => array( 'deprecated_allow_duplicates' => true, 'name' => 'amp-youtube', - 'requires_usage' => 2, + 'requires_usage' => true, 'version' => array( '0.1', 'latest', From 2f9f8b9864ce668c28bdb1efccf8d4efb28959d6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Wed, 13 Nov 2019 14:07:25 -0800 Subject: [PATCH 4/9] Add required_usage info as script data and use to determine which scripts get removed --- bin/amphtml-update.py | 5 ++ includes/amp-helper-functions.php | 45 +++++++++------- includes/class-amp-theme-support.php | 27 +++------- tests/php/test-class-amp-theme-support.php | 63 +++++++++++++++++----- 4 files changed, 87 insertions(+), 53 deletions(-) diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py index 7c88ab8ca21..cb5370db072 100644 --- a/bin/amphtml-update.py +++ b/bin/amphtml-update.py @@ -530,6 +530,11 @@ def GetTagRules(tag_spec): # 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' ) + tag_rules['extension_spec'] = extension_spec if tag_spec.HasField('mandatory'): diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 5dbef925a2f..1228b4dc6ec 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -464,38 +464,43 @@ function amp_register_default_scripts( $wp_scripts ) { ] ); - // Get all AMP components as defined in the spec. - $extensions = []; + // Register all AMP components as defined in the spec. 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( $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec'] ) ) { + continue; } - } - - 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'; - } + $extension_spec = $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec']; + $versions = array_filter( + $extension_spec['version'], + function ( $version ) { + // @todo Why are we including the latest version in the generated spec in the first place? We can just include the highest number version. + return 'latest' !== $version; + } + ); - foreach ( $extensions as $extension => $version ) { $src = sprintf( 'https://cdn.ampproject.org/v0/%s-%s.js', - $extension, - $version + $extension_spec['name'], + array_pop( $versions ) ); $wp_scripts->add( - $extension, + $extension_spec['name'], $src, [ 'amp-runtime' ], null ); + + $wp_scripts->add_data( $extension_spec['name'], 'amp_requires_usage', ! empty( $extension_spec['requires_usage'] ) ); + } + + 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'; } } diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 41b41fa7de6..3fd1e650d40 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1746,27 +1746,14 @@ public static function ensure_required_markup( DOMDocument $dom, $script_handles } // Remove scripts that had already been added but couldn't be detected from output buffering. - foreach ( array_diff( array_keys( $amp_scripts ), $script_handles ) as $superfluous_script_handle ) { - $src = $amp_scripts[ $superfluous_script_handle ]->getAttribute( 'src' ); - - // Skip scripts unrelated to AMP. - if ( ! $src || 0 !== strpos( $src, 'https://cdn.ampproject.org/' ) ) { - continue; - } - - // Skip AMP runtime script. - if ( 'https://cdn.ampproject.org/v0.js' === $src ) { - continue; - } - - $custom_element = $amp_scripts[ $superfluous_script_handle ]->getAttribute( 'custom-element' ); - - // Skip dynamic CSS classes script, it has no associated element. - if ( 'amp-dynamic-css-classes' === $custom_element ) { - continue; + $superfluous_script_handles = array_diff( + array_keys( $amp_scripts ), + array_merge( $script_handles, [ 'amp-runtime' ] ) + ); + foreach ( $superfluous_script_handles as $superfluous_script_handle ) { + if ( true === wp_scripts()->get_data( $superfluous_script_handle, 'amp_requires_usage' ) ) { + unset( $amp_scripts[ $superfluous_script_handle ] ); } - - unset( $amp_scripts[ $superfluous_script_handle ] ); } /* phpcs:ignore Squiz.PHP.CommentedOutCode.Found diff --git a/tests/php/test-class-amp-theme-support.php b/tests/php/test-class-amp-theme-support.php index 890d4fb8c23..74c00c6fa4a 100644 --- a/tests/php/test-class-amp-theme-support.php +++ b/tests/php/test-class-amp-theme-support.php @@ -1431,19 +1431,44 @@ public function test_scripts_get_moved_to_head() { * @covers AMP_Theme_Support::ensure_required_markup() */ public function test_unneeded_scripts_get_removed() { + wp(); + add_theme_support( AMP_Theme_Support::SLUG ); + AMP_Theme_Support::init(); + AMP_Theme_Support::finish_init(); + + // These should all get removed, unless used. + $required_usage_grandfathered = [ + 'amp-anim', + 'amp-ad', + 'amp-mustache', + 'amp-list', + 'amp-youtube', + 'amp-form', + 'amp-live-list', + ]; + + // These also should get removed, unless used. + $required_usage_error = [ + 'amp-facebook-like', + 'amp-date-picker', + 'amp-call-tracking', + ]; + + // These should not get removed, ever. + $required_usage_none = [ + 'amp-bind', // And yet, see https://github.com/ampproject/amphtml/blob/eb05855/extensions/amp-bind/validator-amp-bind.protoascii#L25-L28 + 'amp-dynamic-css-classes', + 'amp-subscriptions', + 'amp-lightbox-gallery', + 'amp-video', + ]; + ob_start(); ?> - - - - + query( '//script[ not( @type ) or @type = "text/javascript" ]' ); - $this->assertSame( 3, $scripts->length ); - $this->assertEquals( 'https://cdn.ampproject.org/v0.js', $scripts->item( 0 )->getAttribute( 'src' ) ); - $this->assertEquals( 'amp-mustache', $scripts->item( 1 )->getAttribute( 'custom-template' ) ); - $this->assertEquals( 'amp-list', $scripts->item( 2 )->getAttribute( 'custom-element' ) ); + /** @var DOMElement $script Script. */ + $actual_script_srcs = []; + foreach ( $xpath->query( '//script[ not( @type ) or @type = "text/javascript" ]' ) as $script ) { + $actual_script_srcs[] = $script->getAttribute( 'src' ); + } + + $expected_script_srcs = [ + wp_scripts()->registered['amp-runtime']->src, + ]; + foreach ( $required_usage_none as $handle ) { + $expected_script_srcs[] = wp_scripts()->registered[ $handle ]->src; + } + + $this->assertEqualSets( + $expected_script_srcs, + $actual_script_srcs + ); } /** From 879cb170f210ed9b997fff56cba9e31fffd23637 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 14 Nov 2019 22:59:55 -0800 Subject: [PATCH 5/9] Improve handling of versions in extension_spec --- bin/amphtml-update.py | 12 ++ includes/amp-helper-functions.php | 11 +- .../class-amp-allowed-tags-generated.php | 123 ------------------ .../class-amp-tag-and-attribute-sanitizer.php | 13 +- 4 files changed, 19 insertions(+), 140 deletions(-) diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py index cb5370db072..ce18afa4d8b 100644 --- a/bin/amphtml-update.py +++ b/bin/amphtml-update.py @@ -535,6 +535,18 @@ def GetTagRules(tag_spec): 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'] + tag_rules['extension_spec'] = extension_spec if tag_spec.HasField('mandatory'): diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 1228b4dc6ec..69adbf8e148 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -470,18 +470,11 @@ function amp_register_default_scripts( $wp_scripts ) { continue; } $extension_spec = $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec']; - $versions = array_filter( - $extension_spec['version'], - function ( $version ) { - // @todo Why are we including the latest version in the generated spec in the first place? We can just include the highest number version. - return 'latest' !== $version; - } - ); $src = sprintf( 'https://cdn.ampproject.org/v0/%s-%s.js', $extension_spec['name'], - array_pop( $versions ) + end( $extension_spec['version'] ) ); $wp_scripts->add( @@ -581,6 +574,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 */ diff --git a/includes/sanitizers/class-amp-allowed-tags-generated.php b/includes/sanitizers/class-amp-allowed-tags-generated.php index 37c88d2551f..a49c2d52778 100644 --- a/includes/sanitizers/class-amp-allowed-tags-generated.php +++ b/includes/sanitizers/class-amp-allowed-tags-generated.php @@ -11552,7 +11552,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -11578,7 +11577,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -11605,7 +11603,6 @@ class AMP_Allowed_Tags_Generated { 'version' => array( '0.1', '0.2', - 'latest', ), ), 'requires_extension' => array( @@ -11634,7 +11631,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), 'requires_extension' => array( @@ -11663,7 +11659,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), 'requires_extension' => array( @@ -11693,7 +11688,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -11752,7 +11746,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -11778,7 +11771,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -11804,7 +11796,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), 'spec_name' => 'amp-ad-custom extension .js script', @@ -11832,7 +11823,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), 'spec_name' => 'amp-ad extension .js script', @@ -11859,7 +11849,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -11886,7 +11875,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -11939,7 +11927,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -11965,7 +11952,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12017,7 +12003,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12044,7 +12029,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12071,7 +12055,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12097,7 +12080,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12123,7 +12105,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12174,7 +12155,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12200,7 +12180,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12226,7 +12205,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12280,7 +12258,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12307,7 +12284,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12334,7 +12310,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12360,7 +12335,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12386,7 +12360,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12414,7 +12387,6 @@ class AMP_Allowed_Tags_Generated { 'version' => array( '0.1', '0.2', - 'latest', ), ), ), @@ -12440,7 +12412,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12466,7 +12437,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12519,7 +12489,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12545,7 +12514,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12571,7 +12539,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12597,7 +12564,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12623,7 +12589,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12650,7 +12615,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12676,7 +12640,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12704,7 +12667,6 @@ class AMP_Allowed_Tags_Generated { 'version' => array( '0.1', '1.0', - 'latest', ), ), ), @@ -12755,7 +12717,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12781,7 +12742,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12807,7 +12767,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12834,7 +12793,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12861,7 +12819,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12888,7 +12845,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12915,7 +12871,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12941,7 +12896,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12968,7 +12922,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -12994,7 +12947,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13047,7 +12999,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13073,7 +13024,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13099,7 +13049,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13125,7 +13074,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13152,7 +13100,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13178,7 +13125,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13205,7 +13151,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13231,7 +13176,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13257,7 +13201,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13283,7 +13226,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13310,7 +13252,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13337,7 +13278,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13363,7 +13303,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13390,7 +13329,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13417,7 +13355,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13443,7 +13380,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13470,7 +13406,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13496,7 +13431,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13548,7 +13482,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13574,7 +13507,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), 'mandatory_parent' => 'head', @@ -13602,7 +13534,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13628,7 +13559,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13654,7 +13584,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13680,7 +13609,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13703,16 +13631,11 @@ class AMP_Allowed_Tags_Generated { 'tag_spec' => array( 'extension_spec' => array( 'deprecated_allow_duplicates' => true, - 'deprecated_version' => array( - '0.1', - ), - 'extension_type' => 2, 'name' => 'amp-mustache', 'requires_usage' => true, 'version' => array( '0.1', '0.2', - 'latest', ), ), ), @@ -13776,7 +13699,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13821,7 +13743,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13848,7 +13769,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13874,7 +13794,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13900,7 +13819,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13926,7 +13844,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13953,7 +13870,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -13979,7 +13895,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14005,7 +13920,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14031,7 +13945,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14058,7 +13971,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14084,7 +13996,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14111,7 +14022,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14137,7 +14047,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14163,7 +14072,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14226,7 +14134,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14253,7 +14160,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14279,7 +14185,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14305,7 +14210,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14332,7 +14236,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14359,7 +14262,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14386,7 +14288,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14408,15 +14309,11 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_version' => array( - '0.1', - ), 'name' => 'amp-sticky-ad', 'requires_usage' => true, 'version' => array( '0.1', '1.0', - 'latest', ), ), ), @@ -14442,7 +14339,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14494,7 +14390,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '1.0', - 'latest', ), ), ), @@ -14591,7 +14486,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14649,7 +14543,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), 'requires_extension' => array( @@ -14678,7 +14571,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14704,7 +14596,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14731,7 +14622,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14757,7 +14647,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14810,7 +14699,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14836,7 +14724,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), 'spec_name' => 'amp-video-docking', @@ -14863,7 +14750,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14889,7 +14775,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => false, 'version' => array( '0.1', - 'latest', ), ), 'spec_name' => 'amp-video extension .js script', @@ -14917,7 +14802,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14944,7 +14828,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14970,7 +14853,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -14996,7 +14878,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -15022,7 +14903,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -15048,7 +14928,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), @@ -15074,7 +14953,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), 'spec_url' => 'https://amp.dev/documentation/components/amp-yotpo', @@ -15102,7 +14980,6 @@ class AMP_Allowed_Tags_Generated { 'requires_usage' => true, 'version' => array( '0.1', - 'latest', ), ), ), diff --git a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php index 6dcd9c3edaf..37766722423 100644 --- a/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php +++ b/includes/sanitizers/class-amp-tag-and-attribute-sanitizer.php @@ -332,27 +332,22 @@ private function get_rule_spec_list_to_validate( DOMElement $node, $rule_spec ) // Expand extension_spec into a set of attr_spec_list. if ( isset( $rule_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec'] ) ) { $extension_spec = $rule_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec']; - $custom_attr = 'amp-mustache' === $extension_spec['name'] ? 'custom-template' : 'custom-element'; + + // This could also be derived from the extension_type in the extension_spec. + $custom_attr = 'amp-mustache' === $extension_spec['name'] ? 'custom-template' : 'custom-element'; $rule_spec[ AMP_Rule_Spec::ATTR_SPEC_LIST ][ $custom_attr ] = [ AMP_Rule_Spec::VALUE => $extension_spec['name'], AMP_Rule_Spec::MANDATORY => true, ]; - $versions = array_unique( - array_merge( - isset( $extension_spec['allowed_versions'] ) ? $extension_spec['allowed_versions'] : [], - isset( $extension_spec['version'] ) ? $extension_spec['version'] : [] - ) - ); - $rule_spec[ AMP_Rule_Spec::ATTR_SPEC_LIST ]['src'] = [ AMP_Rule_Spec::VALUE_REGEX => implode( '', [ '^', preg_quote( 'https://cdn.ampproject.org/v0/' . $extension_spec['name'] . '-' ), // phpcs:ignore WordPress.PHP.PregQuoteDelimiter.Missing - '(' . implode( '|', $versions ) . ')', + '(' . implode( '|', array_merge( $extension_spec['version'], [ 'latest' ] ) ) . ')', '\.js$', ] ), From 17d03ad62d82d247ad9e3648668a6f6d0738fcc4 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 14 Nov 2019 23:06:05 -0800 Subject: [PATCH 6/9] Remove unused deprecated_allow_duplicates from extension_spec --- bin/amphtml-update.py | 3 ++ .../class-amp-allowed-tags-generated.php | 42 ------------------- 2 files changed, 3 insertions(+), 42 deletions(-) diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py index ce18afa4d8b..07dd23af353 100644 --- a/bin/amphtml-update.py +++ b/bin/amphtml-update.py @@ -547,6 +547,9 @@ def GetTagRules(tag_spec): 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'): diff --git a/includes/sanitizers/class-amp-allowed-tags-generated.php b/includes/sanitizers/class-amp-allowed-tags-generated.php index a49c2d52778..3f3c62e0088 100644 --- a/includes/sanitizers/class-amp-allowed-tags-generated.php +++ b/includes/sanitizers/class-amp-allowed-tags-generated.php @@ -11683,7 +11683,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-access', 'requires_usage' => true, 'version' => array( @@ -11741,7 +11740,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-accordion', 'requires_usage' => true, 'version' => array( @@ -11818,7 +11816,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-ad', 'requires_usage' => true, 'version' => array( @@ -11870,7 +11867,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-analytics', 'requires_usage' => true, 'version' => array( @@ -11922,7 +11918,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-anim', 'requires_usage' => true, 'version' => array( @@ -11998,7 +11993,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-apester-media', 'requires_usage' => true, 'version' => array( @@ -12024,7 +12018,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-app-banner', 'requires_usage' => true, 'version' => array( @@ -12050,7 +12043,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-audio', 'requires_usage' => true, 'version' => array( @@ -12279,7 +12271,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-brid-player', 'requires_usage' => true, 'version' => array( @@ -12305,7 +12296,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-brightcove', 'requires_usage' => true, 'version' => array( @@ -12381,7 +12371,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-carousel', 'requires_usage' => true, 'version' => array( @@ -12484,7 +12473,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-dailymotion', 'requires_usage' => true, 'version' => array( @@ -12610,7 +12598,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-dynamic-css-classes', 'requires_usage' => false, 'version' => array( @@ -12661,7 +12648,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-experiment', 'requires_usage' => true, 'version' => array( @@ -12788,7 +12774,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-facebook', 'requires_usage' => true, 'version' => array( @@ -12814,7 +12799,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-fit-text', 'requires_usage' => true, 'version' => array( @@ -12840,7 +12824,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-font', 'requires_usage' => true, 'version' => array( @@ -12866,7 +12849,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-form', 'requires_usage' => true, 'version' => array( @@ -12917,7 +12899,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-fx-flying-carpet', 'requires_usage' => true, 'version' => array( @@ -12994,7 +12975,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-gfycat', 'requires_usage' => true, 'version' => array( @@ -13095,7 +13075,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-iframe', 'requires_usage' => true, 'version' => array( @@ -13146,7 +13125,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-image-lightbox', 'requires_usage' => true, 'version' => array( @@ -13247,7 +13225,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-instagram', 'requires_usage' => true, 'version' => array( @@ -13273,7 +13250,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-install-serviceworker', 'requires_usage' => true, 'version' => array( @@ -13324,7 +13300,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-jwplayer', 'requires_usage' => true, 'version' => array( @@ -13350,7 +13325,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-kaltura-player', 'requires_usage' => true, 'version' => array( @@ -13401,7 +13375,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-lightbox', 'requires_usage' => true, 'version' => array( @@ -13477,7 +13450,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-list', 'requires_usage' => true, 'version' => array( @@ -13630,7 +13602,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-mustache', 'requires_usage' => true, 'version' => array( @@ -13764,7 +13735,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-o2-player', 'requires_usage' => true, 'version' => array( @@ -13865,7 +13835,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-pinterest', 'requires_usage' => true, 'version' => array( @@ -13966,7 +13935,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-reach-player', 'requires_usage' => true, 'version' => array( @@ -14017,7 +13985,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-reddit', 'requires_usage' => true, 'version' => array( @@ -14155,7 +14122,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-sidebar', 'requires_usage' => true, 'version' => array( @@ -14231,7 +14197,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-social-share', 'requires_usage' => true, 'version' => array( @@ -14257,7 +14222,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-soundcloud', 'requires_usage' => true, 'version' => array( @@ -14283,7 +14247,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-springboard-player', 'requires_usage' => true, 'version' => array( @@ -14617,7 +14580,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-twitter', 'requires_usage' => true, 'version' => array( @@ -14694,7 +14656,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-user-notification', 'requires_usage' => true, 'version' => array( @@ -14797,7 +14758,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-vimeo', 'requires_usage' => true, 'version' => array( @@ -14823,7 +14783,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-vine', 'requires_usage' => true, 'version' => array( @@ -14975,7 +14934,6 @@ class AMP_Allowed_Tags_Generated { ), 'tag_spec' => array( 'extension_spec' => array( - 'deprecated_allow_duplicates' => true, 'name' => 'amp-youtube', 'requires_usage' => true, 'version' => array( From 9956c90e2d27f2183be41471cf42275f0b51585f Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Thu, 14 Nov 2019 23:15:38 -0800 Subject: [PATCH 7/9] Implement method to get all extension_specs --- bin/amphtml-update.py | 17 +++++++++++++++++ includes/amp-helper-functions.php | 13 +++---------- includes/class-amp-theme-support.php | 3 ++- .../class-amp-allowed-tags-generated.php | 17 +++++++++++++++++ 4 files changed, 39 insertions(+), 11 deletions(-) diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py index 07dd23af353..1948c4cb592 100644 --- a/bin/amphtml-update.py +++ b/bin/amphtml-update.py @@ -211,6 +211,23 @@ 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() { + $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. * diff --git a/includes/amp-helper-functions.php b/includes/amp-helper-functions.php index 69adbf8e148..91394bfc453 100644 --- a/includes/amp-helper-functions.php +++ b/includes/amp-helper-functions.php @@ -465,26 +465,19 @@ function amp_register_default_scripts( $wp_scripts ) { ); // Register all AMP components as defined in the spec. - foreach ( AMP_Allowed_Tags_Generated::get_allowed_tag( 'script' ) as $script_spec ) { - if ( ! isset( $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_spec'] ) ) { - continue; - } - $extension_spec = $script_spec[ AMP_Rule_Spec::TAG_SPEC ]['extension_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_spec['name'], + $extension_name, end( $extension_spec['version'] ) ); $wp_scripts->add( - $extension_spec['name'], + $extension_name, $src, [ 'amp-runtime' ], null ); - - $wp_scripts->add_data( $extension_spec['name'], 'amp_requires_usage', ! empty( $extension_spec['requires_usage'] ) ); } if ( $wp_scripts->query( 'amp-carousel', 'registered' ) ) { diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index 3fd1e650d40..e881f72f8b0 100644 --- a/includes/class-amp-theme-support.php +++ b/includes/class-amp-theme-support.php @@ -1746,12 +1746,13 @@ public static function ensure_required_markup( DOMDocument $dom, $script_handles } // 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 ( true === wp_scripts()->get_data( $superfluous_script_handle, 'amp_requires_usage' ) ) { + if ( ! empty( $extension_specs[ $superfluous_script_handle ]['requires_usage'] ) ) { unset( $amp_scripts[ $superfluous_script_handle ] ); } } diff --git a/includes/sanitizers/class-amp-allowed-tags-generated.php b/includes/sanitizers/class-amp-allowed-tags-generated.php index 3f3c62e0088..be5345bde8e 100644 --- a/includes/sanitizers/class-amp-allowed-tags-generated.php +++ b/includes/sanitizers/class-amp-allowed-tags-generated.php @@ -17595,6 +17595,23 @@ public static function get_allowed_tags() { 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() { + $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. * From 16f35b4512e5dd151d4afa3757cf75ed2a9eb4e6 Mon Sep 17 00:00:00 2001 From: Weston Ruter Date: Fri, 15 Nov 2019 08:57:50 -0800 Subject: [PATCH 8/9] Cache the collection of the extension_specs --- bin/amphtml-update.py | 8 +++++++- includes/sanitizers/class-amp-allowed-tags-generated.php | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/bin/amphtml-update.py b/bin/amphtml-update.py index 1948c4cb592..c3a0eaf0d45 100644 --- a/bin/amphtml-update.py +++ b/bin/amphtml-update.py @@ -219,12 +219,18 @@ def GenerateFooterPHP(out): * @return array Extension specs, keyed by extension name. */ public static function get_extension_specs() { - $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; } diff --git a/includes/sanitizers/class-amp-allowed-tags-generated.php b/includes/sanitizers/class-amp-allowed-tags-generated.php index be5345bde8e..65d3e7d65be 100644 --- a/includes/sanitizers/class-amp-allowed-tags-generated.php +++ b/includes/sanitizers/class-amp-allowed-tags-generated.php @@ -17603,12 +17603,18 @@ public static function get_allowed_tags() { * @return array Extension specs, keyed by extension name. */ public static function get_extension_specs() { - $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; } From 617d17d874ab5ddac1db033fdcb2117a26a67d2a Mon Sep 17 00:00:00 2001 From: Alain Schlesser Date: Fri, 15 Nov 2019 17:59:22 +0100 Subject: [PATCH 9/9] Remove todo --- includes/class-amp-theme-support.php | 1 - 1 file changed, 1 deletion(-) diff --git a/includes/class-amp-theme-support.php b/includes/class-amp-theme-support.php index e881f72f8b0..11b7d9a3361 100644 --- a/includes/class-amp-theme-support.php +++ b/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.