-
Notifications
You must be signed in to change notification settings - Fork 384
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
Introduce support for a more flexible AMP via Bento with levels of sandboxing enforcement #6443
Comments
We discussed this in our plugin sync today, and per the initial release this will be an experimental feature. We won't have a user interface to reveal the feature, but will instead make it available as a PHP filter for initial testing. |
QA Passed After enabling the experimental feature (via the example plugin), I've followed the QA process described in #6546 (Testing section). The sandboxing level selected on the AMP settings page was set to Loose. All three sample posts passed the specified tests:
Tested on AMP 2.2.0-alpha-20211117T230738Z-094aef8 I'm leaving this issue in "Ready for QA" so that @dhaval-parekh has a chance to do a QA separately. |
✅ Automatically upgrading to sandboxing level if page supports it. ✅ I have tested with the QA process described in PR #6546 (Testing section) and that looks good to me.
However, It works perfectly in "Moderate" and "Strict" mode because of the AMP video component. Other than this all looks good to me. |
@dhaval-parekh Yes, I'm seeing that as well. Thanks for raising that. This seems like it may be a Gutenberg bug. Specifically, I'm seeing that Gutenberg has this style rule: .wp-block-video video {
width: 100%;
} The |
I've opened WordPress/gutenberg#37052 to address this. |
While testing sandboxing level with Strict Mode (develop branch), I noticed comment form is missing |
@milindmore22 I'm not seeing that in |
yes, I was testing those test cases |
The fact that your screenshot is showing a red outline means that something is not working as expected. The test cases should only have green outlines if they are passing. |
JFI : I installed production build v2.2.0-alpha-20211207T172207Z-0ab628a on the test site to confirm |
Can you add me to that site? |
Nevermind, I'm able to reproduce it now. It doesn't happen when I have the Sandboxing level set to Loose and I'm upgraded to Strict since there is nothing AMP-invalid on the page. However, if I've set the level to Strict then it fails to add the |
Simple fix: #6769 |
(And good catch!) |
QA Passed Sandboxing Levels are all well tested and Look good as per the QA process #6546
|
Feature description
The AMP plugin currently offers a binary choice: either you are 100% AMP or you disable AMP. The Bento AMP initiative seeks to make AMP more flexible by allowing you to use AMP components on non-AMP pages. In reality, the AMP plugin has supported this to a limited degree in Standard mode. When invalid markup is kept, the AMP plugin serves an AMP page without the
amp
attribute on thehtml
element (cf. #5549). This in effect allows you to have a page with all the benefits of the AMP runtime, but also with additional non-AMP scripts you may need. The caveat here is that this is not officially supported and since AMP components lack encapsulation, there is no guarantee that it will work. Your mileage may vary. Bento solves this problem by encapsulating the components with shadow DOM so that other scripts on the page can't interfere with their internal behavior.An additional purpose of the Bento project is to make components much easier to author via Preact. In addition to the AMP core components being available as Bento, site owners and plugin authors may create their own Bento components which may not be “certified” for AMP and yet still have an assurance of performance by being created with the Bento framework.
With Bento components in hand, we can relax AMP validation to make pages more flexible. In particular, we can introduce sandbox levels to indicate how strict a site owner wants to be in terms of the markup allowed on the page:
For the initial exploration, the sandbox levels would only be available in Standard mode. Initially it would default to L3, maintaining strict AMP validity. Once Bento matures, the plugin can switch from Reader mode being the default to instead having Standard mode with L1 sandboxing as the default.
Additionally, in the initial sandboxing implementation, the level would largely be static. If you select L1, then that is the level that would be reflected on the rendered document. However, in a subsequent iteration the sandboxing levels should dynamically adjust based on the content of the page. So if you selected L1 and yet there is no invalid AMP markup on the page, then it would automatically be upgraded to L3.
Lastly, in future releases we may allow the sandboxing levels to be used in non-Standard modes as well, in particular Reader mode. If the user selected L1 or L2 sandboxing for Reader mode, then the paired versions would not be linked to as AMP with
link[rel=amphtml]
, since there would be no guarantee that the pages would be valid. Instead, only alink[rel=alternate][media]
would appear as is today when mobile redirection is enabled, as is documented in the Separate URLs documentation on Google Search Console. In other words, Reader mode with these lesser sandboxing levels would provide a “mobile theme” for the site, which would be particularly useful for sites which have a primary theme which is fundamentally not responsive with a poor user experience.Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation brief
amp-carousel
should be converted toamp-base-carousel
.action-xhr
, do not include theamp-form
component.QA testing instructions
Demo
Changelog entry
The text was updated successfully, but these errors were encountered: