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

Fix referrer-policy setup for fetch #17280

Merged

Conversation

JuniorHsu
Copy link
Contributor

Previous setup didn't pass the RP to fetch

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

I think this is a good test, but the test it replaces was also a good test. In particular, if the referrer policy passed to fetch() is the empty string (the default value), the fetching algorithm will pull the value from the request's client, which is the iframe's global object/document which does have a referrer policy, right? So as far as I can tell both should work.

@JuniorHsu
Copy link
Contributor Author

I think this is a good test, but the test it replaces was also a good test. In particular, if the referrer policy passed to fetch() is the empty string (the default value), the fetching algorithm will pull the value from the request's client, which is the iframe's global object/document which does have a referrer policy, right? So as far as I can tell both should work.

For the test I replaces, both browsers do not work as you said, which is noted in fetch 4.1.2.5
However, it distracts the spec we want to modify here. We might need another issue for Referrer-Policy to address this.

@annevk
Copy link
Member

annevk commented Jun 15, 2019

How about we leave the test in and file bugs against browsers? It does seem like something that ought to be fixed.

@JuniorHsu
Copy link
Contributor Author

I leave both tests in this version. Two observations:
(a) We load too many iframes in one test, which could cause TIME-OUT
(b) In current Firefox, |submit.click();| doesn't work sometimes for the form POST test.

@annevk
Copy link
Member

annevk commented Jun 18, 2019

For a) should we split the test somehow then? I could merge this first and then we can do that separately though. Not sure what the problem could be with b).

@JuniorHsu
Copy link
Contributor Author

Let's merge. Thanks!

@annevk annevk merged commit 05ec7bd into web-platform-tests:annevk/origin-header Jun 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants