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

Don't strictly enforce a particular FetchEvent.clientId for navigatio… #9240

Merged
merged 2 commits into from Jan 30, 2018

Conversation

Projects
None yet
5 participants
@wanderview
Copy link
Contributor

wanderview commented Jan 29, 2018

…n requests. (w3c/ServiceWorker#1266)

Note, FetchEvent.clientId is also covered by fetch-event.https.html, so we don't need this coverage here. Since this part of the spec is changing and a bit confused at the moment here it would be nice to let these tests verify their other parts without this check for now. Also, the tests are not well written and currently timeout if this check fails.

@wanderview wanderview force-pushed the wanderview:dev-nav-clientid branch from f0e0ea2 to be08ad7 Jan 29, 2018

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Jan 29, 2018

Build PASSED

Started: 2018-01-30 14:42:37
Finished: 2018-01-30 14:52:04

View more information about this build on:

// NOTE: Until FetchEvent.resultingClientId is implemented some browsers
// return null for the FetchEvent.clientId on navigation requests. Since
// this is not in the spec yet, though, we don't strictly enforce it in the
// test any more.

This comment has been minimized.

Copy link
@mattto

mattto Jan 30, 2018

Member

Yes I think we should use fetch-event.https.html to test this instead (although the current test there is a bit awkward).

See also client-navigate-worker.js which seems to explicitly test the opposite.

I don't know if we really need this comment in this file though... seems better suited for the commit description.

Remove comment and add additional commit message.
Until FetchEvent.resultingClientId is implemented some browsers
return null for the FetchEvent.clientId on navigation requests.  Since
this is not in the spec yet, though, we don't strictly enforce it in the
test any more.
@mattto

mattto approved these changes Jan 30, 2018

Copy link
Member

mattto left a comment

lgtm, I'd also mention in the description we already have test coverage for clientId for navigations in fetch-event.https.html.

@jdm

jdm approved these changes Jan 30, 2018

Copy link
Contributor

jdm left a comment

Based on mattto's approval.

@wanderview wanderview merged commit 608c356 into web-platform-tests:master Jan 30, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
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.