-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Allow to propagate more props to amp-iframe from amp-consent #39745
Comments
@adriafolchmrf thanks for the report. the team will take a look at this and prioritize cc @powerivq |
We would consider opening them up if there are proper use cases. Can you think of the scenario where each one is needed? @adriafolchmrf For payment, are you trying to do a pay-for-not-tracking? |
mainly 2 use cases:
if we work on a PR would you be open to land it? |
@xbeumala I do not think there is a good reason to have them configurable. I would rather just add these into the amp-iframe attributes. However, I do think we need to look at each attribute case-by-case from a security and privacy standpoint. Please list all attributes you intend to add and we will take a look. |
https://developer.mozilla.org/en-US/docs/Web/API/HTMLIFrameElement/allowPaymentRequest FYI allowpaymentrequest is deprecated. allowfullscreen: I do not know why use case would require it. Can you provide a scenario? allow="geolocation": sounds fine to me in general |
PR created adding the attribute for the iframe created via amp-consent. Amp-iframe compoment already allows to configure allow attribute. Let's see what you think 🙏 |
Closed since PR merged. |
Description
A couple of days ago, legal requirements where changed in Spain for the consent, due to that, there are some attributes already available in the amp-iframe found in this list that would be interesting to be able to configure in the
amp-consent
, same way as thesandbox
attribute which allow some modification for the initialamp-iframe
rendered inside it.Alternatives Considered
Allow to extend the json of amp-consent to something like
Additional Context
No response
The text was updated successfully, but these errors were encountered: