-
Notifications
You must be signed in to change notification settings - Fork 381
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update AMP spec to 1d3757a07f #6303
Conversation
* Version 1.0 of amp-experiment is still experimental and requires the user to enable it. | ||
* @todo Revisit once amp-experiment is no longer experimental. | ||
*/ | ||
$wp_scripts->registered['amp-experiment']->src = 'https://cdn.ampproject.org/v0/amp-experiment-0.1.js'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: need to update that in Web Stories as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It turns out that 0.1 is still the latestVersion
, so you don't have to change it. While 1.0 is valid now, the plugin will continue to serve 0.1 by default.
60ba245
to
9e8eb52
Compare
@@ -1254,6 +1254,161 @@ static function ( $script ) { | |||
$this->assertStringContains( '<script type=\'text/javascript\' src=\'https://cdn.ampproject.org/v0/amp-foo-0.1.js\' async custom-element="amp-foo"></script>', $output ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript | |||
} | |||
|
|||
/** @covers ::amp_register_default_scripts() */ | |||
public function test_amp_register_default_scripts() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that all of the versions being checked in this test are the same as in develop
, with the exception of the three new component scripts added:
amp-cache-url
amp-iframely
amp-stream-gallery
amp-render
So there will be no unexpected component version changes if this is part of a 2.1.3 patch release.
For fd72710, there seems to be something missing in the validator spec for the Namely, it currently has:
What appears to be missing is
The generated spec should include: --- a/includes/sanitizers/class-amp-allowed-tags-generated.php
+++ b/includes/sanitizers/class-amp-allowed-tags-generated.php
@@ -7271,6 +7271,9 @@ class AMP_Allowed_Tags_Generated {
'value' => array(
'google',
),
+ 'requires_extension' => array(
+ 'amp-cache-url',
+ ),
),
'controls' => array(
'value' => array( Since there is no |
Correction: It turns out that |
85765bd
to
ac99ccf
Compare
I'm going to rebase to remove the Bento-specific functionality into another PR which will go into 2.2. The commits left in this this PR will then be just related to the spec update. |
f36363d
to
bf9480d
Compare
Codecov Report
@@ Coverage Diff @@
## develop #6303 +/- ##
=============================================
- Coverage 79.84% 75.39% -4.45%
- Complexity 0 5881 +5881
=============================================
Files 48 235 +187
Lines 759 17786 +17027
=============================================
+ Hits 606 13410 +12804
- Misses 153 4376 +4223
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Plugin builds for cac1371 are ready 🛎️!
|
@pierlon This is a bit hard to review. The biggest change in the diff is caused by sorting the scripts in bd31a77. So it's easier to see the actual changes in this diff: https://github.com/ampproject/amp-wp/pull/6303/files/bd31a77598c5de78b55b7ae24d48bc87a756a125..f164cf4f0256603adeaec8d3c30ac5a5cba1cbc0 |
AMP bento components not in changelog:
Also, I see |
'amp-byside-content' => 'v0/amp-byside-content-0.1.js', | ||
'amp-cache-url' => 'v0/amp-cache-url-0.1.js', | ||
'amp-call-tracking' => 'v0/amp-call-tracking-0.1.js', | ||
'amp-carousel' => 'v0/amp-carousel-0.2.js', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amp-carousel
using v0.2 👍
@@ -920,7 +929,7 @@ class AMP_Allowed_Tags_Generated { | |||
'amp-ad', | |||
), | |||
'spec_name' => 'amp-ad with type=custom', | |||
'spec_url' => 'https://github.com/ampproject/amphtml/blob/master/ads/custom.md', | |||
'spec_url' => 'https://github.com/ampproject/amphtml/blob/main/ads/custom.md', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this URL is now a 404. It should be https://github.com/ampproject/amphtml/blob/main/ads/vendors/custom.md.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened a PR to fix that: ampproject/amphtml#34791
Yes, I see that too. It seems due to the lack of I'm checking on that. |
dc24210
to
cac1371
Compare
I've opened ampproject/amphtml#34798 and regenerated the spec off of that. |
QA Passed I added post content as follows: <!-- wp:html -->
<amp-render src="https://blog.amp.dev/wp-json/wp/v2/posts?per_page=1&_fields=title,link" layout="fixed-height" height="100">
<template type="amp-mustache">
Latest pot on AMP blog:
<a href="{{0.link}}">{{0.title.rendered}}</a>
</template>
</amp-render>
<!-- /wp:html -->
<!-- wp:html -->
<amp-iframely width="400" height="225"
data-id="I8NNa1s"
layout="responsive"
data-img>
</amp-iframely>
<!-- /wp:html -->
<!-- wp:html -->
<amp-script id="calendar" script="calendarScript" sandboxed nodom layout="container"></amp-script>
<script id="calendarScript" type="text/plain" target="amp-script">
exportFunction('getData', () => {
return { date: new Date().toLocaleString() };
})
</script>
<amp-render src="amp-script:calendar.getData" layout="fixed-height" height="100">
<template type="amp-mustache">
The current date is: {{date}}
</template>
</amp-render>
<!-- /wp:html --> The result was as expected, with |
Previously #5998.
./bin/amphtml-update.sh
(lando ssh -c 'bash ./bin/amphtml-update.sh vendor/amphtml'
).Changelog
/
delimiters.amp-accordion
amp-base-carousel
, withorientation
attribute replacinghorizontal
.amp-date-countdown
, withcount-up
attribute replacingdata-count-up
.amp-date-display
amp-fit-text
amp-facebook-like
amp-inline-gallery
, withaspect-ratio
attribute replacingaspect-ratio-width
.amp-instagram
amp-lightbox
, withanimation
attribute replacinganimate-in
, andscrollable
being removed.amp-selector
amp-social-share
amp-stream-gallery
amp-timeago
amp-twitter
amp-video-iframe
amp-video
amp-vimeo
amp-youtube
amp-iframely
amp-render
amp-stream-gallery
amp-cache-url
(extension script)amp-nested-menu
.amp-nexxtv-player
: Removedata-origin
while adding values fordata-streamtype
and a newdata-exit-mode
attribute.amp-brid-player
: Adddata-carousel
attribute.amp-script
: Addsandboxed
attributeAddform
attribute to all form input elements.amp-date-display
.amp-story-page-outlink
.amp-story-page-attachment
/amp-story-page-outlink
attributes:cta-accent-color
,cta-accent-element
,cta-image
, andcta-text
.amp-story-page-attachment
attributes:title
,custom
value fortheme
.amp-video
: Allowcache=google
attribute.Details
1bbd5505...1d3757a07f