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

FF Clarify Feature-Policy header not sent #20729

Merged
merged 3 commits into from
Sep 22, 2023

Conversation

hamishwillee
Copy link
Contributor

The Permissions-Policy and Feature-Policy for FF confused me because it said partial support of the header and indicated

"Only supported through the allow attribute on <iframe> elements."

So the header isn't actually supported, just the functionality that the header provides. Someone who knows how this all works might not be confused, but I was. So what I have done is change the note text to:

Header not sent but policy can be set through the allow attribute on <iframe> elements (see bug 1627890)"

The bug links are different - for Feature-Policy I link to the bug where the header was disabled by default. For Permissions-Policy I link to the bug where the header is intended to be updated to Permissions-Policy and be sent. I think that makes sense for the later case, because you mostly want to be able to track when this will be fixed.

This fell out of work done for mdn/content#28848

@github-actions github-actions bot added the data:http Compat data for HTTP features. https://developer.mozilla.org/docs/Web/HTTP label Sep 18, 2023
@hamishwillee hamishwillee changed the title FF Clarify header not sent FF Clarify Feature-Policy header not sent Sep 18, 2023
Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

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

hmmm, so essentially the header is not recognized at all? Maybe it would be better tosay not supported then? (or behind a pref)?

And instead of "Header not sent", I would say "Header not recognized" because I guess people do successfully send this header, some browsers recognize it, but Firefox doesn't.

@hamishwillee
Copy link
Contributor Author

hmmm, so essentially the header is not recognized at all? Maybe it would be better tosay not supported then? (or behind a pref)?

Yes, FF can be made to recognise it with a pref but out of the box ignores it (and that pref will never be set by default). I am happy to make this false, remove the partial implementation but keep the note - that would be accurate.

And instead of "Header not sent", I would say "Header not recognized" because I guess people do successfully send this header, some browsers recognize it, but Firefox doesn't.

That would be more correct.

I'll this tomorrow unless you say "changed my mind". Thank you for the advice.

@Elchi3
Copy link
Member

Elchi3 commented Sep 18, 2023

Yes, FF can be made to recognise it with a pref but out of the box ignores it (and that pref will never be set by default). I am happy to make this false, remove the partial implementation but keep the note - that would be accurate.

Sounds good to me!

@@ -141,7 +139,7 @@
"webview_android": "mirror"
},
"status": {
"experimental": true,
"experimental": false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is OK because the feature is available in multiple browsers. I suspect that this is why it was done as partial implementation.

@hamishwillee
Copy link
Contributor Author

Hi @Elchi3 This proved a bit of a shit.

  • If I set supported to false then I lose the version number, which is obviously useful for knowing when things were implemented.
  • It would be accurate to set the flag/preference - but it turns out that it strips all the notes from android along with making the preference false. Since the allow thingy works irrespective of the setting seems a bad idea.
  • Tried also to set the Feature-Policy to non-standard=true, but can't do that because it has spec urls. Probably should remove Feature-Policy doc altogether, but not sure where it is used."
  • I could set version to false and add a note everywhere:

    "Header not recognized but policy can be set from version Xxx through the allow attribute on <iframe> elements "

What I have done is your original suggestion - just changed text to "recognized". This will all fix itself once the header is sent.

@Elchi3
Copy link
Member

Elchi3 commented Sep 19, 2023

I just noticed we also have #20328.
I don't know how much work is needed on the content side to remove Feature-Policy docs / redirect them to Permission-Policy but I assume it would be good to do that.

@Elchi3
Copy link
Member

Elchi3 commented Sep 19, 2023

That said, I think the changes to Permission-Policy.json are an improvement. The Feature-Policy.json changes will be void soon anyway...

@hamishwillee
Copy link
Contributor Author

I don't know how much work is needed on the content side to remove Feature-Policy docs / redirect them to Permission-Policy but I assume it would be good to do that.

@Elchi3 The docs work was already done - https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Feature-Policy now redirects.

The Feature-Policy json is an orphan - I'm only updating it because it is there and I don't know how it is used. IMO we should just merge #20328 to delete it.

Can you merge this then? As you say, it is better/less ambiguous.

@Elchi3 Elchi3 merged commit 0338141 into mdn:main Sep 22, 2023
@hamishwillee hamishwillee deleted the ff_permission_policy branch September 24, 2023 23:48
Elchi3 pushed a commit to Elchi3/browser-compat-data that referenced this pull request Nov 14, 2023
* FF Clarify header not sent

* Change notes to not-recognised

* Fix typo recognised
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.

2 participants