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

Clean up data for Permissions Policy header #23470

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

queengooborg
Copy link
Collaborator

@queengooborg queengooborg commented Jun 20, 2024

This PR cleans up a lot of data for the Permissions Policy HTTP header. The alternative name was copied down to all subfeatures, which is improper.

Additionally, this marks the header as unsupported in Firefox and Safari, as the header is not supported, and support was only indicated because the values are supported as values to <iframe allow="">. The features were copied to #23487 to untangle this data.

This fixes #15987.

@github-actions github-actions bot added the data:http 🚠 Compat data for HTTP features. https://developer.mozilla.org/docs/Web/HTTP label Jun 20, 2024
@queengooborg queengooborg requested a review from Elchi3 July 9, 2024 08:49
@github-actions github-actions bot added the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Sep 10, 2024
Copy link

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

@github-actions github-actions bot removed the merge conflicts 🚧 This PR needs to merge latest "main" branch to resolve a merge conflict or other issue. label Sep 11, 2024
}
}
},
"battery": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Mind adding battery in a separate PR?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To avoid merge conflicts, I feel it is best to keep it all in the same PR -- but if still desired, I can split it out!

Copy link
Contributor

Choose a reason for hiding this comment

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

I have removed it in 521b2de and we can simply revert that in a separate PR after merging this, which shouldn't cause a merge conflict.

@alexjalonso7777

This comment has been minimized.

}
}
},
"battery": {
Copy link
Contributor

Choose a reason for hiding this comment

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

I have removed it in 521b2de and we can simply revert that in a separate PR after merging this, which shouldn't cause a merge conflict.

Copy link
Contributor

@caugner caugner left a comment

Choose a reason for hiding this comment

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

Originally I approved this PR, but it looks like this PR does more than what's described in the PR description.

In particular, this PR seems to remove the "Feature-Policy", and therefore changes the support data for the gamepad and speaker-selection permissions, causing the lint issues.

@queengooborg
Copy link
Collaborator Author

The changes regarding the removal of "Feature-Policy" were stated in the description, in that the alternative name was removed from all subfeatures.

The failing lint is actually not because of that change, though, but rather because gamepad and speaker-selection were incorrectly stated as supported on the header when they're only supported on the allow iframe attribute (see the notes that were removed). It looks like the data was change for those two features for Chrome/Safari in between which set them to false, so now all browser engines say the feature is unsupported, and thus they should be removed. Doing so now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:http 🚠 Compat data for HTTP features. https://developer.mozilla.org/docs/Web/HTTP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

http.headers.Feature-Policy - should it be marked as "supported" on Fx/Safari?
3 participants