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

[WIP] Update testing of Request's default credentials mode #9911

Merged

Conversation

Projects
None yet
5 participants
@domfarolino
Copy link
Member

domfarolino commented Mar 7, 2018

This is for whatwg/fetch#585.

I'm a little surprised if only these tests have to be updated, so I'm marking this as a WIP for now. Basically what I did though, was:

  • Ran all of the /fetch, /xor, service-workers, and /cors tests on Chromium, and noted failues (none)
  • Modified the Request constructor to use the same-origin credentials fallback instead of the previous omit
  • Ran all of the previous tests again and noted the failures (only two)
  • Updated the affected tests!

@wpt-pr-bot wpt-pr-bot added the fetch label Mar 7, 2018

@wpt-pr-bot wpt-pr-bot requested review from annevk, jdm, mnot, youennf and yutakahirano Mar 7, 2018

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Mar 8, 2018

I guess we don't really have good same-origin cookie/HTTP authentication tests then.

@wanderview does that make sense?

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 8, 2018

Build PASSED

Started: 2018-03-08 11:12:38
Finished: 2018-03-08 11:23:12

Failing Jobs

  • MicrosoftEdge:16.16299

View more information about this build on:

@domfarolino

This comment has been minimized.

Copy link
Member Author

domfarolino commented Apr 4, 2018

Pinging @wanderview here. This seems to be the only thing blocking Chrome's implementation so the sooner we can get this in the better :)

@wanderview
Copy link
Contributor

wanderview left a comment

LGTM. I'm surprised there aren't any service worker tests that check the default credentials via the FetchEvent.

@domfarolino

This comment has been minimized.

Copy link
Member Author

domfarolino commented Apr 4, 2018

Thanks a lot! Me too, should we make an issue to add some at some point? I would expect more tests to be failing here, and we probably want more tests to be doing so.

@annevk

This comment has been minimized.

Copy link
Member

annevk commented Apr 20, 2018

Thoughts on whatwg/fetch#585 (comment) anyone? Sounds like @wanderview wants to go ahead with this in Firefox as well, so I'm happy to land this, just wondering about follow-up work and how to structure that.

@wanderview

This comment has been minimized.

Copy link
Contributor

wanderview commented Apr 20, 2018

BTW, the service worker tests mostly check that the correct value is passed through to FetchEvent. The tests use explicit modes for this. They are not really checking for defaults. I think that its reasonable for the fetch tests to mainly be checking the defaults.

@annevk annevk merged commit 55d647f into web-platform-tests:master Apr 20, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

annevk added a commit to whatwg/fetch that referenced this pull request Apr 20, 2018

Make fetch() use "same-origin" credentials by default
Most features in the platform have this default and not having this default causes multiple connections in contemporary implementations. It also causes confusion as switching from XMLHttpRequest to fetch() is not as seamless as it could be.

Making this change will also make it easier for <script type=module> to have this default as per discussion in whatwg/html#2557.

Tests: web-platform-tests/wpt#9911.

@domfarolino domfarolino deleted the domfarolino:update-requestinit-creds branch Nov 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.