ampproject / ampproject/amp.dev

amp-iframe documentation is confusing regarding allow-same-origin

Open
#2,824 2 comments 0 reactions 0 assignees View on GitHub
Category: Content P1: High
Dominant language
HTML
Stars
600
Forks
675
PR merge metrics
No merged PRs in 30d

Description

## 📖 Missing or out-of-date documentation

**Describe the content that is missing or should be updated**
[amp-iframe documentation](https://github.com/ampproject/amphtml/blob/master/extensions/amp-iframe/amp-iframe.md)
A constant source of confusion for our client partners and developers is amp-iframe, specifically regarding allow-source-origin.

For example, in the spec it is written:

> An amp-iframe must not be in the same origin as the container unless they do not allow allow-same-origin in the sandbox attribute.

"Unless they do not allow allow-same-origin" is confusing and oddly phrased. "Allow allow-same-origin" is an awkward phrase, and who is "they" referring to in this case? When we say "they allow", what does that mean?

If I interpret "allow" to mean that "allow-same-origin" is simply included in the sandbox attribute, this makes it sound like we can be on the same origin if we don't have allow-same-origin. While discouraged, this is true, and also very counterintuitive (allow-same-origin DISABLES usage of the same origin).

In the [iframe origin policy](https://github.com/ampproject/amphtml/blob/master/spec/amp-iframe-origin-policy.md) note, this sentence also proves very confusing:

> AMP thus enforces that they are not on the same origin.
> There is one exception to this. amp-iframe uses a restrictive iframe-sandbox by default.
> If one does not opt into allow-same-origin, then every origin is allowed for the iframe.
> As soon as you add allow-same-origin to the sandbox the origin rules apply.

This again states that adding allow-same-origin introduces the rule that prevents usage of the same origin. This is really confusing! If the iframe-sandbox is restrictive by default, why would adding an "allow-same-origin" exception make it more restrictive?

In addition, "AMP thus enforces that they are not on the same origin" isn't accurate, because by default, **it doesn't enforce it**. You have to add allow-same-origin in order to enforce it.

The AMP validation message is also confusing:

> Origin of \ must not be equal to container if allow-same-origin is set

In order to fix this, I think the following could be tweaked:
1. More clarification on why allow-same-origin should be used. If AMP doesn't enforce this by default, either through errors or validation, then we should explain that this is important for security, ensuring performance in cache, etc.
2. Some acknowledgement that the name is counterintuitive (since allow-same-origin actually prevents same origin).
3. A less confusing statement in the spec (clarifying "they", removing multiple negatives in a sentence, providing more examples could all help)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.