Skip to content
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

Pass amp_is_bento_enabled() to AMP_Tag_And_Attribute_Sanitizer as prefer_bento arg #6537

Merged
merged 1 commit into from
Aug 13, 2021

Conversation

westonruter
Copy link
Member

Given a post that has a Custom HTML block with this content:

<script src="https://cdn.ampproject.org/v0/amp-facebook-page-0.1.mjs" async="" custom-element="amp-facebook-page" type="module" crossorigin="anonymous"></script>

Accessing an AMP page (without Bento enabled) will result in v0.1 of the AMP component added to the page (as expected):

<script src="https://cdn.ampproject.org/v0/amp-facebook-page-0.1.mjs" async="" custom-element="amp-facebook" type="module"></script>

When adding this feature flag plugin to enable bento when ?bento is added to the URL:

if ( isset( $_GET['bento'] ) ) {
	add_filter( 'amp_bento_enabled', '__return_true' );
}

Accessing the same AMP page with Bento still results in the same script being added as opposed to amp-facebook v1.0:

<script src="https://cdn.ampproject.org/v0/amp-facebook-page-0.1.js" async="" custom-element="amp-facebook-page"></script>

With this PR, this is fixed by passing the Bento enabled state to the AMP_Tag_And_Attribute_Sanitizer as the prefer_bento arg. So the resulting page then has:

<script src="https://cdn.ampproject.org/v0/amp-facebook-1.0.js" async="" custom-element="amp-facebook"></script>

(Note that amp-facebook-1.0.js is served rather than amp-facebook-1.0.mjs because SSR for Bento is currently broken per ampproject/amphtml#35485, and so SSR is currently disabled when Bento is enabled.)

Props @pierlon for recognizing this oversight in #6353 (comment).

@westonruter westonruter requested a review from pierlon August 13, 2021 18:31
@westonruter westonruter added this to the v2.2 milestone Aug 13, 2021
@westonruter westonruter enabled auto-merge August 13, 2021 18:32
@github-actions
Copy link
Contributor

Plugin builds for b0a4aca are ready 🛎️!

@westonruter westonruter merged commit 0765bb6 into develop Aug 13, 2021
@westonruter westonruter deleted the update/bento-preference branch August 13, 2021 19:25
@dhaval-parekh dhaval-parekh self-assigned this Dec 13, 2021
@dhaval-parekh
Copy link
Collaborator

QA Passed

✅ Bento versions of scripts are used instead of the AMP version of the script while enabling bento.

@westonruter westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bento Changelogged Whether the issue/PR has been added to release notes. Sandboxing Experiment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants