Skip to content

Clean up data for Permissions Policy header#23470

Merged
caugner merged 11 commits intomainfrom
permissions-policy
Nov 5, 2024
Merged

Clean up data for Permissions Policy header#23470
caugner merged 11 commits intomainfrom
permissions-policy

Conversation

@queengooborg
Copy link
Contributor

@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
@github-actions
Copy link
Contributor

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
Contributor 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
Contributor 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.

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.

LGTM, but shouldn't Permission-Policy and all its sub-features get "experimental": true given that only Chromium supports it?

PS: Apologies for the confusion about "Feature-Policy" removal.

@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 Oct 29, 2024
@github-actions
Copy link
Contributor

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 Oct 30, 2024
queengooborg and others added 2 commits October 30, 2024 17:02
Co-authored-by: Claas Augner <495429+caugner@users.noreply.github.com>
@queengooborg queengooborg requested a review from caugner November 2, 2024 16:08
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.

TY, LGTM!

@caugner caugner merged commit 76a2380 into main Nov 5, 2024
@caugner caugner deleted the permissions-policy branch November 5, 2024 10:41
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