-
Notifications
You must be signed in to change notification settings - Fork 155
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
Major update to spec; rewrite in bikeshed. #54
Conversation
This is a significant update to the Feature Policy spec; in addition to rewriting the spec in Bikeshed, this brings it in line with the explainer document, adds iframe policies, removes explicit references to JFV, moves all of the features into an appendix, (adding a couple of new features, and removing references to features that we're not planning on trying to standardize on and ship in the near future.)
index.bs
Outdated
access to [=Payment interface=].</p> | ||
<p>If the iframe element has an "<code>allow</code>" attribute whose | ||
value contains the token "<code>payment</code>", then the | ||
"<code>allowpaymentrequest</code> attribute should have no effect.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not must?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
index.bs
Outdated
{{requestFullscreen()}}.</p> | ||
<p>If the iframe element has an "<code>allow</code>" attribute whose | ||
value contains the token "<code>fullscreen</code>", then the | ||
"<code>allowfullscreen</code> attribute should have no effect.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not must?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch; I'll fix that.
index.bs
Outdated
attribute's value is the empty string, then abort these steps.</li> | ||
<li>If the [meta] element is not being inserted in into the | ||
{{Document}}'s [head] element, or if is inserted after any [script] or | ||
[link] elements, then abort these steps.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this rule? Is "after" in time, or in tree order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rule, which we used for origin trials as well, is so that scripts running synchronously in the document will always have a consistent, effectively-immutable policy. Without that, a script in the head could run before the meta element is encountered.
'After' is supposed to refer to tree order, but ideally I'd want elements injected via script to be ignored as well, for the same reason. I'm just not sure how to do that in the spec yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this should be an attribute on html
instead, then? Like manifest
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's an interesting possibility -- it would only allow a single policy, and would mean that we abandon the http-equiv idea, but it would certainly be parseable before any scripts get a chance to run.
I'll remove the <meta>
bits from the spec, and bring this up in an issue separately then. Having the ability to set the policy for the top-level frame purely in HTML is nice-to-have, but not, I think, strictly necessary, with the set of features currently listed.
};</pre> | ||
<p><{iframe}> elements have an "<code>allow</code>" attribute, which | ||
contains an unordered set of unique space-separated tokens that are ASCII | ||
case-insensitive. The allowed values are names of features. Unrecognized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why ASCII case-insensitive?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This keeps it in line with other iframe attributes, like sandbox. I didn't think it was useful to insist that these tokens are case-sensitive, while sandbox attributes on the same element are insensitive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related discussion: whatwg/html#1665
These must have no additional effect when the feature in question is already mentioned in the 'allow' attribute value.
This isn't strictly necessary for the operation of the framework, and can be added to the spec afterwards.
Awesome, thanks for the update! There's more stuff to flesh out here, but we can tackle those bits in separate PRs. Merging. |
This is a significant update to the Feature Policy spec; in addition to
rewriting the spec in Bikeshed, this brings it in line with the explainer
document, adds iframe policies, removes explicit references to JFV, moves all
of the features into an appendix, (adding a couple of new features, and removing
references to features that we're not planning on trying to standardize on and
ship in the near future.)