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

feat: allow payment in amp-consent external ui #39855

Merged
merged 2 commits into from
Mar 14, 2024

Conversation

marcmrf
Copy link
Contributor

@marcmrf marcmrf commented Feb 22, 2024

Solves #39745. As described in the issue, new EU regulations demand the sites to offer a reject all option with the same visibility than the accept all one. However, they also open the door for asking for a retribution for accessing the content if user wants to reject cookies.

allow="payment" is required if the iframe want to make use of payment api.

@@ -471,6 +473,7 @@ export class ConsentUI {
const iframe = this.parent_.ownerDocument.createElement('iframe');
const sandbox = this.getSandboxAttribute_(promptUISrc);
iframe.setAttribute('sandbox', sandbox);
iframe.setAttribute('allow', IFRAME_ALLOWED_PERMISSIONS.join('; '));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not doing it via config as commented here #39745 (comment). Should I add any other permission?

@erwinmombay erwinmombay requested review from erwinmombay and powerivq and removed request for alanorozco February 22, 2024 18:24
@powerivq
Copy link
Contributor

@marcmrf Please rebase against the latest main branch. Once it passes the tests, I will be able to merge it.

@marcmrf
Copy link
Contributor Author

marcmrf commented Mar 14, 2024

@powerivq done! 🙏 Thx for the review.

@powerivq powerivq merged commit 21bbf8a into ampproject:main Mar 14, 2024
52 checks passed
eszponder pushed a commit to krzysztofequativ/amphtml that referenced this pull request Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants