search

ampproject / amphtml

The AMP web component framework.
https://amp.dev
Apache License 2.0
14.89k stars 3.89k forks source link

FR: `sandbox` iframes in `amp-facebook` components #34181

Open caroqliu opened 3 years ago

caroqliu commented 3 years ago

We currently exclude amp-facebook, amp-facebook-comments, amp-facebook-like, and amp-facebook-page from applying sandbox attributes to their generated iframe element: https://github.com/ampproject/amphtml/blob/36e818946a3231685e407297b8180a48f869f626/src/3p-frame.js#L158-L161

This is a tracking issue to introduce sandbox attributes for these components, which may* no longer be critical.

*Our manual tests show that embedded Facebook posts of all the above types don't appear to be affected aside from one side affect: an alarming error to the console, likely due to the 3p script altering document.domain, which is a sandbox-non-compliant operation. For all other intents and purposes the component appears to continue to work despite throwing this error. image

It is possible that we were not able to detect potential breakages due to confirmation bias from manually testing our examples exclusively, and that Domain Verification may have something to do with the document.domain access that historically caused sandbox FB iframes to break.

@alanorozco suggests to verify if we can safely introduce sandbox by accessing facebook analytics from embedding comments configured to a certain domain or facebook page.

Related discussion

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.