Skip to content
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

Update Feature-Policy to Permissions-Policy #22439

Merged

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented Nov 21, 2022

Description

This PR aims to do the work of migrating the existing Feature-Policy docs over toPermissions-Policy, as described in #22347

See my research document at https://docs.google.com/document/d/14xHANU3n43PfJR_dCpEwVCiVYv12BAfGqOICNvd79R4/edit#heading=h.mahbkxhoku5t for a list of the Permissions-Policy directives I am intending to implement on MDN as part of this work. This gives us parity with what was previously documented under Feature-Policy, but it adds a few more that are known to be supported, and it removes mention of some that were obviously being worked on at some point, but never seem to have gotten any traction.

Also see the equivalent BCD changes at mdn/browser-compat-data#18250

Notes:

  • I found a note at https://w3c.github.io/permissions/#powerful-features, which has questioned my understanding of the fundamental way in which these policies work. It says "However, requesting permission to use any other feature will be automatically denied because they are not listed in the allow attribute.". I thought the behavior of all features that weren't explicitly listed was governed by the default allowlist value? This note doesn't sound right to me.

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested review from a team as code owners November 21, 2022 06:31
@chrisdavidmills chrisdavidmills requested review from teoli2003 and removed request for a team November 21, 2022 06:31
@github-actions github-actions bot added the Content:HTTP HTTP docs label Nov 21, 2022
@chrisdavidmills chrisdavidmills marked this pull request as draft November 21, 2022 06:32
@github-actions
Copy link
Contributor

github-actions bot commented Nov 21, 2022

Preview URLs (120 pages)
Flaws (43)

Note! 103 documents with no flaws that don't need to be listed. 🎉

URL: /en-US/docs/Web/Security/Same-origin_policy
Title: Same-origin policy
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/URLUtils/href does not exist

URL: /en-US/docs/Web/API/XMLHttpRequest
Title: XMLHttpRequest
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/XMLHttpRequestUpload does not exist

URL: /en-US/docs/Web/API/Screen_Capture_API
Title: Screen Capture API
Flaw count: 3

  • macros:
    • /en-US/docs/Web/API/MediaTrackConstraints/cursor redirects to /en-US/docs/Web/API/MediaTrackConstraints
    • /en-US/docs/Web/API/MediaTrackSupportedConstraints/cursor redirects to /en-US/docs/Web/API/MediaTrackSupportedConstraints
    • /en-US/docs/Web/API/MediaTrackConstraints/cursor redirects to /en-US/docs/Web/API/MediaTrackConstraints

URL: /en-US/docs/Web/API/Screen_Capture_API/Using_Screen_Capture
Title: Using the Screen Capture API
Flaw count: 3

  • macros:
    • /en-US/docs/Web/API/MediaTrackConstraints/cursor redirects to /en-US/docs/Web/API/MediaTrackConstraints
    • The fourth to sixth parameters of 'EmbedLiveSample' are deprecated
  • broken_links:
    • Can't resolve /en-US/docs/Web/API/WebRTC_API/Taking_still_photos

URL: /en-US/docs/Web/API/WebXR_Device_API
Title: WebXR Device API
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/XRPermissionStatus does not exist

URL: /en-US/docs/Web/API/Navigator/requestMIDIAccess
Title: Navigator.requestMIDIAccess()
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/docs/Web/HTTP/Feature_Policy

URL: /en-US/docs/Web/API/Navigator/requestMediaKeySystemAccess
Title: Navigator.requestMediaKeySystemAccess()
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/MediaKeySystemConfiguration redirects to /en-US/docs/Web/API/MediaKeySystemAccess/getConfiguration

URL: /en-US/docs/Web/API/FeaturePolicy/getAllowlistForFeature
Title: FeaturePolicy.getAllowlistForFeature()
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/docs/Web/HTTP/Permissions_Policy/#allowlists

URL: /en-US/docs/Web/API/Fullscreen_API
Title: Fullscreen API
Flaw count: 1

  • broken_links:
    • Link points to the page it's already on

URL: /en-US/docs/Web/API/CredentialsContainer/get
Title: CredentialsContainer.get()
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/CredentialRequestOptions does not exist

URL: /en-US/docs/Web/API/Payment_Request_API
Title: Payment Request API
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/AddressErrors does not exist

URL: /en-US/docs/Web/API/HTMLIFrameElement
Title: HTMLIFrameElement
Flaw count: 22

  • macros:
    • /en-US/docs/Web/API/HTMLIFrameElement/align does not exist
    • /en-US/docs/Web/API/HTMLIFrameElement/allow does not exist
    • /en-US/docs/Web/API/HTMLIFrameElement/allowfullscreen does not exist
    • /en-US/docs/Web/API/HTMLIFrameElement/frameBorder does not exist
    • /en-US/docs/Web/API/HTMLIFrameElement/height does not exist
    • and 7 more flaws omitted
  • bad_bcd_links:
    • no explanation!
    • no explanation!
    • no explanation!
    • no explanation!
    • no explanation!
    • and 5 more flaws omitted

URL: /en-US/docs/Web/API/Picture-in-Picture_API
Title: Picture-in-Picture API
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/PictureInPictureWindow/onresize redirects to /en-US/docs/Web/API/PictureInPictureWindow/resize_event

URL: /en-US/docs/Web/HTTP/Headers/Permissions-Policy
Title: Permissions-Policy
Flaw count: 2

  • broken_links:
    • No need for the pathname in anchor links if it's the same page
    • Can't resolve /en-US/docs/Web/API/Audio_Output_Devices_API

URL: /en-US/docs/Web/HTTP/Headers/Permissions-Policy/display-capture
Title: Permissions-Policy: display-capture
Flaw count: 1

  • bad_bcd_queries:
    • No BCD data for query: http.headers.Permissions-Policy.display-capture

URL: /en-US/docs/Web/HTTP/Headers/Permissions-Policy/usb
Title: Permissions-Policy: usb
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/Navigator/usb does not exist

URL: /en-US/docs/Web/Media/Autoplay_guide
Title: Autoplay guide for media and Web Audio APIs
Flaw count: 1

  • macros:
    • /en-US/docs/Web/API/HTMLMediaElement/allowedToPlay does not exist
External URLs (11)

URL: /en-US/docs/Web/API/XMLHttpRequest
Title: XMLHttpRequest


URL: /en-US/docs/Web/API/AmbientLightSensor/illuminance
Title: AmbientLightSensor.illuminance


URL: /en-US/docs/Web/HTTP/Headers/Permissions-Policy
Title: Permissions-Policy


URL: /en-US/docs/Web/HTTP/Headers/Permissions-Policy/execution-while-not-rendered
Title: Permissions-Policy: execution-while-not-rendered


URL: /en-US/docs/Web/HTTP/Headers/Permissions-Policy/execution-while-out-of-viewport
Title: Permissions-Policy: execution-while-out-of-viewport


URL: /en-US/docs/Web/HTTP/Headers/Content-Security-Policy
Title: Content-Security-Policy


URL: /en-US/docs/Web/HTTP/Permissions_Policy
Title: Permissions Policy

(comment last updated: 2022-12-12 11:50:01)

