-
Notifications
You must be signed in to change notification settings - Fork 11
Require all owning documents to have permission policy enabled #239
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
Conversation
Failure is due to respec.org being down! |
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 idea sounds good, I wonder if this spec is the first to implement this kind of check.
Can you mention this novel approach in the "Policy control" section? Maybe it's worth discussing it in "Mitigation strategies" too.
index.html
Outdated
If |document| is not [=allowed to use=] the [=policy-controlled | ||
feature=] token "compute-pressure", return [=a promise rejected with=] {{NotAllowedError}}. | ||
</ol> | ||
</li> | ||
<aside class="issue"> | ||
<a href="https://github.com/wicg/compute-pressure/issues/110"> |
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.
<a href="https://github.com/wicg/compute-pressure/issues/110"> | |
<a href="https://github.com/w3c/compute-pressure/issues/110"> |
index.html
Outdated
<aside class="issue"> | ||
<a href="https://github.com/wicg/compute-pressure/issues/110"> | ||
Permission policy doesn't support workers yet #110 | ||
</a> | ||
Permission policy doesn't support workers directly yet #110</a>, so they cannot be set per |
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.
Permission policy doesn't support workers directly yet #110</a>, so they cannot be set per | |
Permission policy does not support workers directly yet (#110)</a>, so they cannot be set per |
index.html
Outdated
Shared workers often have multiple owning documents as they can be obtained | ||
by other documents with the [=same origin=]. | ||
|
||
In this case, all owning documents have to have the permission policy allowed |
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.
Is "have the permission policy allowed" a common way of phrasing this? It sounds a bit confusing.
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.
Yeah, I tried changing it a few times, but didn't find a good wording :-) Maybe @anssiko has some good suggestions
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.
How about just using allowed to use like in observe()
?
"[...] all owning documents must be allowed to use the policy-controlled feature defined by this specification", for example.
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.
Sure, done
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.
lgtm with a nit
Co-authored-by: Raphael Kubo da Costa <raphael.kubo.da.costa@intel.com>
@rakuco
Preview | Diff