@github-actions github-actions bot added Content:Glossary Glossary entries Content:HTML Hypertext Markup Language docs Content:Media Media docs Content:Other Any docs not covered by another "Content:" label Content:WebAPI Web API docs labels Nov 21, 2022
@chrisdavidmills chrisdavidmills force-pushed the change-feature-policy-to-permissions-policy branch from 4bfa476 to 58ac7be Compare November 21, 2022 15:58
@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@@ -22,7 +22,7 @@ The **`AbsoluteOrientationSensor`** interface of the [Sensor APIs](/en-US/docs/W

To use this sensor, the user must grant permission to the `'accelerometer'`, `'gyroscope'`, and `'magnetometer'` device sensors through the [Permissions API](/en-US/docs/Web/API/Permissions_API).

If a feature policy blocks use of a feature it is because your code is inconsistent with the policies set on your server. This is not something that would ever be shown to a user. The {{httpheader('Feature-Policy')}} HTTP header article contains implementation instructions.
If a Permissions Policy blocks use of a feature it is because your code is inconsistent with the policies set on your server. This is not something that would ever be shown to a user. Our [Permissions Policy](/en-US/docs/Web/HTTP/Permissions_Policy) article contains implementation instructions.
Copy link
Collaborator

@hamishwillee hamishwillee Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, FWIW, I do not like this note, because I don't know what ". This is not something that would ever be shown to a user." means/the intent.

It is worth saying that this might be blocked though and rolling out everywhere. Something like:

This feature may be blocked by a server Permissions Policy.

... unless there is a nuance I am missing that needs to be captured?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this note is pretty crappy. I've updated all such instances with

This feature may be blocked by a Permissions Policy set on your server.

Or some slight variation of it.

@@ -22,7 +22,7 @@ The **`Accelerometer`** interface of the [Sensor APIs](/en-US/docs/Web/API/Senso

To use this sensor, the user must grant permission to the `'accelerometer'`, device sensor through the [Permissions API](/en-US/docs/Web/API/Permissions_API).

If a feature policy blocks the use of a feature, it is because your code is inconsistent with the policies set on your server. This is not something that would ever be shown to a user. The {{httpheader('Feature-Policy')}} HTTP header article contains implementation instructions.
If a Permissions Policy blocks the use of a feature, it is because your code is inconsistent with the policies set on your server. This is not something that would ever be shown to a user. Our [Permissions Policy](/en-US/docs/Web/HTTP/Permissions_Policy) article contains implementation instructions.
Copy link
Collaborator

@hamishwillee hamishwillee Dec 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated as per comment above.

@hamishwillee hamishwillee requested review from sideshowbarker and schalkneethling and removed request for a team December 8, 2022 23:14
@hamishwillee
Copy link
Collaborator

@chrisdavidmills I have fixed the merge issues, and re-reviewed.

  • Great job of integrating the suggestions.
  • Some minor nits/comments added.
  • Moved out of draft and approved.

Once you've had a chance to look I'd be happy to merge. It's a mammoth PR so I can imagine I have missed things, but I am also sure it is way better than what preceded it.

@github-actions
Copy link
Contributor

This pull request has merge conflicts that must be resolved before it can be merged.

@caugner
Copy link
Contributor

caugner commented Dec 12, 2022

@yin1999 Would you have a chance to look into this error? 🙏

@yin1999
Copy link
Member

yin1999 commented Dec 12, 2022

@yin1999 Would you have a chance to look into this error? 🙏

I'll have a look on it

@yin1999
Copy link
Member

yin1999 commented Dec 12, 2022

@yin1999 Would you have a chance to look into this error? 🙏

I'll have a look on it

Hi @caugner, I found the problem:

When we pass --paginate param, the response of second paginated api call for this PR will not contain files field (all the file changes are listed in the first api call). So the command fails. (Tested in #22897)

I've created #22898 to fix this.

@chrisdavidmills
Copy link
Contributor Author

@caugner @yin1999 This is the best news I've had all day. Thank you so much for fixing this <3

@yin1999
Copy link
Member

yin1999 commented Dec 12, 2022

@caugner @yin1999 This is the best news I've had all day. Thank you so much for fixing this <3

Apology for I haven't tested the workflow on a PR that has more than 100 commits.

@caugner
Copy link
Contributor

caugner commented Dec 12, 2022

Apology for I haven't tested the workflow on a PR that has more than 100 commits.

No worries, thanks for the quick fix! 🎉

@hamishwillee hamishwillee merged commit af967bb into mdn:main Dec 12, 2022
@hamishwillee
Copy link
Collaborator

Let us all bow before @chrisdavidmills !

@chrisdavidmills chrisdavidmills deleted the change-feature-policy-to-permissions-policy branch December 13, 2022 06:46
@chrisdavidmills
Copy link
Contributor Author

@hamishwillee awesome, just awesome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:Glossary Glossary entries Content:HTML Hypertext Markup Language docs Content:HTTP HTTP docs Content:Media Media docs Content:Other Any docs not covered by another "Content:" label Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants